Cross JDK forking #4139

eed3si9n · 2018-05-03T04:39:16Z

This is a refactoring/continuation of #4136 by @2m reflecting review comments by @dwijnand

sbt:helloworld> java++ 10
[info] Reapplying settings...
sbt:helloworld> run
[info] Running (fork) Hello
[info] 10.0.1

sbt:helloworld> java++ 8
[info] Reapplying settings...

sbt:helloworld> run
[info] Running (fork) Hello
[info] 1.8.0_171

original description

This PR proposes a discovery for Java home directories implemented by scanning well known system directories for available java installations. Currently this PR only addresses Linux and MacOS. // @cunei

This PR is a first step towards cross building across different Java versions.

Discovering Java home directories is also useful when a project needs to locate rt.jar of a different java installation (for example when working around -release 8 limitaions in JDK9)

eed3si9n · 2018-05-03T04:40:16Z

main/src/main/contraband/main.contra

@@ -17,3 +17,9 @@ enum PluginTrigger {
  AllRequirements
  NoTrigger
 }
+
+type JavaVersion {


Added pseudo case class for JavaVersion, instead of using String.

eed3si9n · 2018-05-03T04:40:42Z

main/src/main/scala/sbt/Keys.scala

@@ -268,6 +268,10 @@ object Keys {
  val outputStrategy = settingKey[Option[sbt.OutputStrategy]]("Selects how to log output when running a main class.").withRank(DSetting)
  val connectInput = settingKey[Boolean]("If true, connects standard input when running a main class forked.").withRank(CSetting)
  val javaHome = settingKey[Option[File]]("Selects the Java installation used for compiling and forking.  If None, uses the Java installation running the build.").withRank(ASetting)
+  val discoveredJavaHomes = settingKey[Map[JavaVersion, File]]("Discovered Java home directories")
+  val javaHomes = settingKey[Map[JavaVersion, File]]("The user-defined additional Java home directories")


Custom key is named javaHomes.

eed3si9n · 2018-05-03T04:41:36Z

main/src/main/scala/sbt/Keys.scala

@@ -268,6 +268,10 @@ object Keys {
  val outputStrategy = settingKey[Option[sbt.OutputStrategy]]("Selects how to log output when running a main class.").withRank(DSetting)
  val connectInput = settingKey[Boolean]("If true, connects standard input when running a main class forked.").withRank(CSetting)
  val javaHome = settingKey[Option[File]]("Selects the Java installation used for compiling and forking.  If None, uses the Java installation running the build.").withRank(ASetting)
+  val discoveredJavaHomes = settingKey[Map[JavaVersion, File]]("Discovered Java home directories")
+  val javaHomes = settingKey[Map[JavaVersion, File]]("The user-defined additional Java home directories")
+  val fullJavaHomes = taskKey[Map[JavaVersion, File]]("Combines discoveredJavaHomes and custom javaHomes.").withRank(CTask)


fullJavaHomes is a task if someone wants to side effect. (Credit to Dale)

I had to switch this back to a setting since carrying the state around aggregation is pain in the ass when implementing a command.

dwijnand · 2018-05-03T06:41:18Z

I've a yak to shave about JavaVersion.. 😕

Short-term solution: can we make it an sbt.internal type, so we can iterate on it?

Long-term solution: one idea that I was thinking was to experiment modelling "java version" using a sum type to give a selection like jenv, where when I added Oracle JDK 1.8.0.144 I got the following selection of identifiers to choose from:

1.8
1.8.0.144
oracle64-1.8.0.144

So the usual pitfalls need to be avoids:

can't make JavaVersion a sealed trait -> means we can't add new types
can't give JavaVersion or its subtypes unapply

So perhaps a starting point would be to make JavaVersion a public package sbt type, add different constructors on JavaVersion (maybe apply takes "1.8", update takes "1.8.0.144", full takes "oracle64-1.8.0.144") that just return opaque JavaVersion. But that's the creation side, I'm unsure about at the usage side. 🤔

dwijnand · 2018-05-03T07:01:33Z

I'm overthinking it. I'm designing for use cases I don't quite have clear.

Which makes me realise: what are the motivating use cases we're looking to solve with this change? Can we explore that space before committing to a stable public API?

/cc @2m who I assume can provide the Akka use cases

eed3si9n · 2018-05-03T07:45:07Z

Implemented java++ 10

eed3si9n · 2018-05-03T07:51:33Z

@dwijnand We can iterate by changing the implementation of JavaVersion.apply(String).

dwijnand · 2018-05-03T08:04:44Z

Can we remove its other companion object applies and methods from the public API too?

eed3si9n · 2018-05-03T08:14:03Z

I think what you're talking about can be achieved by making bunch of duplicate entries in discoveredJavaHomes like e6fa634 or jEnv without being too paranoid about JavaVersion.

dwijnand · 2018-05-03T08:19:13Z

Can we discuss the use cases before committing to new public API?

dwijnand · 2018-05-03T09:06:57Z

Can we discuss the use cases before committing to new public API?

Or, let's just make JavaVersion sbt.internal and cross that bridge another day.

2m · 2018-05-03T10:01:57Z

We have a couple of use-cases in Akka:

We would use the discovery of JavaHomes to locate the rt.jar of an older version. Currently the -release flag is broken in JDK9 where it does not allow acceess to Unsafe when compiling with JDK9 and with -release 8 flag. The workaround is to compile with "-source", "8", "-target", "8", "-bootclasspath", "/usr/lib/jvm/java-8-openjdk/jre/lib/rt.jar" javacFlags for which we need JavaHome discovery.
Cross testing with JDK8. Since we build with JDK9, we would like to test the compiled classes running Java 8 to be sure that we did not do something that is JDK8 incompatible.

eed3si9n · 2018-05-03T19:02:22Z

Given the uses cases, the minor version and vendor handling of JDK variety should be considered out of scope of this PR.

Also given that JavaVersion("10") -> file(...) needs to go into build.sbt, we do need to keep bincompat, and I see no reason to put that into internal.

eed3si9n · 2018-05-04T08:24:50Z

sbt/src/sbt-test/java/cross/test

+
+> java++ 10!
+> run
+> check 10


Cross JDK testing using jabba installed openjdk 10.

dwijnand · 2018-05-04T09:21:46Z

Also given that JavaVersion("10") -> file(...) needs to go into build.sbt, we do need to keep bincompat, and I see no reason to put that into internal.

We don't need to keep bincompat if we declare the whole feature experimental, which is what I'm proposing.

eed3si9n · 2018-05-04T23:26:09Z

I was thinking how we would document this feature, and realized that we shouldn't expose JavaVersion to javaHomes key because of the compatibility with sbt 1.0, assuming we want to support javaHomes in user-defined global settings.

Keys should use String, but
on the other hand, JavaVersion can now be internal

2m

LGTM. Looking good. The cross building functionality is getting more and more ripe for factoring out to a more general behaviour.

``` sbt:helloworld> java++ 10 [info] Reapplying settings... sbt:helloworld> run [info] Running (fork) Hello [info] 10.0.1 sbt:helloworld> java++ 8 [info] Reapplying settings... sbt:helloworld> run [info] Running (fork) Hello [info] 1.8.0_171 ```

Ref shyiko/jabba#190 Bumping to jabba 0.9.6 fixes sporaditc permission issues.

https://github.com/shyiko/jabba/blob/master/CHANGELOG.md#0101---2018-05-07 Ref shyiko/jabba#190

chbatey · 2018-07-03T07:39:29Z

main/src/main/scala/sbt/internal/CrossJava.scala

+  object JavaDiscoverConfig {
+    val linux = new JavaDiscoverConf {
+      val base: File = file("/usr") / "lib" / "jvm"
+      val JavaHomeDir = """java-([0-9]+)-.*""".r


FYI this regex won't match on a default Fedora Open JDK install. See akka/akka#25298

Ref scala/bug#10831 Ref sbt#4139 (comment)

eed3si9n commented May 3, 2018

View reviewed changes

eed3si9n requested a review from dwijnand May 3, 2018 05:00

eed3si9n changed the title ~~Re: Discovery of java home directories~~ Cross JDK forking May 3, 2018

2m mentioned this pull request May 3, 2018

Discovery of java home directories #4136

Closed

1 task

eed3si9n force-pushed the wip/discover-java-home branch 5 times, most recently from 0478546 to c165d6b Compare May 4, 2018 08:21

eed3si9n commented May 4, 2018

View reviewed changes

sbt/src/sbt-test/java/cross/test

> java++ 10!

> run

> check 10

Copy link

Member Author

eed3si9n May 4, 2018

Choose a reason for hiding this comment

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

Cross JDK testing using jabba installed openjdk 10.

eed3si9n changed the base branch from 1.1.x to 1.x May 4, 2018 08:26

eed3si9n requested a review from cunei May 4, 2018 16:51

eed3si9n force-pushed the wip/discover-java-home branch 2 times, most recently from da4a6ca to fa74f85 Compare May 7, 2018 07:03

eed3si9n mentioned this pull request May 8, 2018

Add semantic version selector API sbt/librarymanagement#239

Merged

eed3si9n force-pushed the wip/discover-java-home branch from 1fed9d8 to ef09829 Compare May 9, 2018 07:47

2m approved these changes May 9, 2018

View reviewed changes

2m and others added 8 commits May 30, 2018 00:16

Discovery of java homes

c31583e

Accept both 1.x and x for 1.1 to 1.8

aff9e01

implement cross JDK forking

2da1aa6

``` sbt:helloworld> java++ 10 [info] Reapplying settings... sbt:helloworld> run [info] Running (fork) Hello [info] 10.0.1 sbt:helloworld> java++ 8 [info] Reapplying settings... sbt:helloworld> run [info] Running (fork) Hello [info] 1.8.0_171 ```

jabba 0.9.6 (no sudo)

951eaa6

Ref shyiko/jabba#190 Bumping to jabba 0.9.6 fixes sporaditc permission issues.

Adjust to upstream change

35e98f5

Formatting

a7d85c8

jabba 0.10.1

72ebdeb

https://github.com/shyiko/jabba/blob/master/CHANGELOG.md#0101---2018-05-07 Ref shyiko/jabba#190

use stringly-typed key so we can define it machine-wide

9b7c224

eed3si9n force-pushed the wip/discover-java-home branch from ef09829 to 9b7c224 Compare May 30, 2018 04:59

eed3si9n merged commit 2848770 into sbt:1.x May 30, 2018

eed3si9n deleted the wip/discover-java-home branch May 30, 2018 17:45

eed3si9n mentioned this pull request Jul 1, 2018

Find out what we can do to release for jdk6 instead of jdk7 scala/scala-collection-compat#77

Closed

chbatey reviewed Jul 3, 2018

View reviewed changes

eed3si9n added a commit to eed3si9n/sbt that referenced this pull request Jul 14, 2018

Fixes contains bug

439e67c

Ref scala/bug#10831 Ref sbt#4139 (comment)

eed3si9n mentioned this pull request Jul 14, 2018

Fixes contains bug #4264

Merged

eed3si9n added the area/jdk_x jdk 9, 10, 11, 17 etc label Aug 21, 2018

eed3si9n mentioned this pull request Mar 26, 2019

detection of SDKMAN managed JDKs #4580

Merged

mkurz mentioned this pull request Apr 9, 2025

Remove unused files playframework/playframework#13239

Merged

Cross JDK forking #4139

Cross JDK forking #4139

Uh oh!

Conversation

eed3si9n commented May 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

original description

Uh oh!

eed3si9n May 3, 2018

Choose a reason for hiding this comment

Uh oh!

eed3si9n May 3, 2018

Choose a reason for hiding this comment

Uh oh!

eed3si9n May 3, 2018

Choose a reason for hiding this comment

Uh oh!

eed3si9n May 3, 2018

Choose a reason for hiding this comment

Uh oh!

dwijnand commented May 3, 2018

Uh oh!

dwijnand commented May 3, 2018

Uh oh!

eed3si9n commented May 3, 2018

Uh oh!

eed3si9n commented May 3, 2018

Uh oh!

dwijnand commented May 3, 2018

Uh oh!

eed3si9n commented May 3, 2018

Uh oh!

dwijnand commented May 3, 2018

Uh oh!

dwijnand commented May 3, 2018

Uh oh!

2m commented May 3, 2018

Uh oh!

eed3si9n commented May 3, 2018

Uh oh!

eed3si9n May 4, 2018

Choose a reason for hiding this comment

Uh oh!

dwijnand commented May 4, 2018

Uh oh!

eed3si9n commented May 4, 2018

Uh oh!

2m left a comment

Choose a reason for hiding this comment

Uh oh!

chbatey Jul 3, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eed3si9n commented May 3, 2018 •

edited

Loading