From b81a9dce080be8e1618e7a979d4823556dced971 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 15 Aug 2023 11:46:53 +0100 Subject: [PATCH 01/10] Fix compilation of Java 17+ sources --- .github/workflows/ci.yml | 22 +++++-- .github/workflows/pr-auditor.yml | 13 ++-- .github/workflows/sourcegraph.yml | 9 ++- .../sbtsourcegraph/SourcegraphEnable.scala | 36 +++++++++++ .../sourcegraph/sbtsourcegraph/Versions.scala | 59 +++++++++++++++++++ src/sbt-test/sbt-sourcegraph/basic/build.sbt | 10 ++++ .../basic/project/build.properties | 2 +- src/sbt-test/sbt-sourcegraph/scala-3/test | 2 - 8 files changed, 136 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3e5d4cf..23d3e76 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,13 +7,27 @@ on: jobs: test: runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest] + java: [8, 11, 17] steps: - - uses: actions/checkout@v2 - - uses: olafurpg/setup-scala@v13 + - uses: actions/checkout@v3 + - uses: actions/setup-java@v3 + with: + distribution: "temurin" + cache: "sbt" + java-version: ${{ matrix.java }} - run: sbt scripted +test + check: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - uses: olafurpg/setup-scala@v13 + - uses: actions/checkout@v3 + - uses: actions/setup-java@v3 + with: + distribution: "temurin" + cache: "sbt" + java-version: 17 - run: sbt checkAll diff --git a/.github/workflows/pr-auditor.yml b/.github/workflows/pr-auditor.yml index bdc06f3..3fe4223 100644 --- a/.github/workflows/pr-auditor.yml +++ b/.github/workflows/pr-auditor.yml @@ -8,14 +8,13 @@ jobs: check-pr: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - with: - repository: 'sourcegraph/pr-auditor' - - uses: actions/setup-go@v4 - with: { go-version: '1.20' } + - uses: actions/checkout@v2 + with: { repository: 'sourcegraph/sourcegraph' } + - uses: actions/setup-go@v2 + with: { go-version: '1.18' } - - run: './check-pr.sh' + - run: ./dev/pr-auditor/check-pr.sh env: GITHUB_EVENT_PATH: ${{ env.GITHUB_EVENT_PATH }} - GITHUB_TOKEN: ${{ github.token }} + GITHUB_TOKEN: ${{ secrets.CODENOTIFY_GITHUB_TOKEN }} GITHUB_RUN_URL: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }} diff --git a/.github/workflows/sourcegraph.yml b/.github/workflows/sourcegraph.yml index 6c6c626..170bc58 100644 --- a/.github/workflows/sourcegraph.yml +++ b/.github/workflows/sourcegraph.yml @@ -10,11 +10,14 @@ jobs: name: "Upload LSIF" steps: - uses: actions/checkout@v2 - - uses: coursier/setup-action@v1.1.2 + - uses: actions/setup-java@v3 with: - jvm: adopt:8 + distribution: "temurin" + cache: "sbt" + java-version: 17 - run: | - cs launch com.sourcegraph:scip-java_2.13:latest.stable -M com.sourcegraph.scip_java.ScipJava -- index + curl -fL "https://github.com/coursier/launchers/raw/master/cs-x86_64-pc-linux.gz" | gzip -d > cs && chmod +x cs + ./cs launch com.sourcegraph:scip-java_2.13:latest.stable -M com.sourcegraph.scip_java.ScipJava -- index - run: yarn global add @sourcegraph/src - run: | src code-intel upload "-commit=${GITHUB_SHA}" "-github-token=${GITHUB_TOKEN}" diff --git a/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala b/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala index 19382d9..8d634d1 100644 --- a/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala +++ b/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala @@ -37,6 +37,13 @@ object SourcegraphEnable { ) enableSemanticdbPlugin = List( + Option( + javacOptions.in(p) ++= { + if (Versions.isJavaAtLeast(17, home = javaHome.in(p).value)) + javacModuleOptions + else Nil + } + ), Option( allDependencies.in(p) += "com.sourcegraph" % "semanticdb-javac" % semanticdbJavacVersion @@ -51,6 +58,11 @@ object SourcegraphEnable { Option(SemanticdbPlugin.semanticdbEnabled.in(p) := true), semanticdbVersion.map(ver => SemanticdbPlugin.semanticdbVersion.in(p) := ver + ), + Option( + javaHome.in(p) := javaHome.in(p).value orElse Some( + new File(System.getProperty("java.home")) + ) ) ).flatten settings <- @@ -101,6 +113,16 @@ object SourcegraphEnable { ) ) }.toSeq + ), + Option( + javacOptions.in(p) ++= { + if (Versions.isJavaAtLeast(17)) javacModuleOptions else Nil + } + ), + Option( + javaHome.in(p) := javaHome.in(p).value orElse Some( + new File(System.getProperty("java.home")) + ) ) ).flatten settings <- @@ -143,4 +165,18 @@ object SourcegraphEnable { semanticdbVersion = Versions .semanticdbVersion(overriddenScalaVersion.getOrElse(projectScalaVersion)) } yield (p, semanticdbVersion, overriddenScalaVersion) + + def javacModuleOptions = + List( + "-J--add-exports", + "-Jjdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED", + "-J--add-exports", + "-Jjdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED", + "-J--add-exports", + "-Jjdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED", + "-J--add-exports", + "-Jjdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED", + "-J--add-exports", + "-Jjdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED" + ) } diff --git a/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala b/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala index 08c107f..ae5f9e0 100644 --- a/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala +++ b/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala @@ -5,6 +5,7 @@ import java.nio.file.Paths import java.util.Properties import scala.collection.JavaConverters._ import scala.sys.process._ +import java.io.File object Versions { def scalametaVersion = "4.4.26" @@ -65,6 +66,64 @@ object Versions { .updated(semanticdbJavacKey, semanticdbJavacVersions.last) } + private val jvmVersionCache = collection.mutable.Map.empty[Option[File], Int] + + def isJavaAtLeast(n: Int, home: Option[File] = None) = { + + val significant = jvmVersionCache.getOrElseUpdate( + home, { + val raw = + home match { + case None => + System.getProperty("java.version") + case Some(javaHome) => + val sb = new StringBuilder + val proc = { + val cmd = + if (scala.util.Properties.isWin) + Paths.get("bin", "javac.exe") + else Paths.get("bin", "javac") + + val stdout = scala.sys.process + .Process(Seq(cmd.toString(), "-version"), cwd = javaHome) + .!!(ProcessLogger(sb.append(_))) + .trim + + // Java 8 sends output to stderr... + if (stdout.isEmpty()) sb.result().trim else stdout + } + + proc.split(" ").toList match { + case "javac" :: version :: Nil => version + case other => + sys.error( + s"Cannot process javac output (in $javaHome): [$proc]" + ) + } + } + + val prop = raw.takeWhile(c => c.isDigit || c == '.') + + val segments = prop.split("\\.").toList + + segments match { + // Java 1.6 - 1.8 + case "1" :: lessThan8 :: _ :: Nil => lessThan8.toInt + // Java 17.0.1, .. + case modern :: _ :: _ :: Nil => modern.toInt + // Java 12 + case modern :: Nil => modern.toInt + case other => + sys.error( + s"Cannot process java.home property, unknown format: [$raw]" + ) + } + } + ) + + significant >= n + } + private def proc(cmd: String*): List[String] = { println(cmd.updated(0, "coursier").mkString("$ ", " ", "")) cmd.!!.linesIterator.toList diff --git a/src/sbt-test/sbt-sourcegraph/basic/build.sbt b/src/sbt-test/sbt-sourcegraph/basic/build.sbt index 96243bf..bd00c16 100644 --- a/src/sbt-test/sbt-sourcegraph/basic/build.sbt +++ b/src/sbt-test/sbt-sourcegraph/basic/build.sbt @@ -17,6 +17,15 @@ lazy val a = project lazy val b = project .dependsOn(a) + .settings( + // Test to ensure the plugin works with explicitly set java home + // On Java 8 the java.home property returns JRE path, not JDK path. + // so we try and work around it hoping that JAVA_HOME is set by executing + // environment + javaHome := Some( + new File(sys.env.getOrElse("JAVA_HOME", System.getProperty("java.home"))) + ) + ) commands += Command.command("checkLsif") { s => val dumpPath = @@ -32,6 +41,7 @@ commands += Command.command("checkLsif") { s => .filterNot(_ == ".") .distinct .sorted + .toList if ( packageNames != List( "jdk", diff --git a/src/sbt-test/sbt-sourcegraph/basic/project/build.properties b/src/sbt-test/sbt-sourcegraph/basic/project/build.properties index 19479ba..52413ab 100644 --- a/src/sbt-test/sbt-sourcegraph/basic/project/build.properties +++ b/src/sbt-test/sbt-sourcegraph/basic/project/build.properties @@ -1 +1 @@ -sbt.version=1.5.2 +sbt.version=1.9.3 diff --git a/src/sbt-test/sbt-sourcegraph/scala-3/test b/src/sbt-test/sbt-sourcegraph/scala-3/test index d318c67..a60e80e 100644 --- a/src/sbt-test/sbt-sourcegraph/scala-3/test +++ b/src/sbt-test/sbt-sourcegraph/scala-3/test @@ -1,4 +1,2 @@ > sourcegraphEnable -> show semanticdbEnabled -> show Compile/semanticdbEnabled > sourcegraphCompile From b3709ba7bcc1532e4eedd65eaab2b2f20b29945d Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 15 Aug 2023 11:53:08 +0100 Subject: [PATCH 02/10] chore: scalafix --- .../com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala | 2 +- src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala b/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala index 8d634d1..73b57d8 100644 --- a/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala +++ b/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala @@ -166,7 +166,7 @@ object SourcegraphEnable { .semanticdbVersion(overriddenScalaVersion.getOrElse(projectScalaVersion)) } yield (p, semanticdbVersion, overriddenScalaVersion) - def javacModuleOptions = + def javacModuleOptions: List[String] = List( "-J--add-exports", "-Jjdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED", diff --git a/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala b/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala index ae5f9e0..77dc60a 100644 --- a/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala +++ b/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala @@ -68,7 +68,7 @@ object Versions { private val jvmVersionCache = collection.mutable.Map.empty[Option[File], Int] - def isJavaAtLeast(n: Int, home: Option[File] = None) = { + def isJavaAtLeast(n: Int, home: Option[File] = None): Boolean = { val significant = jvmVersionCache.getOrElseUpdate( home, { From 41b2f325a0bf201592cef5b4b2bdc24c77dda8ee Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 15 Aug 2023 12:13:47 +0100 Subject: [PATCH 03/10] Come on --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 23d3e76..bb53691 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,9 @@ jobs: distribution: "temurin" cache: "sbt" java-version: ${{ matrix.java }} - - run: sbt scripted +test + - run: | + ls $JAVA_HOME + sbt scripted +test check: runs-on: ubuntu-latest From 4617850f2dd48a0cce0cda9db92db5f2533649a1 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 15 Aug 2023 12:17:48 +0100 Subject: [PATCH 04/10] glargh --- .github/workflows/ci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bb53691..a99513f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,8 +20,9 @@ jobs: cache: "sbt" java-version: ${{ matrix.java }} - run: | - ls $JAVA_HOME - sbt scripted +test + ls $JAVA_HOME + sbt scripted +test + shell: bash check: runs-on: ubuntu-latest From ce2cdd7747078825732ba698ad86af6fcda16319 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 15 Aug 2023 12:52:25 +0100 Subject: [PATCH 05/10] Switch back to calling java -version --- .../sourcegraph/sbtsourcegraph/Versions.scala | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala b/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala index 77dc60a..849dfdd 100644 --- a/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala +++ b/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala @@ -81,24 +81,26 @@ object Versions { val proc = { val cmd = if (scala.util.Properties.isWin) - Paths.get("bin", "javac.exe") - else Paths.get("bin", "javac") + Paths.get("bin", "java") + else Paths.get("bin", "java") - val stdout = scala.sys.process + scala.sys.process .Process(Seq(cmd.toString(), "-version"), cwd = javaHome) .!!(ProcessLogger(sb.append(_))) .trim - - // Java 8 sends output to stderr... - if (stdout.isEmpty()) sb.result().trim else stdout } - proc.split(" ").toList match { - case "javac" :: version :: Nil => version - case other => + val rgx = "version \"(.*?)\"".r + + rgx.findFirstMatchIn( + proc.linesIterator.take(1).mkString("") + ) match { + case None => sys.error( - s"Cannot process javac output (in $javaHome): [$proc]" + s"Cannot process [java -version] output (in $javaHome): [$proc]" ) + case Some(value) => + value.group(1) } } From 8cd2e7bdd7bc7e5b0c7fb07e262ecd0e5df78e7d Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 15 Aug 2023 12:57:16 +0100 Subject: [PATCH 06/10] When will this end --- src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala b/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala index 849dfdd..db4f1ca 100644 --- a/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala +++ b/src/main/scala/com/sourcegraph/sbtsourcegraph/Versions.scala @@ -87,7 +87,8 @@ object Versions { scala.sys.process .Process(Seq(cmd.toString(), "-version"), cwd = javaHome) .!!(ProcessLogger(sb.append(_))) - .trim + + sb.result().trim } val rgx = "version \"(.*?)\"".r From ee0b3e36f9cbe46e25723563b03fc0c14e13e8fb Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 15 Aug 2023 13:51:54 +0100 Subject: [PATCH 07/10] honestly --- src/sbt-test/sbt-sourcegraph/basic/build.sbt | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/sbt-test/sbt-sourcegraph/basic/build.sbt b/src/sbt-test/sbt-sourcegraph/basic/build.sbt index bd00c16..d847b2e 100644 --- a/src/sbt-test/sbt-sourcegraph/basic/build.sbt +++ b/src/sbt-test/sbt-sourcegraph/basic/build.sbt @@ -22,9 +22,14 @@ lazy val b = project // On Java 8 the java.home property returns JRE path, not JDK path. // so we try and work around it hoping that JAVA_HOME is set by executing // environment - javaHome := Some( - new File(sys.env.getOrElse("JAVA_HOME", System.getProperty("java.home"))) - ) + javaHome := { + println(sys.env.get("JAVA_HOME")) + Some( + new File( + sys.env.getOrElse("JAVA_HOME", System.getProperty("java.home")) + ) + ) + } ) commands += Command.command("checkLsif") { s => From 86ec7c424f03538d22ffb2d75503cff7e2130f17 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 15 Aug 2023 14:09:31 +0100 Subject: [PATCH 08/10] WIP --- .../com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala b/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala index 73b57d8..6f72a54 100644 --- a/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala +++ b/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala @@ -60,9 +60,10 @@ object SourcegraphEnable { SemanticdbPlugin.semanticdbVersion.in(p) := ver ), Option( - javaHome.in(p) := javaHome.in(p).value orElse Some( + javaHome.in(p) := javaHome.in(p).value orElse Some { + println("Oopsie daisy, javaHome is not set") new File(System.getProperty("java.home")) - ) + } ) ).flatten settings <- From 4417d62c525f3448f77a8583ded41eeba0d00340 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 15 Aug 2023 14:57:10 +0100 Subject: [PATCH 09/10] Don't enforce javaHome on JDK < 17 --- .../sbtsourcegraph/SourcegraphEnable.scala | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala b/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala index 6f72a54..349cf95 100644 --- a/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala +++ b/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala @@ -31,6 +31,7 @@ object SourcegraphEnable { ) val semanticdbJavacVersion = Versions.semanticdbJavacVersion() + val settings = for { (p, semanticdbVersion, overriddenScalaVersion) <- collectProjects( extracted @@ -60,9 +61,22 @@ object SourcegraphEnable { SemanticdbPlugin.semanticdbVersion.in(p) := ver ), Option( - javaHome.in(p) := javaHome.in(p).value orElse Some { - println("Oopsie daisy, javaHome is not set") - new File(System.getProperty("java.home")) + javaHome.in(p) := { + javaHome.in(p).value orElse { + // We can safely use java.home property + // on JDK 17+ as it won't be pointing to JRE which + // doesn't contain a compiler. + if (Versions.isJavaAtLeast(17)) { + // On JDK 17+ we need to explicitly fork the compiler + // so that we can set the necessary JVM options to access + // jdk.compiler module + Some(new File(System.getProperty("java.home"))) + } else { + // If JDK is below 17, we don't actually need to + // fork the compiler, so we can keep javaHome empty + None + } + } } ) ).flatten From 729040cf5a7c02ab262fec8fc6692eb9c38218cc Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Tue, 15 Aug 2023 15:10:28 +0100 Subject: [PATCH 10/10] Clean up --- .../sbtsourcegraph/SourcegraphEnable.scala | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala b/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala index 349cf95..96843a7 100644 --- a/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala +++ b/src/main/scala/com/sourcegraph/sbtsourcegraph/SourcegraphEnable.scala @@ -62,21 +62,7 @@ object SourcegraphEnable { ), Option( javaHome.in(p) := { - javaHome.in(p).value orElse { - // We can safely use java.home property - // on JDK 17+ as it won't be pointing to JRE which - // doesn't contain a compiler. - if (Versions.isJavaAtLeast(17)) { - // On JDK 17+ we need to explicitly fork the compiler - // so that we can set the necessary JVM options to access - // jdk.compiler module - Some(new File(System.getProperty("java.home"))) - } else { - // If JDK is below 17, we don't actually need to - // fork the compiler, so we can keep javaHome empty - None - } - } + javaHome.in(p).value orElse calculateJavaHome } ) ).flatten @@ -135,9 +121,7 @@ object SourcegraphEnable { } ), Option( - javaHome.in(p) := javaHome.in(p).value orElse Some( - new File(System.getProperty("java.home")) - ) + javaHome.in(p) := javaHome.in(p).value orElse calculateJavaHome ) ).flatten settings <- @@ -194,4 +178,20 @@ object SourcegraphEnable { "-J--add-exports", "-Jjdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED" ) + + private def calculateJavaHome = { + // We can safely use java.home property + // on JDK 17+ as it won't be pointing to JRE which + // doesn't contain a compiler. + if (Versions.isJavaAtLeast(17)) { + // On JDK 17+ we need to explicitly fork the compiler + // so that we can set the necessary JVM options to access + // jdk.compiler module + Some(new File(System.getProperty("java.home"))) + } else { + // If JDK is below 17, we don't actually need to + // fork the compiler, so we can keep javaHome empty + None + } + } }