-
Notifications
You must be signed in to change notification settings - Fork 129
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
Build scala-cli with jvm 11 #212
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to make sure that the runner
, test-runner
, and stub
modules are still fine with Java 8, by passing them the right javac and scalac options (see this SO question, I think we need both its javac and scalac options). These are the modules that are sometimes added to the user class path by Scala CLI.
To be sure it works, we can add a simple test in RunTestDefinitions
say, that passes --jvm 8
to scala-cli
.
Wouldn't upgrading to latest LTS (JDK 17) make sense? Afaik this is quite a problematic release as oracle decided to encapsulate some of its internals therefore some tooling may not be compatible. So if it works on JDK 17 it will probably run on older JDK-s |
For now, we're using the Java 11 version of GraalVM to build the native launcher, so switching to Java 11 isn't a problem. Maybe the upcoming GraalVM version will have Java 17 versions, and allow to switch to Java 17 then. |
ca804c7
to
1866f29
Compare
1ddd820
to
99b932d
Compare
@@ -214,7 +214,7 @@ class NativePackagerTests extends munit.FunSuite { | |||
// format: off | |||
val cmd = Seq[os.Shellable]( | |||
TestUtil.cli, | |||
"package", helloWorldFileName, | |||
"package", "--jvm", "8", helloWorldFileName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should just change the base image in Package.scala
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, but this does not fully resolve the problem. I opened an issue #413
modules/integration/src/test/scala/scala/cli/integration/TestTestDefinitions.scala
Show resolved
Hide resolved
website/docs/commands/package.md
Outdated
@@ -114,7 +114,7 @@ The docker image name parameter `--docker-image-repository` is mandatory. | |||
The following command generates a `hello-docker` image with the `latest` tag: | |||
|
|||
```bash | |||
scala-cli package --docker HelloDocker.scala --docker-image-repository hello-docker | |||
scala-cli package --jvm 8 --docker HelloDocker.scala --docker-image-repository hello-docker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, this shouldn't be needed with a change of base image.
f52bc6b
to
4284c72
Compare
Add to `stubs` Do not add to `TastyLib`
4284c72
to
6a9a45a
Compare
Let's check what happens