Fix mvnup tool issues #7934-#7938

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/ConsoleIcon.java

@@ -0,0 +1,108 @@
+/*
+ * 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.
+ */
+package org.apache.maven.cling.invoker.mvnup;
+
+import java.nio.charset.Charset;
+
+import org.jline.terminal.Terminal;
+
+/**
+ * Console icons for Maven upgrade tool output.
+ * Each icon has a Unicode character and an ASCII fallback.
+ * The appropriate representation is chosen based on the terminal's charset capabilities.
+ */
+public enum ConsoleIcon {
+    /**
+     * Success/completion icon.
+     */
+    SUCCESS('✓', "[OK]"),
+
+    /**
+     * Error/failure icon.
+     */
+    ERROR('✗', "[ERROR]"),
+
+    /**
+     * Warning icon.
+     */
+    WARNING('⚠', "[WARNING]"),
+
+    /**
+     * Detail/bullet point icon.
+     */
+    DETAIL('•', "-"),
+
+    /**
+     * Action/arrow icon.
+     */
+    ACTION('→', ">");
+
+    private final char unicodeChar;
+    private final String asciiFallback;
+
+    ConsoleIcon(char unicodeChar, String asciiFallback) {
+        this.unicodeChar = unicodeChar;
+        this.asciiFallback = asciiFallback;
+    }
+
+    /**
+     * Returns the appropriate icon representation for the given terminal.
+     * Tests if the terminal's charset can encode the Unicode character,
+     * falling back to ASCII if not.
+     *
+     * @param terminal the terminal to get the icon for
+     * @return the Unicode character if supported, otherwise the ASCII fallback
+     */
+    public String getIcon(Terminal terminal) {
+        Charset charset = getTerminalCharset(terminal);
+        return charset.newEncoder().canEncode(unicodeChar) ? String.valueOf(unicodeChar) : asciiFallback;
+    }
+
+    /**
+     * Gets the charset used by the terminal for output.
+     * Falls back to the system default charset if terminal charset is not available.
+     *
+     * @param terminal the terminal to get the charset from
+     * @return the terminal's output charset or the system default charset
+     */
+    private static Charset getTerminalCharset(Terminal terminal) {
+        if (terminal != null && terminal.encoding() != null) {
+            return terminal.encoding();
+        }
+        return Charset.defaultCharset();
+    }
+
+    /**
+     * Returns the Unicode character for this icon.
+     *
+     * @return the Unicode character
+     */
+    public char getUnicodeChar() {
+        return unicodeChar;
+    }
+
+    /**
+     * Returns the ASCII fallback text for this icon.
+     *
+     * @return the ASCII fallback text
+     */
+    public String getAsciiFallback() {
+        return asciiFallback;
+    }
+}

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/UpgradeContext.java

@@ -118,35 +118,35 @@ public void println() {
      * Logs a successful operation with a checkmark icon.
      */
     public void success(String message) {
-        logger.info(getCurrentIndent() + "✓ " + message);
+        logger.info(getCurrentIndent() + ConsoleIcon.SUCCESS.getIcon(terminal) + " " + message);
     }
 
     /**
      * Logs an error with an X icon.
      */
     public void failure(String message) {
-        logger.error(getCurrentIndent() + "✗ " + message);
+        logger.error(getCurrentIndent() + ConsoleIcon.ERROR.getIcon(terminal) + " " + message);
     }
 
     /**
      * Logs a warning with a warning icon.
      */
     public void warning(String message) {
-        logger.warn(getCurrentIndent() + "⚠ " + message);
+        logger.warn(getCurrentIndent() + ConsoleIcon.WARNING.getIcon(terminal) + " " + message);
     }
 
     /**
      * Logs detailed information with a bullet point.
      */
     public void detail(String message) {
-        logger.info(getCurrentIndent() + "• " + message);
+        logger.info(getCurrentIndent() + ConsoleIcon.DETAIL.getIcon(terminal) + " " + message);
     }
 
     /**
      * Logs a performed action with an arrow icon.
      */
     public void action(String message) {
-        logger.info(getCurrentIndent() + "→ " + message);
+        logger.info(getCurrentIndent() + ConsoleIcon.ACTION.getIcon(terminal) + " " + message);
     }
 
     /**

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoal.java

@@ -208,10 +208,9 @@ protected int doUpgrade(UpgradeContext context, String targetModel, Map<Path, Do
         try {
             UpgradeResult result = orchestrator.executeStrategies(context, pomMap);
 
-            // Create .mvn directory if needed (when not upgrading to 4.1.0)
-            if (!MODEL_VERSION_4_1_0.equals(targetModel)) {
-                createMvnDirectoryIfNeeded(context);
-            }
+            // Create .mvn directory if needed to avoid root directory warnings
+            // This is needed for both 4.0.0 and 4.1.0 to help Maven find the project root
+            createMvnDirectoryIfNeeded(context);
 
             return result.success() ? 0 : 1;
         } catch (Exception e) {

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/InferenceStrategy.java

@@ -388,46 +388,85 @@ private boolean trimParentElementFull(
             Map<Path, Document> pomMap) {
         boolean hasChanges = false;
 
-        // First apply limited inference (child elements)
-        hasChanges |= trimParentElementLimited(context, root, parentElement, namespace);
-
-        // Get child GAV
+        // Get child GAV before applying any changes
         String childGroupId = getChildText(root, GROUP_ID, namespace);
         String childVersion = getChildText(root, VERSION, namespace);
 
-        // Remove parent groupId if child has no explicit groupId
-        if (childGroupId == null) {
-            Element parentGroupIdElement = parentElement.getChild(GROUP_ID, namespace);
-            if (parentGroupIdElement != null) {
-                removeElementWithFormatting(parentGroupIdElement);
-                context.detail("Removed: parent groupId (child has no explicit groupId)");
-                hasChanges = true;
+        // First apply limited inference (child elements) - this removes matching child groupId/version
+        hasChanges |= trimParentElementLimited(context, root, parentElement, namespace);
+
+        // Only remove parent elements if the parent is in the same reactor (not external)
+        if (isParentInReactor(parentElement, namespace, pomMap, context)) {
+            // Remove parent groupId if child has no explicit groupId
+            if (childGroupId == null) {
+                Element parentGroupIdElement = parentElement.getChild(GROUP_ID, namespace);
+                if (parentGroupIdElement != null) {
+                    removeElementWithFormatting(parentGroupIdElement);
+                    context.detail("Removed: parent groupId (child has no explicit groupId)");
+                    hasChanges = true;
+                }
             }
-        }
 
-        // Remove parent version if child has no explicit version
-        if (childVersion == null) {
-            Element parentVersionElement = parentElement.getChild(VERSION, namespace);
-            if (parentVersionElement != null) {
-                removeElementWithFormatting(parentVersionElement);
-                context.detail("Removed: parent version (child has no explicit version)");
-                hasChanges = true;
+            // Remove parent version if child has no explicit version
+            if (childVersion == null) {
+                Element parentVersionElement = parentElement.getChild(VERSION, namespace);
+                if (parentVersionElement != null) {
+                    removeElementWithFormatting(parentVersionElement);
+                    context.detail("Removed: parent version (child has no explicit version)");
+                    hasChanges = true;
+                }
             }
-        }
 
-        // Remove parent artifactId if it can be inferred from relativePath
-        if (canInferParentArtifactId(parentElement, namespace, pomMap)) {
-            Element parentArtifactIdElement = parentElement.getChild(ARTIFACT_ID, namespace);
-            if (parentArtifactIdElement != null) {
-                removeElementWithFormatting(parentArtifactIdElement);
-                context.detail("Removed: parent artifactId (can be inferred from relativePath)");
-                hasChanges = true;
+            // Remove parent artifactId if it can be inferred from relativePath
+            if (canInferParentArtifactId(parentElement, namespace, pomMap)) {
+                Element parentArtifactIdElement = parentElement.getChild(ARTIFACT_ID, namespace);
+                if (parentArtifactIdElement != null) {
+                    removeElementWithFormatting(parentArtifactIdElement);
+                    context.detail("Removed: parent artifactId (can be inferred from relativePath)");
+                    hasChanges = true;
+                }
             }
         }
 
         return hasChanges;
     }
 
+    /**
+     * Determines if the parent is part of the same reactor (multi-module project)
+     * vs. an external parent POM by checking if the parent exists in the pomMap.
+     */
+    private boolean isParentInReactor(
+            Element parentElement, Namespace namespace, Map<Path, Document> pomMap, UpgradeContext context) {
+        // If relativePath is explicitly set to empty, parent is definitely external
+        String relativePath = getChildText(parentElement, RELATIVE_PATH, namespace);
+        if (relativePath != null && relativePath.trim().isEmpty()) {
+            return false;
+        }
+
+        // Extract parent GAV
+        String parentGroupId = getChildText(parentElement, GROUP_ID, namespace);
+        String parentArtifactId = getChildText(parentElement, ARTIFACT_ID, namespace);
+        String parentVersion = getChildText(parentElement, VERSION, namespace);
+
+        if (parentGroupId == null || parentArtifactId == null || parentVersion == null) {
+            // Cannot determine parent GAV, assume external
+            return false;
+        }
+
+        GAV parentGAV = new GAV(parentGroupId, parentArtifactId, parentVersion);
+
+        // Check if any POM in our reactor matches the parent GAV using GAVUtils
+        for (Document pomDocument : pomMap.values()) {
+            GAV pomGAV = GAVUtils.extractGAVWithParentResolution(context, pomDocument);
+            if (pomGAV != null && pomGAV.equals(parentGAV)) {
+                return true;
+            }
+        }
+
+        // Parent not found in reactor, must be external
+        return false;
+    }
+
     /**
      * Determines if parent artifactId can be inferred from relativePath.
      */
@@ -438,11 +477,9 @@ private boolean canInferParentArtifactId(Element parentElement, Namespace namesp
             relativePath = DEFAULT_PARENT_RELATIVE_PATH; // Maven default
         }
 
-        // For now, we use a simple heuristic: if relativePath is the default "../pom.xml"
-        // and we have parent POMs in our pomMap, we can likely infer the artifactId.
-        // A more sophisticated implementation would resolve the actual path and check
-        // if the parent POM exists in pomMap.
-        return DEFAULT_PARENT_RELATIVE_PATH.equals(relativePath) && !pomMap.isEmpty();
+        // Only infer artifactId if relativePath is the default and we have multiple POMs
+        // indicating this is likely a multi-module project
+        return DEFAULT_PARENT_RELATIVE_PATH.equals(relativePath) && pomMap.size() > 1;
     }
 
     /**

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/ModelUpgradeStrategy.java

@@ -114,7 +114,9 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
                     context.success("Model upgrade completed");
                     modifiedPoms.add(pomPath);
                 } else {
-                    context.warning("Cannot upgrade from " + currentVersion + " to " + targetModelVersion);
+                    // Treat invalid upgrades (including downgrades) as errors, not warnings
+                    context.failure("Cannot upgrade from " + currentVersion + " to " + targetModelVersion);
+                    errorPoms.add(pomPath);
                 }
             } catch (Exception e) {
                 context.failure("Model upgrade failed: " + e.getMessage());