Copilot commented on code in PR #389:
URL:
https://github.com/apache/maven-build-cache-extension/pull/389#discussion_r4438391675
##########
src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java:
##########
@@ -1055,6 +1059,30 @@ private void recordMojoProperties(CompletedExecution
execution, MojoExecutionEve
}
}
+ private Object normalizeMojoProperty(
+ String propertyName, Object value, MojoExecution mojoExecution,
MavenProject project) {
+ if (!(value instanceof List)
+ || !"compilerArgs".equals(propertyName)
+ || mojoExecution.getPlugin() == null
+ ||
!"maven-compiler-plugin".equals(mojoExecution.getPlugin().getArtifactId())) {
+ return value;
+ }
+
+ List<?> compilerArguments = (List<?>) value;
+ List<Object> normalizedArguments = new
ArrayList<>(compilerArguments.size());
+ for (int i = 0; i < compilerArguments.size(); i++) {
+ Object argument = compilerArguments.get(i);
+ if ("--module-version".equals(argument)
+ && i + 1 < compilerArguments.size()
+ && Objects.equals(project.getVersion(),
compilerArguments.get(i + 1))) {
+ i++;
+ } else {
+ normalizedArguments.add(argument);
+ }
+ }
+ return normalizedArguments.isEmpty() ? null : normalizedArguments;
Review Comment:
Normalization runs only when saving, so an unchanged empty `compilerArgs`
list is stored as `"null"` but compared as `"[]"`. An explicitly configured
`--module-version` pair matching the project version is likewise removed only
from the saved value, causing repeated cache misses. Preserve explicitly
configured arguments and use compatible representations when recording and
comparing them, preferably by snapshotting inputs before execution.
##########
src/main/java/org/apache/maven/buildcache/xml/PluginParameterLoader.java:
##########
@@ -0,0 +1,283 @@
+/*
+ * 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.buildcache.xml;
+
+import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import
org.apache.maven.buildcache.xml.PluginParameterDefinition.GoalParameterDefinition;
+import
org.apache.maven.buildcache.xml.PluginParameterDefinition.ParameterDefinition;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.NodeList;
+import org.xml.sax.SAXException;
+
+/**
+ * Loads plugin parameter definitions from classpath resources.
+ * Definitions are stored in
src/main/resources/plugin-parameters/{artifactId}.xml
+ */
+class PluginParameterLoader {
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PluginParameterLoader.class);
+ private static final String PARAMETER_DIR = "plugin-parameters/";
+
+ private final Map<String, PluginParameterDefinition> definitions = new
ConcurrentHashMap<>();
+
+ /**
+ * Load parameter definitions for a plugin by artifact ID only (no version
matching)
+ */
+ PluginParameterDefinition load(String artifactId) {
+ return load(artifactId, null);
+ }
+
+ /**
+ * Load parameter definitions for a plugin by artifact ID and version.
+ * If version is provided, finds the best matching definition (highest
minVersion <= actual version).
+ * If version is null, returns any definition for the artifactId.
+ */
+ PluginParameterDefinition load(String artifactId, String pluginVersion) {
+ String cacheKey = artifactId + (pluginVersion != null ? ":" +
pluginVersion : "");
+
+ if (definitions.containsKey(cacheKey)) {
+ return definitions.get(cacheKey);
+ }
+
+ String resourcePath = PARAMETER_DIR + artifactId + ".xml";
+ InputStream resourceStream =
getClass().getClassLoader().getResourceAsStream(resourcePath);
+
+ if (resourceStream == null) {
+ LOGGER.debug("No parameter definition found for plugin: {}",
artifactId);
+ return null;
+ }
+
+ try (InputStream inputStream = resourceStream) {
+ List<PluginParameterDefinition> allDefinitions =
parseDefinitions(inputStream);
+
+ PluginParameterDefinition bestMatch =
findBestMatch(allDefinitions, pluginVersion);
+
+ if (bestMatch != null) {
+ definitions.put(cacheKey, bestMatch);
+ LOGGER.debug(
+ "Loaded parameter definition for {}:{} (minVersion:
{}): {} goals, {} total parameters",
+ artifactId,
+ pluginVersion != null ? pluginVersion : "any",
+ bestMatch.getMinVersion() != null ?
bestMatch.getMinVersion() : "none",
+ bestMatch.getGoals().size(),
+ bestMatch.getGoals().values().stream()
+ .mapToInt(goalDefinition ->
+ goalDefinition.getParameters().size())
+ .sum());
+ }
+
+ return bestMatch;
+ } catch (ParserConfigurationException | SAXException | IOException e) {
+ LOGGER.warn("Failed to load parameter definition for {}: {}",
artifactId, e.getMessage(), e);
+ return null;
+ }
+ }
+
+ /**
+ * Find the best matching definition for a plugin version.
+ * Returns the definition with the highest minVersion that is <=
pluginVersion.
+ * If pluginVersion is null, returns the first definition (or the one
without minVersion).
+ */
+ private PluginParameterDefinition
findBestMatch(List<PluginParameterDefinition> definitions, String
pluginVersion) {
+ if (definitions.isEmpty()) {
+ return null;
+ }
+
+ if (pluginVersion == null) {
+ // No version specified, prefer definition without minVersion,
otherwise return first
+ return definitions.stream()
+ .filter(definition -> definition.getMinVersion() == null)
+ .findFirst()
+ .orElse(definitions.get(0));
+ }
+
+ // Find highest minVersion that's <= pluginVersion
+ PluginParameterDefinition bestMatch = null;
+ String bestMinVersion = null;
+
+ for (PluginParameterDefinition definition : definitions) {
+ String minVersion = definition.getMinVersion();
+
+ // Definition without minVersion applies to all versions
+ if (minVersion == null) {
+ if (bestMatch == null) {
+ bestMatch = definition;
+ }
+ continue;
+ }
+
+ // Check if this definition applies to the plugin version
+ if (compareVersions(minVersion, pluginVersion) <= 0) {
+ // minVersion <= pluginVersion, so this definition applies
+ if (bestMinVersion == null || compareVersions(minVersion,
bestMinVersion) > 0) {
+ // This is a better match (higher minVersion)
+ bestMatch = definition;
+ bestMinVersion = minVersion;
+ }
+ }
+ }
+
+ return bestMatch;
+ }
+
+ /**
+ * Compare two version strings.
+ * Returns: negative if the first version is older, zero if equal,
positive if newer.
+ */
+ private int compareVersions(String firstVersion, String secondVersion) {
+ String[] firstVersionParts = firstVersion.split("\\.");
+ String[] secondVersionParts = secondVersion.split("\\.");
+
+ int maxLength = Math.max(firstVersionParts.length,
secondVersionParts.length);
+
+ for (int i = 0; i < maxLength; i++) {
+ int firstPart = i < firstVersionParts.length ?
parseVersionPart(firstVersionParts[i]) : 0;
+ int secondPart = i < secondVersionParts.length ?
parseVersionPart(secondVersionParts[i]) : 0;
+
+ if (firstPart != secondPart) {
+ return Integer.compare(firstPart, secondPart);
+ }
+ }
+
+ return 0;
+ }
+
+ private int parseVersionPart(String part) {
+ try {
+ // Handle qualifiers like "3.8.0-SNAPSHOT" - just use numeric part
+ int dashIndex = part.indexOf('-');
+ if (dashIndex > 0) {
+ part = part.substring(0, dashIndex);
+ }
+ return Integer.parseInt(part);
+ } catch (NumberFormatException e) {
+ return 0;
+ }
+ }
+
+ /**
+ * Parse plugin parameter definitions from XML.
+ * Supports multiple <plugin> elements in a single file for
version-specific definitions.
+ */
+ private List<PluginParameterDefinition> parseDefinitions(InputStream
inputStream)
+ throws ParserConfigurationException, SAXException, IOException {
+ List<PluginParameterDefinition> definitions = new ArrayList<>();
+
+ DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+ factory.setNamespaceAware(true);
+ factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl";,
true);
+
factory.setFeature("http://xml.org/sax/features/external-general-entities";,
false);
+
factory.setFeature("http://xml.org/sax/features/external-parameter-entities";,
false);
+
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd";,
false);
+ factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+ factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
+ factory.setXIncludeAware(false);
+ factory.setExpandEntityReferences(false);
+ DocumentBuilder builder = factory.newDocumentBuilder();
+ Document document = builder.parse(inputStream);
+
+ Element root = document.getDocumentElement();
+
+ // Check if root is a single <plugin> or if we need to look for
multiple
+ if ("plugin".equals(root.getLocalName())) {
+ // Single plugin definition
+ definitions.add(parsePluginDefinition(root));
+ } else {
+ // Look for multiple <plugin> elements
+ NodeList pluginNodes = root.getElementsByTagNameNS("*", "plugin");
+ for (int i = 0; i < pluginNodes.getLength(); i++) {
+ Element pluginElement = (Element) pluginNodes.item(i);
+ definitions.add(parsePluginDefinition(pluginElement));
+ }
+ }
+
+ return definitions;
+ }
+
+ private PluginParameterDefinition parsePluginDefinition(Element
pluginElement) {
+ String groupId = getTextContent(pluginElement, "groupId");
+ String actualArtifactId = getTextContent(pluginElement, "artifactId");
+ String minVersion = getTextContent(pluginElement, "minVersion");
+
+ PluginParameterDefinition definition = new
PluginParameterDefinition(groupId, actualArtifactId, minVersion);
+
+ NodeList goalsNodes = pluginElement.getElementsByTagNameNS("*",
"goals");
+ if (goalsNodes.getLength() > 0) {
+ Element goalsElement = (Element) goalsNodes.item(0);
+ NodeList goalNodes = goalsElement.getElementsByTagNameNS("*",
"goal");
+
+ for (int i = 0; i < goalNodes.getLength(); i++) {
+ Element goalElement = (Element) goalNodes.item(i);
+ parseGoal(goalElement, definition);
+ }
+ }
+
+ return definition;
+ }
+
+ private void parseGoal(Element goalElement, PluginParameterDefinition
definition) {
+ String goalName = getTextContent(goalElement, "name");
+ GoalParameterDefinition goal = new GoalParameterDefinition(goalName);
+
+ NodeList parametersNodes = goalElement.getElementsByTagNameNS("*",
"parameters");
+ if (parametersNodes.getLength() > 0) {
+ Element parametersElement = (Element) parametersNodes.item(0);
+ NodeList parameterNodes =
parametersElement.getElementsByTagNameNS("*", "parameter");
+
+ for (int i = 0; i < parameterNodes.getLength(); i++) {
+ Element parameterElement = (Element) parameterNodes.item(i);
+ ParameterDefinition parameterDefinition =
parseParameter(parameterElement);
+ goal.addParameter(parameterDefinition);
+ }
+ }
+
+ definition.addGoal(goalName, goal);
+ }
+
+ private ParameterDefinition parseParameter(Element parameterElement) {
+ String name = getTextContent(parameterElement, "name");
+ boolean cacheKey =
Boolean.parseBoolean(getTextContent(parameterElement, "cache-key"));
Review Comment:
The supplied schema declares `cache-key` as `xs:boolean`, for which `1` is a
valid spelling of true. `Boolean.parseBoolean("1")` returns false, so a
schema-valid descriptor using that spelling silently disables automatic
tracking for the parameter. Accept both XML boolean spellings of true.
##########
src/main/resources/plugin-parameters/maven-compiler-plugin.xml:
##########
@@ -0,0 +1,318 @@
+<?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.
+-->
+<plugin xmlns="http://maven.apache.org/PLUGIN-PARAMETERS/1.0.0";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://maven.apache.org/PLUGIN-PARAMETERS/1.0.0
plugin-parameters.xsd">
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <minVersion>3.11.0</minVersion>
+
+ <goals>
+ <goal>
+ <name>compile</name>
+ <parameters>
+ <!-- cache-key=true entries explain why a value change must cause a
cache miss. -->
+ <parameter>
+ <name>source</name>
+ <cache-key>true</cache-key>
+ <description>Source JDK version for compilation; changing it can
change generated bytecode or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>target</name>
+ <cache-key>true</cache-key>
+ <description>Target JDK version for compiled bytecode; changing it
changes generated bytecode</description>
+ </parameter>
+ <parameter>
+ <name>release</name>
+ <cache-key>true</cache-key>
+ <description>JDK release version (combines source and target);
changing it changes generated bytecode and API compatibility</description>
+ </parameter>
+ <parameter>
+ <name>encoding</name>
+ <cache-key>true</cache-key>
+ <description>Source file encoding; changing it can change parsed
source and generated bytecode</description>
+ </parameter>
+ <parameter>
+ <name>debug</name>
+ <cache-key>true</cache-key>
+ <description>Include debugging information in compiled bytecode;
changing it changes artifact contents</description>
+ </parameter>
+ <parameter>
+ <name>debuglevel</name>
+ <cache-key>true</cache-key>
+ <description>Level of debugging information (lines, vars, source);
changing it changes artifact contents</description>
+ </parameter>
+ <parameter>
+ <name>optimize</name>
+ <cache-key>true</cache-key>
+ <description>Optimize compiled bytecode; changing it changes
artifact contents</description>
+ </parameter>
+ <parameter>
+ <name>compilerArgs</name>
+ <cache-key>true</cache-key>
+ <description>Additional compiler arguments; changing them can change
generated bytecode or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>compilerArgument</name>
+ <cache-key>true</cache-key>
+ <description>Single additional compiler argument; changing it can
change generated bytecode or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>annotationProcessorPaths</name>
+ <cache-key>true</cache-key>
+ <description>Classpath for annotation processors; changing it can
change generated sources or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>annotationProcessors</name>
+ <cache-key>true</cache-key>
+ <description>Annotation processors to run; changing them can change
generated sources or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>proc</name>
+ <cache-key>true</cache-key>
+ <description>Annotation processing mode (none, only, proc); changing
it can change generated sources or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>executable</name>
+ <cache-key>true</cache-key>
+ <description>Path to javac executable; changing it can change the
compiler output or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>parameters</name>
+ <cache-key>true</cache-key>
+ <description>Generate metadata for method parameters; changing it
changes bytecode contents</description>
+ </parameter>
+ <parameter>
+ <name>enablePreview</name>
+ <cache-key>true</cache-key>
+ <description>Enable preview features; changing it can change
generated bytecode or compilation success</description>
+ </parameter>
+
+ <!-- No cache-key annotation means false; these settings do not
invalidate the cache. -->
+ <parameter>
+ <name>verbose</name>
+ <description>Verbose compiler output</description>
+ </parameter>
+ <parameter>
+ <name>showWarnings</name>
+ <cache-key>true</cache-key>
+ <description>Show compilation warnings (can change the build outcome
with failOnWarning)</description>
+ </parameter>
+ <parameter>
+ <name>showDeprecation</name>
+ <cache-key>true</cache-key>
+ <description>Show deprecation warnings (can change the build outcome
with failOnWarning)</description>
+ </parameter>
+ <parameter>
+ <name>skipMain</name>
+ <cache-key>true</cache-key>
+ <description>Skip compiling main sources; changing it changes
whether main bytecode is produced</description>
+ </parameter>
+ <parameter>
+ <name>failOnError</name>
+ <cache-key>true</cache-key>
+ <description>Fail build on compilation error (changes the build
outcome)</description>
+ </parameter>
+ <parameter>
+ <name>failOnWarning</name>
+ <cache-key>true</cache-key>
+ <description>Fail build on compilation warning (changes the build
outcome)</description>
+ </parameter>
+ <parameter>
+ <name>fork</name>
+ <cache-key>true</cache-key>
+ <description>Fork compiler into separate process (can change the
compiler, output, or outcome)</description>
+ </parameter>
+ <parameter>
+ <name>maxmem</name>
+ <cache-key>true</cache-key>
+ <description>Maximum memory for compiler process (can change the
build outcome)</description>
+ </parameter>
+ <parameter>
+ <name>meminitial</name>
+ <cache-key>true</cache-key>
+ <description>Initial memory for compiler process (can change the
build outcome)</description>
+ </parameter>
+ <parameter>
+ <name>compilerReuseStrategy</name>
+ <description>Strategy for reusing compiler instances</description>
+ </parameter>
+ <parameter>
+ <name>forceJavacCompilerUse</name>
+ <cache-key>true</cache-key>
+ <description>Force use of javac compiler; changing it can affect
compiler output</description>
+ </parameter>
+ <parameter>
+ <name>staleMillis</name>
+ <cache-key>true</cache-key>
+ <description>Staleness check interval for incremental compilation
(can change generated files)</description>
+ </parameter>
+ <parameter>
+ <name>useIncrementalCompilation</name>
+ <cache-key>true</cache-key>
+ <description>Enable incremental compilation (can change generated
files and build outcome)</description>
+ </parameter>
+ </parameters>
+ </goal>
+
+ <goal>
+ <name>testCompile</name>
+ <parameters>
+ <!-- Same parameters as compile goal -->
+ <parameter>
+ <name>source</name>
+ <cache-key>true</cache-key>
+ <description>Source JDK version for test compilation; changing it
can change generated test bytecode or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>target</name>
+ <cache-key>true</cache-key>
+ <description>Target JDK version for compiled test bytecode; changing
it changes generated test bytecode</description>
+ </parameter>
+ <parameter>
+ <name>release</name>
+ <cache-key>true</cache-key>
+ <description>JDK release version for tests; changing it changes
generated test bytecode and API compatibility</description>
+ </parameter>
Review Comment:
The `testCompile` mojo uses `testSource`, `testTarget`, and `testRelease` in
preference to the inherited `source`, `target`, and `release` fields. Those
test-specific parameters are missing here, so changing only
`-Dmaven.compiler.testRelease` can still reuse test bytecode compiled for the
previous release. Track the test-specific fields as well as the inherited
fallbacks.
##########
src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java:
##########
@@ -253,27 +256,181 @@ public boolean isLogAllProperties(MojoExecution
mojoExecution) {
}
private GoalReconciliation findReconciliationConfig(MojoExecution
mojoExecution) {
- if (cacheConfig.getExecutionControl() == null) {
+ if (mojoExecution == null) {
return null;
}
- final ExecutionControl executionControl =
cacheConfig.getExecutionControl();
- if (executionControl.getReconcile() == null) {
+ final String goal = mojoExecution.getGoal();
+ final Plugin plugin = mojoExecution.getPlugin();
+
+ if (plugin == null) {
return null;
}
- final List<GoalReconciliation> reconciliation =
- executionControl.getReconcile().getPlugins();
+ // First check explicit configuration
+ if (cacheConfig.getExecutionControl() != null
+ && cacheConfig.getExecutionControl().getReconcile() != null) {
+ List<GoalReconciliation> explicitConfigs =
+
cacheConfig.getExecutionControl().getReconcile().getPlugins();
+ for (GoalReconciliation config : explicitConfigs) {
+ if (isPluginMatch(plugin, config) && Strings.CS.equals(goal,
config.getGoal())) {
+ // Validate explicit config against parameter definitions
(with version)
+ validateReconciliationConfig(config, plugin);
+ return config;
+ }
+ }
+ }
- for (GoalReconciliation goalReconciliationConfig : reconciliation) {
- final String goal = mojoExecution.getGoal();
+ // Auto-generate from parameter definitions (track all cache-key
parameters)
+ GoalReconciliation autoGenerated =
generateReconciliationFromParameters(plugin, goal);
+ if (autoGenerated != null) {
+ LOGGER.debug(
+ "Auto-generated reconciliation config for {}:{} with {}
cache-key parameters",
+ plugin.getArtifactId(),
+ goal,
+ autoGenerated.getReconciles() != null
+ ? autoGenerated.getReconciles().size()
+ : 0);
+ }
+ return autoGenerated;
+ }
- if (isPluginMatch(mojoExecution.getPlugin(),
goalReconciliationConfig)
- && Strings.CS.equals(goal,
goalReconciliationConfig.getGoal())) {
- return goalReconciliationConfig;
+ /**
+ * Validates a single reconciliation config against plugin parameter
definitions.
+ * Uses plugin version to load the appropriate parameter definition.
+ */
+ private void validateReconciliationConfig(GoalReconciliation config,
Plugin plugin) {
+ String artifactId = config.getArtifactId();
+ String goal = config.getGoal();
+ String pluginVersion = plugin.getVersion();
+
+ // Load parameter definition for this plugin with version
+ PluginParameterDefinition pluginDefinition =
parameterLoader.load(artifactId, pluginVersion);
+
+ if (pluginDefinition == null) {
+ LOGGER.warn(
+ "No parameter definition found for plugin {}:{} version
{}. "
+ + "Cannot validate reconciliation configuration. "
+ + "Consider adding a parameter definition file to
plugin-parameters/{}.xml",
+ artifactId,
+ goal,
+ pluginVersion != null ? pluginVersion : "unknown",
+ artifactId);
+ return;
+ }
+
+ // Get goal definition
+ GoalParameterDefinition goalDefinition =
pluginDefinition.getGoal(goal);
+ if (goalDefinition == null) {
+ LOGGER.warn(
+ "Goal '{}' not found in parameter definition for plugin {}
version {}. "
+ + "Cannot validate reconciliation configuration.",
+ goal,
+ artifactId,
+ pluginVersion != null ? pluginVersion : "unknown");
+ return;
+ }
+
+ // Validate each tracked property
+ List<TrackedProperty> properties = config.getReconciles();
+
+ for (TrackedProperty property : properties) {
+ String propertyName = property.getPropertyName();
+
+ if (!goalDefinition.hasParameter(propertyName)) {
+ LOGGER.warn(
+ "Unknown parameter '{}' in reconciliation config for
{}:{} version {}. "
+ + "This may indicate a plugin version mismatch
or renamed parameter. "
+ + "Consider updating parameter definition or
removing from reconciliation.",
+ propertyName,
+ artifactId,
+ goal,
+ pluginVersion != null ? pluginVersion : "unknown");
+ } else {
+ ParameterDefinition parameterDefinition =
goalDefinition.getParameter(propertyName);
+ if (!parameterDefinition.isCacheKey()) {
+ LOGGER.warn(
+ "Parameter '{}' in reconciliation config for {}:{}
is not marked as a cache key. "
+ + "Changing it will not invalidate the
cache according to the plugin definition.",
+ propertyName,
+ artifactId,
+ goal);
+ }
}
}
- return null;
+ }
+
+ /**
+ * Auto-generates a reconciliation config by tracking all cache-key
parameters
+ * from the plugin parameter definition.
+ * This provides automatic default tracking for any plugin with parameter
definitions.
+ *
+ * @param plugin The plugin to generate config for
+ * @param goal The goal name
+ * @return Auto-generated config tracking all cache-key parameters, or
null if no parameter definition exists
+ */
+ private GoalReconciliation generateReconciliationFromParameters(Plugin
plugin, String goal) {
+ String artifactId = plugin.getArtifactId();
+ String pluginVersion = plugin.getVersion();
+
+ LOGGER.debug(
+ "Attempting to auto-generate reconciliation config for {}:{}
version {}",
+ artifactId,
+ goal,
+ pluginVersion);
+
+ // Load parameter definition for this plugin
+ PluginParameterDefinition pluginDefinition =
parameterLoader.load(artifactId, pluginVersion);
+ if (pluginDefinition == null) {
Review Comment:
Descriptor lookup uses only the artifact ID, so a different plugin such as
`com.example:maven-compiler-plugin` inherits the Apache compiler's tracked
fields and can have every cached build rejected for missing properties.
Explicit reconciliation is also validated against the wrong plugin's descriptor
in `validateReconciliationConfig()`. Check the descriptor's group ID against
the actual plugin in both paths before using it.
##########
src/main/resources/plugin-parameters/maven-compiler-plugin.xml:
##########
@@ -0,0 +1,318 @@
+<?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.
+-->
+<plugin xmlns="http://maven.apache.org/PLUGIN-PARAMETERS/1.0.0";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://maven.apache.org/PLUGIN-PARAMETERS/1.0.0
plugin-parameters.xsd">
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <minVersion>3.11.0</minVersion>
+
+ <goals>
+ <goal>
+ <name>compile</name>
+ <parameters>
+ <!-- cache-key=true entries explain why a value change must cause a
cache miss. -->
+ <parameter>
+ <name>source</name>
+ <cache-key>true</cache-key>
+ <description>Source JDK version for compilation; changing it can
change generated bytecode or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>target</name>
+ <cache-key>true</cache-key>
+ <description>Target JDK version for compiled bytecode; changing it
changes generated bytecode</description>
+ </parameter>
+ <parameter>
+ <name>release</name>
+ <cache-key>true</cache-key>
+ <description>JDK release version (combines source and target);
changing it changes generated bytecode and API compatibility</description>
+ </parameter>
+ <parameter>
+ <name>encoding</name>
+ <cache-key>true</cache-key>
+ <description>Source file encoding; changing it can change parsed
source and generated bytecode</description>
+ </parameter>
+ <parameter>
+ <name>debug</name>
+ <cache-key>true</cache-key>
+ <description>Include debugging information in compiled bytecode;
changing it changes artifact contents</description>
+ </parameter>
+ <parameter>
+ <name>debuglevel</name>
+ <cache-key>true</cache-key>
+ <description>Level of debugging information (lines, vars, source);
changing it changes artifact contents</description>
+ </parameter>
+ <parameter>
+ <name>optimize</name>
+ <cache-key>true</cache-key>
+ <description>Optimize compiled bytecode; changing it changes
artifact contents</description>
+ </parameter>
+ <parameter>
+ <name>compilerArgs</name>
+ <cache-key>true</cache-key>
+ <description>Additional compiler arguments; changing them can change
generated bytecode or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>compilerArgument</name>
+ <cache-key>true</cache-key>
+ <description>Single additional compiler argument; changing it can
change generated bytecode or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>annotationProcessorPaths</name>
+ <cache-key>true</cache-key>
+ <description>Classpath for annotation processors; changing it can
change generated sources or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>annotationProcessors</name>
+ <cache-key>true</cache-key>
+ <description>Annotation processors to run; changing them can change
generated sources or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>proc</name>
+ <cache-key>true</cache-key>
+ <description>Annotation processing mode (none, only, proc); changing
it can change generated sources or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>executable</name>
+ <cache-key>true</cache-key>
+ <description>Path to javac executable; changing it can change the
compiler output or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>parameters</name>
+ <cache-key>true</cache-key>
+ <description>Generate metadata for method parameters; changing it
changes bytecode contents</description>
+ </parameter>
+ <parameter>
+ <name>enablePreview</name>
+ <cache-key>true</cache-key>
+ <description>Enable preview features; changing it can change
generated bytecode or compilation success</description>
+ </parameter>
+
+ <!-- No cache-key annotation means false; these settings do not
invalidate the cache. -->
+ <parameter>
+ <name>verbose</name>
+ <description>Verbose compiler output</description>
+ </parameter>
+ <parameter>
+ <name>showWarnings</name>
+ <cache-key>true</cache-key>
+ <description>Show compilation warnings (can change the build outcome
with failOnWarning)</description>
+ </parameter>
+ <parameter>
+ <name>showDeprecation</name>
+ <cache-key>true</cache-key>
+ <description>Show deprecation warnings (can change the build outcome
with failOnWarning)</description>
+ </parameter>
+ <parameter>
+ <name>skipMain</name>
+ <cache-key>true</cache-key>
+ <description>Skip compiling main sources; changing it changes
whether main bytecode is produced</description>
+ </parameter>
+ <parameter>
+ <name>failOnError</name>
+ <cache-key>true</cache-key>
+ <description>Fail build on compilation error (changes the build
outcome)</description>
+ </parameter>
+ <parameter>
+ <name>failOnWarning</name>
+ <cache-key>true</cache-key>
+ <description>Fail build on compilation warning (changes the build
outcome)</description>
+ </parameter>
+ <parameter>
+ <name>fork</name>
+ <cache-key>true</cache-key>
+ <description>Fork compiler into separate process (can change the
compiler, output, or outcome)</description>
+ </parameter>
+ <parameter>
+ <name>maxmem</name>
+ <cache-key>true</cache-key>
+ <description>Maximum memory for compiler process (can change the
build outcome)</description>
+ </parameter>
+ <parameter>
+ <name>meminitial</name>
+ <cache-key>true</cache-key>
+ <description>Initial memory for compiler process (can change the
build outcome)</description>
+ </parameter>
+ <parameter>
+ <name>compilerReuseStrategy</name>
+ <description>Strategy for reusing compiler instances</description>
+ </parameter>
+ <parameter>
+ <name>forceJavacCompilerUse</name>
+ <cache-key>true</cache-key>
+ <description>Force use of javac compiler; changing it can affect
compiler output</description>
+ </parameter>
+ <parameter>
+ <name>staleMillis</name>
+ <cache-key>true</cache-key>
+ <description>Staleness check interval for incremental compilation
(can change generated files)</description>
+ </parameter>
+ <parameter>
+ <name>useIncrementalCompilation</name>
+ <cache-key>true</cache-key>
+ <description>Enable incremental compilation (can change generated
files and build outcome)</description>
+ </parameter>
+ </parameters>
+ </goal>
+
+ <goal>
+ <name>testCompile</name>
+ <parameters>
+ <!-- Same parameters as compile goal -->
+ <parameter>
+ <name>source</name>
+ <cache-key>true</cache-key>
+ <description>Source JDK version for test compilation; changing it
can change generated test bytecode or compilation success</description>
+ </parameter>
+ <parameter>
+ <name>target</name>
+ <cache-key>true</cache-key>
+ <description>Target JDK version for compiled test bytecode; changing
it changes generated test bytecode</description>
+ </parameter>
+ <parameter>
+ <name>release</name>
+ <cache-key>true</cache-key>
+ <description>JDK release version for tests; changing it changes
generated test bytecode and API compatibility</description>
+ </parameter>
+ <parameter>
+ <name>encoding</name>
+ <cache-key>true</cache-key>
+ <description>Test source file encoding; changing it can change
parsed source and generated test bytecode</description>
+ </parameter>
+ <parameter>
+ <name>debug</name>
+ <cache-key>true</cache-key>
+ <description>Include debugging information in test bytecode;
changing it changes artifact contents</description>
+ </parameter>
+ <parameter>
+ <name>debuglevel</name>
+ <cache-key>true</cache-key>
+ <description>Level of debugging information for tests; changing it
changes artifact contents</description>
+ </parameter>
+ <parameter>
+ <name>optimize</name>
+ <cache-key>true</cache-key>
+ <description>Optimize test bytecode; changing it changes artifact
contents</description>
+ </parameter>
+ <parameter>
+ <name>compilerArgs</name>
+ <cache-key>true</cache-key>
Review Comment:
For JPMS modules with ordinary test sources,
`TestCompilerMojo.preparePaths()` appends `--patch-module` and `--add-reads` to
`compilerArgs` during execution. These defaults record the mutated list, but
reconciliation compares a freshly configured mojo, so unchanged builds
repeatedly miss the cache; removing only `--module-version` does not fix this.
Snapshot tracked inputs before execution, including copies of mutable values,
and record those snapshots instead.
--
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]
