From 5868f937e52cf2e72bb23189b14b3147df97d01d Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 10 Jan 2024 18:05:35 +0100 Subject: [PATCH 1/5] [MNG-7854] Warn if imported dep is ignored --- https://issues.apache.org/jira/browse/MNG-7854 --- .../model/building/DefaultModelProblem.java | 12 ++-- .../DefaultDependencyManagementImporter.java | 72 +++++++++++++++++-- 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProblem.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProblem.java index fd0914d0fc53..00cb37a1bf76 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProblem.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelProblem.java @@ -136,16 +136,16 @@ public Exception getException() { @Override public String getMessage() { - String msg; + String msg = null; - if (message != null && message.length() > 0) { + if (message != null && !message.isEmpty()) { msg = message; - } else { + } else if (exception != null) { msg = exception.getMessage(); + } - if (msg == null) { - msg = ""; - } + if (msg == null) { + msg = ""; } return msg; diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java b/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java index 4456c541f5f9..dd028ae11abd 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java @@ -21,15 +21,16 @@ import javax.inject.Named; import javax.inject.Singleton; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import org.apache.maven.api.model.Dependency; import org.apache.maven.api.model.DependencyManagement; +import org.apache.maven.api.model.Exclusion; import org.apache.maven.api.model.Model; import org.apache.maven.model.building.ModelBuildingRequest; +import org.apache.maven.model.building.ModelProblem; import org.apache.maven.model.building.ModelProblemCollector; +import org.apache.maven.model.building.ModelProblemCollectorRequest; /** * Handles the import of dependency management from other models into the target model. @@ -61,7 +62,14 @@ public Model importManagement( for (DependencyManagement source : sources) { for (Dependency dependency : source.getDependencies()) { String key = dependency.getManagementKey(); - dependencies.putIfAbsent(key, dependency); + Dependency present = dependencies.putIfAbsent(key, dependency); + if (present != null && !equals(dependency, present)) { + // TODO: https://issues.apache.org/jira/browse/MNG-8004 + problems.add(new ModelProblemCollectorRequest( + ModelProblem.Severity.WARNING, ModelProblem.Version.V40) + .setMessage("Ignored POM import for: " + toString(dependency) + " as already imported " + + toString(present))); + } } } @@ -69,4 +77,60 @@ public Model importManagement( } return target; } + + private String toString(Dependency dependency) { + StringBuilder stringBuilder = new StringBuilder(); + stringBuilder + .append(dependency.getGroupId()) + .append(":") + .append(dependency.getArtifactId()) + .append(":") + .append(dependency.getType()); + if (dependency.getClassifier() != null && !dependency.getClassifier().isEmpty()) { + stringBuilder.append(":").append(dependency.getClassifier()); + } + stringBuilder + .append(":") + .append(dependency.getVersion()) + .append("@") + .append(dependency.getScope() == null ? "compile" : dependency.getScope()); + if (dependency.isOptional()) { + stringBuilder.append("[optional]"); + } + if (!dependency.getExclusions().isEmpty()) { + stringBuilder.append("[").append(dependency.getExclusions().size()).append(" exclusions]"); + } + return stringBuilder.toString(); + } + + private boolean equals(Dependency d1, Dependency d2) { + return Objects.equals(d1.getGroupId(), d2.getGroupId()) + && Objects.equals(d1.getArtifactId(), d2.getArtifactId()) + && Objects.equals(d1.getVersion(), d2.getVersion()) + && Objects.equals(d1.getType(), d2.getType()) + && Objects.equals(d1.getClassifier(), d2.getClassifier()) + && Objects.equals(d1.getScope(), d2.getScope()) + && Objects.equals(d1.getSystemPath(), d2.getSystemPath()) + && Objects.equals(d1.getOptional(), d2.getOptional()) + && equals(d1.getExclusions(), d2.getExclusions()); + } + + private boolean equals(Collection ce1, Collection ce2) { + if (ce1.size() == ce2.size()) { + Iterator i1 = ce1.iterator(); + Iterator i2 = ce2.iterator(); + while (i1.hasNext() && i2.hasNext()) { + if (!equals(i1.next(), i2.next())) { + return false; + } + } + return !i1.hasNext() && !i2.hasNext(); + } + return false; + } + + private boolean equals(Exclusion e1, Exclusion e2) { + return Objects.equals(e1.getGroupId(), e2.getGroupId()) + && Objects.equals(e1.getArtifactId(), e2.getArtifactId()); + } } From ada947e2020ca761f036f3cb3c01194006a88dcf Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 1 Feb 2024 11:19:22 +0100 Subject: [PATCH 2/5] Add unit tests with the current (but somewhat unexpected results) --- .../building/DefaultModelBuilderTest.java | 184 +++++++++++++++++- .../resources/poms/depmgmt/distant-import.xml | 23 +++ .../test/resources/poms/depmgmt/import.xml | 22 +++ .../test/resources/poms/depmgmt/junit-0.1.xml | 22 +++ .../test/resources/poms/depmgmt/junit-0.2.xml | 22 +++ .../resources/poms/depmgmt/other-import.xml | 22 +++ .../resources/poms/depmgmt/root-dep-first.xml | 28 +++ .../resources/poms/depmgmt/root-dep-last.xml | 28 +++ .../resources/poms/depmgmt/root-distance.xml | 30 +++ .../poms/depmgmt/root-two-imports.xml | 30 +++ 10 files changed, 409 insertions(+), 2 deletions(-) create mode 100644 maven-model-builder/src/test/resources/poms/depmgmt/distant-import.xml create mode 100644 maven-model-builder/src/test/resources/poms/depmgmt/import.xml create mode 100644 maven-model-builder/src/test/resources/poms/depmgmt/junit-0.1.xml create mode 100644 maven-model-builder/src/test/resources/poms/depmgmt/junit-0.2.xml create mode 100644 maven-model-builder/src/test/resources/poms/depmgmt/other-import.xml create mode 100644 maven-model-builder/src/test/resources/poms/depmgmt/root-dep-first.xml create mode 100644 maven-model-builder/src/test/resources/poms/depmgmt/root-dep-last.xml create mode 100644 maven-model-builder/src/test/resources/poms/depmgmt/root-distance.xml create mode 100644 maven-model-builder/src/test/resources/poms/depmgmt/root-two-imports.xml diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java index 0c2d783496ac..d7219d004938 100644 --- a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java +++ b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java @@ -30,8 +30,7 @@ import org.apache.maven.model.resolution.UnresolvableModelException; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.*; /** */ @@ -163,4 +162,185 @@ void testBuildRawModel() throws Exception { false); assertNotNull(res.get()); } + + /** + * This test has + * - a dependency directly managed to 0.2 + * - then a BOM import which manages that same dep to 0.1 + * This currently results in 0.2 and a warning (the warning should not be present imho) + */ + @Test + void testManagedDependencyBeforeImport() throws Exception { + ModelBuilder builder = new DefaultModelBuilderFactory().newInstance(); + assertNotNull(builder); + + DefaultModelBuildingRequest request = new DefaultModelBuildingRequest(); + request.setModelSource(new FileModelSource(new File( + getClass().getResource("/poms/depmgmt/root-dep-first.xml").getFile()))); + request.setModelResolver(new BaseModelResolver() { + public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) + throws UnresolvableModelException { + switch (dependency.getManagementKey()) { + case "test:import:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/import.xml") + .getFile())); + default: + throw new UnresolvableModelException( + "Cannot resolve", + dependency.getGroupId(), + dependency.getArtifactId(), + dependency.getVersion()); + } + } + }); + + ModelBuildingResult result = builder.build(request); + Dependency dep = result.getEffectiveModel().getDelegate().getDependencyManagement().getDependencies().stream() + .filter(d -> "test:mydep:jar".equals(d.getManagementKey())) + .findFirst() + .get(); + assertEquals("0.2", dep.getVersion()); + assertEquals(1, result.getProblems().size()); + ModelProblem problem = result.getProblems().get(0); + assertTrue(problem.toString().contains("Ignored POM import")); + } + + /** + * This test has + * - a BOM import which manages the dep to 0.1 + * - then a directly managed dependency to 0.2 + * This currently results in 0.2 and a warning (the warning should not be present imho) + */ + @Test + void testManagedDependencyAfterImport() throws Exception { + ModelBuilder builder = new DefaultModelBuilderFactory().newInstance(); + assertNotNull(builder); + + DefaultModelBuildingRequest request = new DefaultModelBuildingRequest(); + request.setModelSource(new FileModelSource(new File( + getClass().getResource("/poms/depmgmt/root-dep-last.xml").getFile()))); + request.setModelResolver(new BaseModelResolver() { + public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) + throws UnresolvableModelException { + switch (dependency.getManagementKey()) { + case "test:import:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/import.xml") + .getFile())); + default: + throw new UnresolvableModelException( + "Cannot resolve", + dependency.getGroupId(), + dependency.getArtifactId(), + dependency.getVersion()); + } + } + }); + + ModelBuildingResult result = builder.build(request); + Dependency dep = result.getEffectiveModel().getDelegate().getDependencyManagement().getDependencies().stream() + .filter(d -> "test:mydep:jar".equals(d.getManagementKey())) + .findFirst() + .get(); + assertEquals("0.2", dep.getVersion()); + assertEquals(1, result.getProblems().size()); + ModelProblem problem = result.getProblems().get(0); + assertTrue(problem.toString().contains("Ignored POM import")); + } + + /** + * This test has + * - a BOM import which manages dep to 0.3 + * - another BOM import which manages the dep to 0.1 + * This currently results in 0.3 (first wins) and a warning + */ + @Test + void testManagedDependencyTwoImports() throws Exception { + ModelBuilder builder = new DefaultModelBuilderFactory().newInstance(); + assertNotNull(builder); + + DefaultModelBuildingRequest request = new DefaultModelBuildingRequest(); + request.setModelSource(new FileModelSource(new File( + getClass().getResource("/poms/depmgmt/root-two-imports.xml").getFile()))); + request.setModelResolver(new BaseModelResolver() { + public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) + throws UnresolvableModelException { + switch (dependency.getManagementKey()) { + case "test:import:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/import.xml") + .getFile())); + case "test:other:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/other-import.xml") + .getFile())); + default: + throw new UnresolvableModelException( + "Cannot resolve", + dependency.getGroupId(), + dependency.getArtifactId(), + dependency.getVersion()); + } + } + }); + + ModelBuildingResult result = builder.build(request); + Dependency dep = result.getEffectiveModel().getDelegate().getDependencyManagement().getDependencies().stream() + .filter(d -> "test:mydep:jar".equals(d.getManagementKey())) + .findFirst() + .get(); + assertEquals("0.3", dep.getVersion()); + assertEquals(1, result.getProblems().size()); + ModelProblem problem = result.getProblems().get(0); + assertTrue(problem.toString().contains("Ignored POM import")); + } + + /** + * This test has + * - a BOM import which itself imports the junit BOM which manages the dep to 0.1 + * - a BOM import to the junit BOM which manages the dep to 0.2 + * This currently results in 0.1 (first wins, unexpected as this is not the closest) and a warning + */ + @Test + void testManagedDependencyDistance() throws Exception { + ModelBuilder builder = new DefaultModelBuilderFactory().newInstance(); + assertNotNull(builder); + + DefaultModelBuildingRequest request = new DefaultModelBuildingRequest(); + request.setLocationTracking(true); + request.setModelSource(new FileModelSource(new File( + getClass().getResource("/poms/depmgmt/root-distance.xml").getFile()))); + request.setModelResolver(new BaseModelResolver() { + public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) + throws UnresolvableModelException { + switch (dependency.getManagementKey()) { + case "org.junit:bom:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/junit-" + dependency.getVersion() + ".xml") + .getFile())); + case "test:other:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/distant-import.xml") + .getFile())); + default: + throw new UnresolvableModelException( + "Cannot resolve", + dependency.getGroupId(), + dependency.getArtifactId(), + dependency.getVersion()); + } + } + }); + + ModelBuildingResult result = builder.build(request); + Dependency dep = result.getEffectiveModel().getDelegate().getDependencyManagement().getDependencies().stream() + .filter(d -> "org.junit:junit:jar".equals(d.getManagementKey())) + .findFirst() + .get(); + assertEquals("0.1", dep.getVersion()); + assertEquals(1, result.getProblems().size()); + ModelProblem problem = result.getProblems().get(0); + assertTrue(problem.toString().contains("Ignored POM import")); + } } diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/distant-import.xml b/maven-model-builder/src/test/resources/poms/depmgmt/distant-import.xml new file mode 100644 index 000000000000..2a19c7fed15d --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/distant-import.xml @@ -0,0 +1,23 @@ + + + 4.1.0 + + test + other + 0.1-SNAPSHOT + + + + + org.junit + bom + 0.1 + pom + import + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/import.xml b/maven-model-builder/src/test/resources/poms/depmgmt/import.xml new file mode 100644 index 000000000000..d4f2490cd346 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/import.xml @@ -0,0 +1,22 @@ + + + 4.1.0 + + test + import + 0.1-SNAPSHOT + pom + + + + + test + mydep + 0.1 + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/junit-0.1.xml b/maven-model-builder/src/test/resources/poms/depmgmt/junit-0.1.xml new file mode 100644 index 000000000000..344fc69a9709 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/junit-0.1.xml @@ -0,0 +1,22 @@ + + + 4.1.0 + + org.junit + bom + 0.1 + pom + + + + + org.junit + junit + 0.1 + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/junit-0.2.xml b/maven-model-builder/src/test/resources/poms/depmgmt/junit-0.2.xml new file mode 100644 index 000000000000..58db6d80f8cd --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/junit-0.2.xml @@ -0,0 +1,22 @@ + + + 4.1.0 + + org.junit + bom + 0.2 + pom + + + + + org.junit + junit + 0.2 + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/other-import.xml b/maven-model-builder/src/test/resources/poms/depmgmt/other-import.xml new file mode 100644 index 000000000000..b110091a03d7 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/other-import.xml @@ -0,0 +1,22 @@ + + + 4.1.0 + + test + other-import + 0.1-SNAPSHOT + pom + + + + + test + mydep + 0.3 + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/root-dep-first.xml b/maven-model-builder/src/test/resources/poms/depmgmt/root-dep-first.xml new file mode 100644 index 000000000000..eb57c4549c4d --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/root-dep-first.xml @@ -0,0 +1,28 @@ + + + 4.1.0 + + test + test + 0.1-SNAPSHOT + + + + + test + mydep + 0.2 + + + test + import + 0.1-SNAPSHOT + pom + import + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/root-dep-last.xml b/maven-model-builder/src/test/resources/poms/depmgmt/root-dep-last.xml new file mode 100644 index 000000000000..bac146e50e85 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/root-dep-last.xml @@ -0,0 +1,28 @@ + + + 4.1.0 + + test + test + 0.1-SNAPSHOT + + + + + test + import + 0.1-SNAPSHOT + pom + import + + + test + mydep + 0.2 + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/root-distance.xml b/maven-model-builder/src/test/resources/poms/depmgmt/root-distance.xml new file mode 100644 index 000000000000..d56bbee188c1 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/root-distance.xml @@ -0,0 +1,30 @@ + + + 4.1.0 + + test + test + 0.1-SNAPSHOT + + + + + test + other + 0.1-SNAPSHOT + pom + import + + + org.junit + bom + 0.2 + pom + import + + + + + diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/root-two-imports.xml b/maven-model-builder/src/test/resources/poms/depmgmt/root-two-imports.xml new file mode 100644 index 000000000000..198a885610ea --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/root-two-imports.xml @@ -0,0 +1,30 @@ + + + 4.1.0 + + test + test + 0.1-SNAPSHOT + + + + + test + other + 0.1-SNAPSHOT + pom + import + + + test + import + 0.1-SNAPSHOT + pom + import + + + + + From 7f71f7f66af63f3c9fac54d2c374d62b821fee6a Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 1 Feb 2024 11:36:17 +0100 Subject: [PATCH 3/5] Fix warning when the dependency is directly managed --- .../DefaultDependencyManagementImporter.java | 18 ++++++++---------- .../building/DefaultModelBuilderTest.java | 12 ++++-------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java b/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java index dd028ae11abd..81a232e4d1c4 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java @@ -49,16 +49,6 @@ public Model importManagement( if (sources != null && !sources.isEmpty()) { Map dependencies = new LinkedHashMap<>(); - DependencyManagement depMgmt = target.getDependencyManagement(); - - if (depMgmt != null) { - for (Dependency dependency : depMgmt.getDependencies()) { - dependencies.put(dependency.getManagementKey(), dependency); - } - } else { - depMgmt = DependencyManagement.newInstance(); - } - for (DependencyManagement source : sources) { for (Dependency dependency : source.getDependencies()) { String key = dependency.getManagementKey(); @@ -73,6 +63,14 @@ public Model importManagement( } } + DependencyManagement depMgmt = target.getDependencyManagement(); + if (depMgmt != null) { + for (Dependency dependency : depMgmt.getDependencies()) { + dependencies.put(dependency.getManagementKey(), dependency); + } + } else { + depMgmt = DependencyManagement.newInstance(); + } return target.withDependencyManagement(depMgmt.withDependencies(dependencies.values())); } return target; diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java index d7219d004938..aa978fcd6b31 100644 --- a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java +++ b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java @@ -167,7 +167,7 @@ void testBuildRawModel() throws Exception { * This test has * - a dependency directly managed to 0.2 * - then a BOM import which manages that same dep to 0.1 - * This currently results in 0.2 and a warning (the warning should not be present imho) + * This currently results in 0.2 and a no warning */ @Test void testManagedDependencyBeforeImport() throws Exception { @@ -201,16 +201,14 @@ public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) .findFirst() .get(); assertEquals("0.2", dep.getVersion()); - assertEquals(1, result.getProblems().size()); - ModelProblem problem = result.getProblems().get(0); - assertTrue(problem.toString().contains("Ignored POM import")); + assertEquals(0, result.getProblems().size()); } /** * This test has * - a BOM import which manages the dep to 0.1 * - then a directly managed dependency to 0.2 - * This currently results in 0.2 and a warning (the warning should not be present imho) + * This currently results in 0.2 and no warning */ @Test void testManagedDependencyAfterImport() throws Exception { @@ -244,9 +242,7 @@ public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) .findFirst() .get(); assertEquals("0.2", dep.getVersion()); - assertEquals(1, result.getProblems().size()); - ModelProblem problem = result.getProblems().get(0); - assertTrue(problem.toString().contains("Ignored POM import")); + assertEquals(0, result.getProblems().size()); } /** From e1aee724d67bc6af75bc06af3a08e4e17efcb2f7 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 1 Feb 2024 11:43:20 +0100 Subject: [PATCH 4/5] The previous fix is wrong, now is a correct one --- .../DefaultDependencyManagementImporter.java | 25 ++++++---- .../building/DefaultModelBuilderTest.java | 48 +++++++++++++++++++ .../poms/depmgmt/root-distance-explicit.xml | 35 ++++++++++++++ 3 files changed, 98 insertions(+), 10 deletions(-) create mode 100644 maven-model-builder/src/test/resources/poms/depmgmt/root-distance-explicit.xml diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java b/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java index 81a232e4d1c4..613e8e160561 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java @@ -49,28 +49,33 @@ public Model importManagement( if (sources != null && !sources.isEmpty()) { Map dependencies = new LinkedHashMap<>(); + DependencyManagement depMgmt = target.getDependencyManagement(); + + if (depMgmt != null) { + for (Dependency dependency : depMgmt.getDependencies()) { + dependencies.put(dependency.getManagementKey(), dependency); + } + } else { + depMgmt = DependencyManagement.newInstance(); + } + + Set directDependencies = new HashSet<>(dependencies.keySet()); + for (DependencyManagement source : sources) { for (Dependency dependency : source.getDependencies()) { String key = dependency.getManagementKey(); Dependency present = dependencies.putIfAbsent(key, dependency); - if (present != null && !equals(dependency, present)) { + if (present != null && !equals(dependency, present) && !directDependencies.contains(key)) { // TODO: https://issues.apache.org/jira/browse/MNG-8004 problems.add(new ModelProblemCollectorRequest( ModelProblem.Severity.WARNING, ModelProblem.Version.V40) .setMessage("Ignored POM import for: " + toString(dependency) + " as already imported " - + toString(present))); + + toString(present) + ". Add a the conflicting managed dependency directly " + + "to the dependencyManagement section of the POM.")); } } } - DependencyManagement depMgmt = target.getDependencyManagement(); - if (depMgmt != null) { - for (Dependency dependency : depMgmt.getDependencies()) { - dependencies.put(dependency.getManagementKey(), dependency); - } - } else { - depMgmt = DependencyManagement.newInstance(); - } return target.withDependencyManagement(depMgmt.withDependencies(dependencies.values())); } return target; diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java index aa978fcd6b31..f41df183df8c 100644 --- a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java +++ b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java @@ -339,4 +339,52 @@ public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) ModelProblem problem = result.getProblems().get(0); assertTrue(problem.toString().contains("Ignored POM import")); } + + + /** + * This test has + * - a BOM import which itself imports the junit BOM which manages the dep to 0.1 + * - a BOM import to the junit BOM which manages the dep to 0.2 + * - a direct managed dependency to the dep at 0.3 (as suggested by the warning) + * This results in 0.3 (explicit managed wins) and no warning + */ + @Test + void testManagedDependencyDistanceWithExplicit() throws Exception { + ModelBuilder builder = new DefaultModelBuilderFactory().newInstance(); + assertNotNull(builder); + + DefaultModelBuildingRequest request = new DefaultModelBuildingRequest(); + request.setLocationTracking(true); + request.setModelSource(new FileModelSource(new File( + getClass().getResource("/poms/depmgmt/root-distance-explicit.xml").getFile()))); + request.setModelResolver(new BaseModelResolver() { + public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) + throws UnresolvableModelException { + switch (dependency.getManagementKey()) { + case "org.junit:bom:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/junit-" + dependency.getVersion() + ".xml") + .getFile())); + case "test:other:pom": + return new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/distant-import.xml") + .getFile())); + default: + throw new UnresolvableModelException( + "Cannot resolve", + dependency.getGroupId(), + dependency.getArtifactId(), + dependency.getVersion()); + } + } + }); + + ModelBuildingResult result = builder.build(request); + Dependency dep = result.getEffectiveModel().getDelegate().getDependencyManagement().getDependencies().stream() + .filter(d -> "org.junit:junit:jar".equals(d.getManagementKey())) + .findFirst() + .get(); + assertEquals("0.3", dep.getVersion()); + assertEquals(0, result.getProblems().size()); + } } diff --git a/maven-model-builder/src/test/resources/poms/depmgmt/root-distance-explicit.xml b/maven-model-builder/src/test/resources/poms/depmgmt/root-distance-explicit.xml new file mode 100644 index 000000000000..737cfd7fabd8 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/depmgmt/root-distance-explicit.xml @@ -0,0 +1,35 @@ + + + 4.1.0 + + test + test + 0.1-SNAPSHOT + + + + + test + other + 0.1-SNAPSHOT + pom + import + + + org.junit + bom + 0.2 + pom + import + + + org.junit + junit + 0.3 + + + + + From 84cd38f85d779ebd151bdaaf1ab76a782812674b Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 1 Feb 2024 11:47:18 +0100 Subject: [PATCH 5/5] Fix code style --- .../maven/model/building/DefaultModelBuilderTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java index f41df183df8c..4c98d88b5081 100644 --- a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java +++ b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java @@ -340,7 +340,6 @@ public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) assertTrue(problem.toString().contains("Ignored POM import")); } - /** * This test has * - a BOM import which itself imports the junit BOM which manages the dep to 0.1 @@ -355,8 +354,9 @@ void testManagedDependencyDistanceWithExplicit() throws Exception { DefaultModelBuildingRequest request = new DefaultModelBuildingRequest(); request.setLocationTracking(true); - request.setModelSource(new FileModelSource(new File( - getClass().getResource("/poms/depmgmt/root-distance-explicit.xml").getFile()))); + request.setModelSource(new FileModelSource(new File(getClass() + .getResource("/poms/depmgmt/root-distance-explicit.xml") + .getFile()))); request.setModelResolver(new BaseModelResolver() { public ModelSource resolveModel(org.apache.maven.model.Dependency dependency) throws UnresolvableModelException {