Skip to content

Commit d2c8071

Browse files
committed
Fix #375: Capture plugin properties at validation time to eliminate timing mismatch
Maven 4 automatically injects --module-version into compiler arguments during execution, but cache validation happens before execution. This creates a timing mismatch that causes cache invalidation. This fix captures properties at validation time for ALL builds and stores ONLY validation-time values in the cache, ensuring consistent reading at the same lifecycle point. Changes: - Modified CacheResult to store validation-time mojo events - Added captureValidationTimeProperties() to BuildCacheMojosExecutionStrategy - Modified execute() to capture properties after findCachedBuild() - Modified save() to store ONLY validation-time events (AssertionError if missing) - Added comprehensive integration tests (Maven4Jpms, ExplicitModuleVersion, NonJpms, MultiModule) Benefits: - No configuration required (vs ignorePattern in PR #391) - Fixes root cause (timing mismatch) not symptom (value mismatch) - Works for ALL Maven 4 auto-injected properties - Preserves execution-time logging for debugging Alternative to PR #391
1 parent 49dc9ff commit d2c8071

File tree

28 files changed

+810
-10
lines changed

28 files changed

+810
-10
lines changed

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

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,19 @@ public void execute(
131131
}
132132
if (cacheState == INITIALIZED || skipCache) {
133133
result = cacheController.findCachedBuild(session, project, mojoExecutions, skipCache);
134+
135+
// Capture validation-time properties for all mojos to ensure consistent property reading
136+
// at the same lifecycle point for all builds (eliminates Maven 4 injection timing issues)
137+
// Always capture when cacheState is INITIALIZED since we may need to save
138+
if (cacheState == INITIALIZED) {
139+
Map<String, MojoExecutionEvent> validationTimeEvents =
140+
captureValidationTimeProperties(session, project, mojoExecutions);
141+
result = CacheResult.rebuilded(result, validationTimeEvents);
142+
LOGGER.debug(
143+
"Captured validation-time properties for {} mojos in project {}",
144+
validationTimeEvents.size(),
145+
projectName);
146+
}
134147
}
135148
} else {
136149
LOGGER.info("Cache is disabled on project level for {}", projectName);
@@ -163,8 +176,16 @@ public void execute(
163176
.isEmpty()) {
164177
LOGGER.info("Cache storing is skipped since there was no \"clean\" phase.");
165178
} else {
166-
final Map<String, MojoExecutionEvent> executionEvents = mojoListener.getProjectExecutions(project);
167-
cacheController.save(result, mojoExecutions, executionEvents);
179+
// Validation-time events must exist for cache storage
180+
// If they don't exist, this indicates a bug in the capture logic
181+
if (result.getValidationTimeEvents() == null || result.getValidationTimeEvents().isEmpty()) {
182+
throw new AssertionError(
183+
"Validation-time properties not captured for project " + projectName
184+
+ ". This is a bug - validation-time capture should always succeed when saving to cache.");
185+
}
186+
LOGGER.debug(
187+
"Using validation-time properties for cache storage (consistent lifecycle point)");
188+
cacheController.save(result, mojoExecutions, result.getValidationTimeEvents());
168189
}
169190
}
170191

@@ -434,6 +455,63 @@ private static String normalizedPath(Path path, Path baseDirPath) {
434455
return normalizedPath;
435456
}
436457

458+
/**
459+
* Captures plugin properties at validation time for all mojo executions.
460+
* This ensures properties are read at the same lifecycle point for all builds,
461+
* eliminating timing mismatches caused by Maven 4's auto-injection of properties
462+
* like --module-version during execution.
463+
*
464+
* @param session Maven session
465+
* @param project Current project
466+
* @param mojoExecutions List of mojo executions to capture properties for
467+
* @return Map of execution key to MojoExecutionEvent captured at validation time
468+
*/
469+
private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
470+
MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions) {
471+
Map<String, MojoExecutionEvent> validationTimeEvents = new java.util.HashMap<>();
472+
473+
for (MojoExecution mojoExecution : mojoExecutions) {
474+
// Skip mojos that don't execute or are in clean phase
475+
if (mojoExecution.getLifecyclePhase() == null
476+
|| !lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())) {
477+
continue;
478+
}
479+
480+
Mojo mojo = null;
481+
try {
482+
mojoExecutionScope.enter();
483+
mojoExecutionScope.seed(MavenProject.class, project);
484+
mojoExecutionScope.seed(MojoExecution.class, mojoExecution);
485+
486+
mojo = mavenPluginManager.getConfiguredMojo(Mojo.class, session, mojoExecution);
487+
MojoExecutionEvent event = new MojoExecutionEvent(session, project, mojoExecution, mojo);
488+
validationTimeEvents.put(mojoExecutionKey(mojoExecution), event);
489+
490+
LOGGER.debug(
491+
"Captured validation-time properties for {}",
492+
mojoExecution.getMojoDescriptor().getFullGoalName());
493+
494+
} catch (PluginConfigurationException | PluginContainerException e) {
495+
LOGGER.warn(
496+
"Cannot capture validation-time properties for {}: {}",
497+
mojoExecution.getMojoDescriptor().getFullGoalName(),
498+
e.getMessage());
499+
} finally {
500+
try {
501+
mojoExecutionScope.exit();
502+
} catch (MojoExecutionException e) {
503+
LOGGER.debug("Error exiting mojo execution scope: {}", e.getMessage());
504+
}
505+
if (mojo != null) {
506+
mavenPluginManager.releaseMojo(mojo, mojoExecution);
507+
}
508+
}
509+
}
510+
511+
LOGGER.debug("Captured validation-time properties for {} mojos", validationTimeEvents.size());
512+
return validationTimeEvents;
513+
}
514+
437515
private enum CacheRestorationStatus {
438516
SUCCESS,
439517
FAILURE,

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

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

21+
import java.util.Map;
22+
2123
import org.apache.maven.buildcache.xml.Build;
2224
import org.apache.maven.buildcache.xml.CacheSource;
25+
import org.apache.maven.execution.MojoExecutionEvent;
2326

2427
import static java.util.Objects.requireNonNull;
2528

@@ -31,49 +34,84 @@ public class CacheResult {
3134
private final RestoreStatus status;
3235
private final Build build;
3336
private final CacheContext context;
37+
private final Map<String, MojoExecutionEvent> validationTimeEvents;
3438

35-
private CacheResult(RestoreStatus status, Build build, CacheContext context) {
39+
private CacheResult(RestoreStatus status, Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
3640
this.status = requireNonNull(status);
3741
this.build = build;
3842
this.context = context;
43+
this.validationTimeEvents = validationTimeEvents;
3944
}
4045

4146
public static CacheResult empty(CacheContext context) {
4247
requireNonNull(context);
43-
return new CacheResult(RestoreStatus.EMPTY, null, context);
48+
return new CacheResult(RestoreStatus.EMPTY, null, context, null);
49+
}
50+
51+
public static CacheResult empty(CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
52+
requireNonNull(context);
53+
return new CacheResult(RestoreStatus.EMPTY, null, context, validationTimeEvents);
4454
}
4555

4656
public static CacheResult empty() {
47-
return new CacheResult(RestoreStatus.EMPTY, null, null);
57+
return new CacheResult(RestoreStatus.EMPTY, null, null, null);
4858
}
4959

5060
public static CacheResult failure(Build build, CacheContext context) {
5161
requireNonNull(build);
5262
requireNonNull(context);
53-
return new CacheResult(RestoreStatus.FAILURE, build, context);
63+
return new CacheResult(RestoreStatus.FAILURE, build, context, null);
64+
}
65+
66+
public static CacheResult failure(Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
67+
requireNonNull(build);
68+
requireNonNull(context);
69+
return new CacheResult(RestoreStatus.FAILURE, build, context, validationTimeEvents);
5470
}
5571

5672
public static CacheResult success(Build build, CacheContext context) {
5773
requireNonNull(build);
5874
requireNonNull(context);
59-
return new CacheResult(RestoreStatus.SUCCESS, build, context);
75+
return new CacheResult(RestoreStatus.SUCCESS, build, context, null);
76+
}
77+
78+
public static CacheResult success(Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
79+
requireNonNull(build);
80+
requireNonNull(context);
81+
return new CacheResult(RestoreStatus.SUCCESS, build, context, validationTimeEvents);
6082
}
6183

6284
public static CacheResult partialSuccess(Build build, CacheContext context) {
6385
requireNonNull(build);
6486
requireNonNull(context);
65-
return new CacheResult(RestoreStatus.PARTIAL, build, context);
87+
return new CacheResult(RestoreStatus.PARTIAL, build, context, null);
88+
}
89+
90+
public static CacheResult partialSuccess(Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
91+
requireNonNull(build);
92+
requireNonNull(context);
93+
return new CacheResult(RestoreStatus.PARTIAL, build, context, validationTimeEvents);
6694
}
6795

6896
public static CacheResult failure(CacheContext context) {
6997
requireNonNull(context);
70-
return new CacheResult(RestoreStatus.FAILURE, null, context);
98+
return new CacheResult(RestoreStatus.FAILURE, null, context, null);
99+
}
100+
101+
public static CacheResult failure(CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
102+
requireNonNull(context);
103+
return new CacheResult(RestoreStatus.FAILURE, null, context, validationTimeEvents);
71104
}
72105

73106
public static CacheResult rebuilded(CacheResult orig, Build build) {
74107
requireNonNull(orig);
75108
requireNonNull(build);
76-
return new CacheResult(orig.status, build, orig.context);
109+
return new CacheResult(orig.status, build, orig.context, orig.validationTimeEvents);
110+
}
111+
112+
public static CacheResult rebuilded(CacheResult orig, Map<String, MojoExecutionEvent> validationTimeEvents) {
113+
requireNonNull(orig);
114+
return new CacheResult(orig.status, orig.build, orig.context, validationTimeEvents);
77115
}
78116

79117
public boolean isSuccess() {
@@ -103,4 +141,8 @@ public RestoreStatus getStatus() {
103141
public boolean isFinal() {
104142
return build != null && build.getDto().is_final();
105143
}
144+
145+
public Map<String, MojoExecutionEvent> getValidationTimeEvents() {
146+
return validationTimeEvents;
147+
}
106148
}
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.maven.buildcache.its;
20+
21+
import org.apache.maven.buildcache.its.junit.IntegrationTest;
22+
import org.apache.maven.it.VerificationException;
23+
import org.apache.maven.it.Verifier;
24+
import org.junit.jupiter.api.Test;
25+
26+
/**
27+
* Integration test for JPMS module compilation with explicit moduleVersion configuration.
28+
*
29+
* <p>This test verifies that the validation-time property capture approach works correctly
30+
* when the moduleVersion is explicitly configured in the POM. Unlike Maven 4's auto-injection
31+
* scenario, this configuration is present at validation time, so there's no timing mismatch.
32+
* However, validation-time capture should still work correctly.
33+
*
34+
* <p>This test verifies:
35+
* <ol>
36+
* <li>First build creates cache entry with validation-time properties</li>
37+
* <li>Second build restores from cache successfully</li>
38+
* <li>Explicit configuration is captured correctly at validation time</li>
39+
* </ol>
40+
*/
41+
@IntegrationTest("src/test/projects/explicit-module-version")
42+
class ExplicitModuleVersionTest {
43+
44+
/**
45+
* Verifies that JPMS module compilation with explicit moduleVersion works with cache restoration.
46+
* This tests that validation-time capture works correctly when moduleVersion is explicitly
47+
* configured in the POM (no Maven 4 auto-injection needed).
48+
*
49+
* @param verifier Maven verifier for running builds
50+
* @throws VerificationException if verification fails
51+
*/
52+
@Test
53+
void testExplicitModuleVersionCacheRestoration(Verifier verifier) throws VerificationException {
54+
verifier.setAutoclean(false);
55+
56+
// First build - should create cache entry with validation-time properties
57+
verifier.setLogFileName("../log-build-1.txt");
58+
verifier.executeGoal("clean");
59+
verifier.executeGoal("compile");
60+
verifier.verifyErrorFreeLog();
61+
62+
// Verify compilation succeeded
63+
verifier.verifyFilePresent("target/classes/module-info.class");
64+
verifier.verifyFilePresent(
65+
"target/classes/org/apache/maven/caching/test/explicit/ExplicitVersionModule.class");
66+
67+
// Second build - should restore from cache
68+
verifier.setLogFileName("../log-build-2.txt");
69+
verifier.executeGoal("clean");
70+
verifier.executeGoal("compile");
71+
verifier.verifyErrorFreeLog();
72+
73+
// Verify cache was used (not rebuilt)
74+
verifier.verifyTextInLog(
75+
"Found cached build, restoring org.apache.maven.caching.test.explicit:explicit-module-version from cache");
76+
77+
// Verify compilation was skipped (restored from cache)
78+
verifier.verifyTextInLog("Skipping plugin execution (cached): compiler:compile");
79+
80+
// Verify output files were restored from cache
81+
verifier.verifyFilePresent("target/classes/module-info.class");
82+
verifier.verifyFilePresent(
83+
"target/classes/org/apache/maven/caching/test/explicit/ExplicitVersionModule.class");
84+
}
85+
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.maven.buildcache.its;
20+
21+
import org.apache.maven.buildcache.its.junit.IntegrationTest;
22+
import org.apache.maven.it.VerificationException;
23+
import org.apache.maven.it.Verifier;
24+
import org.junit.jupiter.api.Test;
25+
26+
/**
27+
* Integration test for Maven 4 JPMS module compilation with automatic --module-version injection.
28+
*
29+
* <p>This test verifies that issue #375 is fixed by the validation-time property capture approach.
30+
* Maven 4 automatically injects {@code --module-version ${project.version}} into compiler arguments
31+
* during execution. Without the fix, this creates a timing mismatch:
32+
* <ul>
33+
* <li>First build: Properties captured during execution (WITH injection)</li>
34+
* <li>Second build: Properties captured during validation (WITHOUT injection yet)</li>
35+
* <li>Result: Cache invalidation due to parameter mismatch</li>
36+
* </ul>
37+
*
38+
* <p>The fix captures properties at validation time for ALL builds, ensuring consistent reading
39+
* at the same lifecycle point. This test verifies:
40+
* <ol>
41+
* <li>First build creates cache entry</li>
42+
* <li>Second build restores from cache (NO cache invalidation)</li>
43+
* <li>NO {@code ignorePattern} configuration required</li>
44+
* </ol>
45+
*/
46+
@IntegrationTest("src/test/projects/maven4-jpms-module")
47+
class Maven4JpmsModuleTest {
48+
49+
/**
50+
* Verifies that Maven 4 JPMS module compilation works with cache restoration.
51+
* Maven 4 auto-injects {@code --module-version} during compilation, but the
52+
* validation-time capture approach ensures this doesn't cause cache invalidation.
53+
*
54+
* @param verifier Maven verifier for running builds
55+
* @throws VerificationException if verification fails
56+
*/
57+
@Test
58+
void testMaven4JpmsModuleCacheRestoration(Verifier verifier) throws VerificationException {
59+
verifier.setAutoclean(false);
60+
61+
// First build - should create cache entry with validation-time properties
62+
verifier.setLogFileName("../log-build-1.txt");
63+
verifier.executeGoal("clean");
64+
verifier.executeGoal("compile");
65+
verifier.verifyErrorFreeLog();
66+
67+
// Verify compilation succeeded
68+
verifier.verifyFilePresent("target/classes/module-info.class");
69+
verifier.verifyFilePresent(
70+
"target/classes/org/apache/maven/caching/test/maven4/HelloMaven4.class");
71+
72+
// Second build - should restore from cache WITHOUT invalidation
73+
// This is the critical test: validation-time properties should match stored properties
74+
verifier.setLogFileName("../log-build-2.txt");
75+
verifier.executeGoal("clean");
76+
verifier.executeGoal("compile");
77+
verifier.verifyErrorFreeLog();
78+
79+
// Verify cache was used (not rebuilt) - this proves the fix works!
80+
verifier.verifyTextInLog(
81+
"Found cached build, restoring org.apache.maven.caching.test.maven4:maven4-jpms-module from cache");
82+
83+
// Verify compilation was skipped (restored from cache)
84+
verifier.verifyTextInLog("Skipping plugin execution (cached): compiler:compile");
85+
86+
// Verify output files were restored from cache
87+
verifier.verifyFilePresent("target/classes/module-info.class");
88+
verifier.verifyFilePresent(
89+
"target/classes/org/apache/maven/caching/test/maven4/HelloMaven4.class");
90+
}
91+
}

0 commit comments

Comments
 (0)