diff --git a/scip-gradle-plugin/src/main/java/org/scip_code/scip_java/gradle/ScipGradlePlugin.java b/scip-gradle-plugin/src/main/java/org/scip_code/scip_java/gradle/ScipGradlePlugin.java index 7bb2c45ab..e4c9845b9 100644 --- a/scip-gradle-plugin/src/main/java/org/scip_code/scip_java/gradle/ScipGradlePlugin.java +++ b/scip-gradle-plugin/src/main/java/org/scip_code/scip_java/gradle/ScipGradlePlugin.java @@ -20,6 +20,8 @@ import org.gradle.api.tasks.Input; import org.gradle.api.tasks.SourceSet; import org.gradle.api.tasks.SourceSetContainer; +import org.gradle.api.tasks.TaskProvider; +import org.gradle.api.tasks.TaskState; import org.gradle.api.tasks.compile.CompileOptions; import org.gradle.api.tasks.compile.JavaCompile; import org.gradle.process.CommandLineArgumentProvider; @@ -137,11 +139,43 @@ private void configureProject(Project project) { } } + // Kotlin sources are not indexed inside kotlinc: after the Kotlin compile + // tasks, their sources and classpath are dumped for the scip-java CLI to + // index after the build, decoupling indexing from the build's Kotlin + // compiler version. The dump is a finalizer rather than an action of the + // compile tasks, because Gradle skips their actions when they are up to + // date or restored from the build cache, while the CLI deletes the + // targetroot before every build. The finalizer declares no outputs, so + // Gradle runs it every time. + List kotlinCompileTasks = new ArrayList<>(); + TaskProvider kotlinConfigs = + project + .getTasks() + .register( + "scipKotlinConfigs", + export -> + export.doLast( + ignored -> { + for (Task task : kotlinCompileTasks) { + // Only tasks of this build that succeeded; up-to-date + // and cached tasks count as executed. + TaskState state = task.getState(); + if (state.getExecuted() && state.getFailure() == null) { + writeKotlinConfig(task, sourceRoot, targetRoot); + } + } + })); // Not limited to the plugin checks above: AGP 9's built-in Kotlin adds // Kotlin compile tasks to Android modules without applying a Kotlin plugin. project .getTasks() - .configureEach(task -> configureKotlinCompileTask(task, sourceRoot, targetRoot)); + .configureEach( + task -> { + if (task.getClass().getSimpleName().contains("KotlinCompile")) { + kotlinCompileTasks.add(task); + task.finalizedBy(kotlinConfigs); + } + }); project.getTasks().create("scipCompileAll").dependsOn(triggers); @@ -237,71 +271,62 @@ private static Set addJavacPlugin(Project project, Object javacPluginDep } /** - * Kotlin sources are not indexed inside kotlinc: after each Kotlin compile task runs, its sources - * and classpath are dumped to {@code /kotlin-configs/.txt} for the scip-java - * CLI to index after the build, decoupling indexing from the build's Kotlin compiler version. + * Dumps the sources and classpath of a Kotlin compile task to {@code kotlin-configs/.txt}. */ - private static void configureKotlinCompileTask(Task task, String sourceRoot, String targetRoot) { - if (!task.getClass().getSimpleName().contains("KotlinCompile")) { - return; - } + private static void writeKotlinConfig(Task task, String sourceRoot, String targetRoot) { + try { + List lines = new ArrayList<>(); + lines.add("sourceroot " + sourceRoot); - task.doLast( - ignored -> { - try { - List lines = new ArrayList<>(); - lines.add("sourceroot " + sourceRoot); - - // Referring to KotlinCompile directly triggers NoClassDefFoundError - - // the plugin classpath is murky and we deliberately don't bundle the - // Kotlin Gradle plugin. So we commit the sins of reflection. - FileCollection libraries; - try { - libraries = (FileCollection) task.getClass().getMethod("getLibraries").invoke(task); - } catch (ReflectiveOperationException exc) { - libraries = (FileCollection) task.getClass().getMethod("getClasspath").invoke(task); - } - for (File entry : libraries.getFiles()) { - lines.add("classpath " + entry.getAbsolutePath()); - } - - int sources = 0; - for (File file : task.getInputs().getSourceFiles().getFiles()) { - if (file.getName().endsWith(".kt") || file.getName().endsWith(".kts")) { - lines.add("source " + file.getAbsolutePath()); - sources++; - } - } - // Java sources of the same module are listed too, so Kotlin - // references to them resolve. The indexer only emits Kotlin files. - try { - FileCollection javaSources = - (FileCollection) task.getClass().getMethod("getJavaSources").invoke(task); - for (File file : javaSources.getFiles()) { - if (file.getName().endsWith(".java")) { - lines.add("source " + file.getAbsolutePath()); - } - } - } catch (ReflectiveOperationException exc) { - // Kotlin Gradle Plugin without javaSources (pre 1.7). - } - if (sources == 0) { - return; - } - - Path configDir = Paths.get(targetRoot, "kotlin-configs"); - Files.createDirectories(configDir); - String fileName = task.getPath().replaceAll("[^A-Za-z0-9]", "-") + ".txt"; - Files.write(configDir.resolve(fileName), lines); - } catch (Exception exc) { - throw new RuntimeException( - "scip-java: failed to extract Kotlin compile configuration for task '" - + task.getName() - + "': " - + exc, - exc); + // Referring to KotlinCompile directly triggers NoClassDefFoundError - + // the plugin classpath is murky and we deliberately don't bundle the + // Kotlin Gradle plugin. So we commit the sins of reflection. + FileCollection libraries; + try { + libraries = (FileCollection) task.getClass().getMethod("getLibraries").invoke(task); + } catch (ReflectiveOperationException exc) { + libraries = (FileCollection) task.getClass().getMethod("getClasspath").invoke(task); + } + for (File entry : libraries.getFiles()) { + lines.add("classpath " + entry.getAbsolutePath()); + } + + int sources = 0; + for (File file : task.getInputs().getSourceFiles().getFiles()) { + if (file.getName().endsWith(".kt") || file.getName().endsWith(".kts")) { + lines.add("source " + file.getAbsolutePath()); + sources++; + } + } + // Java sources of the same module are listed too, so Kotlin + // references to them resolve. The indexer only emits Kotlin files. + try { + FileCollection javaSources = + (FileCollection) task.getClass().getMethod("getJavaSources").invoke(task); + for (File file : javaSources.getFiles()) { + if (file.getName().endsWith(".java")) { + lines.add("source " + file.getAbsolutePath()); } - }); + } + } catch (ReflectiveOperationException exc) { + // Kotlin Gradle Plugin without javaSources (pre 1.7). + } + if (sources == 0) { + return; + } + + Path configDir = Paths.get(targetRoot, "kotlin-configs"); + Files.createDirectories(configDir); + String fileName = task.getPath().replaceAll("[^A-Za-z0-9]", "-") + ".txt"; + Files.write(configDir.resolve(fileName), lines); + } catch (Exception exc) { + throw new RuntimeException( + "scip-java: failed to extract Kotlin compile configuration for task '" + + task.getName() + + "': " + + exc, + exc); + } } private static Object requiredExtra(Map extraProperties, String name) { diff --git a/scip-java/src/main/kotlin/org/scip_code/scip_java/buildtools/GradleBuildTool.kt b/scip-java/src/main/kotlin/org/scip_code/scip_java/buildtools/GradleBuildTool.kt index 511d30834..c95ab9563 100644 --- a/scip-java/src/main/kotlin/org/scip_code/scip_java/buildtools/GradleBuildTool.kt +++ b/scip-java/src/main/kotlin/org/scip_code/scip_java/buildtools/GradleBuildTool.kt @@ -94,6 +94,10 @@ This means our SCIP compiler plugin was not attached to one or more JavaCompile cmd += script cmd += "-Dscip.targetroot=${targetroot()}" cmd += index.finalBuildCommand(listOf("clean", "scipPrintDependencies", "scipCompileAll")) + // The Gradle plugin isn't compatible with the configuration cache, which + // a build can enable in gradle.properties. Last, because the last of + // --configuration-cache and --no-configuration-cache wins. + cmd += "--no-configuration-cache" targetroot().toFile().deleteRecursively() val result = index.app.runProcess(cmd, env = mapOf("TERM" to "dumb")) diff --git a/scip-java/src/test/kotlin/tests/BuildToolHarness.kt b/scip-java/src/test/kotlin/tests/BuildToolHarness.kt index 2ea39aa09..e1692b14b 100644 --- a/scip-java/src/test/kotlin/tests/BuildToolHarness.kt +++ b/scip-java/src/test/kotlin/tests/BuildToolHarness.kt @@ -113,6 +113,7 @@ abstract class BuildToolHarness { targetRoot: String? = null, maxJdk: Int? = null, substitutions: Map = emptyMap(), + runs: Int = 1, ): DynamicTest = dynamicTest(name) { // Some build tools cap the JDK they support; skip rather than fail @@ -143,21 +144,25 @@ abstract class BuildToolHarness { targetroot.toString(), ) + extraArguments - val (exit, output) = runScipJava(workingDirectory, arguments) + // Later runs reuse the checkout, so they cover state left by + // earlier builds (up-to-date tasks, build cache entries). + for (run in 1..runs) { + val (exit, output) = runScipJava(workingDirectory, arguments) - if (expectedError != null) { - assertNotEquals(0, exit, "expected failure, output:\n$output") - expectedError(output) - } else { - assertEquals(0, exit, "expected success, output:\n$output") - } + if (expectedError != null) { + assertNotEquals(0, exit, "expected failure, output:\n$output") + expectedError(output) + } else { + assertEquals(0, exit, "expected success, output:\n$output") + } - val shards = listScipShards(targetroot) - if (shards.size != expectedScipFiles) { - fail( - "Expected $expectedScipFiles SCIP shard(s), got ${shards.size}: " + - "$shards\noutput:\n$output" - ) + val shards = listScipShards(targetroot) + if (shards.size != expectedScipFiles) { + fail( + "Run $run: expected $expectedScipFiles SCIP shard(s), got ${shards.size}: " + + "$shards\noutput:\n$output" + ) + } } if (expectedPackages.isNotEmpty()) { diff --git a/scip-java/src/test/kotlin/tests/GradleBuildToolTest.kt b/scip-java/src/test/kotlin/tests/GradleBuildToolTest.kt index 937095327..cdf0f6bbf 100644 --- a/scip-java/src/test/kotlin/tests/GradleBuildToolTest.kt +++ b/scip-java/src/test/kotlin/tests/GradleBuildToolTest.kt @@ -20,6 +20,7 @@ class GradleBuildToolTest : BuildToolHarness() { expectedPackages: String = "", extraArguments: List = emptyList(), substitutions: Map = emptyMap(), + runs: Int = 1, ): DynamicTest = checkBuild( "$name-gradle-$GRADLE_VERSION", @@ -28,6 +29,7 @@ class GradleBuildToolTest : BuildToolHarness() { expectedPackages = expectedPackages, extraArguments = extraArguments, substitutions = substitutions, + runs = runs, initCommand = { workingDirectory -> // Gradle needs a build script present to generate the wrapper; // the real one is laid down afterwards from the fixture. @@ -161,7 +163,47 @@ class GradleBuildToolTest : BuildToolHarness() { extraArguments = listOf("--", "compileJava"), ) ) + // Regression tests: the configuration cache is disabled, both when the + // build enables it and when the build command asks for it. Otherwise + // scipPrintDependencies fails, and a second run reuses the configuration + // cache entry of the first, which leaves compileJava up to date. + add( + checkGradleBuild( + "configuration-cache", + "gradle/configuration-cache", + expectedScipFiles = 1, + ) + ) + add( + checkGradleBuild( + "java-configuration-cache", + "gradle/build-command", + expectedScipFiles = 1, + extraArguments = listOf("--", "--configuration-cache", "compileJava"), + runs = 2, + ) + ) add(checkGradleBuild("kotlin2", "gradle/kotlin2", expectedScipFiles = 4)) + // Regression tests: Kotlin sources must be indexed again when the Kotlin + // compile tasks are restored from the build cache, or are up to date + // because the build command doesn't clean. + add( + checkGradleBuild( + "kotlin-build-cache", + "gradle/kotlin-build-cache", + expectedScipFiles = 4, + runs = 2, + ) + ) + add( + checkGradleBuild( + "kotlin-up-to-date", + "gradle/kotlin2", + expectedScipFiles = 2, + extraArguments = listOf("--", "compileKotlin", "compileJava"), + runs = 2, + ) + ) add( checkGradleBuild( "implementation-deps", diff --git a/scip-java/src/test/resources/fixtures/gradle/configuration-cache/build.gradle b/scip-java/src/test/resources/fixtures/gradle/configuration-cache/build.gradle new file mode 100644 index 000000000..bbfeb03c2 --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/configuration-cache/build.gradle @@ -0,0 +1 @@ +apply plugin: 'java' diff --git a/scip-java/src/test/resources/fixtures/gradle/configuration-cache/gradle.properties b/scip-java/src/test/resources/fixtures/gradle/configuration-cache/gradle.properties new file mode 100644 index 000000000..5ad69748c --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/configuration-cache/gradle.properties @@ -0,0 +1 @@ +org.gradle.configuration-cache=true diff --git a/scip-java/src/test/resources/fixtures/gradle/configuration-cache/src/main/java/Example.java b/scip-java/src/test/resources/fixtures/gradle/configuration-cache/src/main/java/Example.java new file mode 100644 index 000000000..0f5fefd30 --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/configuration-cache/src/main/java/Example.java @@ -0,0 +1 @@ +public class Example {} diff --git a/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/build.gradle b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/build.gradle new file mode 100644 index 000000000..b559907c9 --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/build.gradle @@ -0,0 +1,9 @@ +plugins { + id 'org.jetbrains.kotlin.jvm' version '2.4.0' +} +kotlin { + jvmToolchain(17) +} +repositories { + mavenCentral() +} diff --git a/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/gradle.properties b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/gradle.properties new file mode 100644 index 000000000..160890028 --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/gradle.properties @@ -0,0 +1 @@ +org.gradle.caching=true diff --git a/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/settings.gradle b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/settings.gradle new file mode 100644 index 000000000..067e7d885 --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/settings.gradle @@ -0,0 +1,7 @@ +// A build cache inside the checkout, so every test starts with an empty +// cache and leaves the user's Gradle home untouched. +buildCache { + local { + directory = file('build-cache') + } +} diff --git a/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/src/main/java/foo/JExample.java b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/src/main/java/foo/JExample.java new file mode 100644 index 000000000..b46e167d9 --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/src/main/java/foo/JExample.java @@ -0,0 +1,2 @@ +package foo; +public class JExample {} diff --git a/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/src/main/kotlin/foo/Example.kt b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/src/main/kotlin/foo/Example.kt new file mode 100644 index 000000000..26f3d6294 --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/src/main/kotlin/foo/Example.kt @@ -0,0 +1,2 @@ +package foo +object Example {} diff --git a/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/src/test/java/foo/JExampleSuite.java b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/src/test/java/foo/JExampleSuite.java new file mode 100644 index 000000000..cc39874dd --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/src/test/java/foo/JExampleSuite.java @@ -0,0 +1,2 @@ +package foo; +public class JExampleSuite {} diff --git a/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/src/test/kotlin/foo/ExampleSuite.kt b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/src/test/kotlin/foo/ExampleSuite.kt new file mode 100644 index 000000000..67303e2ee --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/src/test/kotlin/foo/ExampleSuite.kt @@ -0,0 +1,2 @@ +package foo +class ExampleSuite {}