cowwoc commented on code in PR #389:
URL:
https://github.com/apache/maven-build-cache-extension/pull/389#discussion_r3608969042
##########
src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java:
##########
@@ -974,14 +974,18 @@ private boolean isCachedSegmentPropertiesPresent(
return false;
}
+ // Allow cache restore even if some tracked properties are missing
from the cached build.
+ // The reconciliation check will detect mismatches and trigger
rebuild if needed.
+ // This provides backward compatibility when new properties are
added to tracking.
if (!DtoUtils.containsAllProperties(cachedExecution,
trackedProperties)) {
- LOGGER.warn(
- "Cached build record doesn't contain all tracked
properties. Plugin: {}, goal: {},"
- + " executionId: {}",
+ LOGGER.info(
+ "Cached build record doesn't contain all
currently-tracked properties. "
+ + "Plugin: {}, goal: {}, executionId: {}. "
+ + "Proceeding with cache restore -
reconciliation will verify parameters.",
mojoExecution.getPlugin(),
mojoExecution.getGoal(),
mojoExecution.getExecutionId());
- return false;
+ // Don't reject the cache - let reconciliation check handle it
}
Review Comment:
Addressed in 25e09ea: cache restoration now rejects cached executions
missing tracked properties, forcing a normal rebuild.
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -422,8 +430,18 @@ boolean isParamsMatched(
} catch (IllegalAccessException e) {
LOGGER.error("Cannot extract plugin property {} from mojo {}",
propertyName, mojo, e);
return false;
+ } catch (Exception e) {
+ // Catch all exceptions including NullPointerException when
property doesn't exist in mojo
+ LOGGER.warn(
+ "Property '{}' not found in mojo {} - treating as
null",
+ propertyName,
+ mojo.getClass().getSimpleName());
+ currentValue = "null";
}
Review Comment:
Addressed in 25e09ea: unexpected extraction failures are no longer treated
as "null"; they log and return false so the cache consistency check falls back
to a full build. IllegalAccessException retains its existing error handling.
##########
src/main/java/org/apache/maven/buildcache/xml/PluginParameterLoader.java:
##########
@@ -0,0 +1,271 @@
+/*
+ * 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.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+
+import java.io.InputStream;
+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.apache.maven.buildcache.xml.PluginParameterDefinition.ParameterType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.NodeList;
+
+/**
+ * Loads plugin parameter definitions from classpath resources.
+ * Definitions are stored in
src/main/resources/plugin-parameters/{artifactId}.xml
+ */
+public 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)
+ */
+ public 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.
+ */
+ public 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 is =
getClass().getClassLoader().getResourceAsStream(resourcePath);
+
+ if (is == null) {
+ LOGGER.debug("No parameter definition found for plugin: {}",
artifactId);
+ return null;
+ }
+
+ try {
+ java.util.List<PluginParameterDefinition> allDefinitions =
parseDefinitions(is, artifactId);
+
+ PluginParameterDefinition bestMatch =
findBestMatch(allDefinitions, pluginVersion);
+
+ if (bestMatch != null) {
+ definitions.put(cacheKey, bestMatch);
+ LOGGER.info(
+ "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(g -> g.getParameters().size())
+ .sum());
+ }
+
+ return bestMatch;
+ } catch (Exception e) {
+ LOGGER.warn("Failed to load parameter definition for {}: {}",
artifactId, e.getMessage(), e);
+ return null;
+ }
Review Comment:
Addressed in 25e09ea: the descriptor parser now uses try-with-resources,
closing the parameter definition InputStream on every path.
##########
src/main/java/org/apache/maven/buildcache/xml/PluginParameterLoader.java:
##########
@@ -0,0 +1,271 @@
+/*
+ * 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.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+
+import java.io.InputStream;
+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.apache.maven.buildcache.xml.PluginParameterDefinition.ParameterType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.NodeList;
+
+/**
+ * Loads plugin parameter definitions from classpath resources.
+ * Definitions are stored in
src/main/resources/plugin-parameters/{artifactId}.xml
+ */
+public 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)
+ */
+ public 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.
+ */
+ public 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 is =
getClass().getClassLoader().getResourceAsStream(resourcePath);
+
+ if (is == null) {
+ LOGGER.debug("No parameter definition found for plugin: {}",
artifactId);
+ return null;
+ }
+
+ try {
+ java.util.List<PluginParameterDefinition> allDefinitions =
parseDefinitions(is, artifactId);
+
+ PluginParameterDefinition bestMatch =
findBestMatch(allDefinitions, pluginVersion);
+
+ if (bestMatch != null) {
+ definitions.put(cacheKey, bestMatch);
+ LOGGER.info(
+ "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(g -> g.getParameters().size())
+ .sum());
Review Comment:
Addressed in 25e09ea: the parameter-definition load message is now logged at
DEBUG.
##########
src/main/resources/plugin-parameters/maven-compiler-plugin.xml:
##########
@@ -0,0 +1,327 @@
+<?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>
+ <!-- FUNCTIONAL: Affect compiled bytecode or compilation behavior -->
+ <parameter>
+ <name>source</name>
+ <type>functional</type>
+ <description>Source JDK version for compilation</description>
+ </parameter>
+ <parameter>
+ <name>target</name>
+ <type>functional</type>
+ <description>Target JDK version for compiled bytecode</description>
+ </parameter>
+ <parameter>
+ <name>release</name>
+ <type>functional</type>
+ <description>JDK release version (combines source and
target)</description>
+ </parameter>
+ <parameter>
+ <name>encoding</name>
+ <type>functional</type>
+ <description>Source file encoding</description>
+ </parameter>
+ <parameter>
+ <name>debug</name>
+ <type>functional</type>
+ <description>Include debugging information in compiled
bytecode</description>
+ </parameter>
+ <parameter>
+ <name>debuglevel</name>
+ <type>functional</type>
+ <description>Level of debugging information (lines, vars,
source)</description>
+ </parameter>
+ <parameter>
+ <name>optimize</name>
+ <type>functional</type>
+ <description>Optimize compiled bytecode</description>
+ </parameter>
+ <parameter>
+ <name>compilerArgs</name>
+ <type>functional</type>
+ <description>Additional compiler arguments</description>
+ </parameter>
+ <parameter>
+ <name>compilerArgument</name>
+ <type>functional</type>
+ <description>Single additional compiler argument</description>
+ </parameter>
+ <parameter>
+ <name>annotationProcessorPaths</name>
+ <type>functional</type>
+ <description>Classpath for annotation processors</description>
+ </parameter>
+ <parameter>
+ <name>annotationProcessors</name>
+ <type>functional</type>
+ <description>Annotation processors to run</description>
+ </parameter>
+ <parameter>
+ <name>proc</name>
+ <type>functional</type>
+ <description>Annotation processing mode (none, only,
proc)</description>
+ </parameter>
+ <parameter>
+ <name>executable</name>
+ <type>functional</type>
+ <description>Path to javac executable (different compiler may
produce different output)</description>
+ </parameter>
+ <parameter>
+ <name>parameters</name>
+ <type>functional</type>
+ <description>Generate metadata for method parameters</description>
+ </parameter>
+ <parameter>
+ <name>enablePreview</name>
+ <type>functional</type>
+ <description>Enable preview features</description>
+ </parameter>
+
+ <!-- BEHAVIORAL: Affect how compilation runs but not the output -->
Review Comment:
Addressed in 25e09ea: the XML comment now states that behavioral parameters
are the default and output-affecting exceptions are marked FUNCTIONAL.
--
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]
