Skip to content

Fix compile cache artifact escalation - #498

Open
tfactor2 wants to merge 1 commit into
apache:masterfrom
tfactor2:fix-compile-cache-package-artifact
Open

Fix compile cache artifact escalation#498
tfactor2 wants to merge 1 commit into
apache:masterfrom
tfactor2:fix-compile-cache-package-artifact

Conversation

@tfactor2

@tfactor2 tfactor2 commented Jun 24, 2026

Copy link
Copy Markdown

Summary

Fix compile-only cache entries so target/classes is cached as an attached output archive instead of being serialized as the Maven project's main artifact.

This prevents a mvn compile cache hit from poisoning later mvn package/mvn install runs with a directory-backed main artifact while keeping compile output caching enabled for jar modules. The change also keeps a compatibility guard for legacy cache entries that already contain a directory-backed main artifact.

A zipped standalone reproducer is included under src/test/projects/maven-cache-install-after-compile-repro.zip.

Tests

Copilot AI review requested due to automatic review settings June 24, 2026 20:04
@tfactor2
tfactor2 force-pushed the fix-compile-cache-package-artifact branch from 861f590 to 8b4e4a7 Compare June 24, 2026 20:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a lifecycle phase escalation bug where a mvn compile cache hit could “poison” subsequent mvn package/mvn install runs by restoring target/classes as the project’s main artifact. The change ensures compile outputs are cached/restored as attached output archives instead of being serialized as the main Maven artifact, and adds a compatibility guard for legacy cache entries that stored a directory-backed main artifact.

Changes:

  • Cache compile outputs (target/classes, target/test-classes) as attached outputs (zipped) rather than as the main project artifact.
  • Add restoration-time guards/reset logic to prevent legacy directory-backed main artifacts from satisfying package-or-later requests.
  • Add/extend smoke ITs to assert buildinfo contents for compile -> package and compile -> install escalation scenarios.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

File Description
src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java Stops treating directory-backed artifacts as main artifacts for compile-only cache; attaches compile outputs; adjusts save/restore/staging behavior accordingly.
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java Adds legacy-cache guarding and artifact reset logic when escalating to package or later phases.
src/test/java/org/apache/maven/buildcache/its/lifecyclephases/CompileThenPackageEscalationTest.java Strengthens IT by asserting buildinfo.xml does not store a directory-backed main artifact and that package-level entries point to the JAR.
src/test/java/org/apache/maven/buildcache/its/lifecyclephases/CompileThenInstallEscalationTest.java New IT covering compile -> install escalation with the same buildinfo assertions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1343 to 1347
// 2. Compile outputs and configured attached outputs (if cacheCompile enabled)
if (cacheConfig.isCacheCompile()) {
addExistingOutputPath(paths, project.getBuild().getOutputDirectory());
addExistingOutputPath(paths, project.getBuild().getTestOutputDirectory());
List<DirName> attachedDirs = cacheConfig.getAttachedOutputs();

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

/**
* Saves a directory artifact by zipping it first, then saving the zip to cache.
*/
private void saveDirectoryArtifact(

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[explanation] #Why saveDirectoryArtifact(...) is removed:

The method made a directory-backed Maven project artifact saveable by zipping it and storing it through the main artifact path. That preserved the invalid metadata shape that caused #449: target/classes is a compile output directory, not the project's packaged main artifact.

With this change, saveProjectArtifact(...) only saves regular files produced by package-or-later mojos. Compile outputs are still cached, but through attachCompileOutputs(...) / attached output archives, so jar modules keep compile-cache benefits without letting target/classes poison later package or install executions.

Store compile outputs as attached cache outputs instead of serializing target/classes as the main project artifact. This lets package/install builds restore compiled classes, run packaging mojos, and save the packaged archive as the main artifact.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tfactor2
tfactor2 force-pushed the fix-compile-cache-package-artifact branch from 8b4e4a7 to b534e3c Compare June 24, 2026 20:35
@olamy olamy added the bug Something isn't working label Jun 27, 2026
@olamy olamy added this to the 1.2.4 milestone Jun 27, 2026
@cstamas

cstamas commented Jul 21, 2026

Copy link
Copy Markdown
Member

Please rebase against latest master.

@olamy olamy modified the milestones: 1.3.0, 1.3.1 Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants