Make buildTools/test work on Java 17

olafurpg · olafurpg · commit c3b8f4bba4f0 · 2022-08-16T15:52:08.000+02:00
Previously, we only tested `sbt test` on Java 8 and we had a lot of
failing tests if sbt was started with Java 17. This commit makes the
tests in the `buildTools` project pass on Java 17.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -6,17 +6,18 @@ on:
   pull_request:
 jobs:
   test:
-    runs-on:  ${{ matrix.os }}
+    runs-on: ${{ matrix.os }}
     strategy:
       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"
diff --git a/scip-java/src/main/scala/com/sourcegraph/scip_java/Embedded.scala b/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))
diff --git a/scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/GradleBuildTool.scala b/scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/GradleBuildTool.scala
@@ -126,8 +126,9 @@ class GradleBuildTool(index: IndexCommand) extends BuildTool("Gradle", index) {
   }
 
   private def initScript(toolchains: GradleJavaToolchains, tmp: Path): Path = {
+    val executableJavacPath = toolchains.executableJavacPath()
     val executable =
-      toolchains.executableJavacPath() match {
+      executableJavacPath match {
         case Some(path) =>
           s"options.forkOptions.executable = '$path'"
         case None =>
@@ -144,17 +145,6 @@ class GradleBuildTool(index: IndexCommand) extends BuildTool("Gradle", index) {
     val dependenciesPath = targetroot.resolve("dependencies.txt")
     val kotlinSemanticdbVersion = BuildInfo.semanticdbKotlincVersion
     Files.deleteIfExists(dependenciesPath)
-    val javaModuleOptions =
-      if (Properties.isJavaAtLeast(9)) {
-        // Include --add-exports flags for Java 9+
-        BuildInfo
-          .javacModuleOptions
-          .map(option => s""""$option"""")
-          .mkString(",")
-      } else {
-        // No --add-exports flags for Java 8
-        ""
-      }
     val script =
       s"""|allprojects {
           |  gradle.projectsEvaluated {
@@ -171,7 +161,7 @@ class GradleBuildTool(index: IndexCommand) extends BuildTool("Gradle", index) {
           |      tasks.withType(JavaCompile) {
           |        options.fork = true
           |        options.incremental = false
-          |        options.forkOptions.jvmArgs.addAll([$javaModuleOptions])
+          |        System.out.println("JAVA VERSION " + System.getProperty("java.version"))
           |        $executable
           |      }
           |    }
diff --git a/scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/GradleJavaCompiler.scala b/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")
diff --git a/scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/GradleJavaToolchains.scala b/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
+    }
+  }
 }
diff --git a/scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/MavenBuildTool.scala b/scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/MavenBuildTool.scala
@@ -5,6 +5,7 @@ import java.nio.file.Path
 import java.nio.file.Paths
 
 import scala.collection.mutable.ListBuffer
+import scala.util.Properties
 
 import com.sourcegraph.scip_java.Embedded
 import com.sourcegraph.scip_java.commands.IndexCommand
@@ -39,7 +40,10 @@ class MavenBuildTool(index: IndexCommand) extends BuildTool("Maven", index) {
       val executable = Embedded.customJavac(
         index.workingDirectory,
         index.finalTargetroot(defaultTargetroot),
-        tmp
+        tmp,
+        // TODO: infer Java version from `java -version` because it may not
+        // match the Java version that's used by the current process.
+        Properties.isJavaAtLeast(11)
       )
       buildCommand ++=
         List(
diff --git a/tests/buildTools/src/test/scala/tests/BaseBuildToolSuite.scala b/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 =>
diff --git a/tests/buildTools/src/test/scala/tests/GradleBuildToolSuite.scala b/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'
