gnodet-bot commented on code in PR #13059:
URL: https://github.com/apache/maven/pull/13059#discussion_r4044691488
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java:
##########
@@ -523,49 +540,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()) {
Review Comment:
⚠️ **Non-deterministic cross-POM iteration**
`pomMap` is a `HashMap<Path, Document>` in production (`PomDiscovery.java`
constructs it with `new HashMap<>()`). Iterating `.entrySet()` here gives no
ordering guarantees. The common case (property in parent only) is fine, but if
two sibling submodules independently define the same property, the first one
found in the HashMap wins — which may be a sibling, not the intended parent.
Suggestion: switch `PomDiscovery` to `LinkedHashMap` so that discovery order
(typically parent before children) becomes the iteration order, making
behaviour deterministic and reproducible.
--
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]
