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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
