You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "cowwoc (via GitHub)" <gi...@apache.org> on 2025/10/12 07:28:11 UTC
[PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
cowwoc opened a new pull request, #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395
## Problem
Maven 4 automatically injects `--module-version ${project.version}` into compiler arguments **during execution**, but the build cache validates properties **before execution**. This creates a timing mismatch that causes cache invalidation:
- **First build**: No cache exists → properties captured during/after execution (WITH Maven 4 injection)
- **Second build**: Cache exists → properties captured during validation (WITHOUT injection yet) → mismatch → cache invalidated
## Root Cause Analysis
The cache extension currently reads plugin properties at **different lifecycle points** for different builds:
```
First Build Timeline:
1. findCachedBuild() → no cache found
2. Mojos execute → Maven 4 injects --module-version
3. beforeMojoExecution() fires → captures properties WITH injection
4. save() stores properties from execution events
Second Build Timeline:
1. findCachedBuild() → cache found
2. verifyCacheConsistency() → creates mojo via getConfiguredMojo()
3. Reads properties → WITHOUT injection (too early in lifecycle)
4. Compares to first build's stored values (WITH injection)
5. MISMATCH → cache invalidated!
```
The problem: **validation reads BEFORE injection, storage reads AFTER injection**.
## Solution
Capture properties at **validation time** for ALL builds (even when no cache exists). This ensures both validation and storage read at the same lifecycle point.
### Implementation
1. **Modified `CacheResult`** to store validation-time mojo events
2. **Modified `BuildCacheMojosExecutionStrategy`** to capture properties immediately after `findCachedBuild()`
3. **Modified `save()`** to use validation-time events instead of execution-time events
### Key Code Changes
```java
// In BuildCacheMojosExecutionStrategy.execute():
result = cacheController.findCachedBuild(session, project, mojoExecutions, skipCache);
// NEW: Capture validation-time properties for ALL mojos
Map<String, MojoExecutionEvent> validationTimeEvents =
captureValidationTimeProperties(session, project, mojoExecutions);
// Store in result
result = CacheResult.rebuilded(result, validationTimeEvents);
```
```java
// In save():
// Use validation-time events instead of execution-time events
Map<String, MojoExecutionEvent> propertyEvents =
cacheResult.getValidationTimeEvents() != null
? cacheResult.getValidationTimeEvents() // Use validation-time
: executionEvents; // Fallback to execution-time
List<CompletedExecution> completedExecution =
buildExecutionInfo(mojoExecutions, propertyEvents);
```
### New Timeline (Both Builds)
```
First Build:
1. findCachedBuild() → no cache
2. captureValidationTimeProperties() → reads WITHOUT injection
3. Mojos execute → Maven 4 injects (but we don't use these values)
4. save() uses validation-time properties → stores WITHOUT injection
Second Build:
1. findCachedBuild() → cache found
2. verifyCacheConsistency() → reads WITHOUT injection
3. captureValidationTimeProperties() → reads WITHOUT injection
4. Compares to first build → BOTH without injection → MATCH!
```
## Benefits Over PR #391
| Aspect | PR #391 (`ignorePattern`) | This PR (Validation-Time Capture) |
|--------|--------------------------|-----------------------------------|
| **Approach** | Workaround: Filter mismatched values | Fix: Eliminate timing mismatch |
| **Configuration** | Required | None needed |
| **Flexibility** | Pattern must match version format | Format-agnostic |
| **Maintenance** | Patterns need updates | No maintenance |
| **Scope** | Only fixes known patterns | Fixes ALL auto-injected properties |
## Testing
Created comprehensive integration tests:
1. **Maven4JpmsModuleTest** - JPMS module with Maven 4 auto-injection of `--module-version`
2. **ExplicitModuleVersionTest** - JPMS module with explicit `moduleVersion` configuration
3. **NonJpmsProjectTest** - Regular Java project without JPMS (ensures no regression)
4. **MultiModuleJpmsTest** - Multi-module project with mixed JPMS and non-JPMS modules
All tests verify:
- ✅ First build creates cache
- ✅ Second build restores from cache (NO cache invalidation)
- ✅ NO `ignorePattern` configuration required
## Backward Compatibility
- Fully backward compatible
- Existing caches remain valid
- No configuration changes required
- `ignorePattern` still works but is no longer necessary for Maven 4 injection
## Alternative Approaches Considered
1. **Store validation-time values only** ❌ - Would lose auditability of what actually executed
2. **Delay validation until execution** ❌ - Would lose early cache-miss detection performance
3. **Pattern-based filtering (PR #391)** ❌ - Treats symptom, not root cause
4. **Maven core changes** ❌ - Outside our control
5. **Validation-time capture (this PR)** ✅ - Fixes root cause, no configuration needed
## Related Issues
- Fixes #375
- Alternative to #391
- Related to Maven 4 JPMS module compilation
## Migration
No migration needed. This change is transparent to users. Existing projects will benefit automatically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586555618
##########
src/test/projects/explicit-module-version/.mvn/extensions.xml:
##########
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review Comment:
```suggestion
<?xml version="1.0" encoding="UTF-8"?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to You under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r3608934098
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -183,9 +197,16 @@ public void execute(
.isEmpty()) {
LOGGER.debug("Cache storing is skipped since there was no \"clean\" phase.");
} else {
- final Map<String, MojoExecutionEvent> executionEvents =
- mojoListener.getProjectExecutions(project);
- cacheController.save(result, mojoExecutions, executionEvents);
+ // Only save cache if there are validation-time events to store
+ // When running only clean phase, there are no cacheable mojos
+ if (result.getValidationTimeEvents() == null
+ || result.getValidationTimeEvents().isEmpty()) {
+ LOGGER.debug("Skipping cache storage for {} - no cacheable mojos executed", projectName);
+ } else {
+ LOGGER.debug(
+ "Using validation-time properties for cache storage (consistent lifecycle point)");
+ cacheController.save(result, mojoExecutions, result.getValidationTimeEvents());
+ }
Review Comment:
Addressed in f04d1cc. Validation-time capture raises LifecycleExecutionException for plugin-configuration failures, while the missing validation-event invariant raises AssertionError. The save path verifies that every cacheable mojo has a validation-time event before saving. Clean-only executions remain intentionally skipped. The PR description and code snippet match.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2576995320
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -163,8 +176,16 @@ public void execute(
.isEmpty()) {
LOGGER.info("Cache storing is skipped since there was no \"clean\" phase.");
} else {
- final Map<String, MojoExecutionEvent> executionEvents = mojoListener.getProjectExecutions(project);
- cacheController.save(result, mojoExecutions, executionEvents);
+ // Validation-time events must exist for cache storage
+ // If they don't exist, this indicates a bug in the capture logic
+ if (result.getValidationTimeEvents() == null
+ || result.getValidationTimeEvents().isEmpty()) {
+ throw new AssertionError(
+ "Validation-time properties not captured for project " + projectName
+ + ". This is a bug - validation-time capture should always succeed when saving to cache.");
Review Comment:
run-on sentence
This is a bug. Validation-itme...
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -434,6 +455,63 @@ private static String normalizedPath(Path path, Path baseDirPath) {
return normalizedPath;
}
+ /**
+ * Captures plugin properties at validation time for all mojo executions.
+ * This ensures properties are read at the same lifecycle point for all builds,
+ * eliminating timing mismatches caused by Maven 4's auto-injection of properties
+ * like --module-version during execution.
+ *
+ * @param session Maven session
+ * @param project Current project
+ * @param mojoExecutions List of mojo executions to capture properties for
+ * @return Map of execution key to MojoExecutionEvent captured at validation time
+ */
+ private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
+ MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions) {
+ Map<String, MojoExecutionEvent> validationTimeEvents = new java.util.HashMap<>();
+
+ for (MojoExecution mojoExecution : mojoExecutions) {
+ // Skip mojos that don't execute or are in clean phase
+ if (mojoExecution.getLifecyclePhase() == null
+ || !lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())) {
+ continue;
+ }
+
+ Mojo mojo = null;
Review Comment:
declare this inside the try block where initialized
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -434,6 +455,63 @@ private static String normalizedPath(Path path, Path baseDirPath) {
return normalizedPath;
}
+ /**
+ * Captures plugin properties at validation time for all mojo executions.
+ * This ensures properties are read at the same lifecycle point for all builds,
+ * eliminating timing mismatches caused by Maven 4's auto-injection of properties
+ * like --module-version during execution.
+ *
+ * @param session Maven session
+ * @param project Current project
+ * @param mojoExecutions List of mojo executions to capture properties for
+ * @return Map of execution key to MojoExecutionEvent captured at validation time
+ */
+ private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
+ MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions) {
+ Map<String, MojoExecutionEvent> validationTimeEvents = new java.util.HashMap<>();
Review Comment:
java.util.HashMap --> HashMap
##########
src/main/java/org/apache/maven/buildcache/CacheResult.java:
##########
@@ -31,49 +34,91 @@ public class CacheResult {
private final RestoreStatus status;
private final Build build;
private final CacheContext context;
+ private final Map<String, MojoExecutionEvent> validationTimeEvents;
- private CacheResult(RestoreStatus status, Build build, CacheContext context) {
+ private CacheResult(
+ RestoreStatus status,
+ Build build,
+ CacheContext context,
+ Map<String, MojoExecutionEvent> validationTimeEvents) {
Review Comment:
This exposes mutable private state. Probably should copy for safety
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-5040982018
@cstamas Done. Try now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-6386884840
<!-- license-declaration-check -->
### ⚠️ License declaration missing
The pull request description must contain at least one of the following lines with a **checked** box (`[x]`):
```markdown
- [ ] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](https://www.apache.org/licenses/LICENSE-2.0)
- [ ] I hereby declare that I have filed an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
```
This comment will be hidden automatically once the description is fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586556314
##########
src/test/projects/explicit-module-version/pom.xml:
##########
@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review Comment:
```suggestion
<?xml version="1.0" encoding="UTF-8"?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to You under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-3462925179
@elharo Done. Try now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "AlexanderAshitkin (via GitHub)" <gi...@apache.org>.
AlexanderAshitkin commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-3465729676
Hi
1. I'm trying to understand why `verifyCacheConsistency` is even impacted. The verification logic only checks tracked properties from a user-defined configuration. So, `module-version` is not tracked by default, and should not interfere with this verification logic. Why is it causing issues in this case? Is it tracked in the impacted cache configuration?
```java
boolean isParamsMatched(
MavenProject project, MojoExecution mojoExecution, Mojo mojo, CompletedExecution completedExecution) {
List<TrackedProperty> tracked = cacheConfig.getTrackedProperties(mojoExecution);
for (TrackedProperty trackedProperty : tracked) {
```
2. Version is an unstable input and should not interfere with the cache. Versions are normalized elsewhere, and if Maven 4 introduced such logic for this specific property, I would prefer to have the value normalized to a standard value `<VERSION>` or, at the cache level, completely excluded from the reconciliation process.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-3598688067
@elharo I've pushed a commit that should fix the latest build failures. Try now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-3608650122
Absolutely nothing should be merged on a failing build. A failing build blocks everything.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586591627
##########
src/test/projects/multi-module-jpms/module-a/src/main/java/org/apache/maven/caching/test/multi/modulea/ModuleA.java:
##########
@@ -0,0 +1,7 @@
+package org.apache.maven.caching.test.multi.modulea;
Review Comment:
```suggestion
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.caching.test.multi.modulea;
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586579519
##########
src/test/projects/explicit-module-version/src/main/java/org/apache/maven/caching/test/explicit/ExplicitVersionModule.java:
##########
@@ -0,0 +1,7 @@
+package org.apache.maven.caching.test.explicit;
Review Comment:
```suggestion
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.caching.test.explicit;
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586587806
##########
src/test/projects/non-jpms-project/pom.xml:
##########
@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review Comment:
```suggestion
<?xml version="1.0" encoding="UTF-8"?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to You under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2578137767
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -163,8 +176,16 @@ public void execute(
.isEmpty()) {
LOGGER.info("Cache storing is skipped since there was no \"clean\" phase.");
} else {
- final Map<String, MojoExecutionEvent> executionEvents = mojoListener.getProjectExecutions(project);
- cacheController.save(result, mojoExecutions, executionEvents);
+ // Validation-time events must exist for cache storage
+ // If they don't exist, this indicates a bug in the capture logic
+ if (result.getValidationTimeEvents() == null
+ || result.getValidationTimeEvents().isEmpty()) {
+ throw new AssertionError(
+ "Validation-time properties not captured for project " + projectName
+ + ". This is a bug - validation-time capture should always succeed when saving to cache.");
Review Comment:
Hopefully I understood your comment correctly. See https://github.com/apache/maven-build-cache-extension/pull/395/commits/6cf3a361c988f9acdf97b291f93e8af9a6a0551f
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -434,6 +455,63 @@ private static String normalizedPath(Path path, Path baseDirPath) {
return normalizedPath;
}
+ /**
+ * Captures plugin properties at validation time for all mojo executions.
+ * This ensures properties are read at the same lifecycle point for all builds,
+ * eliminating timing mismatches caused by Maven 4's auto-injection of properties
+ * like --module-version during execution.
+ *
+ * @param session Maven session
+ * @param project Current project
+ * @param mojoExecutions List of mojo executions to capture properties for
+ * @return Map of execution key to MojoExecutionEvent captured at validation time
+ */
+ private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
+ MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions) {
+ Map<String, MojoExecutionEvent> validationTimeEvents = new java.util.HashMap<>();
Review Comment:
Fixed in https://github.com/apache/maven-build-cache-extension/pull/395/commits/6cf3a361c988f9acdf97b291f93e8af9a6a0551f
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586544276
##########
src/test/projects/maven4-jpms-module/pom.xml:
##########
@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review Comment:
```suggestion
<?xml version="1.0" encoding="UTF-8"?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to You under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586546998
##########
src/test/projects/maven4-jpms-module/src/main/java/module-info.java:
##########
@@ -0,0 +1,3 @@
+module org.apache.maven.caching.test.maven4 {
Review Comment:
```suggestion
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
module org.apache.maven.caching.test.maven4 {
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-3616656684
@elharo Agreed, but is anyone taking a look at the failing build?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-5012839393
@cstamas Round 2 complete :) Please re-check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r3611299635
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -444,6 +464,81 @@ boolean isParamsMatched(
return true;
}
+ /**
+ * Captures plugin properties at validation time for all mojo executions.
+ * This ensures properties are read at the same lifecycle point for all builds,
+ * eliminating timing mismatches caused by Maven 4's auto-injection of properties
+ * like --module-version during execution.
+ *
+ * @param session Maven session
+ * @param project Current project
+ * @param mojoExecutions List of mojo executions to capture properties for
+ * @return Map of execution key to MojoExecutionEvent captured at validation time
+ */
+ private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
+ MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions)
+ throws LifecycleExecutionException {
+ Map<String, MojoExecutionEvent> validationTimeEvents = new HashMap<>();
+
+ for (MojoExecution mojoExecution : mojoExecutions) {
+ // Skip mojos that don't execute or are in clean phase
+ if (mojoExecution.getLifecyclePhase() == null
+ || !lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())) {
+ continue;
+ }
+
+ mojoExecutionScope.enter();
+ try {
+ mojoExecutionScope.seed(MavenProject.class, project);
+ mojoExecutionScope.seed(MojoExecution.class, mojoExecution);
+
+ Mojo mojo = mavenPluginManager.getConfiguredMojo(Mojo.class, session, mojoExecution);
+ try {
+ MojoExecutionEvent event = new MojoExecutionEvent(session, project, mojoExecution, mojo);
+ validationTimeEvents.put(mojoExecutionKey(mojoExecution), event);
+
+ LOGGER.debug(
+ "Captured validation-time properties for {}",
+ mojoExecution.getMojoDescriptor().getFullGoalName());
+ } finally {
+ mavenPluginManager.releaseMojo(mojo, mojoExecution);
+ }
Review Comment:
Addressed in f4a1cf2: validation-time Mojo instances are retained until cacheController.save() has reflected over them, then released in execute()'s finally block. Partial capture failures also release already-configured Mojos.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-3462884318
I attempted to run `mvn spotless:apply` but encountered a Java 25 compatibility issue with the Palantir Java formatter:
```
java.lang.NoSuchMethodError: 'java.util.Queue com.sun.tools.javac.util.Log$DeferredDiagnosticHandler.getDiagnostics()'
```
The error occurs when trying to format `BuildCacheMojosExecutionStrategyTest.java`.
This is a known incompatibility between Palantir Java formatter (used by spotless) and Java 25.
Options:
1. Run spotless with Java 17/21 (recommended)
2. Upgrade to a newer version of Palantir formatter that supports Java 25
3. Manually format the affected file
Could you run `mvn spotless:apply` with Java 17 or 21, or would you like me to attempt a manual fix?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2578139704
##########
src/main/java/org/apache/maven/buildcache/CacheResult.java:
##########
@@ -31,49 +34,91 @@ public class CacheResult {
private final RestoreStatus status;
private final Build build;
private final CacheContext context;
+ private final Map<String, MojoExecutionEvent> validationTimeEvents;
- private CacheResult(RestoreStatus status, Build build, CacheContext context) {
+ private CacheResult(
+ RestoreStatus status,
+ Build build,
+ CacheContext context,
+ Map<String, MojoExecutionEvent> validationTimeEvents) {
Review Comment:
I opted to make this method package-private instead of returning a copy. Is that okay? See https://github.com/apache/maven-build-cache-extension/pull/395/commits/6cf3a361c988f9acdf97b291f93e8af9a6a0551f
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-3691620204
@elharo @olamy I've rebased the PR. Please check if the build passes now. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-4586615684
@olamy I fixed the Spotless check failure. Please trigger another build.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r3609477957
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -444,6 +464,81 @@ boolean isParamsMatched(
return true;
}
+ /**
+ * Captures plugin properties at validation time for all mojo executions.
+ * This ensures properties are read at the same lifecycle point for all builds,
+ * eliminating timing mismatches caused by Maven 4's auto-injection of properties
+ * like --module-version during execution.
+ *
+ * @param session Maven session
+ * @param project Current project
+ * @param mojoExecutions List of mojo executions to capture properties for
+ * @return Map of execution key to MojoExecutionEvent captured at validation time
+ */
+ private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
+ MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions)
+ throws LifecycleExecutionException {
+ Map<String, MojoExecutionEvent> validationTimeEvents = new HashMap<>();
+
+ for (MojoExecution mojoExecution : mojoExecutions) {
+ // Skip mojos that don't execute or are in clean phase
+ if (mojoExecution.getLifecyclePhase() == null
+ || !lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())) {
+ continue;
+ }
+
+ mojoExecutionScope.enter();
+ try {
+ mojoExecutionScope.seed(MavenProject.class, project);
+ mojoExecutionScope.seed(MojoExecution.class, mojoExecution);
+
+ Mojo mojo = mavenPluginManager.getConfiguredMojo(Mojo.class, session, mojoExecution);
+ try {
+ MojoExecutionEvent event = new MojoExecutionEvent(session, project, mojoExecution, mojo);
+ validationTimeEvents.put(mojoExecutionKey(mojoExecution), event);
+
+ LOGGER.debug(
+ "Captured validation-time properties for {}",
+ mojoExecution.getMojoDescriptor().getFullGoalName());
+ } finally {
+ mavenPluginManager.releaseMojo(mojo, mojoExecution);
+ }
Review Comment:
`captureValidationTimeProperties()` creates a configured Mojo instance, stores it inside a `MojoExecutionEvent`, and then immediately calls `mavenPluginManager.releaseMojo(mojo, mojoExecution)`. Later, `save()` passes these events to `CacheControllerImpl.recordMojoProperties()`, which reflects over `executionEvent.getMojo()` to read parameter values. Using a Mojo instance after it has been released is unsafe (it may be disposed or reused), and can lead to incorrect cached properties or hard-to-diagnose failures.
##########
src/main/java/org/apache/maven/buildcache/CacheResult.java:
##########
@@ -111,4 +156,8 @@ public RestoreStatus getStatus() {
public boolean isFinal() {
return build != null && build.getDto().is_final();
}
+
+ Map<String, MojoExecutionEvent> getValidationTimeEvents() {
+ return validationTimeEvents;
+ }
}
Review Comment:
`validationTimeEvents` can be provided via multiple public factory methods (e.g., `empty(context, validationTimeEvents)` / `success(..., validationTimeEvents)`), but there is no public accessor for this new state. That makes the new API surface asymmetric and effectively unusable outside the package, while all other CacheResult state has public getters. Consider making the getter public (or alternatively, make the factory overloads package-private/remove them).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r3608943171
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -183,9 +197,16 @@ public void execute(
.isEmpty()) {
LOGGER.debug("Cache storing is skipped since there was no \"clean\" phase.");
} else {
- final Map<String, MojoExecutionEvent> executionEvents =
- mojoListener.getProjectExecutions(project);
- cacheController.save(result, mojoExecutions, executionEvents);
+ // Only save cache if there are validation-time events to store
+ // When running only clean phase, there are no cacheable mojos
+ if (result.getValidationTimeEvents() == null
+ || result.getValidationTimeEvents().isEmpty()) {
+ LOGGER.debug("Skipping cache storage for {} - no cacheable mojos executed", projectName);
+ } else {
+ LOGGER.debug(
+ "Using validation-time properties for cache storage (consistent lifecycle point)");
+ cacheController.save(result, mojoExecutions, result.getValidationTimeEvents());
+ }
Review Comment:
Correction: the missing validation-event case is an internal invariant and now throws AssertionError; LifecycleExecutionException remains reserved for plugin-configuration failures. The PR description and code snippet have been updated accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586547590
##########
src/test/projects/maven4-jpms-module/src/main/java/org/apache/maven/caching/test/maven4/HelloMaven4.java:
##########
@@ -0,0 +1,7 @@
+package org.apache.maven.caching.test.maven4;
Review Comment:
```suggestion
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.caching.test.maven4;
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2578335217
##########
src/main/java/org/apache/maven/buildcache/CacheResult.java:
##########
@@ -31,49 +34,91 @@ public class CacheResult {
private final RestoreStatus status;
private final Build build;
private final CacheContext context;
+ private final Map<String, MojoExecutionEvent> validationTimeEvents;
- private CacheResult(RestoreStatus status, Build build, CacheContext context) {
+ private CacheResult(
+ RestoreStatus status,
+ Build build,
+ CacheContext context,
+ Map<String, MojoExecutionEvent> validationTimeEvents) {
this.status = requireNonNull(status);
this.build = build;
this.context = context;
+ this.validationTimeEvents = validationTimeEvents;
}
public static CacheResult empty(CacheContext context) {
requireNonNull(context);
- return new CacheResult(RestoreStatus.EMPTY, null, context);
+ return new CacheResult(RestoreStatus.EMPTY, null, context, null);
+ }
+
+ public static CacheResult empty(CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+ requireNonNull(context);
+ return new CacheResult(RestoreStatus.EMPTY, null, context, validationTimeEvents);
}
public static CacheResult empty() {
- return new CacheResult(RestoreStatus.EMPTY, null, null);
+ return new CacheResult(RestoreStatus.EMPTY, null, null, null);
}
public static CacheResult failure(Build build, CacheContext context) {
requireNonNull(build);
requireNonNull(context);
- return new CacheResult(RestoreStatus.FAILURE, build, context);
+ return new CacheResult(RestoreStatus.FAILURE, build, context, null);
+ }
+
+ public static CacheResult failure(
+ Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+ requireNonNull(build);
+ requireNonNull(context);
+ return new CacheResult(RestoreStatus.FAILURE, build, context, validationTimeEvents);
}
public static CacheResult success(Build build, CacheContext context) {
requireNonNull(build);
requireNonNull(context);
- return new CacheResult(RestoreStatus.SUCCESS, build, context);
+ return new CacheResult(RestoreStatus.SUCCESS, build, context, null);
+ }
+
+ public static CacheResult success(
+ Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+ requireNonNull(build);
+ requireNonNull(context);
+ return new CacheResult(RestoreStatus.SUCCESS, build, context, validationTimeEvents);
}
public static CacheResult partialSuccess(Build build, CacheContext context) {
requireNonNull(build);
requireNonNull(context);
- return new CacheResult(RestoreStatus.PARTIAL, build, context);
+ return new CacheResult(RestoreStatus.PARTIAL, build, context, null);
+ }
+
+ public static CacheResult partialSuccess(
+ Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+ requireNonNull(build);
+ requireNonNull(context);
+ return new CacheResult(RestoreStatus.PARTIAL, build, context, validationTimeEvents);
}
public static CacheResult failure(CacheContext context) {
requireNonNull(context);
- return new CacheResult(RestoreStatus.FAILURE, null, context);
+ return new CacheResult(RestoreStatus.FAILURE, null, context, null);
+ }
+
+ public static CacheResult failure(CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+ requireNonNull(context);
+ return new CacheResult(RestoreStatus.FAILURE, null, context, validationTimeEvents);
}
public static CacheResult rebuilded(CacheResult orig, Build build) {
Review Comment:
Fixed by https://github.com/apache/maven-build-cache-extension/pull/415
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-5012460817
@cstamas Comment addressed. Please re-run the check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r3608934098
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -183,9 +197,16 @@ public void execute(
.isEmpty()) {
LOGGER.debug("Cache storing is skipped since there was no \"clean\" phase.");
} else {
- final Map<String, MojoExecutionEvent> executionEvents =
- mojoListener.getProjectExecutions(project);
- cacheController.save(result, mojoExecutions, executionEvents);
+ // Only save cache if there are validation-time events to store
+ // When running only clean phase, there are no cacheable mojos
+ if (result.getValidationTimeEvents() == null
+ || result.getValidationTimeEvents().isEmpty()) {
+ LOGGER.debug("Skipping cache storage for {} - no cacheable mojos executed", projectName);
+ } else {
+ LOGGER.debug(
+ "Using validation-time properties for cache storage (consistent lifecycle point)");
+ cacheController.save(result, mojoExecutions, result.getValidationTimeEvents());
+ }
Review Comment:
Addressed in a2bd63c. Validation-time capture now raises LifecycleExecutionException for plugin-configuration failures, and the save path verifies that every cacheable mojo has a validation-time event before saving. Clean-only executions remain intentionally skipped. The PR description and code snippet were updated to match.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-5017266780
@cstamas Please trigger another review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-6379990461
This pull request is stale because it has been waiting for feedback for 60 days. Remove the stale label or comment on this PR, or it will be automatically closed in 30 days.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2578337344
##########
pom.xml:
##########
@@ -59,20 +59,21 @@ under the License.
</distributionManagement>
<properties>
- <project.build.outputTimestamp>2025-10-28T05:05:42Z</project.build.outputTimestamp>
+ <project.build.outputTimestamp>2024-05-09T00:27:12Z</project.build.outputTimestamp>
<javaVersion>8</javaVersion>
<minimalMavenBuildVersion>3.9.0</minimalMavenBuildVersion>
<minimalJavaBuildVersion>11</minimalJavaBuildVersion>
<classWorldsVersion>2.6.0</classWorldsVersion>
+ <version.spotless-maven-plugin>3.0.0</version.spotless-maven-plugin>
Review Comment:
Fixed by https://github.com/apache/maven-build-cache-extension/pull/395/commits/04b624656a4ee8618cf021d217ce9f1df871907f
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586592369
##########
src/test/projects/multi-module-jpms/module-a/src/main/java/module-info.java:
##########
@@ -0,0 +1,3 @@
+module org.apache.maven.caching.test.multi.modulea {
Review Comment:
```suggestion
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
module org.apache.maven.caching.test.multi.modulea {
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-4523893899
@olamy please take another shot at this. It should be ready to merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-5012110735
@olamy @slachiewicz Can you please merge this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r3608847608
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -183,9 +197,16 @@ public void execute(
.isEmpty()) {
LOGGER.debug("Cache storing is skipped since there was no \"clean\" phase.");
} else {
- final Map<String, MojoExecutionEvent> executionEvents =
- mojoListener.getProjectExecutions(project);
- cacheController.save(result, mojoExecutions, executionEvents);
+ // Only save cache if there are validation-time events to store
+ // When running only clean phase, there are no cacheable mojos
+ if (result.getValidationTimeEvents() == null
+ || result.getValidationTimeEvents().isEmpty()) {
+ LOGGER.debug("Skipping cache storage for {} - no cacheable mojos executed", projectName);
+ } else {
+ LOGGER.debug(
+ "Using validation-time properties for cache storage (consistent lifecycle point)");
+ cacheController.save(result, mojoExecutions, result.getValidationTimeEvents());
+ }
Review Comment:
The save path currently just logs and skips cache storage when validation-time events are null/empty. That contradicts the PR description (“MUST exist … fail with AssertionError”) and can also hide partial capture failures: if a mojo with tracked properties is missing from the map, those expected values will be absent in the cache and later comparisons fall back to defaults, potentially producing false cache hits. Consider enforcing that validation-time events are present for every cacheable mojo that has tracked properties, and fail fast (or at least refuse to save) when they’re missing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r3611299589
##########
src/main/java/org/apache/maven/buildcache/CacheResult.java:
##########
@@ -111,4 +156,8 @@ public RestoreStatus getStatus() {
public boolean isFinal() {
return build != null && build.getDto().is_final();
}
+
+ Map<String, MojoExecutionEvent> getValidationTimeEvents() {
+ return validationTimeEvents;
+ }
}
Review Comment:
Addressed in f4a1cf2: getValidationTimeEvents() is now public, matching the other CacheResult accessors and making the validation-time state usable by external callers.
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -444,6 +464,81 @@ boolean isParamsMatched(
return true;
}
+ /**
+ * Captures plugin properties at validation time for all mojo executions.
+ * This ensures properties are read at the same lifecycle point for all builds,
+ * eliminating timing mismatches caused by Maven 4's auto-injection of properties
+ * like --module-version during execution.
+ *
+ * @param session Maven session
+ * @param project Current project
+ * @param mojoExecutions List of mojo executions to capture properties for
+ * @return Map of execution key to MojoExecutionEvent captured at validation time
+ */
+ private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
+ MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions)
+ throws LifecycleExecutionException {
+ Map<String, MojoExecutionEvent> validationTimeEvents = new HashMap<>();
+
+ for (MojoExecution mojoExecution : mojoExecutions) {
+ // Skip mojos that don't execute or are in clean phase
+ if (mojoExecution.getLifecyclePhase() == null
+ || !lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())) {
+ continue;
+ }
+
+ mojoExecutionScope.enter();
+ try {
+ mojoExecutionScope.seed(MavenProject.class, project);
+ mojoExecutionScope.seed(MojoExecution.class, mojoExecution);
+
+ Mojo mojo = mavenPluginManager.getConfiguredMojo(Mojo.class, session, mojoExecution);
+ try {
+ MojoExecutionEvent event = new MojoExecutionEvent(session, project, mojoExecution, mojo);
+ validationTimeEvents.put(mojoExecutionKey(mojoExecution), event);
+
+ LOGGER.debug(
+ "Captured validation-time properties for {}",
+ mojoExecution.getMojoDescriptor().getFullGoalName());
+ } finally {
+ mavenPluginManager.releaseMojo(mojo, mojoExecution);
+ }
Review Comment:
Addressed in f4a1cf2: validation-time Mojo instances are retained until cacheController.save() has reflected over them, then released in execute()\u0027s finally block. Partial capture failures also release already-configured Mojos.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-5038492015
Please rebase against latest master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-6386853393
@slachiewicz Waiting for approval
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586576058
##########
src/test/projects/explicit-module-version/src/main/java/org/apache/maven/caching/test/explicit/ExplicitVersionModule.java:
##########
@@ -0,0 +1,7 @@
+package org.apache.maven.caching.test.explicit;
+
+public class ExplicitVersionModule {
+ public String getMessage() {
+ return "Hello from JPMS module with explicit version!";
+ }
+}
Review Comment:
```suggestion
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.caching.test.explicit;
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586587234
##########
src/test/projects/multi-module-jpms/pom.xml:
##########
@@ -0,0 +1,43 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review Comment:
```suggestion
<?xml version="1.0" encoding="UTF-8"?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to You under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
##########
src/test/projects/non-jpms-project/.mvn/extensions.xml:
##########
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review Comment:
```suggestion
<?xml version="1.0" encoding="UTF-8"?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to You under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586592979
##########
src/test/projects/multi-module-jpms/module-b/src/main/java/org/apache/maven/caching/test/multi/moduleb/ModuleB.java:
##########
@@ -0,0 +1,7 @@
+package org.apache.maven.caching.test.multi.moduleb;
Review Comment:
```suggestion
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.caching.test.multi.moduleb;
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586593729
##########
src/test/projects/non-jpms-project/src/main/java/org/apache/maven/caching/test/nonjpms/RegularJavaClass.java:
##########
@@ -0,0 +1,7 @@
+package org.apache.maven.caching.test.nonjpms;
Review Comment:
```suggestion
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.caching.test.nonjpms;
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586586021
##########
src/test/projects/maven4-jpms-module/.mvn/extensions.xml:
##########
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review Comment:
```suggestion
<?xml version="1.0" encoding="UTF-8"?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to You under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-5017541865
@cstamas The failure seems to be a transient network issue. Can you please trigger another build?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-6391048164
@olamy The merge conflicts have been resolved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586586604
##########
src/test/projects/multi-module-jpms/module-a/pom.xml:
##########
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review Comment:
```suggestion
<?xml version="1.0" encoding="UTF-8"?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to You under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
##########
src/test/projects/multi-module-jpms/module-b/pom.xml:
##########
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
Review Comment:
```suggestion
<?xml version="1.0" encoding="UTF-8"?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to You under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2578166987
##########
src/main/java/org/apache/maven/buildcache/CacheResult.java:
##########
@@ -31,49 +34,91 @@ public class CacheResult {
private final RestoreStatus status;
private final Build build;
private final CacheContext context;
+ private final Map<String, MojoExecutionEvent> validationTimeEvents;
- private CacheResult(RestoreStatus status, Build build, CacheContext context) {
+ private CacheResult(
+ RestoreStatus status,
+ Build build,
+ CacheContext context,
+ Map<String, MojoExecutionEvent> validationTimeEvents) {
this.status = requireNonNull(status);
this.build = build;
this.context = context;
+ this.validationTimeEvents = validationTimeEvents;
}
public static CacheResult empty(CacheContext context) {
requireNonNull(context);
- return new CacheResult(RestoreStatus.EMPTY, null, context);
+ return new CacheResult(RestoreStatus.EMPTY, null, context, null);
+ }
+
+ public static CacheResult empty(CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+ requireNonNull(context);
+ return new CacheResult(RestoreStatus.EMPTY, null, context, validationTimeEvents);
}
public static CacheResult empty() {
- return new CacheResult(RestoreStatus.EMPTY, null, null);
+ return new CacheResult(RestoreStatus.EMPTY, null, null, null);
}
public static CacheResult failure(Build build, CacheContext context) {
requireNonNull(build);
requireNonNull(context);
- return new CacheResult(RestoreStatus.FAILURE, build, context);
+ return new CacheResult(RestoreStatus.FAILURE, build, context, null);
+ }
+
+ public static CacheResult failure(
+ Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+ requireNonNull(build);
+ requireNonNull(context);
+ return new CacheResult(RestoreStatus.FAILURE, build, context, validationTimeEvents);
}
public static CacheResult success(Build build, CacheContext context) {
requireNonNull(build);
requireNonNull(context);
- return new CacheResult(RestoreStatus.SUCCESS, build, context);
+ return new CacheResult(RestoreStatus.SUCCESS, build, context, null);
+ }
+
+ public static CacheResult success(
+ Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+ requireNonNull(build);
+ requireNonNull(context);
+ return new CacheResult(RestoreStatus.SUCCESS, build, context, validationTimeEvents);
}
public static CacheResult partialSuccess(Build build, CacheContext context) {
requireNonNull(build);
requireNonNull(context);
- return new CacheResult(RestoreStatus.PARTIAL, build, context);
+ return new CacheResult(RestoreStatus.PARTIAL, build, context, null);
+ }
+
+ public static CacheResult partialSuccess(
+ Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+ requireNonNull(build);
+ requireNonNull(context);
+ return new CacheResult(RestoreStatus.PARTIAL, build, context, validationTimeEvents);
}
public static CacheResult failure(CacheContext context) {
requireNonNull(context);
- return new CacheResult(RestoreStatus.FAILURE, null, context);
+ return new CacheResult(RestoreStatus.FAILURE, null, context, null);
+ }
+
+ public static CacheResult failure(CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+ requireNonNull(context);
+ return new CacheResult(RestoreStatus.FAILURE, null, context, validationTimeEvents);
}
public static CacheResult rebuilded(CacheResult orig, Build build) {
Review Comment:
not for this PR, but this method name isn't grammatical
##########
pom.xml:
##########
@@ -59,20 +59,21 @@ under the License.
</distributionManagement>
<properties>
- <project.build.outputTimestamp>2025-10-28T05:05:42Z</project.build.outputTimestamp>
+ <project.build.outputTimestamp>2024-05-09T00:27:12Z</project.build.outputTimestamp>
<javaVersion>8</javaVersion>
<minimalMavenBuildVersion>3.9.0</minimalMavenBuildVersion>
<minimalJavaBuildVersion>11</minimalJavaBuildVersion>
<classWorldsVersion>2.6.0</classWorldsVersion>
+ <version.spotless-maven-plugin>3.0.0</version.spotless-maven-plugin>
Review Comment:
this change should be in a separate PR since it's unrelated to timing mismatch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-4006042022
This PR is approved but unmerged. Can someone please merge it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586542281
##########
src/test/projects/multi-module-jpms/.mvn/extensions.xml:
##########
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<extensions>
Review Comment:
```suggestion
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to You under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<extensions>
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#issuecomment-3607771560
It looks like the latest build errors are not related to this PR. Can it be merged? Do you need to do something else to fix the build?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586576058
##########
src/test/projects/explicit-module-version/src/main/java/org/apache/maven/caching/test/explicit/ExplicitVersionModule.java:
##########
@@ -0,0 +1,7 @@
+package org.apache.maven.caching.test.explicit;
+
+public class ExplicitVersionModule {
+ public String getMessage() {
+ return "Hello from JPMS module with explicit version!";
+ }
+}
Review Comment:
```suggestion
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.caching.test.explicit;
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "erik-meuwese-topicus (via GitHub)" <gi...@apache.org>.
erik-meuwese-topicus commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r2586582217
##########
src/test/projects/explicit-module-version/src/main/java/module-info.java:
##########
@@ -0,0 +1,3 @@
+module org.apache.maven.caching.test.explicit {
Review Comment:
```suggestion
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
module org.apache.maven.caching.test.explicit {
```
The code convention requires the ASF license header
https://maven.apache.org/developers/conventions/code.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "cowwoc (via GitHub)" <gi...@apache.org>.
cowwoc commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r3609149948
##########
src/test/projects/explicit-module-version/pom.xml:
##########
@@ -0,0 +1,57 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.caching.test.explicit</groupId>
+ <artifactId>explicit-module-version</artifactId>
+ <version>1.0.0-SNAPSHOT</version>
+ <packaging>jar</packaging>
+
+ <name>Explicit Module Version Test</name>
+ <description>Test project for JPMS module with explicit moduleVersion configuration</description>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>11</maven.compiler.source>
+ <maven.compiler.target>11</maven.compiler.target>
+ </properties>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.13.0</version>
+ <configuration>
+ <!-- Explicit moduleVersion configuration - present at validation time -->
+ <compilerArgs>
+ <arg>--module-version</arg>
+ <arg>${project.version}</arg>
+ <arg>-parameters</arg>
+ <arg>-g</arg>
+ </compilerArgs>
Review Comment:
Addressed in 8ba61a1: the test now configures <moduleVersion>${project.version}</moduleVersion> and leaves compilerArgs for the remaining compiler options.
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -444,6 +466,78 @@ boolean isParamsMatched(
return true;
}
+ /**
+ * Captures plugin properties at validation time for all mojo executions.
+ * This ensures properties are read at the same lifecycle point for all builds,
+ * eliminating timing mismatches caused by Maven 4's auto-injection of properties
+ * like --module-version during execution.
+ *
+ * @param session Maven session
+ * @param project Current project
+ * @param mojoExecutions List of mojo executions to capture properties for
+ * @return Map of execution key to MojoExecutionEvent captured at validation time
+ */
+ private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
+ MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions)
+ throws LifecycleExecutionException {
+ Map<String, MojoExecutionEvent> validationTimeEvents = new HashMap<>();
+
+ for (MojoExecution mojoExecution : mojoExecutions) {
+ // Skip mojos that don't execute or are in clean phase
+ if (mojoExecution.getLifecyclePhase() == null
+ || !lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())) {
+ continue;
+ }
+
+ try {
+ mojoExecutionScope.enter();
+ mojoExecutionScope.seed(MavenProject.class, project);
+ mojoExecutionScope.seed(MojoExecution.class, mojoExecution);
+
+ Mojo mojo = mavenPluginManager.getConfiguredMojo(Mojo.class, session, mojoExecution);
+ try {
+ MojoExecutionEvent event = new MojoExecutionEvent(session, project, mojoExecution, mojo);
+ validationTimeEvents.put(mojoExecutionKey(mojoExecution), event);
+
+ LOGGER.debug(
+ "Captured validation-time properties for {}",
+ mojoExecution.getMojoDescriptor().getFullGoalName());
+ } finally {
+ mavenPluginManager.releaseMojo(mojo, mojoExecution);
+ }
+
+ } catch (PluginConfigurationException | PluginContainerException e) {
+ throw new LifecycleExecutionException(
+ "Cannot capture validation-time properties for "
+ + mojoExecution.getMojoDescriptor().getFullGoalName(),
+ e);
+ } finally {
+ try {
+ mojoExecutionScope.exit();
+ } catch (MojoExecutionException e) {
+ LOGGER.debug("Error exiting mojo execution scope: {}", e.getMessage());
+ }
+ }
+ }
+
+ LOGGER.debug("Captured validation-time properties for {} mojos", validationTimeEvents.size());
+ return validationTimeEvents;
+ }
+
+ private void validateValidationTimeEvents(
+ String projectName,
+ List<MojoExecution> mojoExecutions,
+ Map<String, MojoExecutionEvent> validationTimeEvents) {
+ for (MojoExecution mojoExecution : mojoExecutions) {
+ if (mojoExecution.getLifecyclePhase() != null
+ && lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())
+ && (validationTimeEvents == null
+ || !validationTimeEvents.containsKey(mojoExecutionKey(mojoExecution)))) {
+ throw new AssertionError("Validation-time properties not captured for project " + projectName);
Review Comment:
Addressed in 8ba61a1: the assertion now includes the missing mojo’s full goal name and project name.
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -444,6 +466,78 @@ boolean isParamsMatched(
return true;
}
+ /**
+ * Captures plugin properties at validation time for all mojo executions.
+ * This ensures properties are read at the same lifecycle point for all builds,
+ * eliminating timing mismatches caused by Maven 4's auto-injection of properties
+ * like --module-version during execution.
+ *
+ * @param session Maven session
+ * @param project Current project
+ * @param mojoExecutions List of mojo executions to capture properties for
+ * @return Map of execution key to MojoExecutionEvent captured at validation time
+ */
+ private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
+ MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions)
+ throws LifecycleExecutionException {
+ Map<String, MojoExecutionEvent> validationTimeEvents = new HashMap<>();
+
+ for (MojoExecution mojoExecution : mojoExecutions) {
+ // Skip mojos that don't execute or are in clean phase
+ if (mojoExecution.getLifecyclePhase() == null
+ || !lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())) {
+ continue;
+ }
+
+ try {
+ mojoExecutionScope.enter();
Review Comment:
Addressed in 8ba61a1: mojoExecutionScope.enter() now runs before the try/finally, so exit() is not attempted when enter() fails.
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -134,6 +135,19 @@ public void execute(
}
if (cacheState == INITIALIZED) {
result = cacheController.findCachedBuild(session, project, mojoExecutions, skipCache);
+
+ // Capture validation-time properties for all mojos to ensure consistent property reading
+ // at the same lifecycle point for all builds (eliminates Maven 4 injection timing issues)
+ // Always capture when cacheState is INITIALIZED since we may need to save
+ if (cacheState == INITIALIZED) {
+ Map<String, MojoExecutionEvent> validationTimeEvents =
+ captureValidationTimeProperties(session, project, mojoExecutions);
+ result = CacheResult.rebuilded(result, validationTimeEvents);
+ LOGGER.debug(
+ "Captured validation-time properties for {} mojos in project {}",
+ validationTimeEvents.size(),
+ projectName);
+ }
Review Comment:
Addressed in 8ba61a1: removed the redundant inner cacheState check so capture runs directly within the existing initialized block.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix #375: Capture plugin properties at validation time to eliminate timing mismatch [maven-build-cache-extension]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #395:
URL: https://github.com/apache/maven-build-cache-extension/pull/395#discussion_r3608962908
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -134,6 +135,19 @@ public void execute(
}
if (cacheState == INITIALIZED) {
result = cacheController.findCachedBuild(session, project, mojoExecutions, skipCache);
+
+ // Capture validation-time properties for all mojos to ensure consistent property reading
+ // at the same lifecycle point for all builds (eliminates Maven 4 injection timing issues)
+ // Always capture when cacheState is INITIALIZED since we may need to save
+ if (cacheState == INITIALIZED) {
+ Map<String, MojoExecutionEvent> validationTimeEvents =
+ captureValidationTimeProperties(session, project, mojoExecutions);
+ result = CacheResult.rebuilded(result, validationTimeEvents);
+ LOGGER.debug(
+ "Captured validation-time properties for {} mojos in project {}",
+ validationTimeEvents.size(),
+ projectName);
+ }
Review Comment:
Redundant `if (cacheState == INITIALIZED)` nested inside an outer `if (cacheState == INITIALIZED)` adds noise and makes the control-flow harder to scan. The capture can run unconditionally within the existing outer block.
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -444,6 +466,78 @@ boolean isParamsMatched(
return true;
}
+ /**
+ * Captures plugin properties at validation time for all mojo executions.
+ * This ensures properties are read at the same lifecycle point for all builds,
+ * eliminating timing mismatches caused by Maven 4's auto-injection of properties
+ * like --module-version during execution.
+ *
+ * @param session Maven session
+ * @param project Current project
+ * @param mojoExecutions List of mojo executions to capture properties for
+ * @return Map of execution key to MojoExecutionEvent captured at validation time
+ */
+ private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
+ MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions)
+ throws LifecycleExecutionException {
+ Map<String, MojoExecutionEvent> validationTimeEvents = new HashMap<>();
+
+ for (MojoExecution mojoExecution : mojoExecutions) {
+ // Skip mojos that don't execute or are in clean phase
+ if (mojoExecution.getLifecyclePhase() == null
+ || !lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())) {
+ continue;
+ }
+
+ try {
+ mojoExecutionScope.enter();
Review Comment:
`mojoExecutionScope.enter()` is inside the `try`, but `exit()` runs in `finally`. If `enter()` throws (e.g., due to an inconsistent scope state), the `finally` will still attempt `exit()`, which can trigger a secondary failure and obscure the root cause. This file’s other usage patterns call `enter()` before the `try` to avoid exiting an un-entered scope.
##########
src/test/projects/explicit-module-version/pom.xml:
##########
@@ -0,0 +1,57 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.caching.test.explicit</groupId>
+ <artifactId>explicit-module-version</artifactId>
+ <version>1.0.0-SNAPSHOT</version>
+ <packaging>jar</packaging>
+
+ <name>Explicit Module Version Test</name>
+ <description>Test project for JPMS module with explicit moduleVersion configuration</description>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>11</maven.compiler.source>
+ <maven.compiler.target>11</maven.compiler.target>
+ </properties>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.13.0</version>
+ <configuration>
+ <!-- Explicit moduleVersion configuration - present at validation time -->
+ <compilerArgs>
+ <arg>--module-version</arg>
+ <arg>${project.version}</arg>
+ <arg>-parameters</arg>
+ <arg>-g</arg>
+ </compilerArgs>
Review Comment:
This test project is described as having an explicit `moduleVersion` configuration, but the POM actually hard-codes `--module-version` via `compilerArgs`. That doesn’t exercise the plugin’s `moduleVersion` parameter path described in the PR/issue and can mask the real timing behavior.
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -444,6 +466,78 @@ boolean isParamsMatched(
return true;
}
+ /**
+ * Captures plugin properties at validation time for all mojo executions.
+ * This ensures properties are read at the same lifecycle point for all builds,
+ * eliminating timing mismatches caused by Maven 4's auto-injection of properties
+ * like --module-version during execution.
+ *
+ * @param session Maven session
+ * @param project Current project
+ * @param mojoExecutions List of mojo executions to capture properties for
+ * @return Map of execution key to MojoExecutionEvent captured at validation time
+ */
+ private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
+ MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions)
+ throws LifecycleExecutionException {
+ Map<String, MojoExecutionEvent> validationTimeEvents = new HashMap<>();
+
+ for (MojoExecution mojoExecution : mojoExecutions) {
+ // Skip mojos that don't execute or are in clean phase
+ if (mojoExecution.getLifecyclePhase() == null
+ || !lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())) {
+ continue;
+ }
+
+ try {
+ mojoExecutionScope.enter();
+ mojoExecutionScope.seed(MavenProject.class, project);
+ mojoExecutionScope.seed(MojoExecution.class, mojoExecution);
+
+ Mojo mojo = mavenPluginManager.getConfiguredMojo(Mojo.class, session, mojoExecution);
+ try {
+ MojoExecutionEvent event = new MojoExecutionEvent(session, project, mojoExecution, mojo);
+ validationTimeEvents.put(mojoExecutionKey(mojoExecution), event);
+
+ LOGGER.debug(
+ "Captured validation-time properties for {}",
+ mojoExecution.getMojoDescriptor().getFullGoalName());
+ } finally {
+ mavenPluginManager.releaseMojo(mojo, mojoExecution);
+ }
+
+ } catch (PluginConfigurationException | PluginContainerException e) {
+ throw new LifecycleExecutionException(
+ "Cannot capture validation-time properties for "
+ + mojoExecution.getMojoDescriptor().getFullGoalName(),
+ e);
+ } finally {
+ try {
+ mojoExecutionScope.exit();
+ } catch (MojoExecutionException e) {
+ LOGGER.debug("Error exiting mojo execution scope: {}", e.getMessage());
+ }
+ }
+ }
+
+ LOGGER.debug("Captured validation-time properties for {} mojos", validationTimeEvents.size());
+ return validationTimeEvents;
+ }
+
+ private void validateValidationTimeEvents(
+ String projectName,
+ List<MojoExecution> mojoExecutions,
+ Map<String, MojoExecutionEvent> validationTimeEvents) {
+ for (MojoExecution mojoExecution : mojoExecutions) {
+ if (mojoExecution.getLifecyclePhase() != null
+ && lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())
+ && (validationTimeEvents == null
+ || !validationTimeEvents.containsKey(mojoExecutionKey(mojoExecution)))) {
+ throw new AssertionError("Validation-time properties not captured for project " + projectName);
Review Comment:
The AssertionError thrown when validation-time events are missing doesn’t indicate which mojo is missing, making failures hard to diagnose. Including the execution key (or full goal name) would make the invariant actionable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org