gnodet commented on code in PR #12430:
URL: https://github.com/apache/maven/pull/12430#discussion_r3541642669
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/CommonsCliUpgradeOptions.java:
##########
@@ -144,6 +148,32 @@ protected static class CLIManager extends
CommonsCliOptions.CLIManager {
public static final String PLUGINS = "plugins";
public static final String ALL = "a";
+ /**
+ * Overrides the default strict parsing to silently ignore
unrecognized options.
+ * This is necessary because mvnup inherits the Maven launcher's
argument handling,
+ * which appends options from {@code .mvn/maven.config}. That file
often contains
+ * Maven build options like {@code -ntp}, {@code -U}, or {@code -T}
that mvnup
+ * does not recognize. Without this override, mvnup would abort with a
+ * {@link ParseException} before applying any fixes.
+ */
+ @Override
+ public CommandLine parse(String[] args) throws ParseException {
+ List<String> currentArgs = new ArrayList<>(List.of(args));
+ Set<String> removed = new HashSet<>();
+ while (true) {
+ try {
+ return super.parse(currentArgs.toArray(new String[0]));
+ } catch (UnrecognizedOptionException e) {
+ String badOption = e.getOption();
+ if (removed.contains(badOption) ||
!currentArgs.remove(badOption)) {
+ // Already tried removing this option or can't find it
— give up
+ throw e;
+ }
+ removed.add(badOption);
+ }
+ }
+ }
Review Comment:
Good catch! Fixed in 7444c4d. The retry loop now also removes the trailing
argument value when the next token doesn't start with `-`. For example, `-T 4`
(two tokens) now correctly removes both instead of leaving `4` as a spurious
goal in `getArgList()`.
Added a test `testUnrecognizedOptionWithSeparateArgument` that verifies `-T
4` doesn't leak into the goals list.
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java:
##########
@@ -874,6 +894,138 @@ private record PluginAnalysis(Set<String>
needsManagement, Set<String> needsDire
private record PluginAnalysisResults(
Map<Path, Set<String>> pluginsNeedingManagement, Map<Path,
Set<String>> pluginsNeedingDirectOverride) {}
+ /**
+ * Checks if the given plugin is a Quarkus Maven plugin.
+ */
+ private boolean isQuarkusPlugin(String groupId, String artifactId) {
+ return "quarkus-maven-plugin".equals(artifactId)
+ && ("io.quarkus".equals(groupId) ||
"io.quarkus.platform".equals(groupId));
+ }
+
+ /**
+ * Checks if a property is used as the version of a Quarkus BOM in
dependencyManagement.
+ * Quarkus BOMs are identified by having groupId io.quarkus or
io.quarkus.platform,
+ * type "pom", and scope "import".
+ */
+ private boolean isPropertyUsedByQuarkusBom(Document pomDocument, String
propertyName) {
+ Element root = pomDocument.root();
+ String propertyRef = "${" + propertyName + "}";
+
+ Element depManagement =
root.childElement(DEPENDENCY_MANAGEMENT).orElse(null);
+ if (depManagement == null) {
+ return false;
+ }
+ Element dependencies =
depManagement.childElement(DEPENDENCIES).orElse(null);
+ if (dependencies == null) {
+ return false;
+ }
+
+ return dependencies.childElements(DEPENDENCY).anyMatch(dep -> {
+ String groupId = getChildText(dep, GROUP_ID);
+ String version = getChildText(dep, VERSION);
+ String type = getChildText(dep, "type");
+ String scope = getChildText(dep, "scope");
+ return ("io.quarkus".equals(groupId) ||
"io.quarkus.platform".equals(groupId))
+ && "pom".equals(type)
+ && "import".equals(scope)
+ && propertyRef.equals(version);
+ });
+ }
+
+ /**
+ * Decouples the Quarkus plugin version from a shared BOM property.
+ * Introduces a new property for the plugin version and updates the
plugin's version element,
+ * leaving the BOM property unchanged.
+ */
+ private boolean decoupleQuarkusPluginVersion(
+ Document pomDocument,
+ Element versionElement,
+ String sharedPropertyName,
+ PluginUpgradeInfo upgrade,
+ String sectionName,
+ UpgradeContext context) {
+
+ // Resolve the current version from the shared property
+ Editor editor = new Editor(pomDocument);
+ Element root = editor.root();
+ Element propertiesElement = root.childElement(PROPERTIES).orElse(null);
+ String currentVersion = null;
+ if (propertiesElement != null) {
+ Element sharedProp =
+
propertiesElement.childElement(sharedPropertyName).orElse(null);
+ if (sharedProp != null) {
+ currentVersion = sharedProp.textContentTrimmed();
+ }
+ }
+
+ if (currentVersion != null && !isVersionBelow(currentVersion,
upgrade.minVersion)) {
+ context.debug("Quarkus plugin version (via shared property " +
sharedPropertyName + ") " + currentVersion
+ + " is already >= " + upgrade.minVersion);
+ return false;
+ }
Review Comment:
Good catch! Fixed in 7444c4d. When `currentVersion` is null (property not
declared in the current POM, likely inherited from parent), the method now
returns `false` immediately instead of falling through and unconditionally
introducing `quarkus-plugin.version=3.26.0`.
This prevents an accidental downgrade when the inherited property already
resolves to a version >= 3.26.0.
Added a test `shouldNotDecoupleWhenSharedPropertyIsInherited` that verifies
no `quarkus-plugin.version` property is introduced when the shared property is
only in the parent POM.
--
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]
