Implement :jar (deprecate :require)#22343
Conversation
| } | ||
| } | ||
|
|
||
| // TODO: no idea how to access and reload the class paths |
There was a problem hiding this comment.
This is pretty tricky. There is a method updateClassPath but it's not used anywhere and I'm pretty sure it never worked and was just copy-pasted from Scala 2.
ctx.platform.classPath is used from ctx.platform.rootLoader which is itself used to setup RootClass which corresponds to __root__ from which all imports start. But RootClass is a symbol created once when we initialize the initial context and never again, so updateClassPath won't actually help us import any new symbol
So I think we need to recreate a rootCtx, which is akin to what :settings seems to do:
scala3/compiler/src/dotty/tools/repl/ReplDriver.scala
Lines 542 to 543 in 5176f9f
|
Had to rebase to integrate with the |
|
Per the discussion on #21658 rename |
bracevac
left a comment
There was a problem hiding this comment.
LGTM. Had some questions.
Co-authored-by: Oliver Bračevac <bracevac@users.noreply.github.com>
bracevac
left a comment
There was a problem hiding this comment.
I just checked and I think the exception handling needs to be more robust. For example, if you feed it a file that is not a jar, it will crash the entire REPL session:
scala> :jar fakejar.jar
Exception in thread "main" java.io.IOException: Error accessing fakejar.jar
at dotty.tools.io.FileZipArchive.dotty$tools$io$FileZipArchive$$openZipFile(ZipArchive.scala:126)
at dotty.tools.io.FileZipArchive.$1$$lzyINIT1(ZipArchive.scala:164)
at dotty.tools.io.FileZipArchive.$1$(ZipArchive.scala:161)
at dotty.tools.io.FileZipArchive.root(ZipArchive.scala:161)
at dotty.tools.io.FileZipArchive.iterator(ZipArchive.scala:200)
at dotty.tools.repl.ReplDriver.flatten$1(ReplDriver.scala:527)
at dotty.tools.repl.ReplDriver.interpretCommand(ReplDriver.scala:530)
at dotty.tools.repl.ReplDriver.interpret(ReplDriver.scala:308)
at dotty.tools.repl.ReplDriver.loop$1(ReplDriver.scala:219)
at dotty.tools.repl.ReplDriver.runUntilQuit$$anonfun$1(ReplDriver.scala:222)
at dotty.tools.repl.ReplDriver.withRedirectedOutput(ReplDriver.scala:256)
at dotty.tools.repl.ReplDriver.runBody$$anonfun$1(ReplDriver.scala:230)
at dotty.tools.runner.ScalaClassLoader$.asContext(ScalaClassLoader.scala:80)
at dotty.tools.repl.ReplDriver.runBody(ReplDriver.scala:230)
at dotty.tools.repl.ReplDriver.runUntilQuit(ReplDriver.scala:222)
at dotty.tools.repl.ReplDriver.tryRunning(ReplDriver.scala:159)
at dotty.tools.repl.Main$.main(Main.scala:7)
at dotty.tools.repl.Main.main(Main.scala)
Caused by: java.util.zip.ZipException: zip file is empty
at java.base/java.util.zip.ZipFile$Source.zerror(ZipFile.java:1769)
at java.base/java.util.zip.ZipFile$Source.findEND(ZipFile.java:1553)
at java.base/java.util.zip.ZipFile$Source.initCEN(ZipFile.java:1648)
at java.base/java.util.zip.ZipFile$Source.<init>(ZipFile.java:1486)
at java.base/java.util.zip.ZipFile$Source.get(ZipFile.java:1448)
at java.base/java.util.zip.ZipFile$CleanableResource.<init>(ZipFile.java:718)
at java.base/java.util.zip.ZipFile.<init>(ZipFile.java:252)
at java.base/java.util.zip.ZipFile.<init>(ZipFile.java:181)
at java.base/java.util.zip.ZipFile.<init>(ZipFile.java:195)
at dotty.tools.io.FileZipArchive.dotty$tools$io$FileZipArchive$$openZipFile(ZipArchive.scala:123)
... 17 moreHere, fakejar.jar is an empty file on the filesystem.
Ideally, this should not crash and we can continue using the REPL.
|
Perhaps we should MIME check that it is indeed a JAR file? |
This doesn't really solve the issue. I further tried two more things:
scala> :jar corrupt/HelloWorld.jar
Exception in thread "main" java.lang.IllegalArgumentException
at scala.tools.asm.ClassReader.<init>(ClassReader.java:263)
at scala.tools.asm.ClassReader.<init>(ClassReader.java:180)
at scala.tools.asm.ClassReader.<init>(ClassReader.java:166)
at scala.tools.asm.ClassReader.<init>(ClassReader.java:288)
at dotty.tools.repl.ReplDriver.tryClassLoad$1(ReplDriver.scala:535)
at dotty.tools.repl.ReplDriver.$anonfun$16(ReplDriver.scala:546)
at scala.collection.IterableOnceOps.find(IterableOnce.scala:678)
at scala.collection.IterableOnceOps.find$(IterableOnce.scala:674)
at scala.collection.AbstractIterator.find(Iterator.scala:1306)
at dotty.tools.repl.ReplDriver.interpretCommand(ReplDriver.scala:546)
at dotty.tools.repl.ReplDriver.interpret(ReplDriver.scala:308)
at dotty.tools.repl.ReplDriver.loop$1(ReplDriver.scala:219)
at dotty.tools.repl.ReplDriver.runUntilQuit$$anonfun$1(ReplDriver.scala:222)
at dotty.tools.repl.ReplDriver.withRedirectedOutput(ReplDriver.scala:256)
at dotty.tools.repl.ReplDriver.runBody$$anonfun$1(ReplDriver.scala:230)
at dotty.tools.runner.ScalaClassLoader$.asContext(ScalaClassLoader.scala:80)
at dotty.tools.repl.ReplDriver.runBody(ReplDriver.scala:230)
at dotty.tools.repl.ReplDriver.runUntilQuit(ReplDriver.scala:222)
at dotty.tools.repl.ReplDriver.tryRunning(ReplDriver.scala:159)
at dotty.tools.repl.Main$.main(Main.scala:7)
at dotty.tools.repl.Main.main(Main.scala)I think at the very least, we should have somewhere a catch-all to prevent exceptions from crashing the REPL. About signatures, I guess we should allow unsigned jars. But do we want to optionally check signatures? That could be addressed in a follow-up issue after the cutoff. |
should plan that as a follow-up issue IMO
We can have a catch-all to stop the REPL from crashing, but still perform checks (like MIME) to properly warn where the problem is. |
|
In the interest of making it before the cutoff, I suggest to just implement the catch-all now and look into MIME and signatures afterwards. |
Co-authored-by: Piotr Chabelski <ged.subfan@gmail.com>
|
Added a catch-all around :jar for now. |
odersky
left a comment
There was a problem hiding this comment.
I only could do a superficial review and am relying on a deeper review by others.
From what I could see, the code looks good to me,
#21658