Discovery of java home directories #4136

2m · 2018-05-01T07:09:18Z

I've read the CONTRIBUTING guidelines

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)

dwijnand

Cool. Are these the initial pieces of your hackathon in Lisbon?

I left a few comments. Some less important than others.. 🙂

dwijnand · 2018-05-01T07:34:53Z

main/src/main/scala/sbt/Defaults.scala

+  private final val JavaDiscoverConfigs = Seq(
+    JavaDiscoverConf("/usr/lib/jvm", "java-([0-9]+)-.*".r, ""), // most linux
+    JavaDiscoverConf("/Library/Java/JavaVirtualMachines",
+                     "jdk[1\\.]*([0-9]+)\\..*".r,


Doesn't this match, for instance, /Library/Java/JavaVirtualMachines/jdk-10.jdk/?

Good catch. I'll change it to jdk(1\.|-)?([0-9]+)\..* and extract the second group.

dwijnand · 2018-05-01T07:36:48Z

main/src/main/scala/sbt/Defaults.scala

+            .collect {
+              case dir @ dirRexp(ver) => ver -> Paths.get(root, dir, inner).toFile
+            }
+      }(breakOut)


With breakOut making an exit in Scala 2.13, best implement this with .iterator (after JavaDiscoverConfigs) and a final .toMap

dwijnand · 2018-05-01T07:45:32Z

main/src/main/scala/sbt/Defaults.scala

+        case JavaDiscoverConf(root, dirRexp, inner) =>
+          Option(new File(root).list())
+            .getOrElse(Array.empty[String])
+            .toSeq


Needn't be a seq, it can just be an iterator, with .iterator

dwijnand · 2018-05-01T07:48:09Z

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

@@ -268,6 +268,8 @@ 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[String, File]]("Discovered Java home directories")
+  val additionalJavaHomes = settingKey[Map[String, File]]("Additional Java home directories")


Maybe call this just javaHomes, like resolvers?

dwijnand · 2018-05-01T07:50:37Z

main/src/main/scala/sbt/Defaults.scala

@@ -154,6 +161,8 @@ object Defaults extends BuildCommon {
      scalaHome :== None,
      apiURL := None,
      javaHome :== None,
+      discoveredJavaHomes := discoverJavaHomes.value ++ additionalJavaHomes.value,


This definition isn't intuitive to me. Discovered should only be discovered.

Maybe there should be a fullJavaHomes (like fullResolvers and fullClasspath) with this RHS.

Sounds good. Then fullJavaHomes = discoveredJavaHomes + javaHomes.

My vote is fullJavaHomes = discoveredJavaHomes + unmanagedJavaHomes as we had in Lisbon.

The reason is because the build user might want to either:

append to the default list (like resolvers) or

take over the entire mapping.

Note that for resolvers, we offer an entirely different mechanism ~/.sbt/repositories.

Why unmanagedJavaHomes over just javaHomes?

And if you prefer unmanagedJavaHomes, why discoveredJavaHomes instead of managedJavaHomes then?

Because javaHomes is not clear if it's the unmanaged part of the full.

javaHomes := Map("10" -> something)

I am ok with either discoveredJavaHomes or managedJavaHomes since the build user won't be interacting with them that much.

if you had to change it, that means sbt failed to detect some JDK homes automatically, and you are augmenting it with your machine-dependent info.

I see it as the same as resolvers. You're adding not-so-common custom java homes paths.

Also, like resolvers, it could have "doesn't work for everyone" concerns (e.g company-specific artefact repository URLs and company-specific JVM installation paths).

resolvers typically go into build.sbt whereas your Global / (unmanaged)JavaHomes should go into user-defined global settings; never in build.sbt.

The users almost never have to debug fullResolvers whereas if JDK cross building doesn't work, the users will need to type show Global/fullJavaHomes to debug why it wasn't detected. This is analogous to managed vs unmanaged classpath or source. If there's a key named javaHomes already, wouldn't that be more confusing?

If we really want to use the name javaHomes, I would say we should get rid of fullJavaHomes, and just use javaHomes key where the user can just += into it.

resolvers typically go into build.sbt whereas your Global / (unmanaged)JavaHomes should go into user-defined global settings; never in build.sbt.

I can see both. For instance if you're in a company that uses a custom JDK/JVM installed (e.g at Twitter with GraalVM) in a specific but not-so-common path I can see it being experimented with at a project level, maybe eventually being defined in a company sbt plugin. Not necessarily in user-wide global settings.

And even if it were meant for user-wide global settings, why shouldn't it still be called javaHomes?

whereas if JDK cross building doesn't work, the users will need to type show Global/fullJavaHomes to debug why it wasn't detected.

To debug what was discovered they would look at discoveredJavaHomes.

If we really want to use the name javaHomes, I would say we should get rid of fullJavaHomes, and just use javaHomes key where the user can just += into it.

I don't follow, but I think it's non sequitur from my earlier comments.

We discussed in sbt 1 meeting today. I'll take this PR and make the edits.

2m · 2018-05-01T08:53:30Z

Yes, this is from Lisbon and is a prequel to cross java home building. However it looks like this might be useful on its own as well.

eed3si9n · 2018-05-01T14:23:32Z

main/src/main/scala/sbt/Defaults.scala

@@ -1625,6 +1634,28 @@ object Defaults extends BuildCommon {
          else old
        }
      ))
+
+  private case class JavaDiscoverConf(root: String, dirRexp: Regex, innerDir: String)


These should move to CrossJava.scala, I think.

2m · 2018-05-03T10:02:17Z

Cntd. in #4139

Discovery of java homes

d0f4bbe

eed3si9n added the ready label May 1, 2018

2m mentioned this pull request May 1, 2018

Release with jdk9 akka/akka#23789

Closed

dwijnand reviewed May 1, 2018

View reviewed changes

eed3si9n reviewed May 1, 2018

View reviewed changes

eed3si9n mentioned this pull request May 3, 2018

Cross JDK forking #4139

Merged

2m closed this May 3, 2018

Discovery of java home directories #4136

Discovery of java home directories #4136

Uh oh!

Conversation

2m commented May 1, 2018

Uh oh!

dwijnand left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

2m commented May 1, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

2m commented May 3, 2018

Uh oh!

Uh oh!

eed3si9n May 1, 2018 •

edited

Loading