Skip to content

Commit bdceafa

Browse files
christoph-sgchristoph-sg
authored
fix/gradle: don't skip the Kotlin config export for cached compiles (#1059)
Also disable the configuration cache for indexing runs
1 parent 5a9cbe1 commit bdceafa

14 files changed

Lines changed: 180 additions & 76 deletions

File tree

‎scip-gradle-plugin/src/main/java/org/scip_code/scip_java/gradle/ScipGradlePlugin.java‎

Lines changed: 88 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
import org.gradle.api.tasks.Input;
2121
import org.gradle.api.tasks.SourceSet;
2222
import org.gradle.api.tasks.SourceSetContainer;
23+
import org.gradle.api.tasks.TaskProvider;
24+
import org.gradle.api.tasks.TaskState;
2325
import org.gradle.api.tasks.compile.CompileOptions;
2426
import org.gradle.api.tasks.compile.JavaCompile;
2527
import org.gradle.process.CommandLineArgumentProvider;
@@ -137,11 +139,43 @@ private void configureProject(Project project) {
137139
}
138140
}
139141

142+
// Kotlin sources are not indexed inside kotlinc: after the Kotlin compile
143+
// tasks, their sources and classpath are dumped for the scip-java CLI to
144+
// index after the build, decoupling indexing from the build's Kotlin
145+
// compiler version. The dump is a finalizer rather than an action of the
146+
// compile tasks, because Gradle skips their actions when they are up to
147+
// date or restored from the build cache, while the CLI deletes the
148+
// targetroot before every build. The finalizer declares no outputs, so
149+
// Gradle runs it every time.
150+
List<Task> kotlinCompileTasks = new ArrayList<>();
151+
TaskProvider<Task> kotlinConfigs =
152+
project
153+
.getTasks()
154+
.register(
155+
"scipKotlinConfigs",
156+
export ->
157+
export.doLast(
158+
ignored -> {
159+
for (Task task : kotlinCompileTasks) {
160+
// Only tasks of this build that succeeded; up-to-date
161+
// and cached tasks count as executed.
162+
TaskState state = task.getState();
163+
if (state.getExecuted() && state.getFailure() == null) {
164+
writeKotlinConfig(task, sourceRoot, targetRoot);
165+
}
166+
}
167+
}));
140168
// Not limited to the plugin checks above: AGP 9's built-in Kotlin adds
141169
// Kotlin compile tasks to Android modules without applying a Kotlin plugin.
142170
project
143171
.getTasks()
144-
.configureEach(task -> configureKotlinCompileTask(task, sourceRoot, targetRoot));
172+
.configureEach(
173+
task -> {
174+
if (task.getClass().getSimpleName().contains("KotlinCompile")) {
175+
kotlinCompileTasks.add(task);
176+
task.finalizedBy(kotlinConfigs);
177+
}
178+
});
145179

146180
project.getTasks().create("scipCompileAll").dependsOn(triggers);
147181

@@ -237,71 +271,62 @@ private static Set<String> addJavacPlugin(Project project, Object javacPluginDep
237271
}
238272

239273
/**
240-
* Kotlin sources are not indexed inside kotlinc: after each Kotlin compile task runs, its sources
241-
* and classpath are dumped to {@code <targetroot>/kotlin-configs/<task>.txt} for the scip-java
242-
* CLI to index after the build, decoupling indexing from the build's Kotlin compiler version.
274+
* Dumps the sources and classpath of a Kotlin compile task to {@code kotlin-configs/<task>.txt}.
243275
*/
244-
private static void configureKotlinCompileTask(Task task, String sourceRoot, String targetRoot) {
245-
if (!task.getClass().getSimpleName().contains("KotlinCompile")) {
246-
return;
247-
}
276+
private static void writeKotlinConfig(Task task, String sourceRoot, String targetRoot) {
277+
try {
278+
List<String> lines = new ArrayList<>();
279+
lines.add("sourceroot " + sourceRoot);
248280

249-
task.doLast(
250-
ignored -> {
251-
try {
252-
List<String> lines = new ArrayList<>();
253-
lines.add("sourceroot " + sourceRoot);
254-
255-
// Referring to KotlinCompile directly triggers NoClassDefFoundError -
256-
// the plugin classpath is murky and we deliberately don't bundle the
257-
// Kotlin Gradle plugin. So we commit the sins of reflection.
258-
FileCollection libraries;
259-
try {
260-
libraries = (FileCollection) task.getClass().getMethod("getLibraries").invoke(task);
261-
} catch (ReflectiveOperationException exc) {
262-
libraries = (FileCollection) task.getClass().getMethod("getClasspath").invoke(task);
263-
}
264-
for (File entry : libraries.getFiles()) {
265-
lines.add("classpath " + entry.getAbsolutePath());
266-
}
267-
268-
int sources = 0;
269-
for (File file : task.getInputs().getSourceFiles().getFiles()) {
270-
if (file.getName().endsWith(".kt") || file.getName().endsWith(".kts")) {
271-
lines.add("source " + file.getAbsolutePath());
272-
sources++;
273-
}
274-
}
275-
// Java sources of the same module are listed too, so Kotlin
276-
// references to them resolve. The indexer only emits Kotlin files.
277-
try {
278-
FileCollection javaSources =
279-
(FileCollection) task.getClass().getMethod("getJavaSources").invoke(task);
280-
for (File file : javaSources.getFiles()) {
281-
if (file.getName().endsWith(".java")) {
282-
lines.add("source " + file.getAbsolutePath());
283-
}
284-
}
285-
} catch (ReflectiveOperationException exc) {
286-
// Kotlin Gradle Plugin without javaSources (pre 1.7).
287-
}
288-
if (sources == 0) {
289-
return;
290-
}
291-
292-
Path configDir = Paths.get(targetRoot, "kotlin-configs");
293-
Files.createDirectories(configDir);
294-
String fileName = task.getPath().replaceAll("[^A-Za-z0-9]", "-") + ".txt";
295-
Files.write(configDir.resolve(fileName), lines);
296-
} catch (Exception exc) {
297-
throw new RuntimeException(
298-
"scip-java: failed to extract Kotlin compile configuration for task '"
299-
+ task.getName()
300-
+ "': "
301-
+ exc,
302-
exc);
281+
// Referring to KotlinCompile directly triggers NoClassDefFoundError -
282+
// the plugin classpath is murky and we deliberately don't bundle the
283+
// Kotlin Gradle plugin. So we commit the sins of reflection.
284+
FileCollection libraries;
285+
try {
286+
libraries = (FileCollection) task.getClass().getMethod("getLibraries").invoke(task);
287+
} catch (ReflectiveOperationException exc) {
288+
libraries = (FileCollection) task.getClass().getMethod("getClasspath").invoke(task);
289+
}
290+
for (File entry : libraries.getFiles()) {
291+
lines.add("classpath " + entry.getAbsolutePath());
292+
}
293+
294+
int sources = 0;
295+
for (File file : task.getInputs().getSourceFiles().getFiles()) {
296+
if (file.getName().endsWith(".kt") || file.getName().endsWith(".kts")) {
297+
lines.add("source " + file.getAbsolutePath());
298+
sources++;
299+
}
300+
}
301+
// Java sources of the same module are listed too, so Kotlin
302+
// references to them resolve. The indexer only emits Kotlin files.
303+
try {
304+
FileCollection javaSources =
305+
(FileCollection) task.getClass().getMethod("getJavaSources").invoke(task);
306+
for (File file : javaSources.getFiles()) {
307+
if (file.getName().endsWith(".java")) {
308+
lines.add("source " + file.getAbsolutePath());
303309
}
304-
});
310+
}
311+
} catch (ReflectiveOperationException exc) {
312+
// Kotlin Gradle Plugin without javaSources (pre 1.7).
313+
}
314+
if (sources == 0) {
315+
return;
316+
}
317+
318+
Path configDir = Paths.get(targetRoot, "kotlin-configs");
319+
Files.createDirectories(configDir);
320+
String fileName = task.getPath().replaceAll("[^A-Za-z0-9]", "-") + ".txt";
321+
Files.write(configDir.resolve(fileName), lines);
322+
} catch (Exception exc) {
323+
throw new RuntimeException(
324+
"scip-java: failed to extract Kotlin compile configuration for task '"
325+
+ task.getName()
326+
+ "': "
327+
+ exc,
328+
exc);
329+
}
305330
}
306331

307332
private static Object requiredExtra(Map<String, Object> extraProperties, String name) {

‎scip-java/src/main/kotlin/org/scip_code/scip_java/buildtools/GradleBuildTool.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,10 @@ This means our SCIP compiler plugin was not attached to one or more JavaCompile
9494
cmd += script
9595
cmd += "-Dscip.targetroot=${targetroot()}"
9696
cmd += index.finalBuildCommand(listOf("clean", "scipPrintDependencies", "scipCompileAll"))
97+
// The Gradle plugin isn't compatible with the configuration cache, which
98+
// a build can enable in gradle.properties. Last, because the last of
99+
// --configuration-cache and --no-configuration-cache wins.
100+
cmd += "--no-configuration-cache"
97101

98102
targetroot().toFile().deleteRecursively()
99103
val result = index.app.runProcess(cmd, env = mapOf("TERM" to "dumb"))

‎scip-java/src/test/kotlin/tests/BuildToolHarness.kt‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ abstract class BuildToolHarness {
113113
targetRoot: String? = null,
114114
maxJdk: Int? = null,
115115
substitutions: Map<String, String> = emptyMap(),
116+
runs: Int = 1,
116117
): DynamicTest =
117118
dynamicTest(name) {
118119
// Some build tools cap the JDK they support; skip rather than fail
@@ -143,21 +144,25 @@ abstract class BuildToolHarness {
143144
targetroot.toString(),
144145
) + extraArguments
145146

146-
val (exit, output) = runScipJava(workingDirectory, arguments)
147+
// Later runs reuse the checkout, so they cover state left by
148+
// earlier builds (up-to-date tasks, build cache entries).
149+
for (run in 1..runs) {
150+
val (exit, output) = runScipJava(workingDirectory, arguments)
147151

148-
if (expectedError != null) {
149-
assertNotEquals(0, exit, "expected failure, output:\n$output")
150-
expectedError(output)
151-
} else {
152-
assertEquals(0, exit, "expected success, output:\n$output")
153-
}
152+
if (expectedError != null) {
153+
assertNotEquals(0, exit, "expected failure, output:\n$output")
154+
expectedError(output)
155+
} else {
156+
assertEquals(0, exit, "expected success, output:\n$output")
157+
}
154158

155-
val shards = listScipShards(targetroot)
156-
if (shards.size != expectedScipFiles) {
157-
fail(
158-
"Expected $expectedScipFiles SCIP shard(s), got ${shards.size}: " +
159-
"$shards\noutput:\n$output"
160-
)
159+
val shards = listScipShards(targetroot)
160+
if (shards.size != expectedScipFiles) {
161+
fail(
162+
"Run $run: expected $expectedScipFiles SCIP shard(s), got ${shards.size}: " +
163+
"$shards\noutput:\n$output"
164+
)
165+
}
161166
}
162167

163168
if (expectedPackages.isNotEmpty()) {

‎scip-java/src/test/kotlin/tests/GradleBuildToolTest.kt‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ class GradleBuildToolTest : BuildToolHarness() {
2020
expectedPackages: String = "",
2121
extraArguments: List<String> = emptyList(),
2222
substitutions: Map<String, String> = emptyMap(),
23+
runs: Int = 1,
2324
): DynamicTest =
2425
checkBuild(
2526
"$name-gradle-$GRADLE_VERSION",
@@ -28,6 +29,7 @@ class GradleBuildToolTest : BuildToolHarness() {
2829
expectedPackages = expectedPackages,
2930
extraArguments = extraArguments,
3031
substitutions = substitutions,
32+
runs = runs,
3133
initCommand = { workingDirectory ->
3234
// Gradle needs a build script present to generate the wrapper;
3335
// the real one is laid down afterwards from the fixture.
@@ -161,7 +163,47 @@ class GradleBuildToolTest : BuildToolHarness() {
161163
extraArguments = listOf("--", "compileJava"),
162164
)
163165
)
166+
// Regression tests: the configuration cache is disabled, both when the
167+
// build enables it and when the build command asks for it. Otherwise
168+
// scipPrintDependencies fails, and a second run reuses the configuration
169+
// cache entry of the first, which leaves compileJava up to date.
170+
add(
171+
checkGradleBuild(
172+
"configuration-cache",
173+
"gradle/configuration-cache",
174+
expectedScipFiles = 1,
175+
)
176+
)
177+
add(
178+
checkGradleBuild(
179+
"java-configuration-cache",
180+
"gradle/build-command",
181+
expectedScipFiles = 1,
182+
extraArguments = listOf("--", "--configuration-cache", "compileJava"),
183+
runs = 2,
184+
)
185+
)
164186
add(checkGradleBuild("kotlin2", "gradle/kotlin2", expectedScipFiles = 4))
187+
// Regression tests: Kotlin sources must be indexed again when the Kotlin
188+
// compile tasks are restored from the build cache, or are up to date
189+
// because the build command doesn't clean.
190+
add(
191+
checkGradleBuild(
192+
"kotlin-build-cache",
193+
"gradle/kotlin-build-cache",
194+
expectedScipFiles = 4,
195+
runs = 2,
196+
)
197+
)
198+
add(
199+
checkGradleBuild(
200+
"kotlin-up-to-date",
201+
"gradle/kotlin2",
202+
expectedScipFiles = 2,
203+
extraArguments = listOf("--", "compileKotlin", "compileJava"),
204+
runs = 2,
205+
)
206+
)
165207
add(
166208
checkGradleBuild(
167209
"implementation-deps",

‎scip-java/src/test/resources/fixtures/gradle/configuration-cache/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
apply plugin: 'java'

‎scip-java/src/test/resources/fixtures/gradle/configuration-cache/gradle.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
org.gradle.configuration-cache=true

‎scip-java/src/test/resources/fixtures/gradle/configuration-cache/src/main/java/Example.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
public class Example {}

‎scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/build.gradle‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
plugins {
2+
id 'org.jetbrains.kotlin.jvm' version '2.4.0'
3+
}
4+
kotlin {
5+
jvmToolchain(17)
6+
}
7+
repositories {
8+
mavenCentral()
9+
}

‎scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/gradle.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
org.gradle.caching=true

‎scip-java/src/test/resources/fixtures/gradle/kotlin-build-cache/settings.gradle‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// A build cache inside the checkout, so every test starts with an empty
2+
// cache and leaves the user's Gradle home untouched.
3+
buildCache {
4+
local {
5+
directory = file('build-cache')
6+
}
7+
}

0 commit comments

Comments
 (0)