Skip to content

Commit a421356

Browse files
committed
Fix integration test failures and implement staleness detection
This commit fixes test failures and implements staleness detection via staging directory to prevent caching artifacts with future timestamps due to clock skew or orphaned files from git branch switches. Root Causes Fixed: 1. Issue74Test/DuplicateGoalsTest: hasUsefulArtifacts logic too aggressive 2. Compile-only builds: NullPointerException during artifact restoration 3. Clock skew vulnerability: Future timestamps cause wrong cache entries Staleness Detection Solution - Staging Directory: Instead of timestamp checking (which fails with clock skew), physically separate stale artifacts by moving them to a staging directory. Before mojos run: Move pre-existing artifacts to staging directory - Maven sees clean target/ with no pre-existing artifacts - Maven compiler MUST compile (can't skip based on timestamps) - Fresh correct files created in target/ - save() only sees fresh files (stale ones in staging) After save(): Restore artifacts from staging atomically (prevents TOCTOU race) Scenarios Protected: 1. Future Timestamps from Clock Skew - Moved to staging, forcing recompilation 2. Orphaned Class Files (deleted sources) - Moved to staging, not cached 3. Stale JARs/WARs from previous builds - Moved to staging, fresh ones cached Multimodule Support: - Staging directory structure preserves paths relative to multimodule root - Directory: target/maven-build-cache-extension/{module-path}/target/classes - Fixed state lifecycle bug where save() was removing ProjectCacheState before restoreRenamedArtifacts() could restore files - Updated method signatures to accept MavenSession for multimodule root resolution Changes: - CacheControllerImpl: Staging directory infrastructure with atomic restore - BuildCacheMojosExecutionStrategy: Integrated move before mojos, restore after save - hasUsefulArtifacts: Allow POM projects with plugin executions - artifactDto: Always set filePath for directories - restoreRenamedArtifacts: Use atomic Files.move() to prevent race conditions - Added: GitCheckoutStaleArtifactTest for single-module validation - Added: GitCheckoutStaleMultimoduleTest for multimodule validation All 74 unit tests pass.
1 parent 9e86503 commit a421356

File tree

14 files changed

+729
-48
lines changed

14 files changed

+729
-48
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ out/
1515
.java-version
1616
.checkstyle
1717
.factorypath
18+
null/

src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import javax.inject.Named;
2424

2525
import java.io.File;
26+
import java.io.IOException;
2627
import java.nio.file.Path;
2728
import java.util.HashSet;
2829
import java.util.List;
@@ -138,33 +139,54 @@ public void execute(
138139

139140
boolean restorable = result.isSuccess() || result.isPartialSuccess();
140141
boolean restored = false; // if partially restored need to save increment
142+
141143
if (restorable) {
142144
CacheRestorationStatus cacheRestorationStatus =
143145
restoreProject(result, mojoExecutions, mojoExecutionRunner, cacheConfig);
144146
restored = CacheRestorationStatus.SUCCESS == cacheRestorationStatus;
145147
executeExtraCleanPhaseIfNeeded(cacheRestorationStatus, cleanPhase, mojoExecutionRunner);
146148
}
147-
if (!restored) {
148-
for (MojoExecution mojoExecution : mojoExecutions) {
149-
if (source == Source.CLI
150-
|| mojoExecution.getLifecyclePhase() == null
151-
|| lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())) {
152-
mojoExecutionRunner.run(mojoExecution);
149+
150+
try {
151+
if (!restored) {
152+
// Move pre-existing artifacts to staging directory to prevent caching stale files
153+
// from previous builds (e.g., after git branch switch, or from cache restored
154+
// with clock skew). This ensures save() only sees fresh files built during this session.
155+
try {
156+
cacheController.stagePreExistingArtifacts(session, project);
157+
} catch (IOException e) {
158+
LOGGER.warn("Failed to stage pre-existing artifacts: {}", e.getMessage());
159+
// Continue build - if staging fails, we'll just cache what exists
160+
}
161+
162+
for (MojoExecution mojoExecution : mojoExecutions) {
163+
if (source == Source.CLI
164+
|| mojoExecution.getLifecyclePhase() == null
165+
|| lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())) {
166+
mojoExecutionRunner.run(mojoExecution);
167+
}
153168
}
154169
}
155-
}
156170

157-
if (cacheState == INITIALIZED && (!result.isSuccess() || !restored)) {
158-
if (cacheConfig.isSkipSave()) {
159-
LOGGER.info("Cache saving is disabled.");
160-
} else if (cacheConfig.isMandatoryClean()
161-
&& lifecyclePhasesHelper
162-
.getCleanSegment(project, mojoExecutions)
163-
.isEmpty()) {
164-
LOGGER.info("Cache storing is skipped since there was no \"clean\" phase.");
165-
} else {
166-
final Map<String, MojoExecutionEvent> executionEvents = mojoListener.getProjectExecutions(project);
167-
cacheController.save(result, mojoExecutions, executionEvents);
171+
if (cacheState == INITIALIZED && (!result.isSuccess() || !restored)) {
172+
if (cacheConfig.isSkipSave()) {
173+
LOGGER.info("Cache saving is disabled.");
174+
} else if (cacheConfig.isMandatoryClean()
175+
&& lifecyclePhasesHelper
176+
.getCleanSegment(project, mojoExecutions)
177+
.isEmpty()) {
178+
LOGGER.info("Cache storing is skipped since there was no \"clean\" phase.");
179+
} else {
180+
final Map<String, MojoExecutionEvent> executionEvents =
181+
mojoListener.getProjectExecutions(project);
182+
cacheController.save(result, mojoExecutions, executionEvents);
183+
}
184+
}
185+
} finally {
186+
// Always restore staged files after build completes (whether save ran or not).
187+
// Files that were rebuilt are discarded; files that weren't rebuilt are restored.
188+
if (!restored) {
189+
cacheController.restoreStagedArtifacts(session, project);
168190
}
169191
}
170192

@@ -244,6 +266,7 @@ private CacheRestorationStatus restoreProject(
244266

245267
// Restore project artifacts
246268
ArtifactRestorationReport restorationReport = cacheController.restoreProjectArtifacts(cacheResult);
269+
247270
if (!restorationReport.isSuccess()) {
248271
LOGGER.info("Cannot restore project artifacts, continuing with non cached build");
249272
return restorationReport.isRestoredFilesInProjectDirectory()

src/main/java/org/apache/maven/buildcache/CacheController.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.apache.maven.buildcache;
2020

21+
import java.io.IOException;
2122
import java.util.List;
2223
import java.util.Map;
2324

@@ -45,4 +46,23 @@ void save(
4546
boolean isForcedExecution(MavenProject project, MojoExecution execution);
4647

4748
void saveCacheReport(MavenSession session);
49+
50+
/**
51+
* Move pre-existing artifacts to staging directory to prevent caching stale files.
52+
* Called before mojos run to ensure save() only sees fresh files.
53+
*
54+
* @param session the Maven session
55+
* @param project the Maven project
56+
* @throws IOException if file operations fail
57+
*/
58+
void stagePreExistingArtifacts(MavenSession session, MavenProject project) throws IOException;
59+
60+
/**
61+
* Restore staged artifacts after save() completes.
62+
* Files that were rebuilt are discarded; files that weren't rebuilt are restored.
63+
*
64+
* @param session the Maven session
65+
* @param project the Maven project
66+
*/
67+
void restoreStagedArtifacts(MavenSession session, MavenProject project);
4868
}

0 commit comments

Comments
 (0)