Merge pull request #464 from sourcegraph/olafurpg/java17

Include missing `--add-exports` flags for Java 9+ Gradle builds

.github/workflows/ci.yml

@@ -6,17 +6,19 @@ on:
   pull_request:
 jobs:
   test:
-    runs-on:  ${{ matrix.os }}
+    runs-on: ${{ matrix.os }}
     strategy:
+      fail-fast: false
       matrix:
         # NOTE(olafurpg) Windows is not enabled because it times out due to reasons I don't understand.
         # os: [windows-latest, ubuntu-latest]
         os: [ubuntu-latest]
+        java: [8, 17]
     steps:
       - uses: actions/checkout@v2
       - uses: actions/setup-java@v1
         with:
-          java-version: 8
+          java-version: ${{ matrix.java }}
       - uses: actions/setup-go@v2
         with:
           go-version: "^1.13.1"

scip-java/src/main/scala/com/sourcegraph/scip_java/Embedded.scala

@@ -5,6 +5,7 @@ import java.nio.file.Files
 import java.nio.file.Path
 import java.nio.file.StandardCopyOption
 
+import com.sourcegraph.scip_java.BuildInfo
 import moped.reporters.Reporter
 import os.CommandResult
 
@@ -22,7 +23,12 @@ object Embedded {
 
   private def javacErrorpath(tmp: Path) = tmp.resolve("errorpath.txt")
 
-  def customJavac(sourceroot: Path, targetroot: Path, tmp: Path): Path = {
+  def customJavac(
+      sourceroot: Path,
+      targetroot: Path,
+      tmp: Path,
+      javaAtLeast17: Boolean
+  ): Path = {
     val bin = tmp.resolve("bin")
     val javac = bin.resolve("javac")
     val java = bin.resolve("java")
@@ -38,6 +44,11 @@ object Embedded {
         |""".stripMargin.getBytes(StandardCharsets.UTF_8)
     )
     val newJavacopts = tmp.resolve("javac_newarguments")
+    val javacModuleOptions =
+      if (javaAtLeast17)
+        BuildInfo.javacModuleOptions.mkString(" ")
+      else
+        ""
     val injectSemanticdbArguments = List[String](
       "java",
       s"-Dsemanticdb.errorpath=$errorpath",
@@ -62,9 +73,9 @@ object Embedded {
          |done
          |$injectSemanticdbArguments
          |if [ $${#LAUNCHER_ARGS[@]} -eq 0 ]; then
-         |  javac "@$$NEW_JAVAC_OPTS"
+         |  javac $javacModuleOptions "@$$NEW_JAVAC_OPTS"
          |else
-         |  javac "@$$NEW_JAVAC_OPTS" "$${LAUNCHER_ARGS[@]}"
+         |  javac $javacModuleOptions "@$$NEW_JAVAC_OPTS" "$${LAUNCHER_ARGS[@]}"
          |fi
          |""".stripMargin
     Files.write(javac, script.getBytes(StandardCharsets.UTF_8))

scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/GradleJavaCompiler.scala

@@ -27,7 +27,12 @@ case class GradleJavaCompiler(languageVersion: String, javacPath: Path) {
       pluginPath: Path
   ): Path = {
     val home = tmp.resolve(s"1.$languageVersion")
-    val javac = Embedded.customJavac(index.workingDirectory, targetroot, tmp)
+    val javac = Embedded.customJavac(
+      index.workingDirectory,
+      targetroot,
+      tmp,
+      GradleJavaToolchains.isJavaAtLeast(languageVersion, "17")
+    )
     val agent = Embedded.agentJar(tmp)
     val debugPath = GradleJavaCompiler.debugPath(tmp)
     val javacopts = targetroot.resolve("javacopts.txt")

scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/GradleJavaToolchains.scala

@@ -4,6 +4,7 @@ import java.nio.charset.StandardCharsets
 import java.nio.file.Files
 import java.nio.file.Path
 
+import scala.annotation.tailrec
 import scala.jdk.CollectionConverters._
 
 import com.sourcegraph.scip_java.Embedded
@@ -14,6 +15,7 @@ case class GradleJavaToolchains(
     tool: GradleBuildTool,
     index: IndexCommand,
     gradleVersion: Option[String],
+    javaVersion: Option[String],
     isJavaEnabled: Boolean,
     isScalaEnabled: Boolean,
     isKotlinEnabled: Boolean,
@@ -24,9 +26,22 @@ case class GradleJavaToolchains(
 
   def isEmpty: Boolean = toolchains.isEmpty
 
+  def isJavaAtLeast(version: Int): Boolean = {
+    val actualVersion = javaVersion.getOrElse(sys.props("java.version"))
+    GradleJavaToolchains
+      .isJavaAtLeast(actualVersion, math.max(version, 0).toString())
+  }
+
   def executableJavacPath(): Option[Path] = {
     if (toolchains.isEmpty) {
-      Some(Embedded.customJavac(index.workingDirectory, tool.targetroot, tmp))
+      Some(
+        Embedded.customJavac(
+          index.workingDirectory,
+          tool.targetroot,
+          tmp,
+          isJavaAtLeast(17)
+        )
+      )
     } else {
       None
     }
@@ -62,6 +77,7 @@ object GradleJavaToolchains {
     val javaEnabledPath = tmp.resolve("java-enabled.txt")
     val scalaEnabledPath = tmp.resolve("scala-enabled.txt")
     val kotlinEnabledPath = tmp.resolve("kotlin-enabled.txt")
+    val javaVersionPath = tmp.resolve("java-version.txt")
     val kotlinMultiplatformEnabledPath = tmp
       .resolve("kotlin-multiplatform-enabled.txt")
     val gradleVersionPath = tmp.resolve("gradle-version.txt")
@@ -75,6 +91,12 @@ object GradleJavaToolchains {
           |    [gradle.gradleVersion],
           |    java.nio.file.StandardOpenOption.TRUNCATE_EXISTING,
           |    java.nio.file.StandardOpenOption.CREATE)
+          |  java.nio.file.Files.write(
+          |    java.nio.file.Paths.get(
+          |      java.net.URI.create('${javaVersionPath.toUri}')),
+          |    [System.getProperty("java.version")],
+          |    java.nio.file.StandardOpenOption.TRUNCATE_EXISTING,
+          |    java.nio.file.StandardOpenOption.CREATE)
           |} catch (Exception e) {
           |  // Ignore errors.
           |}
@@ -146,11 +168,19 @@ object GradleJavaToolchains {
         None
       }
 
+    val javaVersion =
+      if (Files.isRegularFile(javaVersionPath)) {
+        Some(new String(Files.readAllBytes(javaVersionPath)).trim)
+      } else {
+        None
+      }
+
     GradleJavaToolchains(
       toolchains,
       tool,
       index,
       gradleVersion = gradleVersion,
+      javaVersion = javaVersion,
       isJavaEnabled = Files.isRegularFile(javaEnabledPath),
       isScalaEnabled = Files.isRegularFile(scalaEnabledPath),
       isKotlinEnabled = Files.isRegularFile(kotlinEnabledPath),
@@ -160,4 +190,68 @@ object GradleJavaToolchains {
       tmp = tmp
     )
   }
+
+  // Copy-pasted from scala.util.Properties.isJavaAtLeast but makes the actual
+  // version a parameterizeable instead of being hard-coded to
+  // `Properties.javaVersionSpec`.
+  def isJavaAtLeast(
+      actualVersion: String,
+      comparisonVersion: String
+  ): Boolean = {
+    def versionOf(s: String, depth: Int): (Int, String) =
+      s.indexOf('.') match {
+        case 0 =>
+          (-2, s.substring(1))
+        case 1 if depth == 0 && s.charAt(0) == '1' =>
+          val r0 = s.substring(2)
+          val (v, r) = versionOf(r0, 1)
+          val n =
+            if (v > 8 || r0.isEmpty)
+              -2
+            else
+              v // accept 1.8, not 1.9 or 1.
+          (n, r)
+        case -1 =>
+          val n =
+            if (!s.isEmpty)
+              s.toInt
+            else if (depth == 0)
+              -2
+            else
+              0
+          (n, "")
+        case i =>
+          val r = s.substring(i + 1)
+          val n =
+            if (depth < 2 && r.isEmpty)
+              -2
+            else
+              s.substring(0, i).toInt
+          (n, r)
+      }
+    @tailrec
+    def compareVersions(s: String, v: String, depth: Int): Int = {
+      if (depth >= 3)
+        0
+      else {
+        val (sn, srest) = versionOf(s, depth)
+        val (vn, vrest) = versionOf(v, depth)
+        if (vn < 0)
+          -2
+        else if (sn < vn)
+          -1
+        else if (sn > vn)
+          1
+        else
+          compareVersions(srest, vrest, depth + 1)
+      }
+    }
+
+    compareVersions(actualVersion, comparisonVersion, 0) match {
+      case -2 =>
+        throw new NumberFormatException(s"Not a version: $comparisonVersion")
+      case i =>
+        i >= 0
+    }
+  }
 }

scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/MavenBuildTool.scala

@@ -35,11 +35,13 @@ class MavenBuildTool(index: IndexCommand) extends BuildTool("Maven", index) {
         else {
           "mvn"
         }
+      val start = System.nanoTime()
       val buildCommand = ListBuffer.empty[String]
       val executable = Embedded.customJavac(
         index.workingDirectory,
         index.finalTargetroot(defaultTargetroot),
-        tmp
+        tmp,
+        GradleJavaToolchains.isJavaAtLeast(SystemJavaVersion.detect(), "11")
       )
       buildCommand ++=
         List(

scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/SystemJavaVersion.scala

@@ -0,0 +1,30 @@
+package com.sourcegraph.scip_java.buildtools
+
+import java.nio.file.Files
+
+import com.sourcegraph.scip_java.Embedded
+
+object SystemJavaVersion {
+  // Returns the output of `System.getProperty("java.version")` from a fresh JVM
+  // instance using the system JVM installation. We can't use this process since
+  // it may be running on a separate JVM process (that's the case when we run
+  // `sbt test` in this build at least).
+  def detect(): String = {
+    val tmp = Files.createTempDirectory("java-version")
+    val jar = Embedded.semanticdbJar(tmp)
+    try {
+      os.proc(
+          "java",
+          "-cp",
+          jar.toString(),
+          "com.sourcegraph.semanticdb_javac.PrintJavaVersion"
+        )
+        .call()
+        .out
+        .text()
+        .trim()
+    } finally {
+      Files.deleteIfExists(jar)
+    }
+  }
+}

semanticdb-javac/src/main/java/com/sourcegraph/semanticdb_javac/PrintJavaVersion.java

@@ -0,0 +1,7 @@
+package com.sourcegraph.semanticdb_javac;
+
+public class PrintJavaVersion {
+  public static void main(String[] args) {
+    System.out.print(System.getProperty("java.version"));
+  }
+}

tests/buildTools/src/test/scala/tests/BaseBuildToolSuite.scala

@@ -18,6 +18,8 @@ import munit.Tag
 import munit.TestOptions
 import os.Shellable
 
+object Java8Only extends munit.Tag("Java8Only")
+
 abstract class BaseBuildToolSuite extends MopedSuite(ScipJava.app) {
   override def environmentVariables: Map[String, String] = sys.env
 
@@ -26,6 +28,14 @@ abstract class BaseBuildToolSuite extends MopedSuite(ScipJava.app) {
   override def munitTestTransforms: List[TestTransform] =
     super.munitTestTransforms ++
       List(
+        new TestTransform(
+          "Java8Only",
+          t =>
+            if (Properties.isJavaAtLeast(9) && t.tags(Java8Only))
+              t.tag(munit.Ignore)
+            else
+              t
+        ),
         new TestTransform(
           "SkipWindows",
           t =>

tests/buildTools/src/test/scala/tests/GradleBuildToolSuite.scala

@@ -27,7 +27,10 @@ class GradleBuildToolSuite extends BaseBuildToolSuite {
   List("latest" -> "implementation", "4.0" -> "compile").foreach {
     case (version, config) =>
       checkBuild(
-        s"basic-$version",
+        if (version == "latest")
+          "basic-latest"
+        else
+          s"basic-$version".tag(Java8Only),
         s"""|/build.gradle
             |apply plugin: 'java'
             |repositories {
@@ -55,7 +58,7 @@ class GradleBuildToolSuite extends BaseBuildToolSuite {
 
   List("3.3", "2.2.1").foreach { version =>
     checkBuild(
-      s"legacy-$version",
+      s"legacy-$version".tag(Java8Only),
       s"""|/build.gradle
           |apply plugin: 'java'
           |/src/main/java/Example.java
@@ -90,7 +93,7 @@ class GradleBuildToolSuite extends BaseBuildToolSuite {
   )
 
   checkBuild(
-    "toolchains-6.7",
+    "toolchains-6.7".tag(Java8Only),
     """|/build.gradle
        |apply plugin: 'java'
        |java {
@@ -114,7 +117,7 @@ class GradleBuildToolSuite extends BaseBuildToolSuite {
   )
 
   checkBuild(
-    "toolchains-6.8",
+    "toolchains-6.8".tag(Java8Only),
     """|/build.gradle
        |apply plugin: 'java'
        |java {
@@ -158,7 +161,7 @@ class GradleBuildToolSuite extends BaseBuildToolSuite {
   )
 
   checkBuild(
-    "playframework",
+    "playframework".tag(Java8Only),
     """|/build.gradle
        |plugins {
        |  id 'org.gradle.playframework' version '0.11'
@@ -209,7 +212,7 @@ class GradleBuildToolSuite extends BaseBuildToolSuite {
   )
 
   checkBuild(
-    "checkerframework",
+    "checkerframework".tag(Java8Only),
     """|/build.gradle
        |plugins {
        |    id 'java'