mvnup: Quarkus plugin upgrade, CLI robustness, and Maven 4 compatibility warnings#12430
mvnup: Quarkus plugin upgrade, CLI robustness, and Maven 4 compatibility warnings#12430gnodet wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the mvnup plugin-upgrade logic to specifically handle Quarkus’ quarkus-maven-plugin (both io.quarkus and io.quarkus.platform) to ensure Maven 4 compatibility, including safe handling of shared BOM/version properties and user-facing warnings about platform/plugin version gaps.
Changes:
- Add predefined
PluginUpgradeentries forquarkus-maven-pluginwith a minimum version of3.26.0and an explicit Maven 4 compatibility rationale. - Introduce Quarkus-specific decoupling: when the plugin version is sourced from a property also used by an imported Quarkus BOM, create/use
quarkus-plugin.versioninstead of modifying the BOM property. - Emit a warning when the Quarkus platform BOM property remains behind after decoupling, and add comprehensive tests covering the new behaviors (including pluginManagement).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java | Adds Quarkus plugin upgrade entries and implements BOM-property decoupling + version-gap warning behavior. |
| impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java | Adds targeted unit tests for Quarkus plugin upgrade, decoupling scenarios, warning emission, and pluginManagement handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private boolean decoupleQuarkusPluginVersion( | ||
| Document pomDocument, | ||
| Element pluginElement, | ||
| Element versionElement, | ||
| String sharedPropertyName, | ||
| PluginUpgradeInfo upgrade, | ||
| String sectionName, | ||
| UpgradeContext context) { |
There was a problem hiding this comment.
The method signature is actually:
private boolean decoupleQuarkusPluginVersion(
Document pomDocument,
Element versionElement,
String sharedPropertyName,
PluginUpgradeInfo upgrade,
String sectionName,
UpgradeContext context)There is no pluginElement parameter — the parameter at that position is versionElement, and it IS used at line 983: editor.setTextContent(versionElement, "${" + newPropertyName + "}"). All parameters are used.
Add PluginUpgrade entries for quarkus-maven-plugin (both io.quarkus and io.quarkus.platform groupIds) with minimum version 3.26.0. When the plugin version references the same property as a Quarkus BOM, the strategy decouples it by introducing a separate quarkus-plugin.version property. After upgrading, emits a warning if the platform BOM version is significantly older than the plugin version. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…maven.config mvnup inherits the Maven launcher's argument handling, which appends options from .mvn/maven.config. That file often contains build flags like -ntp, -U, or -T that mvnup does not recognize, causing a ParseException before any upgrades are applied. Override CLIManager.parse() to catch UnrecognizedOptionException and retry after removing the offending option, so mvnup works in projects with a .mvn/maven.config file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gmavenplus-plugin (all versions up to 4.1.1) calls mutating methods on immutable lists returned by the Maven 4 API, causing UnsupportedOperationException on goals like removeStubs. Add a warning-only check to CompatibilityFixStrategy that detects known incompatible plugins and emits a warning with a link to the upstream issue tracker. The check does not modify the POM. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Maven 4's prefix-based artifact filtering may block resolution from repositories that do not publish a prefixes.txt file. When Maven 4 auto-generates an incomplete prefix list, legitimate artifacts are rejected with ArtifactFilteredOutException. Add a warning for non-Maven-Central repositories detected in POMs, advising users to check prefix configuration if builds fail. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Maven 4 validates module paths during POM parsing, before property
interpolation occurs. Paths like <module>spark-${spark.version}</module>
are rejected because the literal string (with the unresolved
expression) does not correspond to an existing directory.
Add a warning for module/subproject elements containing ${...}
expressions, both at root level and inside profiles.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…cy versions
In CI-friendly projects using ${revision}, ${sha1}, or ${changelist}
as the parent version, child modules with versionless dependencies
may fail with 'dependencies.dependency.version is missing' because
Maven 4 validates dependency completeness before fully resolving the
parent's dependencyManagement chain.
Add a warning that detects this pattern: a parent with a CI-friendly
version expression and dependencies without explicit <version> elements.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gnodet
force-pushed
the
fix-https-github-com-apache-maven-issues-12429
branch
from
a431530 to
df2ec27
Compare
gnodet
changed the title
| @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); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this 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.
| if (currentVersion != null && !isVersionBelow(currentVersion, upgrade.minVersion)) { | ||
| context.debug("Quarkus plugin version (via shared property " + sharedPropertyName + ") " + currentVersion | ||
| + " is already >= " + upgrade.minVersion); | ||
| return false; | ||
| } |
There was a problem hiding this 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.
| if (url != null && !url.contains("${")) { | ||
| String normalizedUrl = url.trim().replaceAll("/+$", ""); | ||
| if (!WELL_KNOWN_REPO_URLS.contains(normalizedUrl)) { | ||
| context.warning("Repository '" + (id != null ? id : normalizedUrl) | ||
| + "' is a third-party repository. Maven 4's prefix-based artifact" | ||
| + " filtering may block resolution if the repository does not" | ||
| + " publish a prefixes.txt file. If builds fail with" | ||
| + " 'ArtifactFilteredOutException', check repository" | ||
| + " prefix configuration."); | ||
| } | ||
| } |
There was a problem hiding this comment.
Agreed — this warning was too aggressive. After investigating the actual Maven 4 behavior, the prefix filter defaults to permissive when no prefixes.txt exists (noInputOutcome=true). Repositories without a prefix index are not blocked at all.
The entire #12433 warning has been reverted in e1642cf. The real issue is narrower (auto-discovered prefix files with incomplete content), and Maven already provides per-repo disable options and helpful hints in ArtifactFilteredOutException messages.
Maven 4's prefix filtering defaults to permissive behavior when a repository has no prefixes.txt — artifacts are allowed through, not blocked. The warning was based on a misunderstanding of the default noInputOutcome=true setting. The actual issue is narrower: only auto-discovered prefix files with incomplete content can cause false negatives, and Maven already provides per-repository disable options and helpful hints in ArtifactFilteredOutException messages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rty guard - CLI option stripping: also remove trailing argument values for options like "-T 4" (two tokens) to prevent the value from being treated as a spurious goal. - BOM property decoupling: skip when the shared property is not declared in the current POM (inherited from parent), since we cannot resolve its actual value and introducing a new property could downgrade an already-sufficient inherited version. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
In-Depth PR ReviewI've reviewed all assertions in this PR about Maven 4 behavior, checked upstream sources, and verified edge cases. Here's a detailed analysis. 🔴 Critical: gmavenplus-plugin claim is factually incorrectThe
This is wrong. The latest version is 5.1.0 (released 2026-07-02), and the Maven 4 fix was merged in 4.2.0 — via groovy/GMavenPlus#328, authored by @gnodet. The 4.2.1 release notes explicitly say "Support Maven 4 (#328) Thanks to @gnodet for this PR!" Additionally, the Maven-side issue MNG-8657 was also fixed (resolved 2025-05-27) via MNG-8662 which added the missing A follow-up issue #341 about Recommendation: Convert 🟡 Questionable: "3.31.x ceiling" claim is unsubstantiatedThe Javadoc on
And the warning message tells users:
I could not verify this claim:
Recommendation: Either provide evidence for the 3.31.x ceiling or soften the warning to something like: "Consider upgrading the platform to match — mismatched plugin and platform versions may cause unexpected behavior." A false hard boundary can lead users to avoid valid upgrades. 🟡 Wrong upstream reference in PR descriptionThe PR body references
The minimum version 3.26.0 is correct. 🟢 Verified: Quarkus 3.26.0 minimum versionConfirmed via upstream: quarkusio/quarkus#37627 was fixed by PR #48248, milestoned 3.26.0.CR1. The fix addresses Aether API changes in Maven 4 where 🟢 Verified: CLI retry loop (CommonsCliUpgradeOptions)The retry-loop approach for handling unrecognized options from Minor edge case to be aware of: Boolean flags (e.g., 🟢 Verified: BOM property decoupling logicThe Quarkus BOM decoupling logic is correct:
🟢 Verified: Warning checks
Summary
|
…n version gap warning - Move gmavenplus-plugin from KNOWN_INCOMPATIBLE_PLUGINS (permanent warning) to PluginUpgrade with minimum version 4.2.0. The Maven 4 incompatibility was fixed in groovy/GMavenPlus#328 (released in 4.2.0), and the Maven-side fix landed in MNG-8662. - Soften the Quarkus version gap warning: remove the unsubstantiated claim about binary incompatibility at 3.32.0 and replace with general version alignment advice. - Fix wrong Quarkus upstream reference in PR description (#46889 → #37627). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
This PR adds several mvnup improvements:
quarkus-maven-pluginupgrade support (bothio.quarkusandio.quarkus.platformgroupIds) with BOM property decoupling and version gap warnings.mvn/maven.config(e.g.-ntp,-U,-T) instead of crashing with aParseExceptiongmavenplus-pluginto 4.2.0+ for Maven 4 compatibility (fixed via groovy/GMavenPlus#328)<module>paths containing${...}property expressions, which Maven 4 rejects because it validates paths before interpolation${revision}projects where child dependencies lack explicit<version>elements, causing Maven 4 validation failuresDetails
Quarkus plugin upgrade (#12429)
Quarkus plugin versions < 3.26.0 use
smallrye-beanbagto bootstrap the Maven Resolver, which is incompatible with Maven 4's reorganized Aether API (see quarkusio/quarkus#37627, fixed by PR #48248, milestone 3.26.0.CR1). Many Quarkus projects reuse a single property (e.g.${quarkus.platform.version}) for both the platform BOMs and the plugin version. When mvnup bumps the plugin version, it introduces a separatequarkus-plugin.versionproperty to avoid altering the BOM-managed dependency tree.CLI robustness (#12431)
mvnup inherits the Maven launcher's argument handling, which appends options from
.mvn/maven.config. The fix overridesCLIManager.parse()to catchUnrecognizedOptionExceptionand retry after removing the unrecognized option.gmavenplus-plugin upgrade (#12432)
gmavenplus-plugin versions < 4.2.0 call mutating methods on immutable lists returned by the Maven 4 API (see MNG-8657). This was fixed in gmavenplus 4.2.0 via groovy/GMavenPlus#328. mvnup now auto-upgrades to 4.2.0 as a minimum version.
Maven 4 compatibility warnings (#12434, #12435)
Two warning-only checks in
CompatibilityFixStrategythat detect patterns known to fail with Maven 4 but which cannot be auto-fixed. These emit warnings viacontext.warning()without modifying POMs or affecting thehasIssuesflag.Note: an initial implementation of a prefix-filtering warning (#12433) was included and then reverted — Maven 4's prefix filter defaults to permissive behavior (
noInputOutcome=true) when noprefixes.txtexists, so the blanket warning was incorrect.Test plan
-ntp,-U,-T4,--no-transfer-progresswith mvnup options${revision}, non-CI-friendly no warn, explicit versions,${changelist}, no parent)🤖 Generated with Claude Code