Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix executable jars #13263

Merged
merged 7 commits into from
Aug 16, 2021
Merged

Fix executable jars #13263

merged 7 commits into from
Aug 16, 2021

Conversation

BarkingBad
Copy link
Contributor

Coming back with fixes for the compiler to produce runnable jars.

@som-snytt, you can play with the compiler now, for example, you can try these commands

$ bin/scalac -d hi.jar compiler/test-coursier/run/myfile.scala
$ bin/scala hi.jar
Hello
$ bin/scalac -d hi2.jar -Xmain-class run.myfile compiler/test-coursier/run/myfile.scala
$ bin/scala hi2.jar
Hello

Unfortunately, there is some hacking:

  1. We have to manually write the MANIFEST file. Currently, to write to jars we create the custom object JarArchive which sets up a new file system for writing files, then we write to it directly both bytecode and tasty files. JDK gives an interface called JarOutputStream which is convenient to produce Jar files and takes care of the creation of the manifest file, but with our current approach, it's hard to determine the manifest properties. We would have to store these files temporarily somewhere else then write them all using JarOutputStream (scala 2 has this pipeline afaik).
  2. The above hack forces us to find the MANIFEST file manually, because JDK assumes that the MANIFEST file should be the very first entry in the Jar file (https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/jar/JarInputStream.java#L74). Interesting fact is that it only is required for reading jars using JarInputStream, when we try to run this jar using java -jar ... it would start normally, even though MANIFEST is at the end of the archive.

Thigs to reconsider:

  • attaching classpath to MANIFEST (or at least scala stdlib) so user can run it from java -jar ... command (running from bin/scala provides stdlib by default, so it is not problem for most of the simple scripts).

Comment on lines 79 to 101
val mainClass = Option.when(!ctx.settings.XmainClass.isDefault)(ctx.settings.XmainClass.value).orElse {
val _mainClassesBuffer = new mutable.HashSet[String]
units.map { unit =>
unit.tpdTree.foreachSubTree { tree =>
val sym = tree.symbol
import dotty.tools.dotc.core.NameOps.stripModuleClassSuffix
val name = sym.fullName.stripModuleClassSuffix.toString
if (sym.isStatic && !sym.is(Flags.Trait) && ctx.platform.hasMainMethod(sym)) {
// If sym is an object, all main methods count, otherwise only @static ones count.
_mainClassesBuffer += name
}
}
}
_mainClassesBuffer.toList.match
case List(mainClass) =>
Some(mainClass)
case Nil =>
report.warning("No Main-Class designated or discovered.")
None
case mcs =>
report.warning(s"No Main-Class due to multiple entry points:\n ${mcs.mkString("\n ")}")
None
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is copied from compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala I was thinking whether we could extract it for reuse rather than copy-paste, yet I wasn't sure where should I place it, maybe somewhere under dotty.tools.dotc.util

Copy link
Member

@smarter smarter Aug 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please feel free to extract any code for reuse yes, doesn't matter too much where it ends up :). However, having to fully traverse every single compilation unit just to get main method names out is going to be fairly expensive and should be avoided, especially for information that we should already know. Maybe ExtractAPI (or some other phase which is always run if we want this to work even outside of sbt) should collect this information and store it in the Context. In fact, we used to have a phase which was supposed to do that but I deleted it because it was dead code: 3ebf919

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the advice, I will look into that :D

@BarkingBad BarkingBad linked an issue Aug 6, 2021 that may be closed by this pull request
@BarkingBad
Copy link
Contributor Author

I recommend using an immutable collection, otherwise one needs to be very careful about not reusing the same EntryPoints instance in multiple contexts.

What is the correct way to update such context then or to pass fresh-cloned context with the updated state to the next phases?

@smarter
Copy link
Member

smarter commented Aug 10, 2021

You can use updateStore to replace the value of a Location in the Context. But if you move CollectEntryPoints to be just before the backend and only used by it, then you could also put the state in GenBCode like CollectSuperCalls does: https://github.com/lampepfl/dotty/blob/38b983c1b4a61d4c45f314ae895b95317c322c62/compiler/src/dotty/tools/backend/jvm/CollectSuperCalls.scala#L38

@BarkingBad
Copy link
Contributor Author

I've seen updateStore, but it's only for FreshContext and I didn't dare to type check/cast the context and then invoke the updating function. The second solution like the CollectSuperCalls seems OK, though I just fixed myself on putting that into the context as you proposed earlier. Will change

@BarkingBad
Copy link
Contributor Author

@smarter could you take a final look and give approve if everything is OK :)

* Small phase to be run to collect main classes and store them in the context.
* The general rule to run this phase is either:
* - The output of compilation is JarArchive and there is no `-Xmain-class` defined
* - The compiler is run from sbt and is forced by flags forcing `ExtractorAPI`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this should be removed along with any code related to sbt in this phase since it no longer occurs before ExtractAPI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@BarkingBad BarkingBad merged commit 4ed3ba5 into scala:master Aug 16, 2021
@Kordyjan Kordyjan added this to the 3.1.0 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scala runner doesn't do executable jars
3 participants