Cache Maven and Gradle wrapper distributions separately from the dependency cache#1097
Open
brunoborges wants to merge 8 commits into
Open
Cache Maven and Gradle wrapper distributions separately from the dependency cache#1097brunoborges wants to merge 8 commits into
brunoborges wants to merge 8 commits into
Conversation
The Maven wrapper distribution (~/.m2/wrapper/dists) was cached in the same entry as the local Maven repository (~/.m2/repository), keyed on a hash of **/pom.xml (plus wrapper properties and extensions). Because pom.xml changes frequently and no restoreKeys are used (by design, #269), almost every change produces a full cache miss and the wrapper distribution is re-downloaded via mvnw — which intermittently fails due to upstream rate limiting. The wrapper distribution only depends on maven-wrapper.properties, which changes very rarely. Give it its own cache entry keyed solely on **/.mvn/wrapper/maven-wrapper.properties so it survives the frequent pom.xml changes that rotate the main dependency cache key. - Add a generic additionalCaches concept to PackageManager, restored and saved independently with name-scoped state keys. - Move ~/.m2/wrapper/dists out of the main maven path into a maven-wrapper additional cache; skip silently when the project does not use mvnw. - Keep cache-hit / cache-primary-key outputs driven by the main cache. - Update tests, docs, and rebuild dist bundles. Fixes #1095 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
5 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves cache: maven reliability by splitting the Maven Wrapper distribution (~/.m2/wrapper/dists) into its own independently-keyed cache entry, so wrapper downloads are not evicted by frequent pom.xml-driven key rotations in the main Maven dependency cache.
Changes:
- Introduces a generic
additionalCachesmechanism in the caching layer to restore/save auxiliary caches independently from the main dependency cache. - Moves Maven Wrapper distributions to a separate cache entry keyed only on
**/.mvn/wrapper/maven-wrapper.properties. - Updates documentation and unit tests to reflect and validate the new caching behavior.
Show a summary per file
| File | Description |
|---|---|
| src/cache.ts | Adds additionalCaches support and splits Maven wrapper dists into a separately-keyed cache entry. |
| tests/cache.test.ts | Updates/extends cache tests for Maven and adds coverage for wrapper cache restore/save behavior. |
| README.md | Documents that the Maven wrapper distribution cache is stored separately from the main Maven cache. |
| docs/advanced-usage.md | Updates advanced caching docs to describe the new separate wrapper cache entry and its rationale. |
| dist/setup/index.js | Updates the built distribution output to include the new caching logic. |
| dist/cleanup/index.js | Updates the built distribution output to include the new caching logic. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
src/cache.ts:145
- The cache-key format described here doesn’t match the actual key produced by buildCacheKey (it includes the
setup-javaprefix,RUNNER_OS, andprocess.arch). This makes the documentation misleading for users/debugging.
/**
* A function that generates a cache key to use.
* Format of the generated key will be "${{ platform }}-${{ id }}-${{ fileHash }}"".
* @see {@link https://docs.github.com/en/actions/guides/caching-dependencies-to-speed-up-workflows#matching-a-cache-key|spec of cache key}
*/
- Files reviewed: 4/6 changed files
- Comments generated: 3
- Review effort level: Low
3 tasks
#1098) The Gradle wrapper distribution (~/.gradle/wrapper) only depends on gradle-wrapper.properties, which changes rarely, but it was previously cached in the same entry as ~/.gradle/caches, keyed on volatile **/*.gradle* files with no restoreKeys (issue #269). Every dependency change therefore re-downloaded the wrapper. Move ~/.gradle/wrapper into a dedicated `gradle-wrapper` additional cache keyed only on **/gradle-wrapper.properties, reusing the additionalCaches infrastructure introduced for the Maven wrapper fix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The autofix commits dropped the `} catch (error) {` line in
saveAdditionalCache, leaving a `try` block without a catch and a dangling
`error` reference, which broke compilation and Prettier. Restore the catch
clause, reformat, and regenerate the dist bundles.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- saveAdditionalCache: handle @actions/cache ValidationError (thrown when the cache paths do not resolve, e.g. the wrapper distribution was never downloaded) by skipping instead of failing the post step. Add a test. - Point the Gradle wrapper cache comment at issue #269 (Gradle wrapper cache churn) instead of the Maven-specific #1095. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add a parallel "Gradle Wrapper" note alongside the Maven Wrapper note, so the new behavior for cache: 'gradle' (caching ~/.gradle/wrapper in a separate entry keyed on **/gradle-wrapper.properties) is documented. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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
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.
Description:
Each build tool's wrapper distribution was cached in the same cache entry as its dependency cache, keyed on volatile dependency files. Because those files change frequently and no
restoreKeysare used (by design, see #269), almost every change rotates the key and produces a full cache miss. On a miss nothing is restored, including the rarely-changing wrapper distribution, so./mvnw/./gradlewre-download the build tool, which intermittently fails due to upstream rate limiting.Maven: The wrapper distribution (
~/.m2/wrapper/dists) shared the~/.m2/repositoryentry, keyed on**/pom.xml(plus wrapper properties and extensions). I confirmed the impact from the linked JRuby run: thesetup-javastep loggedmaven cache is not found, then./mvnwfailed withwget: Failed to fetch https://repo1.maven.org/.../apache-maven-3.9.14-bin.zip. JRuby'smaven-wrapper.propertieschanged only twice in about two years, while the repo has 31pom.xmlfiles that each rotate the shared key, so the wrapper was evicted far more often than it actually changes.Gradle: The wrapper distribution (
~/.gradle/wrapper) shared the~/.gradle/cachesentry, keyed on volatile files such as**/*.gradle*, with the same net effect — the wrapper was re-downloaded on nearly every dependency change.Approach:
Give each build-tool wrapper distribution its own cache entry, keyed only on its wrapper properties file, so it survives the frequent dependency changes that invalidate the main dependency cache.
additionalCachesconcept toPackageManager, restored and saved independently with name-scoped state keys so restore/save state does not collide with the main cache.~/.m2/wrapper/distsout of the main mavenpathinto amaven-wrapperadditional cache keyed on**/.mvn/wrapper/maven-wrapper.properties.~/.gradle/wrapperout of the main gradlepathinto agradle-wrapperadditional cache keyed on**/gradle-wrapper.properties.cache-hitandcache-primary-keyoutputs driven by the main dependency cache, so existing behavior and outputs are unchanged.Related issue:
Fixes: #1095
Check list:
npm run checklocally (format, lint, build, test) and all checks pass.