From eea306f03047dc3a9604f167a8a33dbff8d46f0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 20 Apr 2021 11:14:34 +0200 Subject: [PATCH] Add support for the Gradle Playframework plugin. Previously, the `index` command didn't work with Gradle projects that use the Playframework plugin. The codebase would compile, but no LSIF index would be created. Now, everything should work as expected. The problem was that the Playframework plugin uses the Scala plugin to compile auto-generated template and routes files. The Scala plugin runs Zinc (Scala incremental compiler) in a daemon process behind the scenes so it ignores the `javac` fork settings that `lsif-java index` adds to the build. The fix is to enable the SemanticDB Java agent on the Zinc daemon process and to add one more injection point to ensure that the SemanticDB compiler plugin is always on the classpath for all projects. --- .../lsif_java/buildtools/BuildTool.scala | 1 + .../buildtools/GradleBuildTool.scala | 35 ++++++++-- .../buildtools/GradleJavaToolchains.scala | 4 +- .../lsif_java/buildtools/MavenBuildTool.scala | 2 +- .../lsif_java/commands/IndexCommand.scala | 12 ++-- .../lsif_semanticdb/LsifSemanticdb.java | 15 ++++- .../LsifSemanticdbReporter.java | 4 ++ .../semanticdb_javac/SemanticdbAgent.java | 67 ++++++++++++++++++- .../GradleDefaultJvmLanguageCompileSpec.scala | 12 ++++ ... GradleJavaCompilerArgumentsBuilder.scala} | 2 +- .../scala/tests/GradleBuildToolSuite.scala | 51 ++++++++++++++ ...leDefaultJvmLanguageCompileSpecSuite.scala | 29 ++++++++ ...leJavaCompilerArgumentsBuilderSuite.scala} | 5 +- 13 files changed, 218 insertions(+), 21 deletions(-) create mode 100644 tests/buildTools/src/main/scala/tests/GradleDefaultJvmLanguageCompileSpec.scala rename tests/buildTools/src/main/scala/tests/{GradleOptionsBuilder.scala => GradleJavaCompilerArgumentsBuilder.scala} (63%) create mode 100644 tests/buildTools/src/test/scala/tests/GradleDefaultJvmLanguageCompileSpecSuite.scala rename tests/buildTools/src/test/scala/tests/{GradleOptionsBuilderSuite.scala => GradleJavaCompilerArgumentsBuilderSuite.scala} (89%) diff --git a/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/BuildTool.scala b/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/BuildTool.scala index 51384037..0a07212b 100644 --- a/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/BuildTool.scala +++ b/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/BuildTool.scala @@ -16,6 +16,7 @@ abstract class BuildTool(val name: String, index: IndexCommand) { def indexJdk(): Boolean = true + final def sourceroot: Path = index.workingDirectory final def targetroot: Path = AbsolutePath .of(index.targetroot.getOrElse(defaultTargetroot), index.workingDirectory) diff --git a/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/GradleBuildTool.scala b/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/GradleBuildTool.scala index d8a944a2..04a3301b 100644 --- a/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/GradleBuildTool.scala +++ b/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/GradleBuildTool.scala @@ -75,7 +75,7 @@ class GradleBuildTool(index: IndexCommand) extends BuildTool("Gradle", index) { buildCommand ++= index.finalBuildCommand(List("clean", "compileTestJava")) buildCommand += lsifJavaDependencies - val result = index.process(buildCommand.toList: _*) + val result = index.process(buildCommand) printDebugLogs(toolchains.tmp) Embedded .reportUnexpectedJavacErrors(index.app.reporter, toolchains.tmp) @@ -103,6 +103,9 @@ class GradleBuildTool(index: IndexCommand) extends BuildTool("Gradle", index) { case None => "" } + + val agentpath = Embedded.agentJar(tmp) + val pluginpath = Embedded.semanticdbJar(tmp) val dependenciesPath = targetroot.resolve("dependencies.txt") Files.deleteIfExists(dependenciesPath) val script = @@ -111,16 +114,36 @@ class GradleBuildTool(index: IndexCommand) extends BuildTool("Gradle", index) { | boolean isJavaEnabled = project.plugins.any { | it.getClass().getName().endsWith("org.gradle.api.plugins.JavaPlugin") | } - | if (!isJavaEnabled) return - | tasks.withType(JavaCompile) { - | options.fork = true - | options.incremental = false - | $executable + | boolean isScalaEnabled = project.plugins.any { + | it.getClass().getName().endsWith("org.gradle.api.plugins.scala.ScalaPlugin") + | } + | if (isJavaEnabled) { + | tasks.withType(JavaCompile) { + | options.fork = true + | options.incremental = false + | $executable + | } + | } + | if (isScalaEnabled) { + | // The Scala plugin runs Zinc, an incremental compiler, in a separate daemon process. + | // Zinc invokes the Java compiler directly to compile mixed Java/Scala projects + | // instead of respecting the `JavaCompile` fork options from the Gradle Java plugin. + | // By enabling the SemanticDB Java agent on the Zinc daemon process, we manage + | // to configure Zinc to use the semanticdb-javac compiler plugin for Java compilation. + | tasks.withType(ScalaCompile) { + | scalaCompileOptions.forkOptions.with { + | jvmArgs << '-javaagent:$agentpath' + | jvmArgs << '-Dsemanticdb.pluginpath=$pluginpath' + | jvmArgs << '-Dsemanticdb.sourceroot=$sourceroot' + | jvmArgs << '-Dsemanticdb.targetroot=$targetroot' + | } + | } | } | } | task $lsifJavaDependencies { | def depsOut = java.nio.file.Paths.get('$dependenciesPath') | doLast { + | java.nio.file.Files.createDirectories(depsOut.getParent()) | tasks.withType(JavaCompile) { | try { | configurations.each { config -> diff --git a/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/GradleJavaToolchains.scala b/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/GradleJavaToolchains.scala index 5e9fccef..63e9c197 100644 --- a/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/GradleJavaToolchains.scala +++ b/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/GradleJavaToolchains.scala @@ -92,7 +92,9 @@ object GradleJavaToolchains { |} |""".stripMargin Files.write(scriptPath, script.getBytes(StandardCharsets.UTF_8)) - index.process(gradleCommand, "--init-script", scriptPath.toString, taskName) + index.process( + List(gradleCommand, "--init-script", scriptPath.toString, taskName) + ) val toolchains: List[GradleJavaCompiler] = if (Files.isRegularFile(toolchainsPath)) { Files diff --git a/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/MavenBuildTool.scala b/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/MavenBuildTool.scala index 4b0bf204..dae2e463 100644 --- a/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/MavenBuildTool.scala +++ b/lsif-java/src/main/scala/com/sourcegraph/lsif_java/buildtools/MavenBuildTool.scala @@ -53,7 +53,7 @@ class MavenBuildTool(index: IndexCommand) extends BuildTool("Maven", index) { ) ) - val exit = index.process(buildCommand.toList: _*) + val exit = index.process(buildCommand) Embedded .reportUnexpectedJavacErrors(index.app.reporter, tmp) .getOrElse(exit) diff --git a/lsif-java/src/main/scala/com/sourcegraph/lsif_java/commands/IndexCommand.scala b/lsif-java/src/main/scala/com/sourcegraph/lsif_java/commands/IndexCommand.scala index 1a84d16d..10c092a8 100644 --- a/lsif-java/src/main/scala/com/sourcegraph/lsif_java/commands/IndexCommand.scala +++ b/lsif-java/src/main/scala/com/sourcegraph/lsif_java/commands/IndexCommand.scala @@ -61,15 +61,19 @@ case class IndexCommand( app: Application = Application.default ) extends Command { - def process(shellable: String*): CommandResult = { - app.info(shellable.mkString(" ")) + def process( + shellable: Shellable, + env: Map[String, String] = Map.empty + ): CommandResult = { + app.info(shellable.value.mkString(" ")) app - .process(Shellable(shellable)) + .process(shellable) .call( check = false, stdout = Inherit, stderr = Inherit, - cwd = workingDirectory + cwd = workingDirectory, + env = env ) } diff --git a/lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifSemanticdb.java b/lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifSemanticdb.java index 2a01a10e..99c399c1 100644 --- a/lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifSemanticdb.java +++ b/lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifSemanticdb.java @@ -37,12 +37,21 @@ public static void run(LsifSemanticdbOptions options) throws IOException { private void run() throws IOException { PackageTable packages = new PackageTable(options, writer); - writer.emitMetaData(); - int projectId = writer.emitProject(options.language); - List files = SemanticdbWalker.findSemanticdbFiles(options); if (options.reporter.hasErrors()) return; + if (files.isEmpty()) { + options.reporter.error( + "No SemanticDB files found. " + + "This typically means that `lsif-java` is unable to automatically " + + "index this codebase. If you are using Gradle or Maven, please report an issue to " + + "https://github.com/sourcegraph/lsif-java and include steps to reproduce. " + + "If you are using a different build tool, make sure that you have followed all " + + "of the steps from https://sourcegraph.github.io/lsif-java/docs/manual-configuration.html"); + return; + } options.reporter.startProcessing(files.size()); + writer.emitMetaData(); + int projectId = writer.emitProject(options.language); Set isExportedSymbol = exportSymbols(files); List documentIds = diff --git a/lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifSemanticdbReporter.java b/lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifSemanticdbReporter.java index bbf56725..a2a8e3e0 100644 --- a/lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifSemanticdbReporter.java +++ b/lsif-semanticdb/src/main/java/com/sourcegraph/lsif_semanticdb/LsifSemanticdbReporter.java @@ -9,6 +9,10 @@ public abstract class LsifSemanticdbReporter { public void error(Throwable e) {} + public void error(String message) { + error(new RuntimeException(message)); + } + public void startProcessing(int taskSize) {} public void processedOneItem() {} diff --git a/semanticdb-agent/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbAgent.java b/semanticdb-agent/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbAgent.java index ccf94cc8..297d478a 100644 --- a/semanticdb-agent/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbAgent.java +++ b/semanticdb-agent/src/main/java/com/sourcegraph/semanticdb_javac/SemanticdbAgent.java @@ -5,10 +5,12 @@ import java.io.File; import java.io.IOException; +import java.io.OutputStream; import java.io.PrintStream; import java.lang.instrument.Instrumentation; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardOpenOption; import java.util.ArrayList; @@ -21,18 +23,77 @@ public class SemanticdbAgent { public static void premain(String agentArgs, Instrumentation inst) { + // NOTE(olafur): Uncoment below if you want see all the loaded classes. + // PrintStream logger = newLogger(); + // inst.addTransformer( + // new ClassFileTransformer() { + // @Override + // public byte[] transform( + // ClassLoader loader, + // String className, + // Class classBeingRedefined, + // ProtectionDomain protectionDomain, + // byte[] classfileBuffer) { + // logger.println(className); + // return classfileBuffer; + // } + // }); + new AgentBuilder.Default() + .disableClassFormatChanges() + .type( + named("org.gradle.api.internal.tasks.compile.DefaultJvmLanguageCompileSpec") + .or(named("tests.GradleDefaultJvmLanguageCompileSpec"))) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice( + named("getCompileClasspath"), + DefaultJvmLanguageCompileSpecAdvice.class.getName())) + .installOn(inst); new AgentBuilder.Default() .type( named("org.gradle.api.internal.tasks.compile.JavaCompilerArgumentsBuilder") - .or(named("tests.GradleOptionsBuilder"))) + .or(named("tests.GradleJavaCompilerArgumentsBuilder"))) .transform( new AgentBuilder.Transformer.ForAdvice() - .advice(named("build"), GradleAdvice.class.getName())) + .advice(named("build"), JavaCompilerArgumentsBuilderAdvice.class.getName())) .installOn(inst); } + private static PrintStream newLogger() { + Path path = Paths.get(System.getProperty("user.home"), ".lsif-java", "logs.txt"); + try { + Files.createDirectories(path.getParent()); + OutputStream fos = + Files.newOutputStream( + path, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING); + return new PrintStream(fos); + } catch (IOException e) { + return new PrintStream( + new OutputStream() { + @Override + public void write(int b) {} + }); + } + } + + @SuppressWarnings("all") + public static class DefaultJvmLanguageCompileSpecAdvice { + @Advice.OnMethodExit + public static void getClasspath( + @Advice.Return(readOnly = false, typing = DYNAMIC) List classpath) { + String PLUGINPATH = System.getProperty("semanticdb.pluginpath"); + if (PLUGINPATH == null) throw new NoSuchElementException("-Dsemanticdb.pluginpath"); + File semanticdbJar = new File(PLUGINPATH); + if (!classpath.contains(semanticdbJar)) { + List newClasspath = new ArrayList<>(classpath); + newClasspath.add(semanticdbJar); + classpath = newClasspath; + } + } + } + @SuppressWarnings("all") - public static class GradleAdvice { + public static class JavaCompilerArgumentsBuilderAdvice { /** * The bytecode of this method gets injected at the end of the following method in Gradle: diff --git a/tests/buildTools/src/main/scala/tests/GradleDefaultJvmLanguageCompileSpec.scala b/tests/buildTools/src/main/scala/tests/GradleDefaultJvmLanguageCompileSpec.scala new file mode 100644 index 00000000..4043df00 --- /dev/null +++ b/tests/buildTools/src/main/scala/tests/GradleDefaultJvmLanguageCompileSpec.scala @@ -0,0 +1,12 @@ +package tests + +import java.io.File +import java.util + +import scala.jdk.CollectionConverters._ + +class GradleDefaultJvmLanguageCompileSpec(base: List[String]) { + def getCompileClasspath: util.List[File] = { + base.map(new File(_)).asJava + } +} diff --git a/tests/buildTools/src/main/scala/tests/GradleOptionsBuilder.scala b/tests/buildTools/src/main/scala/tests/GradleJavaCompilerArgumentsBuilder.scala similarity index 63% rename from tests/buildTools/src/main/scala/tests/GradleOptionsBuilder.scala rename to tests/buildTools/src/main/scala/tests/GradleJavaCompilerArgumentsBuilder.scala index 1fca3f06..b59aa614 100644 --- a/tests/buildTools/src/main/scala/tests/GradleOptionsBuilder.scala +++ b/tests/buildTools/src/main/scala/tests/GradleJavaCompilerArgumentsBuilder.scala @@ -2,6 +2,6 @@ package tests import scala.jdk.CollectionConverters._ -class GradleOptionsBuilder(base: List[String]) { +class GradleJavaCompilerArgumentsBuilder(base: List[String]) { def build(): java.util.List[String] = base.asJava } diff --git a/tests/buildTools/src/test/scala/tests/GradleBuildToolSuite.scala b/tests/buildTools/src/test/scala/tests/GradleBuildToolSuite.scala index d6d8c9fe..f333989b 100644 --- a/tests/buildTools/src/test/scala/tests/GradleBuildToolSuite.scala +++ b/tests/buildTools/src/test/scala/tests/GradleBuildToolSuite.scala @@ -150,4 +150,55 @@ class GradleBuildToolSuite extends BaseBuildToolSuite { 1, extraArguments = List("--", "compileJava") ) + + checkBuild( + "playframework", + """|/build.gradle + |plugins { + | id 'org.gradle.playframework' version '0.11' + | id 'idea' + |} + | + |play { + | platform { + | playVersion = '2.6.7' + | scalaVersion = '2.12' + | javaVersion = JavaVersion.VERSION_1_8 + | } + | injectedRoutesGenerator = true + |} + |dependencies { + | implementation "com.typesafe.play:play-guice_2.12:2.6.7" + |} + | + |repositories { + | mavenCentral() + | maven { + | name "lightbend-maven-releases" + | url "https://repo.lightbend.com/lightbend/maven-release" + | } + | ivy { + | name "lightbend-ivy-release" + | url "https://repo.lightbend.com/lightbend/ivy-releases" + | layout "ivy" + | } + |} + |/app/controllers/HomeController.java + |package controllers; + |import play.mvc.*; + |import views.html.*; + |public class HomeController extends Controller { + | public Result index() { + | return ok(index.render("Your new application is ready.")); + | } + |} + |/app/views/index.scala.html + |@(message: String) + |

@message

+ |/conf/routes + |GET / controllers.HomeController.index + |""".stripMargin, + 2, // Two files because `conf/routes` generates a Java file. + initCommand = gradleVersion("6.8") + ) } diff --git a/tests/buildTools/src/test/scala/tests/GradleDefaultJvmLanguageCompileSpecSuite.scala b/tests/buildTools/src/test/scala/tests/GradleDefaultJvmLanguageCompileSpecSuite.scala new file mode 100644 index 00000000..dd96add9 --- /dev/null +++ b/tests/buildTools/src/test/scala/tests/GradleDefaultJvmLanguageCompileSpecSuite.scala @@ -0,0 +1,29 @@ +package tests + +import scala.jdk.CollectionConverters._ + +import munit.FunSuite +import munit.TestOptions + +class GradleDefaultJvmLanguageCompileSpecSuite extends FunSuite { + val pluginpath = System.getProperty("semanticdb.pluginpath") + def check( + options: TestOptions, + original: List[String], + expected: List[String] + ): Unit = { + test(options) { + val obtained = + new GradleDefaultJvmLanguageCompileSpec(original) + .getCompileClasspath + .asScala + .map(_.toString) + .toList + assertEquals(obtained, expected) + } + } + + check("empty", List(), List(pluginpath)) + check("non-empty", List("foo"), List("foo", pluginpath)) + +} diff --git a/tests/buildTools/src/test/scala/tests/GradleOptionsBuilderSuite.scala b/tests/buildTools/src/test/scala/tests/GradleJavaCompilerArgumentsBuilderSuite.scala similarity index 89% rename from tests/buildTools/src/test/scala/tests/GradleOptionsBuilderSuite.scala rename to tests/buildTools/src/test/scala/tests/GradleJavaCompilerArgumentsBuilderSuite.scala index fe57d264..3f6aa45f 100644 --- a/tests/buildTools/src/test/scala/tests/GradleOptionsBuilderSuite.scala +++ b/tests/buildTools/src/test/scala/tests/GradleJavaCompilerArgumentsBuilderSuite.scala @@ -7,7 +7,7 @@ import scala.jdk.CollectionConverters._ import munit.FunSuite import munit.TestOptions -class GradleOptionsBuilderSuite extends FunSuite { +class GradleJavaCompilerArgumentsBuilderSuite extends FunSuite { val targetroot = System.getProperty("semanticdb.targetroot") val sourceroot = System.getProperty("semanticdb.sourceroot") val pluginpath = System.getProperty("semanticdb.pluginpath") @@ -19,7 +19,8 @@ class GradleOptionsBuilderSuite extends FunSuite { expected: List[String] ): Unit = { test(options) { - val obtained = new GradleOptionsBuilder(original).build().asScala.toList + val obtained = + new GradleJavaCompilerArgumentsBuilder(original).build().asScala.toList assertEquals(obtained, expected) } }