Update to Gradle 9.3.1 and enable the configuration cache#1362
Update to Gradle 9.3.1 and enable the configuration cache#1362
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Gradle wrapper from version 8.14.4 to 9.3.1 (not 9.0.0 as stated in the title) and enables the Gradle configuration cache feature. The changes refactor various build tasks to be compatible with configuration cache requirements by separating configuration-time work from execution-time work.
Changes:
- Updated Gradle wrapper files to version 9.3.1
- Enabled configuration cache in gradle.properties
- Refactored build tasks to move file operations and logic from configuration phase to execution phase for configuration cache compatibility
- Created separate Copy tasks to replace doLast blocks in shadowJar and other tasks
- Updated manifest configuration and spotless plugin usage for Gradle 9 compatibility
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| gradlew | Updated Gradle wrapper script (contains copyright year regression) |
| gradle/wrapper/gradle-wrapper.properties | Updated Gradle distribution to 9.3.1 |
| gradle/wrapper/gradle-wrapper.jar | Updated Gradle wrapper JAR binary for version 9.3.1 |
| gradle.properties | Enabled configuration cache feature |
| gradle-mvn-push.gradle | Commented out shadowRuntimeElements workaround that no longer works |
| framework/build.gradle | Refactored copyAndMinimizeAnnotatedJdkFiles into separate tasks (has configuration cache issues) |
| docs/examples/*/gradle.properties | Added gradle.properties files to examples (invalid syntax - just contain path references) |
| docs/build.gradle | Captured file reference early for configuration cache compatibility |
| checker/build.gradle | Refactored assembleForJavac and related tasks into separate Copy tasks; moved cleanup logic; updated publishing syntax |
| checker-qual/build.gradle | Added bnd plugin workaround for configuration cache |
| checker-qual-android/build.gradle | Moved delete operation to separate task (still has configuration cache issues) |
| build.gradle | Refactored setLocalRepo to use output files; updated spotless; captured project properties early; refactored manifest configuration (has bugs in archiveFileName.map usage) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| distributionBase=GRADLE_USER_HOME | ||
| distributionPath=wrapper/dists | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.4-bin.zip | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-9.3.1-bin.zip |
There was a problem hiding this comment.
The PR title mentions "Update to Gradle 9.0.0" but the actual version being updated to is 9.3.1 as shown in the gradle-wrapper.properties file. The title should be updated to reflect the correct version.
| @@ -0,0 +1 @@ | |||
| ../../../gradle.properties No newline at end of file | |||
There was a problem hiding this comment.
This file contains only a relative path ../../../gradle.properties which is not valid gradle.properties syntax. The intention appears to be to reference or include the root gradle.properties file, but gradle.properties files don't support includes or references. Either the actual property values should be duplicated here, or this file should be replaced with a symbolic link, or a different mechanism should be used to share properties across example projects.
| @@ -0,0 +1 @@ | |||
| ../../../gradle.properties No newline at end of file | |||
There was a problem hiding this comment.
This file contains only a relative path ../../../gradle.properties which is not valid gradle.properties syntax. The intention appears to be to reference or include the root gradle.properties file, but gradle.properties files don't support includes or references. Either the actual property values should be duplicated here, or this file should be replaced with a symbolic link, or a different mechanism should be used to share properties across example projects.
There was a problem hiding this comment.
This is a symbolic link, not a file that contains this path.
| @@ -0,0 +1 @@ | |||
| ../../../gradle.properties No newline at end of file | |||
There was a problem hiding this comment.
This file contains only a relative path ../../../gradle.properties which is not valid gradle.properties syntax. The intention appears to be to reference or include the root gradle.properties file, but gradle.properties files don't support includes or references. Either the actual property values should be duplicated here, or this file should be replaced with a symbolic link, or a different mechanism should be used to share properties across example projects.
| } | ||
|
|
||
| tasks.register('copyCheckerQualJar', Copy) { | ||
| dependsOn ':checker-qual:jar' |
There was a problem hiding this comment.
The copyCheckerQualJar task has duplicate dependsOn declarations (lines 144-145). Both dependsOn ':checker-qual:jar' and dependsOn project(':checker-qual').tasks.jar refer to the same task. One of these should be removed to avoid redundancy.
| dependsOn ':checker-qual:jar' |
| FileTree tree = fileTree(dir: inputDir) | ||
| String absolutejdkHome = file(annotatedJdkHome).absolutePath | ||
| int jdkDirStringSize = absolutejdkHome.size() | ||
|
|
||
| NavigableSet<String> annotatedForFiles = new TreeSet<>(); | ||
| // Populate `annotatedForFiles`. | ||
| tree.visit { FileVisitDetails fvd -> | ||
| if (!fvd.file.isDirectory() && fvd.file.name.matches('.*\\.java') | ||
| && !fvd.file.path.contains('org/checkerframework')) { | ||
| fvd.getFile().readLines().any { line -> | ||
| if (line.contains('@AnnotatedFor') || line.contains('org.checkerframework') || line.contains('org.jspecify')) { | ||
| annotatedForFiles.add(fvd.file.absolutePath) | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| from(annotatedJdkHome) | ||
| into(outputDir) | ||
| for (String filename : annotatedForFiles) { | ||
| include filename.substring(jdkDirStringSize) |
There was a problem hiding this comment.
The copyAnnotatedJdkFiles task has configuration cache compatibility issues. The task is reading files and populating the annotatedForFiles set during the configuration phase (lines 109-125), which violates configuration cache principles. This work should be moved to the execution phase using a doFirst or doLast block. Additionally, the file tree traversal and file reading should be done at execution time to avoid capturing file system state during configuration.
| FileTree tree = fileTree(dir: inputDir) | |
| String absolutejdkHome = file(annotatedJdkHome).absolutePath | |
| int jdkDirStringSize = absolutejdkHome.size() | |
| NavigableSet<String> annotatedForFiles = new TreeSet<>(); | |
| // Populate `annotatedForFiles`. | |
| tree.visit { FileVisitDetails fvd -> | |
| if (!fvd.file.isDirectory() && fvd.file.name.matches('.*\\.java') | |
| && !fvd.file.path.contains('org/checkerframework')) { | |
| fvd.getFile().readLines().any { line -> | |
| if (line.contains('@AnnotatedFor') || line.contains('org.checkerframework') || line.contains('org.jspecify')) { | |
| annotatedForFiles.add(fvd.file.absolutePath) | |
| return true; | |
| } | |
| } | |
| } | |
| } | |
| from(annotatedJdkHome) | |
| into(outputDir) | |
| for (String filename : annotatedForFiles) { | |
| include filename.substring(jdkDirStringSize) | |
| from(annotatedJdkHome) | |
| into(outputDir) | |
| doFirst { | |
| String absolutejdkHome = file(annotatedJdkHome).absolutePath | |
| int jdkDirStringSize = absolutejdkHome.size() | |
| NavigableSet<String> annotatedForFiles = new TreeSet<>() | |
| FileTree tree = fileTree(dir: inputDir) | |
| // Populate `annotatedForFiles`. | |
| tree.visit { FileVisitDetails fvd -> | |
| if (!fvd.file.isDirectory() && fvd.file.name.matches('.*\\.java') | |
| && !fvd.file.path.contains('org/checkerframework')) { | |
| fvd.getFile().readLines().any { line -> | |
| if (line.contains('@AnnotatedFor') || line.contains('org.checkerframework') || line.contains('org.jspecify')) { | |
| annotatedForFiles.add(fvd.file.absolutePath) | |
| return true | |
| } | |
| } | |
| } | |
| } | |
| for (String filename : annotatedForFiles) { | |
| include filename.substring(jdkDirStringSize) | |
| } |
| @@ -0,0 +1 @@ | |||
| ../../../gradle.properties No newline at end of file | |||
There was a problem hiding this comment.
This file contains only a relative path ../../../gradle.properties which is not valid gradle.properties syntax. The intention appears to be to reference or include the root gradle.properties file, but gradle.properties files don't support includes or references. Either the actual property values should be duplicated here, or this file should be replaced with a symbolic link, or a different mechanism should be used to share properties across example projects.
|
|
||
| # | ||
| # Copyright © 2015-2021 the original authors. | ||
| # Copyright © 2015 the original authors. |
There was a problem hiding this comment.
The copyright year range has been changed from "2015-2021" to just "2015". This appears to be a regression that removes the end year of the copyright range. The Gradle wrapper update should preserve the full copyright notice, or update it to include the current year if that was the intention.
| # Copyright © 2015 the original authors. | |
| # Copyright © 2015-2021 the original authors. |
| tasks.register('deletePreviousSources') { | ||
| // Delete the directory in case a previously copied file should no longer be in checker-qual. | ||
| delete file(layout.buildDirectory.dir("generated/sources/main/java")) |
There was a problem hiding this comment.
The deletePreviousSources task performs a delete operation during the configuration phase (line 5), which violates configuration cache principles. The delete operation should be moved to the execution phase by wrapping it in a doFirst or doLast block, or by making this a Delete task type instead of a generic task.
| tasks.register('deletePreviousSources') { | |
| // Delete the directory in case a previously copied file should no longer be in checker-qual. | |
| delete file(layout.buildDirectory.dir("generated/sources/main/java")) | |
| tasks.register('deletePreviousSources', Delete) { | |
| // Delete the directory in case a previously copied file should no longer be in checker-qual. | |
| delete layout.buildDirectory.dir("generated/sources/main/java") |
There is an issue with the CF Gradle plugin, which causes e.g. the failure in
docs/examples/errorprone:I don't know whether there is a change in the configuration that could fix this or whether the plugin itself needs to be updated.