Enable Gradle build caching and upgrade to Gradle 9.4.1#352
Open
holodorum wants to merge 2 commits intokson-org:mainfrom
Open
Enable Gradle build caching and upgrade to Gradle 9.4.1#352holodorum wants to merge 2 commits intokson-org:mainfrom
holodorum wants to merge 2 commits intokson-org:mainfrom
Conversation
Many tasks re-executed on every `./gradlew check` even when nothing
changed, because Gradle requires both inputs and outputs to determine
up-to-date status.
For each affected task, declare the source files/configs as inputs.
For tasks that produce build artifacts (cli native image, LSP compiled
output), declare those as outputs. For verification tasks that don't
produce artifacts (pytest, cargo test, pyright), use a "stamp file"
pattern: write a marker file on success and declare it as the output.
Also fix `generateConstants` which used `upToDateWhen { false }` as a
blunt workaround — replaced with proper `inputs.property()` for the
version and config values it depends on.
The IntelliJ Gradle Plugin v1 (`org.jetbrains.intellij` 1.17.2) had a
`classpathIndexCleanup` task that always ran and deleted classpath.index
files, causing `instrumentCode`, `instrumentTestCode`, and `test` to
re-execute every build. Additionally, v1 is incompatible with Gradle 9.
This commit:
- Upgrades the Gradle wrapper from 8.6 to 9.4.1
- Migrates from `org.jetbrains.intellij` v1 to
`org.jetbrains.intellij.platform` v2 (2.13.1), which properly
declares task inputs/outputs for caching
- Updates `PixiExecTask` to use injected `ExecOperations` instead of
the removed `Project.exec()` method
- Updates `kotlinx-serialization-json` in buildSrc from 1.6.1 to 1.8.1
for compatibility with Kotlin 2.2.20 (the 1.6.1 version had a binary
incompatibility with the serialization compiler plugin)
- Adds `junit:junit:4.13.2` test dependency for JetBrains plugin tests
(v2 doesn't transitively expose JUnit like v1 did)
The generated Schema test suite files have minor formatting changes
(empty `{}` instead of multi-line) due to the serialization library
update.
aochagavia
force-pushed
the
gradle-caching
branch
from
1bea55a to
bb66bd7
Compare
aochagavia
reviewed
Mar 25, 2026
Collaborator
aochagavia
left a comment
aochagavia
left a comment
There was a problem hiding this comment.
I love the idea of being more aggressive with caching in our builds, but I find it challenging from a maintainability perspective. Let me explain my thought process and suggest potential solutions.
Challenges
- Many of the proposed changes track inputs in a way that breaks encapsulation. For example, gradle needs to know which file dependencies are used inside
package.json, and we will most probably forget to update the gradle side when we change thepackage.jsonfile in the future. - It is difficult to be 100% sure we are properly tracking all the inputs and outputs. Also, the potential downside of missing an input could be huge, since we could end up with outdated build artifacts! Getting this wrong could maybe even lead to our CI passing when a clean checkout would result in test failures, because our CI is cached.
So... I'd very much like to avoid the risk of caching issues 😅.
What can we do?
Some alternatives I've thought of:
- Find well-maintained Gradle plugins for
npm,cargo, etc. that we can trust properly track inputs/outputs. I don't think this exists, at least not for npm. - Have
npmand friends spit out a specification of the inputs/outputs used when running a build or test, then use that to configure Gradle inputs/outputs. I don't think this is possible, at least not for npm. - Rely on
npmand friends' built-in caching mechanisms, without attempting to Gradle-ify them (this is what we have been doing until this PR).
From a pragmatic standpoint, I'd say we could live with alternative 3. After all, npm, cargo and uv know to stop immediately if there is no work to do. The only caveats are:
- Running
npm ciis somewhat slow (up to 5 seconds on my machine). This happens becausenpm cithrows away the existingnode_modulesand recreates it from scratch. We could probably bring this down to less than a second if we switched to pnpm (which is npm-compatible, focuses on performance, and has a strong community). - Tests always run, including heavy ones (in constrast to Gradle, which skips tests when their inputs haven't changed). Most tests complete in an instant, but
:tooling:lsp-clients:run_npm_testtakes ~30 seconds on my machine (assuming the previous build was cached). I guess this an issue if you run./gradlew checkfrequently during local development, but we could maybe introduce a flag that allows skipping those tests (e.g.,./gradlew check --skip-heavy-tests).
With the two mitigations above, I think we should see ./gradlew check --skip-heavy-tests complete in ~5 seconds, which sounds acceptable to me. I'm currently testing this and will push to a separate branch once I'm done, for comparison.
@holodorum what do you think?
| tasks.named("nativeTest") { | ||
| inputs.file(layout.buildDirectory.file("native/nativeTestCompile/native-tests-tests")) | ||
| outputs.dir(layout.buildDirectory.dir("test-results/test-native")) | ||
| } |
Collaborator
There was a problem hiding this comment.
Why is this necessary? We are using Kotlin's built-in testing mechanism, so I'd hope everything works out-of-the-box without extra plumbing...
| val outputDir = layout.buildDirectory.dir("native/nativeCompile").get().asFile | ||
| val outputFile = file("$outputDir/$cliName") | ||
|
|
||
| inputs.files(tasks.jar.map { it.archiveFile }) |
Collaborator
There was a problem hiding this comment.
Now we are adding this, we could get rid of dependsOn(tasks.jar) since Gradle can deduce the dependency automatically (I just discovered this is indeed recommended as a best practice in their docs).
| val outputFile = file("$outputDir/$cliName") | ||
|
|
||
| inputs.files(tasks.jar.map { it.archiveFile }) | ||
| inputs.files(configurations.runtimeClasspath) |
Collaborator
There was a problem hiding this comment.
Why is this last line necessary? If it's obvious and I'm missing it due to lack of knowledge, please enlighten me! And if it's not obvious... Then we should probably have a comment explaining the why :)
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
./gradlew checkeven when nothing changed. Tasks that produce artifacts declare them as outputs; verification-only tasks use a stamp file pattern.org.jetbrains.intellij) to v2 (org.jetbrains.intellij.platform). The v1 plugin had aclasspathIndexCleanuptask that always invalidated downstream tasks, and was incompatible with Gradle 9.