Unconditionally add agent in Gradle plugin, add protobuf codegen test (…

…#630) Also: * Add a protobuf generator testcase to gradle auto-indexing
sourcegraph · Aug 8, 2023 · 30378f1 · 30378f1
1 parent 0fe40e9
commit 30378f1
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 53 deletions.
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
@@ -24,20 +24,20 @@ 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;
-    //          }
-    //        });
+    // 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(
@@ -156,6 +156,7 @@ public static void build(
       }
 
       boolean isProcessorpathUpdated = false;
+      boolean semanticdbAlreadyAdded = false;
       String previousOption = "";
 
       ArrayList<String> newOptions = new ArrayList<>();
@@ -172,6 +173,10 @@ public static void build(
           case "-Xlint":
             break;
           default:
+            if (option.startsWith("-Xplugin:semanticdb")) {
+              semanticdbAlreadyAdded = true;
+              break;
+            }
             if (option.startsWith("-Xplugin:ErrorProne")) {
               break;
             }
@@ -180,33 +185,36 @@ public static void build(
         }
         previousOption = option;
       }
-      if (!isProcessorpathUpdated) {
-        newOptions.add("-classpath");
-        newOptions.add(PLUGINPATH);
-      }
-      newOptions.add(
-          String.format(
-              "-Xplugin:semanticdb -sourceroot:%s -targetroot:%s", SOURCEROOT, TARGETROOT));
-
-      if (DEBUGPATH != null) {
-        ArrayList<String> debuglines = new ArrayList<>();
-        debuglines.add("============== Java Home: " + System.getProperty("java.home"));
-        debuglines.add("============== Old Options");
-        debuglines.addAll(arguments);
-        debuglines.add("============== New Options");
-        debuglines.addAll(newOptions);
+      if (!semanticdbAlreadyAdded) {
 
-        try {
-          Files.write(
-              Paths.get(DEBUGPATH),
-              debuglines,
-              StandardOpenOption.CREATE,
-              StandardOpenOption.APPEND);
-        } catch (IOException e) {
+        if (!isProcessorpathUpdated) {
+          newOptions.add("-classpath");
+          newOptions.add(PLUGINPATH);
         }
-      }
+        newOptions.add(
+            String.format(
+                "-Xplugin:semanticdb -sourceroot:%s -targetroot:%s", SOURCEROOT, TARGETROOT));
+
+        if (DEBUGPATH != null) {
+          ArrayList<String> debuglines = new ArrayList<>();
+          debuglines.add("============== Java Home: " + System.getProperty("java.home"));
+          debuglines.add("============== Old Options");
+          debuglines.addAll(arguments);
+          debuglines.add("============== New Options");
+          debuglines.addAll(newOptions);
 
-      arguments = newOptions;
+          try {
+            Files.write(
+                Paths.get(DEBUGPATH),
+                debuglines,
+                StandardOpenOption.CREATE,
+                StandardOpenOption.APPEND);
+          } catch (IOException e) {
+          }
+        }
+
+        arguments = newOptions;
+      }
     }
   }
 }
diff --git a/semanticdb-gradle-plugin/src/main/scala/SemanticdbGradlePlugin.scala b/semanticdb-gradle-plugin/src/main/scala/SemanticdbGradlePlugin.scala
@@ -154,7 +154,7 @@ class SemanticdbGradlePlugin extends Plugin[Project] {
             task.getOptions().setFork(true)
             task.getOptions().setIncremental(false)
 
-            if (compilerPluginAdded)
+            if (compilerPluginAdded) {
               task
                 .getOptions()
                 .getCompilerArgs()
@@ -169,21 +169,32 @@ class SemanticdbGradlePlugin extends Plugin[Project] {
                     s"-Xplugin:semanticdb -targetroot:$targetRoot -sourceroot:$sourceRoot"
                   ).asJava
                 )
-            else
-              agentJar.foreach { agentpath =>
-                javacPluginJar.foreach { pluginpath =>
-                  val jvmArgs = task.getOptions.getForkOptions.getJvmArgs
-
-                  jvmArgs.addAll(
-                    List(
-                      s"-javaagent:$agentpath",
-                      s"-Dsemanticdb.pluginpath=$pluginpath",
-                      s"-Dsemanticdb.sourceroot=$sourceRoot",
-                      s"-Dsemanticdb.targetroot=$targetRoot"
-                    ).asJava
-                  )
-                }
+            }
+
+            /**
+             * In some yet to be understood cases we see that compiler plugin
+             * can be added successfully, but the correct flags are still not
+             * propagated.
+             *
+             * To work around it, we enable the agent unconditionally, and then
+             * if necessary deduplicate the arguments.
+             *
+             * TODO: figure out why this is necessary
+             */
+            agentJar.foreach { agentpath =>
+              javacPluginJar.foreach { pluginpath =>
+                val jvmArgs = task.getOptions.getForkOptions.getJvmArgs
+
+                jvmArgs.addAll(
+                  List(
+                    s"-javaagent:$agentpath",
+                    s"-Dsemanticdb.pluginpath=$pluginpath",
+                    s"-Dsemanticdb.sourceroot=$sourceRoot",
+                    s"-Dsemanticdb.targetroot=$targetRoot"
+                  ).asJava
+                )
               }
+            }
 
           }
       }

diff --git a/tests/buildTools/src/test/scala/tests/GradleBuildToolSuite.scala b/tests/buildTools/src/test/scala/tests/GradleBuildToolSuite.scala
@@ -157,6 +157,44 @@ abstract class GradleBuildToolSuite(allGradle: List[String])
     )
   }
 
+  checkGradleBuild(
+    "protobuf-generator",
+    """|/build.gradle
+       |plugins {
+       |  id "java"
+       |  id "com.google.protobuf" version "0.9.4"
+       |}
+       |dependencies {
+       |  implementation 'com.google.protobuf:protobuf-javalite:3.8.0'
+       |}
+       |protobuf {
+       |  protoc {
+       |    artifact = 'com.google.protobuf:protoc:3.23.4'
+       |  }
+       |  generateProtoTasks {
+       |    all().configureEach { task ->
+       |      task.builtins {
+       |        java {
+       |          option "lite"
+       |        }
+       |      }
+       |    }
+       |  }
+       |}
+       |/src/main/proto/message.proto
+       |syntax = "proto3";
+       |message SearchRequest {
+       |  string query = 1;
+       |  int32 page_number = 2;
+       |  int32 results_per_page = 3;
+       |}
+       |/src/main/java/Example.java
+       |public class Example {}
+       |""".stripMargin,
+    expectedSemanticdbFiles = 2,
+    gradleVersions = List(Gradle8, Gradle7, Gradle67)
+  )
+
   checkGradleBuild(
     "explicit",
     """|/build.gradle