Merge pull request #16 from TomasHofman/inline-versions-by-default

By default rewrite versions in the version elements, instead of overr…
wildfly-extras · Jul 9, 2024 · d3f9d05 · d3f9d05
2 parents 088a2d1 + c63cfae
commit d3f9d05
Show file tree

Hide file tree

Showing 9 changed files with 265 additions and 57 deletions.
diff --git a/integration-tests/src/test/java/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT.java b/integration-tests/src/test/java/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT.java
@@ -37,6 +37,7 @@ public class UpgradeComponentsMojoIT {
     @SystemProperty(value = "localRepository", content = "${maven.repo.local}")
     @SystemProperty(value = "remoteRepositories", content = "file://${maven.repo.local}")
     @SystemProperty(value = "ignoreStreams", content = "org.jboss:ignored-dep")
+    @SystemProperty(value = "inlineUpgradedVersions", content = "false")
     @MavenTest
     void basic_project_test_case(MavenExecutionResult result) {
         assertThat(result).isSuccessful();
@@ -93,6 +94,7 @@ void basic_project_test_case(MavenExecutionResult result) {
      */
     @MavenGoal("${project.groupId}:wildfly-channel-maven-plugin:${project.version}:upgrade")
     @SystemProperty(value = "manifestFile", content = "manifest.yaml")
+    @SystemProperty(value = "inlineUpgradedVersions", content = "false")
     @MavenTest
     void eap_bom_test_case(MavenExecutionResult result) throws MalformedURLException {
         assertThat(result).isSuccessful();
@@ -132,6 +134,7 @@ void eap_bom_test_case(MavenExecutionResult result) throws MalformedURLException
     @MavenGoal("${project.groupId}:wildfly-channel-maven-plugin:${project.version}:upgrade")
     @SystemProperty(value = "manifestFile", content = "manifest.yaml")
     @SystemProperty(value = "overrideProperties", content = "undertow.version=2.2.5.Final-Overridden")
+    @SystemProperty(value = "inlineUpgradedVersions", content = "false")
     @MavenTest
     void override_property_test_case(MavenExecutionResult result) {
         assertThat(result).isSuccessful();
@@ -156,6 +159,7 @@ void override_property_test_case(MavenExecutionResult result) {
     @MavenGoal("${project.groupId}:wildfly-channel-maven-plugin:${project.version}:upgrade")
     @SystemProperty(value = "manifestFile", content = "manifest.yaml")
     @SystemProperty(value = "overrideDependencies", content = "io.undertow:undertow-core:2.2.5.Final-Overridden")
+    @SystemProperty(value = "inlineUpgradedVersions", content = "false")
     @MavenTest
     void override_dependency_test_case(MavenExecutionResult result) {
         assertThat(result).isSuccessful();
@@ -180,6 +184,7 @@ void override_dependency_test_case(MavenExecutionResult result) {
      */
     @MavenGoal("${project.groupId}:wildfly-channel-maven-plugin:${project.version}:upgrade")
     @SystemProperty(value = "manifestFile", content = "manifest.yaml")
+    @SystemProperty(value = "inlineUpgradedVersions", content = "false")
     @MavenTest
     void external_properties_test_case(MavenExecutionResult result) {
         assertThat(result).isSuccessful();
@@ -197,6 +202,32 @@ void external_properties_test_case(MavenExecutionResult result) {
                 });
     }
 
+    @MavenGoal("${project.groupId}:wildfly-channel-maven-plugin:${project.version}:upgrade")
+    @SystemProperty(value = "manifestFile", content = "manifest.yaml")
+    @MavenTest
+    void inline_override_test_case(MavenExecutionResult result) {
+        assertThat(result).isSuccessful();
+
+        Model model = result.getMavenProjectResult().getModel();
+        DependencyModel dependencyModel = new DependencyModel(model);
+
+        assertThat(dependencyModel.getDependency("org.apache.maven.plugins", "maven-clean-plugin", "pom", null))
+                .satisfies(o -> {
+                    assertThat(o).isPresent();
+                    assertThat(o.get().getVersion()).isEqualTo("3.3.2");
+                });
+        assertThat(dependencyModel.getDependency("io.undertow", "undertow-core", "jar", null))
+                .satisfies(o -> {
+                    assertThat(o).isPresent();
+                    assertThat(o.get().getVersion()).isEqualTo("2.2.17.Final");
+                });
+        assertThat(dependencyModel.getDependency("io.undertow", "undertow-servlet", "jar", null))
+                .satisfies(o -> {
+                    assertThat(o).isPresent();
+                    assertThat(o.get().getVersion()).isEqualTo("2.2.17.Final");
+                });
+    }
+
     @MavenGoal("${project.groupId}:wildfly-channel-maven-plugin:${project.version}:upgrade")
     @SystemProperty(value = "manifestFile", content = "manifest.yaml")
     @SystemProperty(value = "doNotDowngrade", content = "true")

diff --git a/.../wildfly/channelplugin/it/UpgradeComponentsMojoIT/inline_override_test_case/manifest.yaml b/.../wildfly/channelplugin/it/UpgradeComponentsMojoIT/inline_override_test_case/manifest.yaml
@@ -0,0 +1,12 @@
+schemaVersion: 1.0.0
+name: Sample Manifest
+streams:
+  - groupId: org.apache.maven.plugins
+    artifactId: maven-clean-plugin
+    version: "3.3.2"
+  - groupId: io.undertow
+    artifactId: undertow-core
+    version: "2.2.17.Final"
+  - groupId: io.undertow
+    artifactId: undertow-servlet
+    version: "2.2.17.Final"
diff --git a/...ts/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT/inline_override_test_case/pom.xml b/...ts/org/wildfly/channelplugin/it/UpgradeComponentsMojoIT/inline_override_test_case/pom.xml
@@ -0,0 +1,71 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.jboss</groupId>
+        <artifactId>jboss-parent</artifactId>
+        <version>40</version>
+    </parent>
+
+    <groupId>org.wildfly</groupId>
+    <artifactId>test-project</artifactId>
+    <version>1.0.0-SNAPSHOT</version>
+    <packaging>pom</packaging>
+
+    <properties>
+        <maven.compiler.source>11</maven.compiler.source>
+        <maven.compiler.target>11</maven.compiler.target>
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+        <undertow.version>2.2.5.Final</undertow.version>
+    </properties>
+
+    <dependencyManagement>
+        <dependencies>
+            <dependency>
+                <groupId>io.undertow</groupId>
+                <artifactId>undertow-servlet</artifactId>
+                <version>2.2.5.Final</version>
+            </dependency>
+        </dependencies>
+    </dependencyManagement>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.maven.plugins</groupId>
+            <artifactId>maven-clean-plugin</artifactId>
+            <version>${version.clean.plugin}</version>
+            <type>pom</type>
+        </dependency>
+        <dependency>
+            <groupId>io.undertow</groupId>
+            <artifactId>undertow-core</artifactId>
+            <version>${undertow.version}</version>
+        </dependency>
+    </dependencies>
+
+    <profiles>
+        <profile>
+            <id>profile</id>
+            <dependencyManagement>
+                <dependencies>
+                    <dependency>
+                        <groupId>io.undertow</groupId>
+                        <artifactId>undertow-core</artifactId>
+                        <version>${undertow.version}</version>
+                    </dependency>
+                </dependencies>
+            </dependencyManagement>
+            <dependencies>
+                <dependency>
+                    <groupId>io.undertow</groupId>
+                    <artifactId>undertow-core</artifactId>
+                    <version>${undertow.version}</version>
+                </dependency>
+            </dependencies>
+        </profile>
+    </profiles>
+
+</project>
diff --git a/plugin/src/main/java/org/wildfly/channelplugin/UpgradeComponentsMojo.java b/plugin/src/main/java/org/wildfly/channelplugin/UpgradeComponentsMojo.java
@@ -1,5 +1,7 @@
 package org.wildfly.channelplugin;
 
+import groovy.lang.Tuple;
+import groovy.lang.Tuple3;
 import org.apache.commons.lang3.tuple.Pair;
 import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
 import org.apache.maven.model.Dependency;
@@ -116,11 +118,11 @@ public class UpgradeComponentsMojo extends AbstractChannelMojo {
     List<String> overrideDependencies;
 
     /**
-     * Replace property reference in dependency version element with inlined version string, when it's not possible to
-     * override property value due to conflicting versions for different dependencies that use the property.
+     * If true, upgraded versions would be inlined in the dependency version element, possibly replacing a property
+     * reference if it was originally used. If false, the property would be updated instead (less robust option).
      */
-    @Parameter(property = "inlineVersionOnConflict", defaultValue = "true")
-    boolean inlineVersionOnConflict;
+    @Parameter(property = "inlineUpgradedVersions", defaultValue = "true")
+    boolean inlineUpgradedVersions;
 
     /**
      * If true, transitive dependencies of the project that are also declared in the channel will be injected into root
@@ -268,10 +270,11 @@ private void processModule(Project pmeProject, PomManipulator manipulator)
         List<String> overriddenProperties = performHardPropertyOverrides(manipulator);
         List<Dependency> overriddenDependencies = performHardDependencyOverrides(resolvedProjectDependencies, manipulator);
 
-        List<Pair<Dependency, String>> dependenciesToUpgrade = findDependenciesToUpgrade(resolvedProjectDependencies);
-        for (Pair<Dependency, String> upgrade: dependenciesToUpgrade) {
-            String newVersion = upgrade.getRight();
-            Dependency locatedDependency = upgrade.getLeft();
+        List<Tuple3<Dependency, ArtifactRef, String>> dependenciesToUpgrade = findDependenciesToUpgrade(resolvedProjectDependencies);
+        for (Tuple3<Dependency, ArtifactRef, String> upgrade: dependenciesToUpgrade) {
+            String newVersion = upgrade.getV3();
+            Dependency locatedDependency = upgrade.getV1();
+            ArtifactRef resolvedDependency = upgrade.getV2();
 
             @SuppressWarnings("UnnecessaryLocalVariable")
             Dependency d = locatedDependency;
@@ -281,7 +284,7 @@ private void processModule(Project pmeProject, PomManipulator manipulator)
                 continue;
             }
 
-            if (VersionUtils.isProperty(locatedDependency.getVersion())) { // dependency version is set from a property
+            if (VersionUtils.isProperty(locatedDependency.getVersion()) && !inlineUpgradedVersions) { // dependency version is set from a property
                 String originalVersionString = locatedDependency.getVersion();
                 String versionPropertyName = VersionUtils.extractPropertyName(originalVersionString);
 
@@ -319,20 +322,12 @@ private void processModule(Project pmeProject, PomManipulator manipulator)
                         // property has already been changed to different value
                         String propertyName = projectProperty.getRight();
                         String currentPropertyValue = upgradedProperties.get(projectProperty);
-                        if (inlineVersionOnConflict) {
-                            getLog().warn(String.format("Inlining version string for %s:%s:%s, new version '%s'. " +
-                                    "The original version property '%s' has already been modified to '%s'.",
-                                    d.getGroupId(), d.getArtifactId(), d.getVersion(), newVersion, propertyName,
-                                    currentPropertyValue));
-                            manipulator.overrideDependencyVersion(d.getGroupId(), d.getArtifactId(),
-                                    originalVersionString, newVersion);
-                        } else {
-                            getLog().warn(String.format(
-                                    "Can't upgrade %s:%s:%s to '%s', property '%s' was already upgraded to '%s'.",
-                                    d.getGroupId(), d.getArtifactId(), d.getVersion(), newVersion,
-                                    propertyName,
-                                    currentPropertyValue));
-                        }
+                        getLog().warn(String.format("Inlining version string for %s:%s:%s, new version '%s'. " +
+                                "The original version property '%s' has already been modified to '%s'.",
+                                d.getGroupId(), d.getArtifactId(), d.getVersion(), newVersion, propertyName,
+                                currentPropertyValue));
+                        manipulator.overrideDependencyVersion(d.getGroupId(), d.getArtifactId(),
+                                originalVersionString, newVersion);
                         continue; // do not override the property again
                     }
                 }
@@ -358,7 +353,7 @@ private void processModule(Project pmeProject, PomManipulator manipulator)
                             d.getGroupId(), d.getArtifactId(), d.getVersion(), newVersion, targetPropertyName));
                 }
             } else { // dependency version is inlined in version element, can be directly overwritten
-                manipulator.overrideDependencyVersion(toArtifactRef(locatedDependency), newVersion);
+                manipulator.overrideDependencyVersionWithComment(resolvedDependency, newVersion);
             }
         }
 
@@ -451,76 +446,76 @@ private Map<ArtifactRef, Dependency> collectResolvedProjectDependencies(Project
         return projectDependencies;
     }
 
-    private List<Pair<Dependency, String>> findDependenciesToUpgrade(
+    private List<Tuple3<Dependency, ArtifactRef, String>> findDependenciesToUpgrade(
             Map<ArtifactRef, Dependency> resolvedProjectDependencies) {
-        List<Pair<Dependency, String>> dependenciesToUpgrade = new ArrayList<>();
+        List<Tuple3<Dependency, ArtifactRef, String>> dependenciesToUpgrade = new ArrayList<>();
         for (Map.Entry<ArtifactRef, Dependency> entry : resolvedProjectDependencies.entrySet()) {
-            ArtifactRef artifactRef = entry.getKey();
+            ArtifactRef resolvedArtifact = entry.getKey();
             Dependency dependency = entry.getValue();
 
-            Objects.requireNonNull(artifactRef);
+            Objects.requireNonNull(resolvedArtifact);
             Objects.requireNonNull(dependency);
 
-            if (projectGavs.contains(artifactRef.asProjectVersionRef())) {
+            if (projectGavs.contains(resolvedArtifact.asProjectVersionRef())) {
                 getLog().debug("Ignoring in-project dependency: "
-                        + artifactRef.asProjectVersionRef().toString());
+                        + resolvedArtifact.asProjectVersionRef().toString());
                 continue;
             }
-            if (!unignoredStreams.contains(artifactRef.asProjectRef())) {
-                if (ignoredStreams.contains(artifactRef.asProjectRef())) {
+            if (!unignoredStreams.contains(resolvedArtifact.asProjectRef())) {
+                if (ignoredStreams.contains(resolvedArtifact.asProjectRef())) {
                     getLog().info("Skipping dependency (ignored stream): "
-                            + artifactRef.asProjectVersionRef().toString());
+                            + resolvedArtifact.asProjectVersionRef().toString());
                     continue;
                 }
-                ProjectRef wildCardIgnoredProjectRef = new SimpleProjectRef(artifactRef.getGroupId(), "*");
+                ProjectRef wildCardIgnoredProjectRef = new SimpleProjectRef(resolvedArtifact.getGroupId(), "*");
                 if (ignoredStreams.contains(wildCardIgnoredProjectRef)) {
                     getLog().info("Skipping dependency (ignored stream): "
-                            + artifactRef.asProjectVersionRef().toString());
+                            + resolvedArtifact.asProjectVersionRef().toString());
                     continue;
                 }
             }
-            if (artifactRef.getVersionString() == null) {
+            if (resolvedArtifact.getVersionString() == null) {
                 // this is not expected to happen
-                getLog().error("Resolved dependency has null version: " + artifactRef);
+                getLog().error("Resolved dependency has null version: " + resolvedArtifact);
                 continue;
             }
-            if (VersionUtils.isProperty(artifactRef.getVersionString())) {
+            if (VersionUtils.isProperty(resolvedArtifact.getVersionString())) {
                 // hack: PME doesn't seem to resolve properties from external parent poms
                 Pair<String, String> externalProperty = resolveExternalProperty(mavenProject,
-                        VersionUtils.extractPropertyName(artifactRef.getVersionString()));
+                        VersionUtils.extractPropertyName(resolvedArtifact.getVersionString()));
                 if (externalProperty != null) {
-                    artifactRef = new SimpleArtifactRef(artifactRef.getGroupId(), artifactRef.getArtifactId(),
-                            externalProperty.getRight(), artifactRef.getType(), artifactRef.getClassifier());
+                    resolvedArtifact = new SimpleArtifactRef(resolvedArtifact.getGroupId(), resolvedArtifact.getArtifactId(),
+                            externalProperty.getRight(), resolvedArtifact.getType(), resolvedArtifact.getClassifier());
                 } else {
                     // didn't manage to resolve dependency version, this is not expected to happen
-                    getLog().error("Resolved dependency has version with property: " + artifactRef);
+                    getLog().error("Resolved dependency has version with property: " + resolvedArtifact);
                     continue;
                 }
             }
             if ("test".equals(dependency.getScope()) && ignoreTestDependencies) {
                 getLog().info("Skipping dependency (ignored scope): "
-                        + artifactRef.asProjectVersionRef().toString());
+                        + resolvedArtifact.asProjectVersionRef().toString());
                 continue;
             }
 
 
             try {
-                VersionResult versionResult = channelSession.findLatestMavenArtifactVersion(artifactRef.getGroupId(),
-                        artifactRef.getArtifactId(), artifactRef.getType(), artifactRef.getClassifier(),
-                        artifactRef.getVersionString());
+                VersionResult versionResult = channelSession.findLatestMavenArtifactVersion(resolvedArtifact.getGroupId(),
+                        resolvedArtifact.getArtifactId(), resolvedArtifact.getType(), resolvedArtifact.getClassifier(),
+                        resolvedArtifact.getVersionString());
                 String channelVersion = versionResult.getVersion();
 
-                int comparison = compareVersions(channelVersion, artifactRef.getVersionString());
+                int comparison = compareVersions(channelVersion, resolvedArtifact.getVersionString());
                 if (comparison > 0 || (!doNotDowngrade && comparison < 0)) {
-                    getLog().info("Updating dependency " + artifactRef.getGroupId()
-                            + ":" + artifactRef.getArtifactId() + ":" + artifactRef.getVersionString()
+                    getLog().info("Updating dependency " + resolvedArtifact.getGroupId()
+                            + ":" + resolvedArtifact.getArtifactId() + ":" + resolvedArtifact.getVersionString()
                             + " to version " + channelVersion);
-                    dependenciesToUpgrade.add(Pair.of(dependency, channelVersion));
+                    dependenciesToUpgrade.add(Tuple.tuple(dependency, resolvedArtifact, channelVersion));
                 }
             } catch (UnresolvedMavenArtifactException e) {
                 // this produces a lot of noise due to many of e.g. test artifacts not being managed by channels, so keep it
                 // at the debug level
-                getLog().debug("Can't resolve artifact: " + artifactRef, e);
+                getLog().debug("Can't resolve artifact: " + resolvedArtifact, e);
             }
         }
         return dependenciesToUpgrade;