elharo commented on code in PR #12454:
URL: https://github.com/apache/maven/pull/12454#discussion_r3558865515
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java:
##########
@@ -81,6 +85,15 @@ public class CompatibilityFixStrategy extends
AbstractUpgradeStrategy {
private static final Set<String> VALID_COMBINE_CHILDREN_VALUES =
Set.of(COMBINE_APPEND, COMBINE_MERGE);
+ /**
+ * Known incompatible plugins where even the latest version fails with
Maven 4.
+ * Maps plugin key (groupId:artifactId) to a description of the
incompatibility.
+ * <p>
+ * Note: plugins that have a fixed version available should be added to
+ * {@link PluginUpgradeStrategy} instead, so mvnup can auto-upgrade them.
+ */
+ private static final Map<String, String> KNOWN_INCOMPATIBLE_PLUGINS =
Map.of();
Review Comment:
I appreciate special casing known incompatibilities. I wish we did more of
this.
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java:
##########
@@ -816,4 +662,148 @@ private Path findParentPomInMap(
.map(Map.Entry::getKey)
.orElse(null);
}
+
+ // ---- Warning-only checks for Maven 4 compatibility issues that cannot
be auto-fixed ----
+
+ /**
+ * Warns about plugins known to be incompatible with Maven 4 even at their
latest version.
+ * These plugins call methods on immutable API objects or use removed
internal APIs
Review Comment:
Arte these the only cases? Since 4.x is a major upgrade it's not a surprise
that the API has changed incompatibly in various ways. Maybe just say something
like "These plugins do not yet work with Maven 4." or "There is not a version
of this plugin that works with Maven 4 at the time of writing."
##########
pom.xml:
##########
@@ -146,7 +146,7 @@ under the License.
<byteBuddyVersion>1.18.11</byteBuddyVersion>
<classWorldsVersion>2.12.0</classWorldsVersion>
<commonsCliVersion>1.11.0</commonsCliVersion>
- <domtripVersion>1.5.2</domtripVersion>
+ <domtripVersion>1.6.0</domtripVersion>
Review Comment:
This should likely be a separate PR
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/DeduplicateDependenciesStrategy.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * 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.goals;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Stream;
+
+import eu.maveniverse.domtrip.Document;
+import eu.maveniverse.domtrip.Element;
+import eu.maveniverse.domtrip.maven.MavenPomElements;
+import org.apache.maven.api.cli.mvnup.UpgradeOptions;
+import org.apache.maven.api.di.Named;
+import org.apache.maven.api.di.Priority;
+import org.apache.maven.api.di.Singleton;
+import org.apache.maven.cling.invoker.mvnup.UpgradeContext;
+
+import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.BUILD;
+import static
eu.maveniverse.domtrip.maven.MavenPomElements.Elements.DEPENDENCIES;
+import static
eu.maveniverse.domtrip.maven.MavenPomElements.Elements.DEPENDENCY;
+import static
eu.maveniverse.domtrip.maven.MavenPomElements.Elements.DEPENDENCY_MANAGEMENT;
+import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGIN;
+import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGINS;
+import static
eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGIN_MANAGEMENT;
+import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROFILE;
+import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROFILES;
+import static
eu.maveniverse.domtrip.maven.MavenPomElements.Plugins.DEFAULT_MAVEN_PLUGIN_GROUP_ID;
+import static
eu.maveniverse.domtrip.maven.MavenPomElements.Plugins.MAVEN_PLUGIN_PREFIX;
+
+/**
+ * Strategy for deduplicating dependency and plugin declarations in POM files.
+ *
+ * <p>Maven 4 rejects duplicate {@code <dependency>} entries (same groupId,
artifactId,
+ * type, and classifier) that Maven 3 silently accepted. This strategy scans
each POM's
+ * {@code <dependencies>} and {@code <dependencyManagement>/<dependencies>}
sections and
+ * removes duplicates using last-wins semantics (keeping the last
declaration). Note that
+ * Maven 3's dependency resolver uses first-wins for same-depth conflicts;
last-wins is
Review Comment:
I wouldn't assume that at all. When I've added a duplicate dependency (which
I've done more than a few times) it's landed almost anywhere in the list
depending mostly on where and what I was copying and pasting into. There is no
expectation that dependencies are added only at the end. I think it's better to
keep the first to more closely reflect the current dependency tree.
--
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]
