Replace Gradle init scripts with a proper Gradle plugin #565

keynmol · 2023-04-25T16:35:28Z

This PR replicates the functionality of the init script injection with a proper Gradle plugin + a much smaller init script.

Benefits:

You can edit the plugin code in IDE - it's a normal SBT module
Cleaner and doesn't rely as much on values injected as strings
More flexible as it can be added to the builds as both an assembly jar (for auto-indexing) or a maven central dependency (in the future)
Works for gradle 6, 7, and 8, toolchains included (tested on jdk 8, 11, 17)
Most of the logic is done inside the plugin, not in combination of scip-java and init script

Additionally:

Run CI for Java 11
Run tests for 3 major gradle versions
Produce a meaningful message if we get an exception from javac

Old TODO list

TODO: - [x] Restore kotlin multi-platform support - [x] Add dependency dump task - [x] Modify tests to run on various gradle versions - [x] Disable publishing of the gradle plugin for _now_ Currently it's written in Scala as it allows for much faster prototyping. With it not being published but rather injected via assembly, it doesn't matter what language it's written in.

Once we have this merged and tested in the wild, we can start the process of rewriting the plugin in Java (for maximum compatibility) and publish it - after that, the docs can be modified and all manual steps removed.

Old description from where I was just a young pup helplessly thrashing about

TODO: - [x] Kotlin support - [x] Rewrite init script > One main limitation of init scripts is that they cannot access classes in the buildSrc project (see [Using buildSrc to extract imperative logic](https://docs.gradle.org/current/userguide/organizing_gradle_projects.html#sec:build_sources) for details of this feature). ![](https://media.giphy.com/media/vyTnNTrs3wqQ0UIvwE/giphy.gif)

Current plan:

Make a proper gradle plugin that does all the steps that Manual Configuration for Gradle performs
Inject plugin from GradleBuildTool
Convert to SBT
See https://github.com/scalacenter/gradle-bloop/blob/main/build.sbt as an example of publishing a Gradle plugin from SBT

Test plan

Extra tests for Gradle versions: 6.7 (introduced toolchains), 7.5.1 (latest in 7.x lineage), 8.1.1 (latest)
Confirm all old tests are passing

toolchains

Our approach can fail if the build modifies the compile classpath as part of the build definition: > Cannot change dependencies of dependency configuration ':compileOnly' after it has been included in dependency resolution. In this case we go the agent route

keynmol · 2023-05-05T12:14:18Z

semanticdb-gradle-plugin/src/main/scala/SemanticdbGradlePlugin.scala

+      val javacDep = javacPluginJar
+        .map[Object](jar => project.files(jar))
+        .getOrElse(s"com.sourcegraph:semanticdb-javac:${javacPluginVersion}")


We fallback to javac plugin published to maven if there is no jar specified

Could have been a decent inline comment instead of PR comment

keynmol · 2023-05-05T12:15:18Z

semanticdb-gradle-plugin/src/main/scala/SemanticdbGradlePlugin.scala

+              System
+                .err
+                .println(
+                  s"Failed to add compiler plugin to javac, will go through the agent route: ${exc.getMessage()}"


At the time when the plugin is loaded, compileOnly configuration might already be resolved, if triggered by something in the build: https://sourcegraph.com/github.com/awslabs/smithy-language-server/-/blob/build.gradle?L212

keynmol · 2023-05-05T12:15:43Z

semanticdb-gradle-plugin/src/main/scala/SemanticdbGradlePlugin.scala

+              println(
+                task
+                  .asInstanceOf[{
+                      def getToolChain(): Any
+                    }
+                  ]
+                  .getToolChain()
+              )


TODO: see what we can do about this.

Given Gradle's compatibility matrix (only 7.3+ supports Java 17) this branch doesn't need anything.

I have a spearate PR coming that will clean this up among other things

keynmol · 2023-05-05T12:16:00Z

semanticdb-gradle-plugin/src/main/scala/SemanticdbGradlePlugin.scala

+              val host = System
+                .getProperty("java.version")
+                .split("\\.")
+                .headOption
+                .map(_.toInt)


only at play where there is no toolchain provided

olafurpg

This looks really really good 👏🏻 Nice work @keynmol

I love the plugin approach, it's a much cleaner solution than the unmaintainable init script. I can imagine us doing a lot more with Gradle using this plugin approach.

No blocking comments, just two nitpick suggestions to add type annotations. I'm fine with deleting the init script and moving into this bright new future 🌞

olafurpg · 2023-05-09T09:15:05Z

scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/GradleBuildTool.scala

+      s"""
+         | initscript {
+         |     dependencies{ 
+         |         classpath(files("${gradlePluginPath}"))


TIL very cool 👍🏻

olafurpg · 2023-05-09T09:15:23Z

scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/GradleBuildTool.scala

+         |   project.extra["javacPluginJar"] = "$pluginpath"
+         |   project.extra["dependenciesOut"] = "$dependenciesPath"
+         |   project.extra["javacAgentPath"] = "$agentpath"
+         |   apply<SemanticdbGradlePlugin>()


🥳 🥳 😍

olafurpg · 2023-05-09T09:16:07Z

semanticdb-gradle-plugin/src/main/scala/SemanticdbGradlePlugin.scala

+      val javacDep = javacPluginJar
+        .map[Object](jar => project.files(jar))
+        .getOrElse(s"com.sourcegraph:semanticdb-javac:${javacPluginVersion}")


Could have been a decent inline comment instead of PR comment

olafurpg · 2023-05-09T09:18:36Z

semanticdb-gradle-plugin/src/main/scala/SemanticdbGradlePlugin.scala

+              s"-Xplugin:$semanticdbScalac",
+              s"-P:semanticdb:sourceroot:$sourceRoot",
+              s"-P:semanticdb:targetroot:$targetRoot",
+              s"-P:semanticdb:exclude:(src/play/twirl|src/play/routes|src/${System.currentTimeMillis()})", // Ignore autogenerated Playframework files


Nice 👍🏻

olafurpg · 2023-05-09T09:19:11Z

semanticdb-gradle-plugin/src/main/scala/SemanticdbGradlePlugin.scala

+              //
+              // Instead, we commit the sins of reflection for our limited
+              // needs.
+              val compilerArgs = task


this is fine

scip-java/src/main/scala/com/sourcegraph/scip_java/buildtools/GradleBuildTool.scala

semanticdb-gradle-plugin/src/main/scala/SemanticdbGradlePlugin.scala

Because compilation is much more likely to fail halfway through

keynmol added 5 commits May 4, 2023 09:22

WIP: Injectable gradle plugin

ffc3919

Run tests on CI

8c0b510

Actually run the tests

2a9011c

WIP

6ce3e82

Add Gradle plugin written in Scala

b8e12a1

keynmol force-pushed the gradle-plugin branch from 9c4b8dc to b8e12a1 Compare May 4, 2023 09:47

keynmol added 14 commits May 4, 2023 12:16

Target Gradle 7, but handle earlier versions as well

8cd632d

Generate semanticdb in the proper build directory

85e116f

Add random timestamp flag to force Gradle's recompilation

0b3e255

Embed gradle plugin assembly in cli resources

1d33b0d

Configurable semanticdb root, correctly handle jvm args without

26693a4

toolchains

Use javacplugin jar instead of maven dependency

6d66811

No boom please

ef21548

Restore kotlin support!

6d4b9fb

Delete kotlin version of the plugin

85cf780

delete gradle tests from workflows

e875f43

Restore kotlin multiplatform support

f9a37eb

Restore dependency printing support

308ec56

Add tests on a grid of Gradle versions

8f786bc

Clean up tests

3b30c7a

keynmol marked this pull request as ready for review May 5, 2023 09:43

keynmol added 9 commits May 5, 2023 10:46

Huh?

986cd9e

Add Java 11 to tests

9e9ee8c

Don't use new APIs on java.util.List

7eef259

Compile gradle plugin with JDK 8

b2f6bf9

Disable non-local publishing of Gradle plugin

631e347

Extract Kotlin Gradle plugin version

778b78e

Merge branch 'main' into gradle-plugin

b94fd27

Inject java agent into the forked Zinc JVM args

5801d20

keynmol added 6 commits May 5, 2023 11:58

Don't run Gradle 6.7 tests on anything other java 8 or 11

2b44ac7

temp: run only the broken tests on 17

bad92f4

Fix test run command

7b8cca3

Build agent on Java 8?

623eb63

Revert back to Java 8 only tag

d063538

Restore test matrix

77b4c0e

keynmol changed the title ~~WIP: Injectable gradle plugin~~ Replace Gradle init scripts with a proper Gradle plugin May 5, 2023

keynmol commented May 5, 2023

View reviewed changes

olafurpg self-requested a review May 9, 2023 06:31

olafurpg approved these changes May 9, 2023

View reviewed changes

keynmol added 3 commits May 9, 2023 10:53

Print dependencies first, and then compile

ab5f10d

Because compilation is much more likely to fail halfway through

Add comments and return type annotations

c3e124e

chore: formatting

baafb70

keynmol mentioned this pull request May 9, 2023

WIP: Fix gradle tool to work with Gradle 8.0+ #549

Closed

Merge branch 'main' into gradle-plugin

325d07d

keynmol merged commit 3ea8d94 into main May 10, 2023

keynmol deleted the gradle-plugin branch May 10, 2023 09:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace Gradle init scripts with a proper Gradle plugin #565

Replace Gradle init scripts with a proper Gradle plugin #565

keynmol commented Apr 25, 2023 •

edited

Loading

keynmol May 5, 2023

olafurpg May 9, 2023

keynmol May 9, 2023

keynmol May 5, 2023

keynmol May 5, 2023

keynmol May 10, 2023

keynmol May 5, 2023

olafurpg left a comment

olafurpg May 9, 2023

olafurpg May 9, 2023

olafurpg May 9, 2023

olafurpg May 9, 2023

olafurpg May 9, 2023

Replace Gradle init scripts with a proper Gradle plugin #565

Replace Gradle init scripts with a proper Gradle plugin #565

Conversation

keynmol commented Apr 25, 2023 • edited Loading

Test plan

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olafurpg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keynmol commented Apr 25, 2023 •

edited

Loading