Skip to content

Fix mvnup compatibility: jar-plugin 3.4.2, exec-plugin submodule coverage, toolchain warning #13059

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gnodet wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fix mvnup compatibility: jar-plugin 3.4.2, exec-plugin submodule coverage, toolchain warning #13059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bea123f
Fix mvnup compatibility issues: jar-plugin target, exec-plugin submod…
gnodet Sep 6, 2026
d19e12d
Address review findings: fix Files.walk resource leak and verify warning
gnodet Sep 7, 2026
700038a
Fix property-based plugin version resolution across multi-module proj…
gnodet Sep 8, 2026
63c472c
Fix spurious warning when property already at target version
gnodet Sep 8, 2026
05c3a06
Fix mvnup regressions from compat test run on maven-4.0.x-mvnup-fixes
gnodet Sep 9, 2026
b7a5b55
Fix debug log messages: 'needs upgrade to' -> 'needs version change to'
gnodet Sep 11, 2026
4363887
spotless:apply
gnodet Sep 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
192 changes: 138 additions & 54 deletions ...n-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,12 @@ public class PluginUpgradeStrategy extends AbstractUpgradeStrategy {
DEFAULT_MAVEN_PLUGIN_GROUP_ID,
"maven-resources-plugin",
"3.3.1",
"4.0.0-beta-1",
"Pre-release versions compiled against different Maven 4 API signatures"),
"maven-resources-plugin 4.0.0-beta-1 has API incompatibilities at runtime"
+ " (NoSuchMethodError: ProjectManager.getResources); use stable 3.3.1"),
new PluginUpgrade(
DEFAULT_MAVEN_PLUGIN_GROUP_ID,
"maven-jar-plugin",
"3.5.0",
"3.3.1",
"4.0.0-beta-1",
"Pre-release versions compiled against different Maven 4 API signatures"),
new PluginUpgrade(
Expand Down Expand Up @@ -252,7 +252,7 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
boolean hasUpgrades = false;

// Apply direct plugin upgrades in the document
hasUpgrades |= upgradePluginsInDocument(pomDocument, context);
hasUpgrades |= upgradePluginsInDocument(pomDocument, pomMap, context);

// Add plugin management based on effective model analysis
Set<String> pluginsForManagement =
Expand Down Expand Up @@ -303,7 +303,7 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
* Upgrades plugins in the document. Checks both build/plugins and build/pluginManagement/plugins sections. Only
* processes plugins explicitly defined in the current POM document.
*/
private boolean upgradePluginsInDocument(Document pomDocument, UpgradeContext context) {
private boolean upgradePluginsInDocument(Document pomDocument, Map<Path, Document> pomMap, UpgradeContext context) {
Element root = pomDocument.root();
boolean hasUpgrades = false;

Expand All @@ -316,7 +316,7 @@ private boolean upgradePluginsInDocument(Document pomDocument, UpgradeContext co
Element pluginsElement = buildElement.childElement(PLUGINS).orElse(null);
if (pluginsElement != null) {
hasUpgrades |= upgradePluginsInSection(
pluginsElement, pluginUpgrades, pomDocument, BUILD + "/" + PLUGINS, context);
pluginsElement, pluginUpgrades, pomDocument, pomMap, BUILD + "/" + PLUGINS, context);
}

// Check build/pluginManagement/plugins
Expand All @@ -330,6 +330,7 @@ private boolean upgradePluginsInDocument(Document pomDocument, UpgradeContext co
managedPluginsElement,
pluginUpgrades,
pomDocument,
pomMap,
BUILD + "/" + PLUGIN_MANAGEMENT + "/" + PLUGINS,
context);
}
Expand Down Expand Up @@ -361,6 +362,7 @@ private boolean upgradePluginsInSection(
Element pluginsElement,
Map<String, PluginUpgradeInfo> pluginUpgrades,
Document pomDocument,
Map<Path, Document> pomMap,
String sectionName,
UpgradeContext context) {

Expand Down Expand Up @@ -389,13 +391,13 @@ private boolean upgradePluginsInSection(
PluginUpgradeInfo upgrade = pluginUpgrades.get(pluginKey);

if (upgrade != null) {
upgraded =
upgradePluginVersion(pluginElement, upgrade, pomDocument, sectionName, context);
upgraded = upgradePluginVersion(
pluginElement, upgrade, pomDocument, pomMap, sectionName, context);
}
}
}

upgraded |= upgradePluginDependencies(pluginElement, pomDocument, sectionName, context);
upgraded |= upgradePluginDependencies(pluginElement, pomDocument, pomMap, sectionName, context);

return upgraded;
})
Expand All @@ -409,6 +411,7 @@ private boolean upgradePluginVersion(
Element pluginElement,
PluginUpgradeInfo upgrade,
Document pomDocument,
Map<Path, Document> pomMap,
String sectionName,
UpgradeContext context) {
Element versionElement = pluginElement.childElement(VERSION).orElse(null);
Expand Down Expand Up @@ -487,20 +490,32 @@ && isPropertyUsedByQuarkusBom(pomDocument, propertyName)) {
pomDocument, versionElement, propertyName, upgrade, sectionName, context);
}
// Update property value if it's below minimum version
return upgradePropertyVersion(pomDocument, propertyName, upgrade, sectionName, context);
return upgradePropertyVersion(pomDocument, pomMap, propertyName, upgrade, sectionName, context);
} else {
// Check for Maven 4 pre-release versions (alpha/beta/rc) that should be
// upgraded to the latest available pre-release rather than downgraded to 3.x.
if (isMaven4PreRelease(currentVersion) && upgrade.latestPreRelease != null) {
if (isVersionBelow(context, currentVersion, upgrade.latestPreRelease)) {
// Check for Maven 4 pre-release versions (alpha/beta/rc).
if (isMaven4PreRelease(currentVersion)) {
if (upgrade.latestPreRelease != null) {
// Upgrade to the latest pre-release (don't downgrade to 3.x).
if (isVersionBelow(context, currentVersion, upgrade.latestPreRelease)) {
Editor editor = new Editor(pomDocument);
editor.setTextContent(versionElement, upgrade.latestPreRelease);
context.detail("Upgraded " + upgrade.groupId + ":" + upgrade.artifactId + " from pre-release "
+ currentVersion + " to " + upgrade.latestPreRelease + " in " + sectionName);
return true;
} else {
context.debug("Plugin " + upgrade.groupId + ":" + upgrade.artifactId + " version "
+ currentVersion + " is already >= " + upgrade.latestPreRelease);
}
} else {
// No stable 4.x pre-release line — downgrade to the stable minVersion.
// 4.0.0-beta-x versions compiled against a different API snapshot are
// incompatible at runtime; they must be pinned to the stable release.
Editor editor = new Editor(pomDocument);
editor.setTextContent(versionElement, upgrade.latestPreRelease);
context.detail("Upgraded " + upgrade.groupId + ":" + upgrade.artifactId + " from pre-release "
+ currentVersion + " to " + upgrade.latestPreRelease + " in " + sectionName);
editor.setTextContent(versionElement, upgrade.minVersion);
context.detail("Downgraded " + upgrade.groupId + ":" + upgrade.artifactId + " from incompatible "
+ "pre-release " + currentVersion + " to stable " + upgrade.minVersion
+ " in " + sectionName);
return true;
} else {
context.debug("Plugin " + upgrade.groupId + ":" + upgrade.artifactId + " version " + currentVersion
+ " is already >= " + upgrade.latestPreRelease);
}
return false;
}
Expand All @@ -523,49 +538,106 @@ && isPropertyUsedByQuarkusBom(pomDocument, propertyName)) {

/**
* Upgrades a property value if it represents a plugin version below the minimum.
* First checks the current POM's properties, then searches other POMs in the project
* (e.g., parent POMs) if the property is not found locally.
*/
private boolean upgradePropertyVersion(
Document pomDocument,
Map<Path, Document> pomMap,
String propertyName,
PluginUpgradeInfo upgrade,
String sectionName,
UpgradeContext context) {
Editor editor = new Editor(pomDocument);
Element root = editor.root();
// First, try the current POM's properties
if (upgradePropertyInDocument(pomDocument, propertyName, upgrade, sectionName, context)) {
return true;
}

// Check if property exists in the current POM but is already at/above minimum (no upgrade needed).
// In that case, skip the cross-POM search and the warning — the property IS defined.
Element currentRoot = pomDocument.root();
Element currentProps = currentRoot.childElement(PROPERTIES).orElse(null);
if (currentProps != null && currentProps.childElement(propertyName).isPresent()) {
return false; // Found in current POM, no upgrade needed
}

// Property not in current POM — search other POMs in the project (e.g., parent POM)
for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
Document otherDoc = entry.getValue();
if (otherDoc == pomDocument) {
continue; // Skip the current POM, already checked
}
if (upgradePropertyInDocument(otherDoc, propertyName, upgrade, sectionName, context)) {
return true;
}
// Check if property exists in this POM but already at/above minimum
Element otherRoot = otherDoc.root();
Element otherProps = otherRoot.childElement(PROPERTIES).orElse(null);
if (otherProps != null && otherProps.childElement(propertyName).isPresent()) {
return false; // Found in another POM, no upgrade needed
}
}

// Property not found anywhere in the project
context.warning("Property " + propertyName + " not found in any project POM properties");
Comment on lines +551 to +582

@gnodet gnodet Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Spurious warning when property is already at target version.

upgradePropertyInDocument() returns false for two distinct cases:

  1. Property element not found → fall-through to search other POMs is correct
  2. Property found but already at/above minimum → fall-through is wrong

In case 2, the loop finds nothing in sibling POMs (the property is in the current POM, not there), falls through to this line, and emits "Property X not found in any project POM properties" — which is factually wrong. The property was found; it just didn't need upgrading.

Concrete scenario: Root POM has <exec.maven.version>3.5.0</exec.maven.version> (already at target). Assembly submodule uses <version>${exec.maven.version}</version>. User runs mvnup. upgradePropertyInDocument on the current POM returns false (already at min), the pomMap loop finds nothing, warning fires. User sees a confusing "not found" message for a property that is perfectly defined.

Fix: track whether the property was found (regardless of upgrade outcome) to suppress the false warning. Add an existence check before the search loop:

Suggested change
// First, try the current POM's properties
if (upgradePropertyInDocument(pomDocument, propertyName, upgrade, sectionName, context)) {
return true;
}
// Property not found or not upgradable in current POM — search other POMs in the project
for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
Document otherDoc = entry.getValue();
if (otherDoc == pomDocument) {
continue; // Skip the current POM, already checked
}
if (upgradePropertyInDocument(otherDoc, propertyName, upgrade, sectionName, context)) {
return true;
}
}
// Property not found anywhere in the project
context.warning("Property " + propertyName + " not found in any project POM properties");
// First, try the current POM's properties
if (upgradePropertyInDocument(pomDocument, propertyName, upgrade, sectionName, context)) {
return true;
}
// Check if property exists in the current POM but is already at/above min (no upgrade needed).
// In that case, skip the cross-POM search and the warning — the property IS defined.
Element currentRoot = pomDocument.root();
Element currentProps = currentRoot.childElement(PROPERTIES).orElse(null);
if (currentProps != null && currentProps.childElement(propertyName).isPresent()) {
return false; // Found in current POM, no upgrade needed
}
// Property not in current POM — search other POMs in the project (e.g., parent POM)
for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
Document otherDoc = entry.getValue();
if (otherDoc == pomDocument) {
continue; // Skip the current POM, already checked
}
if (upgradePropertyInDocument(otherDoc, propertyName, upgrade, sectionName, context)) {
return true;
}
}
// Property not found anywhere in the project
context.warning("Property " + propertyName + " not found in any project POM properties");
return false;

Also needs a test: single POM with <exec.maven.version>3.5.0</exec.maven.version> + submodule using ${exec.maven.version} → no warning emitted, no modification.

@gnodet gnodet Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 63c472c. Added an existence check after upgradePropertyInDocument returns false — if the property is present in the current POM's <properties> (or any sibling POM's), we return false immediately without the cross-POM search or the spurious warning. Also added a test for the exact scenario (property already at 3.5.0 + submodule reference → no warning).

return false;
}

/**
* Attempts to upgrade a property value in a single document's properties section.
* Returns {@code true} if the property was found and upgraded, {@code false} otherwise
* (property not found, or already at/above minimum version).
*/
private boolean upgradePropertyInDocument(
Document document,
String propertyName,
PluginUpgradeInfo upgrade,
String sectionName,
UpgradeContext context) {
Element root = document.root();
Element propertiesElement = root.childElement(PROPERTIES).orElse(null);
if (propertiesElement == null) {
return false;
}

if (propertiesElement != null) {
Element propertyElement =
propertiesElement.childElement(propertyName).orElse(null);
if (propertyElement != null) {
String currentVersion = propertyElement.textContentTrimmed();
// For 4.x pre-release versions, upgrade to latest pre-release (not 3.x)
if (isMaven4PreRelease(currentVersion) && upgrade.latestPreRelease != null) {
if (isVersionBelow(context, currentVersion, upgrade.latestPreRelease)) {
editor.setTextContent(propertyElement, upgrade.latestPreRelease);
context.detail("Upgraded property " + propertyName + " (for " + upgrade.groupId + ":"
+ upgrade.artifactId + ") from pre-release " + currentVersion + " to "
+ upgrade.latestPreRelease + " in " + sectionName);
return true;
} else {
context.debug("Property " + propertyName + " version " + currentVersion + " is already >= "
+ upgrade.latestPreRelease);
}
} else if (isVersionBelow(context, currentVersion, upgrade.minVersion)) {
editor.setTextContent(propertyElement, upgrade.minVersion);
context.detail(
"Upgraded property " + propertyName + " (for " + upgrade.groupId + ":" + upgrade.artifactId
+ ") from " + currentVersion + " to " + upgrade.minVersion + " in " + sectionName);
Element propertyElement = propertiesElement.childElement(propertyName).orElse(null);
if (propertyElement == null) {
return false;
}

Editor editor = new Editor(document);
String currentVersion = propertyElement.textContentTrimmed();
// For 4.x pre-release versions, handle specially
if (isMaven4PreRelease(currentVersion)) {
if (upgrade.latestPreRelease != null) {
// Upgrade to the latest pre-release (don't downgrade to 3.x)
if (isVersionBelow(context, currentVersion, upgrade.latestPreRelease)) {
editor.setTextContent(propertyElement, upgrade.latestPreRelease);
context.detail("Upgraded property " + propertyName + " (for " + upgrade.groupId + ":"
+ upgrade.artifactId + ") from pre-release " + currentVersion + " to "
+ upgrade.latestPreRelease + " in " + sectionName);
return true;
} else {
context.debug("Property " + propertyName + " version " + currentVersion + " is already >= "
+ upgrade.minVersion);
+ upgrade.latestPreRelease);
}
} else {
context.warning("Property " + propertyName + " not found in POM properties");
// No stable 4.x pre-release line — downgrade to the stable minVersion
editor.setTextContent(propertyElement, upgrade.minVersion);
context.detail("Downgraded property " + propertyName + " (for " + upgrade.groupId + ":"
+ upgrade.artifactId + ") from incompatible pre-release " + currentVersion
+ " to stable " + upgrade.minVersion + " in " + sectionName);
return true;
}
return false;
} else if (isVersionBelow(context, currentVersion, upgrade.minVersion)) {
editor.setTextContent(propertyElement, upgrade.minVersion);
context.detail("Upgraded property " + propertyName + " (for " + upgrade.groupId + ":" + upgrade.artifactId
+ ") from " + currentVersion + " to " + upgrade.minVersion + " in " + sectionName);
return true;
} else {
context.warning("No properties section found in POM for property " + propertyName);
context.debug(
"Property " + propertyName + " version " + currentVersion + " is already >= " + upgrade.minVersion);
}

return false;
Expand Down Expand Up @@ -632,7 +704,11 @@ public static List<PluginMigration> getPluginMigrations() {
* Upgrades plugin dependencies (e.g., extra-enforcer-rules inside maven-enforcer-plugin).
*/
private boolean upgradePluginDependencies(
Element pluginElement, Document pomDocument, String sectionName, UpgradeContext context) {
Element pluginElement,
Document pomDocument,
Map<Path, Document> pomMap,
String sectionName,
UpgradeContext context) {
Element dependenciesElement = pluginElement.childElement(DEPENDENCIES).orElse(null);
if (dependenciesElement == null) {
return false;
Expand All @@ -652,7 +728,12 @@ private boolean upgradePluginDependencies(

if (upgrade != null) {
return upgradePluginVersion(
depElement, upgrade, pomDocument, sectionName + "/plugin/dependencies", context);
depElement,
upgrade,
pomDocument,
pomMap,
sectionName + "/plugin/dependencies",
context);
}
}
return false;
Expand Down Expand Up @@ -927,7 +1008,8 @@ private PluginAnalysis analyzePluginsFromEffectiveModel(
continue;
}
String effectiveVersion = plugin.getVersion();
if (isVersionBelow(context, effectiveVersion, upgrade.minVersion())) {
if (isVersionBelow(context, effectiveVersion, upgrade.minVersion())
|| (isMaven4PreRelease(effectiveVersion) && upgrade.latestPreRelease() == null)) {
needsManagement.add(pluginKey);
String managedVersion = managedVersions.get(pluginKey);
if (managedVersion == null || !managedVersion.equals(effectiveVersion)) {
Expand All @@ -937,10 +1019,11 @@ private PluginAnalysis analyzePluginsFromEffectiveModel(
needsDirectOverride.add(pluginKey);
context.debug("Plugin " + pluginKey + " version " + effectiveVersion
+ " has explicit version in inherited build/plugins"
+ " — needs direct override to " + upgrade.minVersion());
+ " — needs direct version override to " + upgrade.minVersion());
} else {
context.debug("Plugin " + pluginKey + " version " + effectiveVersion
+ " is managed via pluginManagement — needs upgrade to " + upgrade.minVersion());
+ " is managed via pluginManagement — needs version change to "
+ upgrade.minVersion());
}
}
}
Expand All @@ -963,10 +1046,11 @@ private PluginAnalysis analyzePluginsFromEffectiveModel(
continue;
}
String effectiveVersion = plugin.getVersion();
if (isVersionBelow(context, effectiveVersion, upgrade.minVersion())) {
if (isVersionBelow(context, effectiveVersion, upgrade.minVersion())
|| (isMaven4PreRelease(effectiveVersion) && upgrade.latestPreRelease() == null)) {
needsManagement.add(pluginKey);
context.debug("Managed plugin " + pluginKey + " version " + effectiveVersion
+ " needs upgrade to " + upgrade.minVersion());
+ " needs version change to " + upgrade.minVersion());
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions ...cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/ToolchainPluginStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ protected UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomM
modifiedPoms.add(pomPath);
context.success("Added maven-toolchains-plugin with " + SELECT_JDK_TOOLCHAIN_GOAL + " goal (--source "
+ sourceLevel + " requires JDK <= " + latestJdk + ")");
context.warning("A JDK <= " + latestJdk
+ " must be installed and discoverable by the toolchains plugin"
+ " for the build to succeed. If no matching JDK is found,"
+ " the build will fail with a toolchain resolution error."
+ " See https://maven.apache.org/plugins/maven-toolchains-plugin/");
} catch (Exception e) {
context.failure("Failed to add toolchains plugin: " + e.getMessage());
errorPoms.add(pomPath);
Expand Down
Loading
Loading