Copilot commented on code in PR #389: URL: https://github.com/apache/maven-build-cache-extension/pull/389#discussion_r3611404952
########## src/site/markdown/how-to.md: ########## @@ -169,6 +169,175 @@ Add `executionControl/runAlways` section: </executionControl> ``` +### Default Reconciliation Behavior + +The build cache extension automatically tracks certain critical plugin properties by default, even without explicit +`executionControl` configuration. These defaults are derived from plugin parameter descriptors under +`plugin-parameters/`: + +* **maven-compiler-plugin** (`compile` and `testCompile` goals): Tracks functional parameters defined in + `plugin-parameters/maven-compiler-plugin.xml` +* **maven-install-plugin** (`install` and `install-file` goals): Tracks functional parameters defined in + `plugin-parameters/maven-install-plugin.xml` + +This default behavior prevents common cache invalidation issues, particularly in multi-module JPMS (Java Platform Module System) +projects where compiler version changes can cause compilation failures. + +**Overriding Defaults:** When you explicitly configure `executionControl` for a plugin, your explicit configuration completely +overrides the defaults for that plugin. For example, to track only the `release` property for maven-compiler-plugin instead +of the functional parameters from `plugin-parameters/maven-compiler-plugin.xml`: + +```xml +<cache xmlns="http://maven.apache.org/BUILD-CACHE-CONFIG/1.2.0";> + <configuration> + ... + </configuration> + <executionControl> + <reconcile> + <plugins> + <plugin artifactId="maven-compiler-plugin" goal="compile"> + <reconciles> + <reconcile propertyName="release"/> + </reconciles> + </plugin> + </plugins> + </reconcile> + </executionControl> +</cache> +``` + +This configuration in your `.mvn/maven-build-cache-config.xml` file replaces the built-in defaults. You can also define +reconciliation configurations for plugins that don't have built-in defaults using the same syntax. + +### Parameter Validation and Categorization + +The build cache extension includes a parameter validation system that categorizes plugin parameters and validates +reconciliation configurations against known parameter definitions. + +#### Parameter Categories + +All plugin parameters are categorized into two types: + +* **Functional Parameters**: Affect the compiled output or build artifacts (e.g., `source`, `target`, `release`, `encoding`) +* **Behavioral Parameters**: Affect how the build runs but not the output (e.g., `verbose`, `fork`, `maxmem`, `skip`) + +Only **functional** parameters should be tracked in reconciliation configurations, as behavioral parameters don't affect +the build output and shouldn't invalidate the cache. + +#### Validation Features + +The extension automatically validates reconciliation configurations and logs warnings/errors for: + +* **Unknown parameters**: Parameters not defined in the plugin's parameter definition (ERROR level) Review Comment: This section states unknown parameters are logged at **ERROR** level, but the implementation logs them at `WARN` (see `CacheConfigImpl.validateReconciliationConfig()` which uses `LOGGER.warn(...)` for unknown parameters). The doc should match the actual log level to avoid confusing users. ########## src/test/projects/default-reconciliation-with-other-plugin/.mvn/maven-build-cache-config.xml: ########## @@ -0,0 +1,32 @@ +<?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. +--> +<cache xmlns="http://maven.apache.org/BUILD-CACHE-CONFIG/1.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/BUILD-CACHE-CONFIG/1.0.0 https://maven.apache.org/xsd/build-cache-config-1.0.0.xsd";> + <!-- Has executionControl but only for surefire plugin, not compiler - defaults should still apply --> + <executionControl> + <reconcile> + <plugins> + <plugin artifactId="maven-surefire-plugin"> Review Comment: `goal` is a required attribute for reconciliation plugin entries in the build-cache config model/schema (GoalReconciliation extends GoalId with required `goal`). Omitting it here can lead to config parse/validation failures or the reconcile rule being ignored. Add the intended surefire goal explicitly. ########## src/main/resources/plugin-parameters/plugin-parameters.xsd: ########## @@ -0,0 +1,68 @@ +<?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. +--> +<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"; + targetNamespace="http://maven.apache.org/PLUGIN-PARAMETERS/1.0.0"; + xmlns="http://maven.apache.org/PLUGIN-PARAMETERS/1.0.0"; + elementFormDefault="qualified"> + + <xs:element name="plugin" type="PluginType"/> + Review Comment: The documentation and tests use a `<plugins>` wrapper element to support multiple version-specific `<plugin>` definitions, but this XSD only declares `<plugin>` as a valid root element. If users try to validate their descriptor (or if validation is added later), `<plugins>` documents will be rejected. ########## src/site/markdown/how-to.md: ########## @@ -169,6 +169,175 @@ Add `executionControl/runAlways` section: </executionControl> ``` +### Default Reconciliation Behavior + +The build cache extension automatically tracks certain critical plugin properties by default, even without explicit +`executionControl` configuration. These defaults are derived from plugin parameter descriptors under +`plugin-parameters/`: + +* **maven-compiler-plugin** (`compile` and `testCompile` goals): Tracks functional parameters defined in + `plugin-parameters/maven-compiler-plugin.xml` +* **maven-install-plugin** (`install` and `install-file` goals): Tracks functional parameters defined in + `plugin-parameters/maven-install-plugin.xml` + +This default behavior prevents common cache invalidation issues, particularly in multi-module JPMS (Java Platform Module System) +projects where compiler version changes can cause compilation failures. + +**Overriding Defaults:** When you explicitly configure `executionControl` for a plugin, your explicit configuration completely +overrides the defaults for that plugin. For example, to track only the `release` property for maven-compiler-plugin instead +of the functional parameters from `plugin-parameters/maven-compiler-plugin.xml`: + +```xml +<cache xmlns="http://maven.apache.org/BUILD-CACHE-CONFIG/1.2.0";> + <configuration> + ... + </configuration> + <executionControl> + <reconcile> + <plugins> + <plugin artifactId="maven-compiler-plugin" goal="compile"> + <reconciles> + <reconcile propertyName="release"/> + </reconciles> + </plugin> + </plugins> + </reconcile> + </executionControl> +</cache> +``` + +This configuration in your `.mvn/maven-build-cache-config.xml` file replaces the built-in defaults. You can also define +reconciliation configurations for plugins that don't have built-in defaults using the same syntax. + +### Parameter Validation and Categorization + +The build cache extension includes a parameter validation system that categorizes plugin parameters and validates +reconciliation configurations against known parameter definitions. + +#### Parameter Categories + +All plugin parameters are categorized into two types: + +* **Functional Parameters**: Affect the compiled output or build artifacts (e.g., `source`, `target`, `release`, `encoding`) +* **Behavioral Parameters**: Affect how the build runs but not the output (e.g., `verbose`, `fork`, `maxmem`, `skip`) + +Only **functional** parameters should be tracked in reconciliation configurations, as behavioral parameters don't affect +the build output and shouldn't invalidate the cache. + +#### Validation Features + +The extension automatically validates reconciliation configurations and logs warnings/errors for: + +* **Unknown parameters**: Parameters not defined in the plugin's parameter definition (ERROR level) + - May indicate a plugin version mismatch or renamed parameter + - Suggests updating parameter definitions or removing the parameter from reconciliation + +* **Behavioral parameters in reconciliation**: Parameters categorized as behavioral (WARN level) + - Suggests that the parameter likely shouldn't affect cache invalidation + - Consider removing if it doesn't actually affect build output + +#### Adding Parameter Definitions for New Plugins + +Parameter definitions are stored in `src/main/resources/plugin-parameters/{artifactId}.xml`. To add validation for a new plugin: + +1. Create an XML file following the schema in `plugin-parameters.xsd`: + +```xml +<?xml version="1.0" encoding="UTF-8"?> +<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-example-plugin</artifactId> + + <goals> + <goal> + <name>example-goal</name> + <parameters> + <parameter> + <name>outputDirectory</name> + <type>functional</type> + <description>Directory where output is written</description> + </parameter> + <parameter> + <name>verbose</name> + <type>behavioral</type> + <description>Enable verbose logging</description> + </parameter> + </parameters> + </goal> + </goals> +</plugin> +``` + +2. Place the file in the classpath at `plugin-parameters/{artifactId}.xml` + +3. The extension will automatically load and validate against this definition + +#### Version-Specific Parameter Definitions + +The parameter validation system supports version-specific definitions to handle plugins that change parameters across versions. This allows accurate validation even when plugin APIs evolve. + +**How Version Matching Works:** + +- Definitions include a `minVersion` element specifying the minimum plugin version they apply to +- At runtime, the extension selects the definition with the highest `minVersion` that is ≤ the actual plugin version +- Multiple version-specific definitions can exist in a single file + +**Example with version-specific parameters:** + +```xml +<?xml version="1.0" encoding="UTF-8"?> +<plugins> + <!-- Parameters for versions 1.0.0 through 2.x --> + <plugin xmlns="http://maven.apache.org/PLUGIN-PARAMETERS/1.0.0";> Review Comment: The version-specific example claims to follow `plugin-parameters.xsd`, but the root `<plugins>` element in this snippet has no namespace. With the current schema (target namespace + `elementFormDefault="qualified"`), a non-namespaced root element won't validate. Consider adding the namespace to `<plugins>` so the example is schema-valid. ########## src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java: ########## @@ -401,6 +401,14 @@ boolean isParamsMatched( CompletedExecution completedExecution) { List<TrackedProperty> tracked = cacheConfig.getTrackedProperties(mojoExecution); Review Comment: `cacheConfig.getTrackedProperties()` is annotated/expected to be non-null, but callers (and the validation code) treat `getReconciles()` as potentially null. This method immediately uses the returned list (e.g., later `tracked.size()` and the foreach), so a null here would throw an NPE and abort cache validation. Normalize to an empty list defensively before any use. -- 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]
