gnodet-bot commented on code in PR #13249: URL: https://github.com/apache/maven/pull/13249#discussion_r4081516927
########## api/maven-api-core/src/main/mdo/reactor.mdo: ########## @@ -0,0 +1,237 @@ +<?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. + +--> + +<model xmlns="http://codehaus-plexus.github.io/MODELLO/2.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://codehaus-plexus.github.io/MODELLO/2.0.0 https://codehaus-plexus.github.io/modello/xsd/modello-2.0.0.xsd"; + xml.namespace="http://maven.apache.org/REACTOR/${version}"; + xml.schemaLocation="https://maven.apache.org/xsd/reactor-${version}.xsd";> + + <id>reactor</id> + <name>ReactorConfig</name> + <description><![CDATA[ + <p>This is a reference for the Reactor Configuration descriptor.</p> + <p>The default location for the Reactor Configuration descriptor file is <code>${maven.projectBasedir}/.mvn/reactor.xml</code></p> + <p>It consolidates reactor-scoped build configuration previously split across <code>.mvn/maven.config</code> + (CLI flag defaults) and <code>.mvn/extensions.xml</code> (core extensions), and adds support for + named build aliases and reactor-level lifecycle phase injection.</p> + ]]></description> + + <defaults> + <default> + <key>package</key> + <value>org.apache.maven.api.reactor</value> + </default> + </defaults> + + <classes> + <class rootElement="true" xml.tagName="reactor" xsd.compositor="sequence"> + <name>ReactorConfig</name> + <description>Reactor-scoped build configuration for a Maven project.</description> + <version>1.0.0+</version> + <fields> + <field> + <name>options</name> + <description>CLI options to prepend to every invocation. Replaces .mvn/maven.config when present. + Expressed as whitespace-separated, quote-aware inline text (e.g. {@code -T4 --no-transfer-progress}). + For options with spaces in their values, use the structured form via {@link #getOptionArgs()}.</description> + <version>1.0.0+</version> + <required>false</required> + <type>String</type> + </field> + <field> + <name>optionArgs</name> + <description>Structured CLI options — each <arg> element is one option token. + Use instead of {@link #getOptions()} when option values contain spaces. + Mutually exclusive with options.</description> + <version>1.0.0+</version> + <required>false</required> + <association xml.itemsStyle="flat" xml.tagName="arg"> + <type>String</type> + <multiplicity>*</multiplicity> + </association> + </field> + <field> + <name>extensions</name> + <description>Core extensions to load. Replaces .mvn/extensions.xml when present.</description> + <version>1.0.0+</version> + <required>false</required> + <association xml.itemsStyle="flat" xml.tagName="extension"> + <type>ReactorExtension</type> + <multiplicity>*</multiplicity> + </association> + </field> + <field> + <name>aliases</name> + <description>Named build aliases. The first non-flag argument on the command line is matched against + alias names; if found it is replaced by the alias expansion.</description> Review Comment: ⚠️ **Documentation/implementation mismatch: alias expansion semantics** The description says *"The first non-flag argument on the command line is matched against alias names"*, but the actual implementation in `MavenParser.expandAlias()` expands **every** argument that matches an alias name — not just the first non-flag one. The PR description and the code's own Javadoc both correctly describe the all-args behavior. This description will end up in the generated Javadoc and XSD documentation, misleading users who define multiple aliases and expect all of them to be expanded in a single invocation (e.g. `mvn ci rel` where both `ci` and `rel` are aliases). ########## api/maven-api-core/src/main/mdo/reactor.mdo: ########## @@ -0,0 +1,237 @@ +<?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. + +--> + +<model xmlns="http://codehaus-plexus.github.io/MODELLO/2.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://codehaus-plexus.github.io/MODELLO/2.0.0 https://codehaus-plexus.github.io/modello/xsd/modello-2.0.0.xsd"; + xml.namespace="http://maven.apache.org/REACTOR/${version}"; + xml.schemaLocation="https://maven.apache.org/xsd/reactor-${version}.xsd";> + + <id>reactor</id> + <name>ReactorConfig</name> + <description><![CDATA[ + <p>This is a reference for the Reactor Configuration descriptor.</p> + <p>The default location for the Reactor Configuration descriptor file is <code>${maven.projectBasedir}/.mvn/reactor.xml</code></p> + <p>It consolidates reactor-scoped build configuration previously split across <code>.mvn/maven.config</code> + (CLI flag defaults) and <code>.mvn/extensions.xml</code> (core extensions), and adds support for + named build aliases and reactor-level lifecycle phase injection.</p> + ]]></description> + + <defaults> + <default> + <key>package</key> + <value>org.apache.maven.api.reactor</value> + </default> + </defaults> + + <classes> + <class rootElement="true" xml.tagName="reactor" xsd.compositor="sequence"> + <name>ReactorConfig</name> + <description>Reactor-scoped build configuration for a Maven project.</description> + <version>1.0.0+</version> + <fields> + <field> + <name>options</name> + <description>CLI options to prepend to every invocation. Replaces .mvn/maven.config when present. + Expressed as whitespace-separated, quote-aware inline text (e.g. {@code -T4 --no-transfer-progress}). + For options with spaces in their values, use the structured form via {@link #getOptionArgs()}.</description> + <version>1.0.0+</version> + <required>false</required> + <type>String</type> + </field> + <field> + <name>optionArgs</name> + <description>Structured CLI options — each <arg> element is one option token. + Use instead of {@link #getOptions()} when option values contain spaces. + Mutually exclusive with options.</description> + <version>1.0.0+</version> + <required>false</required> + <association xml.itemsStyle="flat" xml.tagName="arg"> + <type>String</type> + <multiplicity>*</multiplicity> + </association> + </field> + <field> + <name>extensions</name> + <description>Core extensions to load. Replaces .mvn/extensions.xml when present.</description> + <version>1.0.0+</version> + <required>false</required> + <association xml.itemsStyle="flat" xml.tagName="extension"> + <type>ReactorExtension</type> + <multiplicity>*</multiplicity> + </association> + </field> + <field> + <name>aliases</name> + <description>Named build aliases. The first non-flag argument on the command line is matched against + alias names; if found it is replaced by the alias expansion.</description> + <version>1.0.0+</version> + <required>false</required> + <association xml.itemsStyle="flat" xml.tagName="alias"> + <type>Alias</type> + <multiplicity>*</multiplicity> + </association> + </field> + <field> + <name>phases</name> + <description>Custom lifecycle phase injections. Phases are registered in the lifecycle DAG + with the declared after/before edges.</description> + <version>1.0.0+</version> + <required>false</required> + <association xml.itemsStyle="flat" xml.tagName="phase"> + <type>PhaseInjection</type> + <multiplicity>*</multiplicity> + </association> + </field> + </fields> + </class> + + <class xml.tagName="extension"> + <name>ReactorExtension</name> + <description>Describes a core extension to load.</description> + <version>1.0.0+</version> + <fields> + <field> + <name>groupId</name> + <description>The group ID of the extension's artifact.</description> + <version>1.0.0+</version> + <required>true</required> + <type>String</type> + </field> + <field> + <name>artifactId</name> + <description>The artifact ID of the extension.</description> + <version>1.0.0+</version> + <required>true</required> + <type>String</type> + </field> + <field> + <name>version</name> + <description>The version of the extension.</description> + <version>1.0.0+</version> + <required>true</required> + <type>String</type> + </field> + <field> + <name>classLoadingStrategy</name> + <description>The class loading strategy: 'self-first' (the default), 'parent-first' (loads classes from the + parent, then from the extension) or 'plugin' (follows the rules from extensions defined as plugins).</description> + <version>1.0.0+</version> + <defaultValue>self-first</defaultValue> + <required>false</required> + <type>String</type> + </field> + <field> + <name>configuration</name> + <description>Extension-specific configuration passed to the extension during initialization. + The structure is extension-defined.</description> + <version>1.0.0+</version> + <required>false</required> + <type>DOM</type> + </field> + </fields> + <codeSegments> + <codeSegment> + <version>1.0.0+</version> + <code> + <![CDATA[ + /** + * Gets the identifier of the extension. + * + * @return The extension id in the form {@code <groupId>:<artifactId>:<version>}, never {@code null}. + */ + public String getId() { + return (getGroupId() == null ? "[unknown-group-id]" : getGroupId()) + + ":" + (getArtifactId() == null ? "[unknown-artifact-id]" : getArtifactId()) + + ":" + (getVersion() == null ? "[unknown-version]" : getVersion()); + } + ]]> + </code> + </codeSegment> + </codeSegments> + </class> + + <class xml.tagName="alias"> + <name>Alias</name> + <description>A named build alias. When the first non-flag argument matches the alias name, + it is replaced by the alias expansion. Supports both inline text (shorthand, whitespace-separated) Review Comment: ⚠️ **Same doc mismatch on the `Alias` class itself** Same issue as the `aliases` field description: says "first non-flag argument" but the implementation expands all matching arguments. ########## impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenParser.java: ########## @@ -76,6 +174,21 @@ protected MavenOptions parseMavenAtFileOptions(Path atFile) { } } + protected MavenOptions parseMavenReactorOptions(List<String> args) { + try { + MavenOptions options = parseArgs("reactor.xml", args); + if (options.goals().isPresent()) { + // <options> can only contain options, not goals/phases + throw new IllegalArgumentException("Unrecognized entries in reactor.xml <options>: " + + options.goals().get()); + } Review Comment: 💡 ** drops the itself as the cause** `org.apache.commons.cli.ParseException` has no cause (it is the root exception); `e.getCause()` returns `null`, so the `IllegalArgumentException` is constructed with a `null` cause. The original exception is swallowed — only its message survives. The same pattern appears in `parseMavenCliOptions` and `parseMavenAtFileOptions`, so this is pre-existing and presumably intentional, but it means stack traces are lost when reactor option parsing fails. If intentional (Commons CLI parse errors carry enough info in the message alone), the three catch blocks should have a matching comment explaining the choice. If not, the fix is: -- 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]
