From 75b238b83d83532e9a783de81af3a1bf5ecaaf8b Mon Sep 17 00:00:00 2001 From: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com> Date: Thu, 18 Jul 2024 18:43:58 +0100 Subject: [PATCH 1/2] workflows: update pr-auditor workflow (#720) --- .github/workflows/pr-auditor.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pr-auditor.yml b/.github/workflows/pr-auditor.yml index cdd92bfc..03f531a0 100644 --- a/.github/workflows/pr-auditor.yml +++ b/.github/workflows/pr-auditor.yml @@ -11,11 +11,12 @@ jobs: steps: - uses: actions/checkout@v4 with: - repository: 'sourcegraph/pr-auditor' + repository: 'sourcegraph/devx-service' + token: ${{ secrets.PR_AUDITOR_TOKEN }} - uses: actions/setup-go@v4 - with: { go-version: '1.21' } + with: { go-version: '1.22' } - - run: './check-pr.sh' + - run: 'go run ./cmd/pr-auditor' env: GITHUB_EVENT_PATH: ${{ env.GITHUB_EVENT_PATH }} GITHUB_TOKEN: ${{ secrets.PR_AUDITOR_TOKEN }} From b8c11c4c959328eacb4bc20ad91843a6984b31e8 Mon Sep 17 00:00:00 2001 From: Anton Sviridov Date: Thu, 1 Aug 2024 16:32:33 +0100 Subject: [PATCH 2/2] Allow passing JVM args to javac in ScipBuildTool (#728) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A canonical usage for this is passing `--add-opens` flags to the _launcher_ of javac to make sure annotation processors work. To pass these arguments to the launcher, they have to be perfixed with -J – but arguments like this cannot be passed through the argfile that we use for javacOptions (see https://docs.oracle.com/en/java/javase/17/docs/specs/man/javac.html#command-line-argument-files) so we need to pass them to the command itself. For that purpose, we add a `jvmOptions` field to scip-java.json config – these options will have `-J` added to them and passed to the `javac` command. The test to verify this behaviour relies on an [old version of lombok that requires these options](https://github.com/projectlombok/lombok/issues/2681#issuecomment-748616687) Additionally, a hidden option `--strict-compilation` is added to the CLI, to prevent error recovery: sometimes scip-java can just ignore javac errors and produce semanticdb artifacts regardless. This complicates testing, so I need an escape hatch). ### Test plan - New test to ScipBuildToolSuite --- .../main/resources/scip-java/scip_java.bzl | 11 ++++- .../scip_java/buildtools/ScipBuildTool.scala | 10 ++++- .../scip_java/commands/IndexCommand.scala | 5 +++ .../test/scala/tests/ScipBuildToolSuite.scala | 43 +++++++++++++++++++ 4 files changed, 66 insertions(+), 3 deletions(-) diff --git a/scip-java/src/main/resources/scip-java/scip_java.bzl b/scip-java/src/main/resources/scip-java/scip_java.bzl index 1bc2bf1e..948ee683 100644 --- a/scip-java/src/main/resources/scip-java/scip_java.bzl +++ b/scip-java/src/main/resources/scip-java/scip_java.bzl @@ -71,11 +71,20 @@ def _scip_java(target, ctx): processorpath += [j.path for j in annotations.processor_classpath.to_list()] processors = annotations.processor_classnames + launcher_javac_flags = [] + compiler_javac_flags = [] + for value in compilation.javac_options: + if value.startswith("-J"): + launcher_javac_flags.append(value) + else: + compiler_javac_flags.append(value) + build_config = struct(**{ "javaHome": ctx.var["java_home"], "classpath": classpath, "sourceFiles": source_files, - "javacOptions": compilation.javac_options, + "javacOptions": compiler_javac_flags, + "jvmOptions": launcher_javac_flags, "processors": processors, "processorpath": processorpath, "bootclasspath": bootclasspath, diff --git a/scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/ScipBuildTool.scala b/scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/ScipBuildTool.scala index 0719f73d..d0477aa8 100644 --- a/scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/ScipBuildTool.scala +++ b/scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/ScipBuildTool.scala @@ -202,7 +202,9 @@ class ScipBuildTool(index: IndexCommand) extends BuildTool("SCIP", index) { } val isSemanticdbGenerated = Files .isDirectory(targetroot.resolve("META-INF")) - if (errors.nonEmpty && !isSemanticdbGenerated) { + if ( + errors.nonEmpty && (index.strictCompilation || !isSemanticdbGenerated) + ) { errors.foreach { error => index.app.reporter.log(Diagnostic.exception(error)) } @@ -558,8 +560,11 @@ class ScipBuildTool(index: IndexCommand) extends BuildTool("SCIP", index) { BuildInfo.javacModuleOptions else Nil + + val jvmOptions = config.jvmOptions.map("-J" + _) + val result = os - .proc(javac.toString, s"@$argsfile", javacModuleOptions) + .proc(javac.toString, s"@$argsfile", javacModuleOptions, jvmOptions) .call( stdout = pipe, stderr = pipe, @@ -815,6 +820,7 @@ class ScipBuildTool(index: IndexCommand) extends BuildTool("SCIP", index) { processorpath: List[String] = Nil, processors: List[String] = Nil, javacOptions: List[String] = Nil, + jvmOptions: List[String] = Nil, jvm: String = "17", kind: String = "" ) diff --git a/scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexCommand.scala b/scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexCommand.scala index 770754a6..1ae41c71 100644 --- a/scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexCommand.scala +++ b/scip-java/src/main/scala/com/sourcegraph/scip_java/commands/IndexCommand.scala @@ -83,6 +83,11 @@ case class IndexCommand( "Defaults to a build-specific command. For example, the default command for Maven command is 'clean verify -DskipTests'." + "To override the default, pass in the build command after a double dash: 'scip-java index -- compile test:compile'" ) + + @Hidden + @Description( + "Fail command invocation if compiler produces any errors" + ) strictCompilation: Boolean = false, @TrailingArguments() buildCommand: List[String] = Nil, @Hidden indexSemanticdb: IndexSemanticdbCommand = IndexSemanticdbCommand(), diff --git a/tests/buildTools/src/test/scala/tests/ScipBuildToolSuite.scala b/tests/buildTools/src/test/scala/tests/ScipBuildToolSuite.scala index ae9db2b5..01b37f83 100644 --- a/tests/buildTools/src/test/scala/tests/ScipBuildToolSuite.scala +++ b/tests/buildTools/src/test/scala/tests/ScipBuildToolSuite.scala @@ -110,6 +110,49 @@ class ScipBuildToolSuite extends BaseBuildToolSuite { ) ) + checkBuild( + "jvm-args", { + // In this test we verify that JVM args and Javac options are passed + // correctly. + // Lombok modules need to be passed with -J prefix, and javacOptions should + // be passed unchanged + // For this test to work the lombok version HAS to be relatively old, + // so that it requires all these opens. + // The list is taken from here: https://github.com/projectlombok/lombok/issues/2681#issuecomment-748616687 + val lombokModules = """ + --add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED + --add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED + --add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED + --add-opens=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED + --add-opens=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED + --add-opens=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED + --add-opens=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED + --add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED + --add-opens=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED + --add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED + """.trim.split("\n").map(_.trim).mkString("\"", "\", \"", "\"") + + s"""|/lsif-java.json + |{"jvmOptions": [$lombokModules], "javacOptions": ["--add-exports=java.base/sun.util=ALL-UNNAMED"], "dependencies": ["org.projectlombok:lombok:1.18.16"]} + |/foo/Example.java + |package foo; + |import sun.util.BuddhistCalendar; + |public class Example extends BuddhistCalendar { + | public static void hello() { + | BuddhistCalendar calendar = new BuddhistCalendar(); + | } + |} + |""".stripMargin + }, + expectedSemanticdbFiles = 1, + // somehow it seems the actual compilation error from javac + // does not stop semanticdb-javac from producing the file. + // we explicitly disable this lenient mode so that if there + // are any compilation errors, it will be reflected in failed + // CLI command. + extraArguments = List("--strict-compilation") + ) + checkBuild( "basic", """|/lsif-java.json