Copilot commented on code in PR #12430:
URL: https://github.com/apache/maven/pull/12430#discussion_r3540261661
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java:
##########
@@ -874,6 +894,138 @@ private record PluginAnalysis(Set<String>
needsManagement, Set<String> needsDire
private record PluginAnalysisResults(
Map<Path, Set<String>> pluginsNeedingManagement, Map<Path,
Set<String>> pluginsNeedingDirectOverride) {}
+ /**
+ * Checks if the given plugin is a Quarkus Maven plugin.
+ */
+ private boolean isQuarkusPlugin(String groupId, String artifactId) {
+ return "quarkus-maven-plugin".equals(artifactId)
+ && ("io.quarkus".equals(groupId) ||
"io.quarkus.platform".equals(groupId));
+ }
+
+ /**
+ * Checks if a property is used as the version of a Quarkus BOM in
dependencyManagement.
+ * Quarkus BOMs are identified by having groupId io.quarkus or
io.quarkus.platform,
+ * type "pom", and scope "import".
+ */
+ private boolean isPropertyUsedByQuarkusBom(Document pomDocument, String
propertyName) {
+ Element root = pomDocument.root();
+ String propertyRef = "${" + propertyName + "}";
+
+ Element depManagement =
root.childElement(DEPENDENCY_MANAGEMENT).orElse(null);
+ if (depManagement == null) {
+ return false;
+ }
+ Element dependencies =
depManagement.childElement(DEPENDENCIES).orElse(null);
+ if (dependencies == null) {
+ return false;
+ }
+
+ return dependencies.childElements(DEPENDENCY).anyMatch(dep -> {
+ String groupId = getChildText(dep, GROUP_ID);
+ String version = getChildText(dep, VERSION);
+ String type = getChildText(dep, "type");
+ String scope = getChildText(dep, "scope");
+ return ("io.quarkus".equals(groupId) ||
"io.quarkus.platform".equals(groupId))
+ && "pom".equals(type)
+ && "import".equals(scope)
+ && propertyRef.equals(version);
+ });
+ }
+
+ /**
+ * Decouples the Quarkus plugin version from a shared BOM property.
+ * Introduces a new property for the plugin version and updates the
plugin's version element,
+ * leaving the BOM property unchanged.
+ */
+ private boolean decoupleQuarkusPluginVersion(
+ Document pomDocument,
+ Element versionElement,
+ String sharedPropertyName,
+ PluginUpgradeInfo upgrade,
+ String sectionName,
+ UpgradeContext context) {
+
+ // Resolve the current version from the shared property
+ Editor editor = new Editor(pomDocument);
+ Element root = editor.root();
+ Element propertiesElement = root.childElement(PROPERTIES).orElse(null);
+ String currentVersion = null;
+ if (propertiesElement != null) {
+ Element sharedProp =
+
propertiesElement.childElement(sharedPropertyName).orElse(null);
+ if (sharedProp != null) {
+ currentVersion = sharedProp.textContentTrimmed();
+ }
+ }
+
+ if (currentVersion != null && !isVersionBelow(currentVersion,
upgrade.minVersion)) {
+ context.debug("Quarkus plugin version (via shared property " +
sharedPropertyName + ") " + currentVersion
+ + " is already >= " + upgrade.minVersion);
+ return false;
+ }
Review Comment:
If the shared BOM property is inherited (not declared in the current POM),
currentVersion stays null and this method will still introduce
quarkus-plugin.version=3.26.0 and switch the plugin to use it. That can
accidentally downgrade projects whose inherited platform/plugin property is
already > 3.26.0 (because the actual value isn’t checked).
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/CommonsCliUpgradeOptions.java:
##########
@@ -144,6 +148,32 @@ protected static class CLIManager extends
CommonsCliOptions.CLIManager {
public static final String PLUGINS = "plugins";
public static final String ALL = "a";
+ /**
+ * Overrides the default strict parsing to silently ignore
unrecognized options.
+ * This is necessary because mvnup inherits the Maven launcher's
argument handling,
+ * which appends options from {@code .mvn/maven.config}. That file
often contains
+ * Maven build options like {@code -ntp}, {@code -U}, or {@code -T}
that mvnup
+ * does not recognize. Without this override, mvnup would abort with a
+ * {@link ParseException} before applying any fixes.
+ */
+ @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);
+ }
+ }
+ }
Review Comment:
This retry loop removes only the unrecognized option token (e.g. "-T") but
leaves any associated argument behind when the option is provided as a separate
token (e.g. "-T", "4"). With Commons CLI parsing, that leftover value ends up
in commandLine.getArgList() and will be treated as an extra goal/phase (e.g.
"4"), changing mvnup behavior instead of just ignoring the Maven option.
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java:
##########
@@ -816,4 +848,193 @@ 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
+ * and require upstream fixes before they can work with Maven 4.
+ *
+ * @see <a href="https://github.com/apache/maven/issues/12432";>#12432</a>
+ */
+ private void warnAboutIncompatiblePlugins(Document pomDocument,
UpgradeContext context) {
+ Element root = pomDocument.root();
+
+ Stream<Element> pluginContainers = Stream.concat(
+ // Root level build
+ root.childElement(BUILD).stream()
+ .flatMap(build -> Stream.concat(
+ build.childElement(PLUGINS).stream(),
+ build.childElement(PLUGIN_MANAGEMENT).stream()
+ .flatMap(pm ->
pm.childElement(PLUGINS).stream()))),
+ // Profile builds
+ root.childElement(PROFILES).stream()
+ .flatMap(profiles -> profiles.childElements(PROFILE))
+ .flatMap(profile ->
profile.childElement(BUILD).stream())
+ .flatMap(build -> Stream.concat(
+ build.childElement(PLUGINS).stream(),
+ build.childElement(PLUGIN_MANAGEMENT).stream()
+ .flatMap(pm ->
pm.childElement(PLUGINS).stream()))));
+
+ pluginContainers.forEach(pluginsElement -> pluginsElement
+ .childElements(PLUGIN)
+ .forEach(pluginElement -> {
+ String groupId =
pluginElement.childText(MavenPomElements.Elements.GROUP_ID);
+ String artifactId =
pluginElement.childText(MavenPomElements.Elements.ARTIFACT_ID);
+
+ if (groupId == null && artifactId != null &&
artifactId.startsWith(MAVEN_PLUGIN_PREFIX)) {
+ groupId = DEFAULT_MAVEN_PLUGIN_GROUP_ID;
+ }
+
+ if (groupId != null && artifactId != null) {
+ String pluginKey = groupId + ":" + artifactId;
+ String warning =
KNOWN_INCOMPATIBLE_PLUGINS.get(pluginKey);
+ if (warning != null) {
+ context.warning("Known Maven 4 incompatibility: "
+ pluginKey + " — " + warning);
+ }
+ }
+ }));
+ }
+
+ /**
+ * Warns about third-party repositories (non-Maven-Central) that may be
affected by
+ * Maven 4's prefix-based artifact filtering. When a repository does not
publish a
+ * {@code prefixes.txt} file, Maven 4 may auto-generate an incomplete
prefix list
+ * that blocks legitimate artifact resolution.
+ *
+ * @see <a href="https://github.com/apache/maven/issues/12433";>#12433</a>
+ */
+ private void warnAboutThirdPartyRepositoryPrefixFiltering(Document
pomDocument, UpgradeContext context) {
+ Element root = pomDocument.root();
+
+ Stream<Element> repositoryContainers = Stream.concat(
+ Stream.of(
+ root.childElement(REPOSITORIES).orElse(null),
+
root.childElement(PLUGIN_REPOSITORIES).orElse(null))
+ .filter(Objects::nonNull),
+ root.childElement(PROFILES).stream()
+ .flatMap(profiles -> profiles.childElements(PROFILE))
+ .flatMap(profile -> Stream.of(
+
profile.childElement(REPOSITORIES).orElse(null),
+
profile.childElement(PLUGIN_REPOSITORIES)
+ .orElse(null))
+ .filter(Objects::nonNull)));
+
+ repositoryContainers.forEach(container -> {
+ String elementType = container.name().equals(REPOSITORIES) ?
REPOSITORY : PLUGIN_REPOSITORY;
+ container.childElements(elementType).forEach(repository -> {
+ String url = repository.childText("url");
+ String id = repository.childText("id");
+
+ 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.");
+ }
+ }
Review Comment:
This warns for any repository URL that isn’t in WELL_KNOWN_REPO_URLS,
including non-HTTP(S) repositories such as file: URLs. Maven 4 prefix filtering
is an HTTP repository concern; warning on local/file repositories will create
false positives and noise.
--
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]
