Skip to content

Commit

Permalink
Allow passing JVM args to javac in ScipBuildTool (#728)
Browse files Browse the repository at this point in the history
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](projectlombok/lombok#2681 (comment))

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
  • Loading branch information
antonsviridov-src authored Aug 1, 2024
1 parent 75b238b commit b8c11c4
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 3 deletions.
11 changes: 10 additions & 1 deletion scip-java/src/main/resources/scip-java/scip_java.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 = ""
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
43 changes: 43 additions & 0 deletions tests/buildTools/src/test/scala/tests/ScipBuildToolSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b8c11c4

Please sign in to comment.