From 5a30d53aed4412a3bf8a602c5a3535ac04bd5ba7 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 22 Mar 2023 08:57:16 +0100 Subject: [PATCH 01/13] [MRESOLVER-345] BD and DF collect differences with ranges The two collector behave differently when in transitive dependencies ranges are in use. --- https://issues.apache.org/jira/browse/MRESOLVER-345 --- .../DependencyCollectorDelegateTestSupport.java | 10 ++++++++++ .../impl/collect/bf/BfDependencyCollectorTest.java | 6 ++---- .../impl/collect/df/DfDependencyCollectorTest.java | 6 ++---- .../artifact-descriptions/menforcer426Result.txt | 8 ++++++++ .../artifact-descriptions/menforcer426_aid_1.ini | 3 +++ .../artifact-descriptions/menforcer426_bid_1.ini | 2 ++ .../artifact-descriptions/menforcer426_cid_1.ini | 1 + .../artifact-descriptions/menforcer426_cid_2.ini | 1 + .../artifact-descriptions/menforcer426_cid_3.ini | 1 + 9 files changed, 30 insertions(+), 8 deletions(-) create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426Result.txt create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_aid_1.ini create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_bid_1.ini create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_1.ini create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_2.ini create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_3.ini diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegateTestSupport.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegateTestSupport.java index 5cf4d7f23..dcae8e7c7 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegateTestSupport.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegateTestSupport.java @@ -497,6 +497,16 @@ public void testDependencyManagement_DefaultDependencyManager() throws Dependenc assertEqualSubtree(expectedTree, toDependencyResult(result.getRoot(), "compile", null)); } + @Test + public void testMenforcer426case() throws DependencyCollectionException, IOException { + DependencyNode root = parser.parseResource("menforcer426Result.txt"); + Dependency dependency = root.getDependency(); + CollectRequest request = new CollectRequest(dependency, singletonList(repository)); + + CollectResult result = collector.collectDependencies(session, request); + assertEqualSubtree(root, result.getRoot()); + } + private DependencyNode toDependencyResult( final DependencyNode root, final String rootScope, final Boolean optional) { // Make the root artifact resultion result a dependency resolution result for the subtree check. diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollectorTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollectorTest.java index 353030fdd..aa903d6fe 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollectorTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollectorTest.java @@ -59,10 +59,8 @@ public static List parameters() { protected void setupCollector() { session.setConfigProperty(BfDependencyCollector.CONFIG_PROP_SKIPPER, useSkipper); - collector = new BfDependencyCollector(); - collector.setArtifactDescriptorReader(newReader("")); - collector.setVersionRangeResolver(new StubVersionRangeResolver()); - collector.setRemoteRepositoryManager(new StubRemoteRepositoryManager()); + collector = new BfDependencyCollector( + new StubRemoteRepositoryManager(), newReader(""), new StubVersionRangeResolver()); } private Dependency newDep(String coords, String scope, Collection exclusions) { diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollectorTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollectorTest.java index f5e808b66..81b5f8d92 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollectorTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollectorTest.java @@ -28,9 +28,7 @@ public class DfDependencyCollectorTest extends DependencyCollectorDelegateTestSupport { @Override protected void setupCollector() { - collector = new DfDependencyCollector(); - collector.setArtifactDescriptorReader(newReader("")); - collector.setVersionRangeResolver(new StubVersionRangeResolver()); - collector.setRemoteRepositoryManager(new StubRemoteRepositoryManager()); + collector = new DfDependencyCollector( + new StubRemoteRepositoryManager(), newReader(""), new StubVersionRangeResolver()); } } diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426Result.txt b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426Result.txt new file mode 100644 index 000000000..a86b64600 --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426Result.txt @@ -0,0 +1,8 @@ +menforcer426:aid:ext:1 compile ++- menforcer426:bid:ext:1 compile +| +- menforcer426:cid:ext:3 compile +| +- menforcer426:cid:ext:2 compile +| \- menforcer426:cid:ext:1 compile ++- menforcer426:cid:ext:3 compile ++- menforcer426:cid:ext:2 compile +\- menforcer426:cid:ext:1 compile diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_aid_1.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_aid_1.ini new file mode 100644 index 000000000..4ee283828 --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_aid_1.ini @@ -0,0 +1,3 @@ +[dependencies] +menforcer426:bid:ext:1 +menforcer426:cid:ext:[1,3] diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_bid_1.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_bid_1.ini new file mode 100644 index 000000000..8ce71c3bd --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_bid_1.ini @@ -0,0 +1,2 @@ +[dependencies] +menforcer426:cid:ext:[1,3] diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_1.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_1.ini new file mode 100644 index 000000000..61a252c23 --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_1.ini @@ -0,0 +1 @@ +[dependencies] diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_2.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_2.ini new file mode 100644 index 000000000..61a252c23 --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_2.ini @@ -0,0 +1 @@ +[dependencies] diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_3.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_3.ini new file mode 100644 index 000000000..61a252c23 --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_3.ini @@ -0,0 +1 @@ +[dependencies] From 50d5778ac5abd4941c981934d963c560ceb72d4b Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 22 Mar 2023 09:39:32 +0100 Subject: [PATCH 02/13] Rename tests properly --- .../collect/DependencyCollectorDelegateTestSupport.java | 4 ++-- .../artifact-descriptions/menforcer426Result.txt | 8 -------- .../artifact-descriptions/menforcer426_aid_1.ini | 3 --- .../artifact-descriptions/menforcer426_bid_1.ini | 2 -- .../transitiveDepsUseRangesDirtyTreeResult.txt | 8 ++++++++ .../transitiveDepsUseRangesDirtyTree_aid_1.ini | 3 +++ .../transitiveDepsUseRangesDirtyTree_bid_1.ini | 2 ++ ...d_1.ini => transitiveDepsUseRangesDirtyTree_cid_1.ini} | 0 ...d_2.ini => transitiveDepsUseRangesDirtyTree_cid_2.ini} | 0 ...d_3.ini => transitiveDepsUseRangesDirtyTree_cid_3.ini} | 0 10 files changed, 15 insertions(+), 15 deletions(-) delete mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426Result.txt delete mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_aid_1.ini delete mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_bid_1.ini create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult.txt create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_aid_1.ini create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_bid_1.ini rename maven-resolver-impl/src/test/resources/artifact-descriptions/{menforcer426_cid_1.ini => transitiveDepsUseRangesDirtyTree_cid_1.ini} (100%) rename maven-resolver-impl/src/test/resources/artifact-descriptions/{menforcer426_cid_2.ini => transitiveDepsUseRangesDirtyTree_cid_2.ini} (100%) rename maven-resolver-impl/src/test/resources/artifact-descriptions/{menforcer426_cid_3.ini => transitiveDepsUseRangesDirtyTree_cid_3.ini} (100%) diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegateTestSupport.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegateTestSupport.java index dcae8e7c7..60518beda 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegateTestSupport.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegateTestSupport.java @@ -498,8 +498,8 @@ public void testDependencyManagement_DefaultDependencyManager() throws Dependenc } @Test - public void testMenforcer426case() throws DependencyCollectionException, IOException { - DependencyNode root = parser.parseResource("menforcer426Result.txt"); + public void testTransitiveDepsUseRangesDirtyTree() throws DependencyCollectionException, IOException { + DependencyNode root = parser.parseResource("transitiveDepsUseRangesDirtyTreeResult.txt"); Dependency dependency = root.getDependency(); CollectRequest request = new CollectRequest(dependency, singletonList(repository)); diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426Result.txt b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426Result.txt deleted file mode 100644 index a86b64600..000000000 --- a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426Result.txt +++ /dev/null @@ -1,8 +0,0 @@ -menforcer426:aid:ext:1 compile -+- menforcer426:bid:ext:1 compile -| +- menforcer426:cid:ext:3 compile -| +- menforcer426:cid:ext:2 compile -| \- menforcer426:cid:ext:1 compile -+- menforcer426:cid:ext:3 compile -+- menforcer426:cid:ext:2 compile -\- menforcer426:cid:ext:1 compile diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_aid_1.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_aid_1.ini deleted file mode 100644 index 4ee283828..000000000 --- a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_aid_1.ini +++ /dev/null @@ -1,3 +0,0 @@ -[dependencies] -menforcer426:bid:ext:1 -menforcer426:cid:ext:[1,3] diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_bid_1.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_bid_1.ini deleted file mode 100644 index 8ce71c3bd..000000000 --- a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_bid_1.ini +++ /dev/null @@ -1,2 +0,0 @@ -[dependencies] -menforcer426:cid:ext:[1,3] diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult.txt b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult.txt new file mode 100644 index 000000000..b1711a4bd --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult.txt @@ -0,0 +1,8 @@ +transitiveDepsUseRangesDirtyTree:aid:ext:1 compile ++- transitiveDepsUseRangesDirtyTree:bid:ext:1 compile +| +- transitiveDepsUseRangesDirtyTree:cid:ext:3 compile +| +- transitiveDepsUseRangesDirtyTree:cid:ext:2 compile +| \- transitiveDepsUseRangesDirtyTree:cid:ext:1 compile ++- transitiveDepsUseRangesDirtyTree:cid:ext:3 compile ++- transitiveDepsUseRangesDirtyTree:cid:ext:2 compile +\- transitiveDepsUseRangesDirtyTree:cid:ext:1 compile diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_aid_1.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_aid_1.ini new file mode 100644 index 000000000..ed282a9c1 --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_aid_1.ini @@ -0,0 +1,3 @@ +[dependencies] +transitiveDepsUseRangesDirtyTree:bid:ext:1 +transitiveDepsUseRangesDirtyTree:cid:ext:[1,3] diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_bid_1.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_bid_1.ini new file mode 100644 index 000000000..e9a5d76b2 --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_bid_1.ini @@ -0,0 +1,2 @@ +[dependencies] +transitiveDepsUseRangesDirtyTree:cid:ext:[1,3] diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_1.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_1.ini similarity index 100% rename from maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_1.ini rename to maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_1.ini diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_2.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_2.ini similarity index 100% rename from maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_2.ini rename to maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_2.ini diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_3.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_3.ini similarity index 100% rename from maven-resolver-impl/src/test/resources/artifact-descriptions/menforcer426_cid_3.ini rename to maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_3.ini From ea36033f19b5c37e9d4aa3e21f651b556abcf140 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 22 Mar 2023 09:39:45 +0100 Subject: [PATCH 03/13] Add fix --- .../aether/internal/impl/collect/df/DfDependencyCollector.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollector.java index f6d9900f4..67d3cd679 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollector.java @@ -207,6 +207,9 @@ private void processDependency( return; } + // resolve newer version first (to align with BF, with this the two produce same dirty tree) + Collections.reverse(versions); + for (Version version : versions) { Artifact originalArtifact = dependency.getArtifact().setVersion(version.toString()); Dependency d = dependency.setArtifact(originalArtifact); From 8712ea2b2bddb6448c7115c165c6309af676e4ae Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 22 Mar 2023 10:21:27 +0100 Subject: [PATCH 04/13] Push demo --- .../resolver/examples/AllResolverDemos.java | 1 + .../DependencyHierarchyWithRanges.java | 85 +++++++++++++++++++ .../demo/mresolver345/a/1.0/a-1.0.pom | 40 +++++++++ .../demo/mresolver345/b/1.0/b-1.0.pom | 35 ++++++++ .../demo/mresolver345/c/1.0/c-1.0.pom | 27 ++++++ .../demo/mresolver345/c/2.0/c-2.0.pom | 27 ++++++ .../demo/mresolver345/c/3.0/c-3.0.pom | 27 ++++++ .../demo/mresolver345/c/maven-metadata.xml | 33 +++++++ 8 files changed, 275 insertions(+) create mode 100644 maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java create mode 100644 maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/a/1.0/a-1.0.pom create mode 100644 maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/b/1.0/b-1.0.pom create mode 100644 maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/1.0/c-1.0.pom create mode 100644 maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/2.0/c-2.0.pom create mode 100644 maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/3.0/c-3.0.pom create mode 100644 maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/maven-metadata.xml diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/AllResolverDemos.java b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/AllResolverDemos.java index 1e5bab32a..f947f8543 100644 --- a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/AllResolverDemos.java +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/AllResolverDemos.java @@ -35,6 +35,7 @@ public static void main(String[] args) throws Exception { GetDirectDependencies.main(args); GetDependencyTree.main(args); GetDependencyHierarchy.main(args); + DependencyHierarchyWithRanges.main(args); ResolveArtifact.main(args); ResolveTransitiveDependencies.main(args); ReverseDependencyTree.main(args); diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java new file mode 100644 index 000000000..a638ac8cf --- /dev/null +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java @@ -0,0 +1,85 @@ +/* + * 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.resolver.examples; + +import java.io.File; +import java.util.Collections; + +import org.apache.maven.resolver.examples.util.Booter; +import org.apache.maven.resolver.examples.util.ConsoleDependencyGraphDumper; +import org.eclipse.aether.DefaultRepositorySystemSession; +import org.eclipse.aether.RepositorySystem; +import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.artifact.DefaultArtifact; +import org.eclipse.aether.collection.CollectRequest; +import org.eclipse.aether.collection.CollectResult; +import org.eclipse.aether.repository.RemoteRepository; +import org.eclipse.aether.repository.RepositoryPolicy; +import org.eclipse.aether.resolution.ArtifactDescriptorRequest; +import org.eclipse.aether.resolution.ArtifactDescriptorResult; +import org.eclipse.aether.util.graph.manager.DependencyManagerUtils; +import org.eclipse.aether.util.graph.transformer.ConflictResolver; + +/** + * Visualizes the transitive dependencies of an artifact similar to m2e's dependency hierarchy view. Artifact in this + * test is not "plain" one as is original "demo" {@link GetDependencyHierarchy}, but specially crafted for case + * described in MRESOLVER-345. + * + * @see MRESOLVER-345 + */ +public class DependencyHierarchyWithRanges { + + /** + * Main. + */ + public static void main(String[] args) throws Exception { + System.out.println("------------------------------------------------------------"); + System.out.println(DependencyHierarchyWithRanges.class.getSimpleName()); + + RepositorySystem system = Booter.newRepositorySystem(Booter.selectFactory(args)); + + DefaultRepositorySystemSession session = Booter.newRepositorySystemSession(system); + + session.setChecksumPolicy(RepositoryPolicy.CHECKSUM_POLICY_IGNORE); // to not bother with checksums + session.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, true); + session.setConfigProperty(DependencyManagerUtils.CONFIG_PROP_VERBOSE, true); + + // this artifact is in "remote" repository in src/main/resources/remote-repository + Artifact artifact = new DefaultArtifact("org.apache.maven.resolver.demo.mresolver345:a:1.0"); + + File remoteRepoBasedir = new File("src/main/remote-repository"); + + ArtifactDescriptorRequest descriptorRequest = new ArtifactDescriptorRequest(); + descriptorRequest.setArtifact(artifact); + descriptorRequest.setRepositories(Collections.singletonList(new RemoteRepository.Builder( + "remote", "default", remoteRepoBasedir.toURI().toASCIIString()) + .build())); + ArtifactDescriptorResult descriptorResult = system.readArtifactDescriptor(session, descriptorRequest); + + CollectRequest collectRequest = new CollectRequest(); + collectRequest.setRootArtifact(descriptorResult.getArtifact()); + collectRequest.setDependencies(descriptorResult.getDependencies()); + collectRequest.setManagedDependencies(descriptorResult.getManagedDependencies()); + collectRequest.setRepositories(descriptorRequest.getRepositories()); + + CollectResult collectResult = system.collectDependencies(session, collectRequest); + + collectResult.getRoot().accept(new ConsoleDependencyGraphDumper()); + } +} diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/a/1.0/a-1.0.pom b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/a/1.0/a-1.0.pom new file mode 100644 index 000000000..918e0eb18 --- /dev/null +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/a/1.0/a-1.0.pom @@ -0,0 +1,40 @@ + + + + 4.0.0 + + org.apache.maven.resolver.demo.mresolver345 + a + 1.0 + + + + org.apache.maven.resolver.demo.mresolver345 + b + 1.0 + + + org.apache.maven.resolver.demo.mresolver345 + c + [1.0, 3.0] + + + + diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/b/1.0/b-1.0.pom b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/b/1.0/b-1.0.pom new file mode 100644 index 000000000..02805ea95 --- /dev/null +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/b/1.0/b-1.0.pom @@ -0,0 +1,35 @@ + + + + 4.0.0 + + org.apache.maven.resolver.demo.mresolver345 + b + 1.0 + + + + org.apache.maven.resolver.demo.mresolver345 + c + [1.0, 3.0] + + + + diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/1.0/c-1.0.pom b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/1.0/c-1.0.pom new file mode 100644 index 000000000..a875fe07a --- /dev/null +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/1.0/c-1.0.pom @@ -0,0 +1,27 @@ + + + + 4.0.0 + + org.apache.maven.resolver.demo.mresolver345 + c + 1.0 + + diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/2.0/c-2.0.pom b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/2.0/c-2.0.pom new file mode 100644 index 000000000..9f961ae0d --- /dev/null +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/2.0/c-2.0.pom @@ -0,0 +1,27 @@ + + + + 4.0.0 + + org.apache.maven.resolver.demo.mresolver345 + c + 2.0 + + diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/3.0/c-3.0.pom b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/3.0/c-3.0.pom new file mode 100644 index 000000000..44f9af58c --- /dev/null +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/3.0/c-3.0.pom @@ -0,0 +1,27 @@ + + + + 4.0.0 + + org.apache.maven.resolver.demo.mresolver345 + c + 3.0 + + diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/maven-metadata.xml b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/maven-metadata.xml new file mode 100644 index 000000000..7d3618d73 --- /dev/null +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/remote-repository/org/apache/maven/resolver/demo/mresolver345/c/maven-metadata.xml @@ -0,0 +1,33 @@ + + + + org.apache.maven.resolver.demo.mresolver345 + c + + 3.0 + 3.0 + + 1.0 + 2.0 + 3.0 + + 20230320074804 + + From a3787a29686178b338ad0fd9c76475e36fcc90bd Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 22 Mar 2023 15:47:44 +0100 Subject: [PATCH 05/13] Cover area with tests --- .../DependencyHierarchyWithRanges.java | 2 +- .../util/ConsoleDependencyGraphDumper.java | 103 +------------- .../collect/df/DfDependencyCollector.java | 3 - .../graph/visitor/DependencyGraphDumper.java | 128 ++++++++++++++++++ .../transformer/ConflictResolverTest.java | 102 ++++++++++++++ 5 files changed, 234 insertions(+), 104 deletions(-) create mode 100644 maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java index a638ac8cf..b53892449 100644 --- a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java @@ -60,7 +60,7 @@ public static void main(String[] args) throws Exception { session.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, true); session.setConfigProperty(DependencyManagerUtils.CONFIG_PROP_VERBOSE, true); - // this artifact is in "remote" repository in src/main/resources/remote-repository + // this artifact is in "remote" repository in src/main/remote-repository Artifact artifact = new DefaultArtifact("org.apache.maven.resolver.demo.mresolver345:a:1.0"); File remoteRepoBasedir = new File("src/main/remote-repository"); diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/ConsoleDependencyGraphDumper.java b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/ConsoleDependencyGraphDumper.java index e94ae2c32..5e31c765c 100644 --- a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/ConsoleDependencyGraphDumper.java +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/ConsoleDependencyGraphDumper.java @@ -18,111 +18,14 @@ */ package org.apache.maven.resolver.examples.util; -import java.io.PrintStream; -import java.util.ArrayList; -import java.util.Iterator; -import java.util.List; - -import org.eclipse.aether.artifact.Artifact; -import org.eclipse.aether.graph.Dependency; -import org.eclipse.aether.graph.DependencyNode; -import org.eclipse.aether.graph.DependencyVisitor; -import org.eclipse.aether.util.artifact.ArtifactIdUtils; -import org.eclipse.aether.util.graph.manager.DependencyManagerUtils; -import org.eclipse.aether.util.graph.transformer.ConflictResolver; +import org.eclipse.aether.util.graph.visitor.DependencyGraphDumper; /** * A dependency visitor that dumps the graph to the console. */ -public class ConsoleDependencyGraphDumper implements DependencyVisitor { - - private final PrintStream out; - - private final List childInfos = new ArrayList<>(); +public class ConsoleDependencyGraphDumper extends DependencyGraphDumper { public ConsoleDependencyGraphDumper() { - this(null); - } - - public ConsoleDependencyGraphDumper(PrintStream out) { - this.out = (out != null) ? out : System.out; - } - - public boolean visitEnter(DependencyNode node) { - out.println(formatIndentation() + formatNode(node)); - childInfos.add(new ChildInfo(node.getChildren().size())); - return true; - } - - private String formatIndentation() { - StringBuilder buffer = new StringBuilder(128); - for (Iterator it = childInfos.iterator(); it.hasNext(); ) { - buffer.append(it.next().formatIndentation(!it.hasNext())); - } - return buffer.toString(); - } - - private String formatNode(DependencyNode node) { - StringBuilder buffer = new StringBuilder(128); - Artifact a = node.getArtifact(); - Dependency d = node.getDependency(); - buffer.append(a); - if (d != null && d.getScope().length() > 0) { - buffer.append(" [").append(d.getScope()); - if (d.isOptional()) { - buffer.append(", optional"); - } - buffer.append("]"); - } - String premanaged = DependencyManagerUtils.getPremanagedVersion(node); - if (premanaged != null && !premanaged.equals(a.getBaseVersion())) { - buffer.append(" (version managed from ").append(premanaged).append(")"); - } - - premanaged = DependencyManagerUtils.getPremanagedScope(node); - if (premanaged != null && !premanaged.equals(d.getScope())) { - buffer.append(" (scope managed from ").append(premanaged).append(")"); - } - DependencyNode winner = (DependencyNode) node.getData().get(ConflictResolver.NODE_DATA_WINNER); - if (winner != null && !ArtifactIdUtils.equalsId(a, winner.getArtifact())) { - Artifact w = winner.getArtifact(); - buffer.append(" (conflicts with "); - if (ArtifactIdUtils.toVersionlessId(a).equals(ArtifactIdUtils.toVersionlessId(w))) { - buffer.append(w.getVersion()); - } else { - buffer.append(w); - } - buffer.append(")"); - } - return buffer.toString(); - } - - public boolean visitLeave(DependencyNode node) { - if (!childInfos.isEmpty()) { - childInfos.remove(childInfos.size() - 1); - } - if (!childInfos.isEmpty()) { - childInfos.get(childInfos.size() - 1).index++; - } - return true; - } - - private static class ChildInfo { - - final int count; - - int index; - - ChildInfo(int count) { - this.count = count; - } - - public String formatIndentation(boolean end) { - boolean last = index + 1 >= count; - if (end) { - return last ? "\\- " : "+- "; - } - return last ? " " : "| "; - } + super(System.out::println); } } diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollector.java index 67d3cd679..f6d9900f4 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollector.java @@ -207,9 +207,6 @@ private void processDependency( return; } - // resolve newer version first (to align with BF, with this the two produce same dirty tree) - Collections.reverse(versions); - for (Version version : versions) { Artifact originalArtifact = dependency.getArtifact().setVersion(version.toString()); Dependency d = dependency.setArtifact(originalArtifact); diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java new file mode 100644 index 000000000..7f50d49f7 --- /dev/null +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java @@ -0,0 +1,128 @@ +/* + * 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.eclipse.aether.util.graph.visitor; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.function.Consumer; + +import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.graph.Dependency; +import org.eclipse.aether.graph.DependencyNode; +import org.eclipse.aether.graph.DependencyVisitor; +import org.eclipse.aether.util.artifact.ArtifactIdUtils; +import org.eclipse.aether.util.graph.manager.DependencyManagerUtils; +import org.eclipse.aether.util.graph.transformer.ConflictResolver; + +import static java.util.Objects.requireNonNull; + +/** + * A dependency visitor that dumps the graph to any {@link Consumer}. Meant for diagnostic and testing, as + * it may output the graph to standard output, error or even some logging interface. + */ +public class DependencyGraphDumper implements DependencyVisitor { + + private final Consumer consumer; + + private final List childInfos = new ArrayList<>(); + + public DependencyGraphDumper(Consumer consumer) { + this.consumer = requireNonNull(consumer); + } + + @Override + public boolean visitEnter(DependencyNode node) { + consumer.accept(formatIndentation() + formatNode(node)); + childInfos.add(new ChildInfo(node.getChildren().size())); + return true; + } + + private String formatIndentation() { + StringBuilder buffer = new StringBuilder(128); + for (Iterator it = childInfos.iterator(); it.hasNext(); ) { + buffer.append(it.next().formatIndentation(!it.hasNext())); + } + return buffer.toString(); + } + + private String formatNode(DependencyNode node) { + StringBuilder buffer = new StringBuilder(128); + Artifact a = node.getArtifact(); + Dependency d = node.getDependency(); + buffer.append(a); + if (d != null && d.getScope().length() > 0) { + buffer.append(" [").append(d.getScope()); + if (d.isOptional()) { + buffer.append(", optional"); + } + buffer.append("]"); + } + String premanaged = DependencyManagerUtils.getPremanagedVersion(node); + if (premanaged != null && !premanaged.equals(a.getBaseVersion())) { + buffer.append(" (version managed from ").append(premanaged).append(")"); + } + + premanaged = DependencyManagerUtils.getPremanagedScope(node); + if (premanaged != null && !premanaged.equals(d.getScope())) { + buffer.append(" (scope managed from ").append(premanaged).append(")"); + } + DependencyNode winner = (DependencyNode) node.getData().get(ConflictResolver.NODE_DATA_WINNER); + if (winner != null && !ArtifactIdUtils.equalsId(a, winner.getArtifact())) { + Artifact w = winner.getArtifact(); + buffer.append(" (conflicts with "); + if (ArtifactIdUtils.toVersionlessId(a).equals(ArtifactIdUtils.toVersionlessId(w))) { + buffer.append(w.getVersion()); + } else { + buffer.append(w); + } + buffer.append(")"); + } + return buffer.toString(); + } + + public boolean visitLeave(DependencyNode node) { + if (!childInfos.isEmpty()) { + childInfos.remove(childInfos.size() - 1); + } + if (!childInfos.isEmpty()) { + childInfos.get(childInfos.size() - 1).index++; + } + return true; + } + + private static class ChildInfo { + + final int count; + + int index; + + ChildInfo(int count) { + this.count = count; + } + + public String formatIndentation(boolean end) { + boolean last = index + 1 >= count; + if (end) { + return last ? "\\- " : "+- "; + } + return last ? " " : "| "; + } + } +} diff --git a/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java b/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java index 01b288000..be8f9637a 100644 --- a/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java +++ b/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java @@ -22,6 +22,7 @@ import java.util.Arrays; import java.util.List; +import org.eclipse.aether.DefaultRepositorySystemSession; import org.eclipse.aether.RepositoryException; import org.eclipse.aether.artifact.DefaultArtifact; import org.eclipse.aether.graph.DefaultDependencyNode; @@ -30,6 +31,7 @@ import org.eclipse.aether.internal.test.util.TestUtils; import org.eclipse.aether.internal.test.util.TestVersion; import org.eclipse.aether.internal.test.util.TestVersionConstraint; +import org.eclipse.aether.util.graph.visitor.DependencyGraphDumper; import org.junit.Test; import static org.junit.Assert.assertEquals; @@ -77,6 +79,106 @@ public void versionClash() throws RepositoryException { assertSame(baz1Node, fooNode.getChildren().get(1)); } + @Test + public void versionRangeClashAscOrderVerbose() throws RepositoryException { + ConflictResolver resolver = makeDefaultResolver(); + + // A -> B -> C[1..2] + // \--> C[1..2] + DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); + DependencyNode b = makeDependencyNode("some-group", "b", "1.0"); + DependencyNode c1 = makeDependencyNode("some-group", "c", "1.0"); + DependencyNode c2 = makeDependencyNode("some-group", "c", "2.0"); + a.setChildren(mutableList(b, c1, c2)); + b.setChildren(mutableList(c1, c2)); + + DependencyNode ta = versionRangeClash(a); + + assertSame(a, ta); + assertEquals(2, a.getChildren().size()); + assertSame(b, a.getChildren().get(0)); + assertSame(c2, a.getChildren().get(1)); + assertEquals(1, b.getChildren().size()); + assertEqualsDependencyNode(c2, b.getChildren().get(0)); + } + + @Test + public void versionRangeClashDescOrderVerbose() throws RepositoryException { + ConflictResolver resolver = makeDefaultResolver(); + + // A -> B -> C[1..2] + // \--> C[1..2] + DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); + DependencyNode b = makeDependencyNode("some-group", "b", "1.0"); + DependencyNode c1 = makeDependencyNode("some-group", "c", "1.0"); + DependencyNode c2 = makeDependencyNode("some-group", "c", "2.0"); + a.setChildren(mutableList(b, c2, c1)); + b.setChildren(mutableList(c2, c1)); + + DependencyNode ta = versionRangeClash(a); + + assertSame(a, ta); + assertEquals(2, a.getChildren().size()); + assertSame(b, a.getChildren().get(0)); + assertSame(c2, a.getChildren().get(1)); + assertEquals(1, b.getChildren().size()); + assertEqualsDependencyNode(c2, b.getChildren().get(0)); + } + + @Test + public void versionRangeClashMixedOrderVerbose() throws RepositoryException { + ConflictResolver resolver = makeDefaultResolver(); + + // A -> B -> C[1..2] + // \--> C[1..2] + DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); + DependencyNode b = makeDependencyNode("some-group", "b", "1.0"); + DependencyNode c1 = makeDependencyNode("some-group", "c", "1.0"); + DependencyNode c2 = makeDependencyNode("some-group", "c", "2.0"); + a.setChildren(mutableList(b, c2, c1)); + b.setChildren(mutableList(c1, c2)); + + DependencyNode ta = versionRangeClash(a); + + assertSame(a, ta); + assertEquals(2, a.getChildren().size()); + assertSame(b, a.getChildren().get(0)); + assertSame(c2, a.getChildren().get(1)); + assertEquals(1, b.getChildren().size()); + assertEqualsDependencyNode(c2, b.getChildren().get(0)); + } + + /** + * Conflict resolution may replace {@link DependencyNode} instances with copies to keep them stateful on different + * levels of graph, hence here we merely assert that node IS what we expect. + */ + private void assertEqualsDependencyNode(DependencyNode node1, DependencyNode node2) { + assertEquals(node1.getDependency().getArtifact(), node2.getDependency().getArtifact()); + assertEquals(node1.getDependency().getScope(), node2.getDependency().getScope()); + assertEquals(node1.getDependency().getOptional(), node2.getDependency().getOptional()); + } + + /** + * Performs a verbose conflict resolution on passed in root. + */ + private DependencyNode versionRangeClash(DependencyNode root) throws RepositoryException { + ConflictResolver resolver = makeDefaultResolver(); + + System.out.println(); + System.out.println("Input node:"); + root.accept(new DependencyGraphDumper(System.out::println)); // TODO: remove + + DefaultRepositorySystemSession session = TestUtils.newSession(); + session.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, true); + DependencyNode transformedRoot = resolver.transformGraph(root, TestUtils.newTransformationContext(session)); + + System.out.println(); + System.out.println("Transformed node:"); + transformedRoot.accept(new DependencyGraphDumper(System.out::println)); + + return transformedRoot; + } + @Test public void derivedScopeChange() throws RepositoryException { ConflictResolver resolver = makeDefaultResolver(); From 2735cf95d08aa225a0801f9eb00eca83d489e158 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 22 Mar 2023 16:05:04 +0100 Subject: [PATCH 06/13] Ass sinc --- .../aether/util/graph/visitor/DependencyGraphDumper.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java index 7f50d49f7..888730c96 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java @@ -36,6 +36,8 @@ /** * A dependency visitor that dumps the graph to any {@link Consumer}. Meant for diagnostic and testing, as * it may output the graph to standard output, error or even some logging interface. + * + * @since 1.9.8 */ public class DependencyGraphDumper implements DependencyVisitor { From f1f103133629320be7735680f9e6be0b960cd546 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 22 Mar 2023 16:25:01 +0100 Subject: [PATCH 07/13] Add more UT --- .../transformer/ConflictResolverTest.java | 96 ++++++++++++++++--- 1 file changed, 84 insertions(+), 12 deletions(-) diff --git a/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java b/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java index be8f9637a..633fb8b9a 100644 --- a/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java +++ b/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java @@ -79,6 +79,28 @@ public void versionClash() throws RepositoryException { assertSame(baz1Node, fooNode.getChildren().get(1)); } + @Test + public void versionRangeClashAscOrder() throws RepositoryException { + ConflictResolver resolver = makeDefaultResolver(); + + // A -> B -> C[1..2] + // \--> C[1..2] + DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); + DependencyNode b = makeDependencyNode("some-group", "b", "1.0"); + DependencyNode c1 = makeDependencyNode("some-group", "c", "1.0"); + DependencyNode c2 = makeDependencyNode("some-group", "c", "2.0"); + a.setChildren(mutableList(b, c1, c2)); + b.setChildren(mutableList(c1, c2)); + + DependencyNode ta = versionRangeClash(a, false); + + assertSame(a, ta); + assertEquals(2, a.getChildren().size()); + assertSame(b, a.getChildren().get(0)); + assertSame(c2, a.getChildren().get(1)); + assertEquals(0, b.getChildren().size()); + } + @Test public void versionRangeClashAscOrderVerbose() throws RepositoryException { ConflictResolver resolver = makeDefaultResolver(); @@ -92,14 +114,38 @@ public void versionRangeClashAscOrderVerbose() throws RepositoryException { a.setChildren(mutableList(b, c1, c2)); b.setChildren(mutableList(c1, c2)); - DependencyNode ta = versionRangeClash(a); + DependencyNode ta = versionRangeClash(a, true); + + assertSame(a, ta); + assertEquals(3, a.getChildren().size()); + assertSame(b, a.getChildren().get(0)); + assertEqualsDependencyNode(c1, a.getChildren().get(1)); + assertSame(c2, a.getChildren().get(2)); + assertEquals(2, b.getChildren().size()); + assertEqualsDependencyNode(c1, b.getChildren().get(0)); + assertEqualsDependencyNode(c2, b.getChildren().get(1)); + } + + @Test + public void versionRangeClashDescOrder() throws RepositoryException { + ConflictResolver resolver = makeDefaultResolver(); + + // A -> B -> C[1..2] + // \--> C[1..2] + DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); + DependencyNode b = makeDependencyNode("some-group", "b", "1.0"); + DependencyNode c1 = makeDependencyNode("some-group", "c", "1.0"); + DependencyNode c2 = makeDependencyNode("some-group", "c", "2.0"); + a.setChildren(mutableList(b, c2, c1)); + b.setChildren(mutableList(c2, c1)); + + DependencyNode ta = versionRangeClash(a, false); assertSame(a, ta); assertEquals(2, a.getChildren().size()); assertSame(b, a.getChildren().get(0)); assertSame(c2, a.getChildren().get(1)); - assertEquals(1, b.getChildren().size()); - assertEqualsDependencyNode(c2, b.getChildren().get(0)); + assertEquals(0, b.getChildren().size()); } @Test @@ -115,18 +161,20 @@ public void versionRangeClashDescOrderVerbose() throws RepositoryException { a.setChildren(mutableList(b, c2, c1)); b.setChildren(mutableList(c2, c1)); - DependencyNode ta = versionRangeClash(a); + DependencyNode ta = versionRangeClash(a, true); assertSame(a, ta); - assertEquals(2, a.getChildren().size()); + assertEquals(3, a.getChildren().size()); assertSame(b, a.getChildren().get(0)); assertSame(c2, a.getChildren().get(1)); - assertEquals(1, b.getChildren().size()); + assertEqualsDependencyNode(c1, a.getChildren().get(2)); + assertEquals(2, b.getChildren().size()); assertEqualsDependencyNode(c2, b.getChildren().get(0)); + assertEqualsDependencyNode(c1, b.getChildren().get(1)); } @Test - public void versionRangeClashMixedOrderVerbose() throws RepositoryException { + public void versionRangeClashMixedOrder() throws RepositoryException { ConflictResolver resolver = makeDefaultResolver(); // A -> B -> C[1..2] @@ -138,14 +186,38 @@ public void versionRangeClashMixedOrderVerbose() throws RepositoryException { a.setChildren(mutableList(b, c2, c1)); b.setChildren(mutableList(c1, c2)); - DependencyNode ta = versionRangeClash(a); + DependencyNode ta = versionRangeClash(a, false); assertSame(a, ta); assertEquals(2, a.getChildren().size()); assertSame(b, a.getChildren().get(0)); assertSame(c2, a.getChildren().get(1)); - assertEquals(1, b.getChildren().size()); - assertEqualsDependencyNode(c2, b.getChildren().get(0)); + assertEquals(0, b.getChildren().size()); + } + + @Test + public void versionRangeClashMixedOrderVerbose() throws RepositoryException { + ConflictResolver resolver = makeDefaultResolver(); + + // A -> B -> C[1..2] + // \--> C[1..2] + DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); + DependencyNode b = makeDependencyNode("some-group", "b", "1.0"); + DependencyNode c1 = makeDependencyNode("some-group", "c", "1.0"); + DependencyNode c2 = makeDependencyNode("some-group", "c", "2.0"); + a.setChildren(mutableList(b, c2, c1)); + b.setChildren(mutableList(c1, c2)); + + DependencyNode ta = versionRangeClash(a, true); + + assertSame(a, ta); + assertEquals(3, a.getChildren().size()); + assertSame(b, a.getChildren().get(0)); + assertSame(c2, a.getChildren().get(1)); + assertEqualsDependencyNode(c1, a.getChildren().get(2)); + assertEquals(2, b.getChildren().size()); + assertEqualsDependencyNode(c1, b.getChildren().get(0)); + assertEqualsDependencyNode(c2, b.getChildren().get(1)); } /** @@ -161,7 +233,7 @@ private void assertEqualsDependencyNode(DependencyNode node1, DependencyNode nod /** * Performs a verbose conflict resolution on passed in root. */ - private DependencyNode versionRangeClash(DependencyNode root) throws RepositoryException { + private DependencyNode versionRangeClash(DependencyNode root, boolean verbose) throws RepositoryException { ConflictResolver resolver = makeDefaultResolver(); System.out.println(); @@ -169,7 +241,7 @@ private DependencyNode versionRangeClash(DependencyNode root) throws RepositoryE root.accept(new DependencyGraphDumper(System.out::println)); // TODO: remove DefaultRepositorySystemSession session = TestUtils.newSession(); - session.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, true); + session.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, verbose); DependencyNode transformedRoot = resolver.transformGraph(root, TestUtils.newTransformationContext(session)); System.out.println(); From fc6cff40cff7ac608af06179db562be28d992d3f Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 22 Mar 2023 20:34:00 +0100 Subject: [PATCH 08/13] Fix in place --- .../resolver/examples/resolver/Resolver.java | 15 +- ...ependencyCollectorDelegateTestSupport.java | 9 +- .../collect/bf/BfDependencyCollectorTest.java | 5 + .../collect/df/DfDependencyCollectorTest.java | 5 + ...sitiveDepsUseRangesDirtyTreeResult_BF.txt} | 0 ...nsitiveDepsUseRangesDirtyTreeResult_DF.txt | 8 + .../graph/transformer/ConflictResolver.java | 141 ++++++++++++++---- .../AbstractDepthFirstNodeListGenerator.java | 2 + .../visitor/CloningDependencyVisitor.java | 2 + .../graph/visitor/DependencyGraphDumper.java | 19 ++- .../visitor/FilteringDependencyVisitor.java | 4 +- .../PathRecordingDependencyVisitor.java | 2 + .../visitor/PostorderNodeListGenerator.java | 6 +- .../visitor/PreorderNodeListGenerator.java | 2 +- .../graph/visitor/TreeDependencyVisitor.java | 2 + .../transformer/ConflictResolverTest.java | 92 +++++++++--- 16 files changed, 243 insertions(+), 71 deletions(-) rename maven-resolver-impl/src/test/resources/artifact-descriptions/{transitiveDepsUseRangesDirtyTreeResult.txt => transitiveDepsUseRangesDirtyTreeResult_BF.txt} (100%) create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult_DF.txt diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/resolver/Resolver.java b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/resolver/Resolver.java index a5a01f76d..bf1d37954 100644 --- a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/resolver/Resolver.java +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/resolver/Resolver.java @@ -20,9 +20,10 @@ import java.io.ByteArrayOutputStream; import java.io.PrintStream; +import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; import org.apache.maven.resolver.examples.util.Booter; -import org.apache.maven.resolver.examples.util.ConsoleDependencyGraphDumper; import org.eclipse.aether.DefaultRepositorySystemSession; import org.eclipse.aether.RepositorySystem; import org.eclipse.aether.RepositorySystemSession; @@ -40,6 +41,7 @@ import org.eclipse.aether.repository.RemoteRepository; import org.eclipse.aether.resolution.DependencyRequest; import org.eclipse.aether.resolution.DependencyResolutionException; +import org.eclipse.aether.util.graph.visitor.DependencyGraphDumper; import org.eclipse.aether.util.graph.visitor.PreorderNodeListGenerator; import org.eclipse.aether.util.repository.AuthenticationBuilder; @@ -121,8 +123,13 @@ public void deploy(Artifact artifact, Artifact pom, String remoteRepository) thr } private void displayTree(DependencyNode node, StringBuilder sb) { - ByteArrayOutputStream os = new ByteArrayOutputStream(1024); - node.accept(new ConsoleDependencyGraphDumper(new PrintStream(os))); - sb.append(os.toString()); + try { + ByteArrayOutputStream os = new ByteArrayOutputStream(1024); + PrintStream ps = new PrintStream(os, true, StandardCharsets.UTF_8.name()); + node.accept(new DependencyGraphDumper(ps::println)); + sb.append(os); + } catch (UnsupportedEncodingException e) { + throw new RuntimeException(e); + } } } diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegateTestSupport.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegateTestSupport.java index 60518beda..89a2291e6 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegateTestSupport.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegateTestSupport.java @@ -499,7 +499,12 @@ public void testDependencyManagement_DefaultDependencyManager() throws Dependenc @Test public void testTransitiveDepsUseRangesDirtyTree() throws DependencyCollectionException, IOException { - DependencyNode root = parser.parseResource("transitiveDepsUseRangesDirtyTreeResult.txt"); + // Note: DF depends on version order (ultimately the order of versions as returned by VersionRangeResolver + // that in case of Maven, means order as in maven-metadata.xml + // BF on the other hand explicitly sorts versions from range in descending order + // + // Hence, the "dirty tree" of two will not match. + DependencyNode root = parser.parseResource(getTransitiveDepsUseRangesDirtyTreeResource()); Dependency dependency = root.getDependency(); CollectRequest request = new CollectRequest(dependency, singletonList(repository)); @@ -507,6 +512,8 @@ public void testTransitiveDepsUseRangesDirtyTree() throws DependencyCollectionEx assertEqualSubtree(root, result.getRoot()); } + protected abstract String getTransitiveDepsUseRangesDirtyTreeResource(); + private DependencyNode toDependencyResult( final DependencyNode root, final String rootScope, final Boolean optional) { // Make the root artifact resultion result a dependency resolution result for the subtree check. diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollectorTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollectorTest.java index aa903d6fe..870d2ef61 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollectorTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollectorTest.java @@ -63,6 +63,11 @@ protected void setupCollector() { new StubRemoteRepositoryManager(), newReader(""), new StubVersionRangeResolver()); } + @Override + protected String getTransitiveDepsUseRangesDirtyTreeResource() { + return "transitiveDepsUseRangesDirtyTreeResult_BF.txt"; + } + private Dependency newDep(String coords, String scope, Collection exclusions) { Dependency d = new Dependency(new DefaultArtifact(coords), scope); return d.setExclusions(exclusions); diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollectorTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollectorTest.java index 81b5f8d92..f07fdc335 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollectorTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/df/DfDependencyCollectorTest.java @@ -31,4 +31,9 @@ protected void setupCollector() { collector = new DfDependencyCollector( new StubRemoteRepositoryManager(), newReader(""), new StubVersionRangeResolver()); } + + @Override + protected String getTransitiveDepsUseRangesDirtyTreeResource() { + return "transitiveDepsUseRangesDirtyTreeResult_DF.txt"; + } } diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult.txt b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult_BF.txt similarity index 100% rename from maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult.txt rename to maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult_BF.txt diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult_DF.txt b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult_DF.txt new file mode 100644 index 000000000..ef148ca9a --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult_DF.txt @@ -0,0 +1,8 @@ +transitiveDepsUseRangesDirtyTree:aid:ext:1 compile ++- transitiveDepsUseRangesDirtyTree:bid:ext:1 compile +| +- transitiveDepsUseRangesDirtyTree:cid:ext:1 compile +| +- transitiveDepsUseRangesDirtyTree:cid:ext:2 compile +| \- transitiveDepsUseRangesDirtyTree:cid:ext:3 compile ++- transitiveDepsUseRangesDirtyTree:cid:ext:1 compile ++- transitiveDepsUseRangesDirtyTree:cid:ext:2 compile +\- transitiveDepsUseRangesDirtyTree:cid:ext:3 compile diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java index 7289c9bc1..fe31df529 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java @@ -18,26 +18,17 @@ */ package org.eclipse.aether.util.graph.transformer; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.IdentityHashMap; -import java.util.Iterator; -import java.util.List; -import java.util.ListIterator; -import java.util.Map; +import java.util.*; import org.eclipse.aether.RepositoryException; +import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.artifact.Artifact; import org.eclipse.aether.collection.DependencyGraphTransformationContext; import org.eclipse.aether.collection.DependencyGraphTransformer; import org.eclipse.aether.graph.DefaultDependencyNode; import org.eclipse.aether.graph.Dependency; import org.eclipse.aether.graph.DependencyNode; -import org.eclipse.aether.util.ConfigUtils; +import org.eclipse.aether.util.artifact.ArtifactIdUtils; import static java.util.Objects.requireNonNull; @@ -66,9 +57,66 @@ public final class ConflictResolver implements DependencyGraphTransformer { /** * The key in the repository session's {@link org.eclipse.aether.RepositorySystemSession#getConfigProperties() * configuration properties} used to store a {@link Boolean} flag controlling the transformer's verbose mode. + * Accepted values are {@link Boolean} type, {@link String} type (where "true" would be interpreted as {@code true} + * or {@link Verbosity} enum instances. */ public static final String CONFIG_PROP_VERBOSE = "aether.conflictResolver.verbose"; + /** + * The enum representing verbosity levels of conflict resolver. + * + * @since 1.9.8 + */ + public enum Verbosity { + /** + * Verbosity level to be used in all "common" resolving use cases (ie. dependencies to build class path). The + * {@link ConflictResolver} in this mode will trim down the graph to the barest minimum: will not leave + * any conflicting node in place, hence no conflicts will be present in transformed graph either. + */ + NONE, + + /** + * Verbosity level to be used in "analyze" resolving use cases (ie. dependency convergence calculations). The + * {@link ConflictResolver} in this mode will remove any redundant collected nodes, in turn it will leave one + * with recorded conflicting information. This mode corresponds tp "classic verbose" mode when + * {@link #CONFIG_PROP_VERBOSE} was set to {@code true}. Obviously, the resulting dependency tree is not + * suitable for artifact resolution unless a filter is employed to exclude the duplicate dependencies. + */ + STANDARD, + + /** + * Verbosity level to be used in "analyze" resolving use cases (ie. dependency convergence calculations). The + * {@link ConflictResolver} in this mode will not remove any collected node, in turn it will record on all + * eliminated nodes the conflicting information. Obviously, the resulting dependency tree is not suitable + * * for artifact resolution unless a filter is employed to exclude the duplicate dependencies. + */ + FULL + } + + /** + * Helper method that uses {@link RepositorySystemSession} and {@link #CONFIG_PROP_VERBOSE} key to figure out + * current {@link Verbosity}: if {@link Boolean} or {@code String} found, returns {@link Verbosity#STANDARD} + * or {@link Verbosity#NONE}, depending on value (string is parsed with {@link Boolean#parseBoolean(String)} + * for {@code true} or {@code false} correspondingly. This is to retain "existing" behavior, where the config + * key accepted only these values. + * Since 1.9.8 release, this key may contain {@link Verbosity} enum instance as well, in which case that instance + * is returned. + * This method never returns {@code null}. + */ + private static Verbosity getVerbosity(RepositorySystemSession session) { + final Object verbosityValue = session.getConfigProperties().get(CONFIG_PROP_VERBOSE); + if (verbosityValue instanceof Boolean) { + return (Boolean) verbosityValue ? Verbosity.STANDARD : Verbosity.NONE; + } else if (verbosityValue instanceof String) { + return Boolean.parseBoolean(verbosityValue.toString()) ? Verbosity.STANDARD : Verbosity.NONE; + } else if (verbosityValue instanceof Verbosity) { + return (Verbosity) verbosityValue; + } else if (verbosityValue != null) { + throw new IllegalArgumentException("Unsupported Verbosity configuration: " + verbosityValue); + } + return Verbosity.NONE; + } + /** * The key in the dependency node's {@link DependencyNode#getData() custom data} under which a reference to the * {@link DependencyNode} which has won the conflict is stored. @@ -173,14 +221,14 @@ public DependencyNode transformGraph(DependencyNode node, DependencyGraphTransfo DependencyNode winner = ctx.winner.node; state.scopeSelector.selectScope(ctx); - if (state.verbose) { + if (Verbosity.NONE != state.verbosity) { winner.setData( NODE_DATA_ORIGINAL_SCOPE, winner.getDependency().getScope()); } winner.setScope(ctx.scope); state.optionalitySelector.selectOptionality(ctx); - if (state.verbose) { + if (Verbosity.NONE != state.verbosity) { winner.setData( NODE_DATA_ORIGINAL_OPTIONALITY, winner.getDependency().isOptional()); @@ -232,11 +280,12 @@ private boolean gatherConflictItems(DependencyNode node, State state) throws Rep return true; } - private void removeLosers(State state) { + private static void removeLosers(State state) { ConflictItem winner = state.conflictCtx.winner; + String winnerId = ArtifactIdUtils.toId(winner.node.getArtifact()); List previousParent = null; ListIterator childIt = null; - boolean conflictVisualized = false; + ArrayList toRemoveIds = new ArrayList<>(); for (ConflictItem item : state.items) { if (item == winner) { continue; @@ -244,30 +293,58 @@ private void removeLosers(State state) { if (item.parent != previousParent) { childIt = item.parent.listIterator(); previousParent = item.parent; - conflictVisualized = false; } while (childIt.hasNext()) { DependencyNode child = childIt.next(); if (child == item.node) { - if (state.verbose && !conflictVisualized && item.parent != winner.parent) { - conflictVisualized = true; - DependencyNode loser = new DefaultDependencyNode(child); - loser.setData(NODE_DATA_WINNER, winner.node); - loser.setData( - NODE_DATA_ORIGINAL_SCOPE, loser.getDependency().getScope()); - loser.setData( - NODE_DATA_ORIGINAL_OPTIONALITY, - loser.getDependency().isOptional()); - loser.setScope(item.getScopes().iterator().next()); - loser.setChildren(Collections.emptyList()); - childIt.set(loser); - } else { + if (Verbosity.NONE == state.verbosity) { childIt.remove(); + break; + } + + if (Verbosity.STANDARD == state.verbosity) { + String childId = ArtifactIdUtils.toId(child.getArtifact()); + if (!Objects.equals(winnerId, childId)) { + toRemoveIds.add(childId); + } } + + DependencyNode loser = new DefaultDependencyNode(child); + loser.setData(NODE_DATA_WINNER, winner.node); + loser.setData( + NODE_DATA_ORIGINAL_SCOPE, loser.getDependency().getScope()); + loser.setData( + NODE_DATA_ORIGINAL_OPTIONALITY, + loser.getDependency().isOptional()); + loser.setScope(item.getScopes().iterator().next()); + loser.setChildren(Collections.emptyList()); + childIt.set(loser); break; } } } + + // 2nd pass to apply "standard" verbosity: leaving only 1 loser, but with care + if (Verbosity.STANDARD == state.verbosity) { + for (ConflictItem item : state.items) { + if (item == winner) { + continue; + } + if (item.parent != previousParent) { + childIt = item.parent.listIterator(); + previousParent = item.parent; + } + while (childIt.hasNext()) { + DependencyNode child = childIt.next(); + String childId = ArtifactIdUtils.toId(child.getArtifact()); + if (toRemoveIds.contains(childId)) { + childIt.remove(); + break; + } + } + } + } + // there might still be losers beneath the winner (e.g. in case of cycles) // those will be nuked during future graph walks when we include the winner in the recursion } @@ -361,7 +438,7 @@ final class State { /** * Flag whether we should keep losers in the graph to enable visualization/troubleshooting of conflicts. */ - final boolean verbose; + final Verbosity verbosity; /** * A mapping from conflict id to winner node, helps to recognize nodes that have their effective @@ -458,7 +535,7 @@ final class State { DependencyGraphTransformationContext context) throws RepositoryException { this.conflictIds = conflictIds; - verbose = ConfigUtils.getBoolean(context.getSession(), false, CONFIG_PROP_VERBOSE); + this.verbosity = getVerbosity(context.getSession()); potentialAncestorIds = new HashSet<>(conflictIdCount * 2); resolvedIds = new HashMap<>(conflictIdCount * 2); items = new ArrayList<>(256); diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/AbstractDepthFirstNodeListGenerator.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/AbstractDepthFirstNodeListGenerator.java index 97b3342a3..6293dda08 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/AbstractDepthFirstNodeListGenerator.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/AbstractDepthFirstNodeListGenerator.java @@ -156,7 +156,9 @@ protected boolean setVisited(DependencyNode node) { return visitedNodes.put(node, Boolean.TRUE) == null; } + @Override public abstract boolean visitEnter(DependencyNode node); + @Override public abstract boolean visitLeave(DependencyNode node); } diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/CloningDependencyVisitor.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/CloningDependencyVisitor.java index 0da5649fa..ab1406e8e 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/CloningDependencyVisitor.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/CloningDependencyVisitor.java @@ -66,6 +66,7 @@ protected DependencyNode clone(DependencyNode node) { return new DefaultDependencyNode(node); } + @Override public final boolean visitEnter(DependencyNode node) { boolean recurse = true; @@ -90,6 +91,7 @@ public final boolean visitEnter(DependencyNode node) { return recurse; } + @Override public final boolean visitLeave(DependencyNode node) { parents.pop(); diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java index 888730c96..60a12727f 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java @@ -86,19 +86,24 @@ private String formatNode(DependencyNode node) { buffer.append(" (scope managed from ").append(premanaged).append(")"); } DependencyNode winner = (DependencyNode) node.getData().get(ConflictResolver.NODE_DATA_WINNER); - if (winner != null && !ArtifactIdUtils.equalsId(a, winner.getArtifact())) { - Artifact w = winner.getArtifact(); - buffer.append(" (conflicts with "); - if (ArtifactIdUtils.toVersionlessId(a).equals(ArtifactIdUtils.toVersionlessId(w))) { - buffer.append(w.getVersion()); + if (winner != null) { + if (ArtifactIdUtils.equalsId(a, winner.getArtifact())) { + buffer.append(" (nearer exists)"); } else { - buffer.append(w); + Artifact w = winner.getArtifact(); + buffer.append(" (conflicts with "); + if (ArtifactIdUtils.toVersionlessId(a).equals(ArtifactIdUtils.toVersionlessId(w))) { + buffer.append(w.getVersion()); + } else { + buffer.append(w); + } + buffer.append(")"); } - buffer.append(")"); } return buffer.toString(); } + @Override public boolean visitLeave(DependencyNode node) { if (!childInfos.isEmpty()) { childInfos.remove(childInfos.size() - 1); diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/FilteringDependencyVisitor.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/FilteringDependencyVisitor.java index 16a851028..202c2598e 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/FilteringDependencyVisitor.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/FilteringDependencyVisitor.java @@ -26,7 +26,7 @@ /** * A dependency visitor that delegates to another visitor if nodes match a filter. Note that in case of a mismatching - * node, the children of that node are still visisted and presented to the filter. + * node, the children of that node are still visited and presented to the filter. */ public final class FilteringDependencyVisitor implements DependencyVisitor { @@ -69,6 +69,7 @@ public DependencyFilter getFilter() { return filter; } + @Override public boolean visitEnter(DependencyNode node) { boolean accept = filter == null || filter.accept(node, parents); @@ -83,6 +84,7 @@ public boolean visitEnter(DependencyNode node) { } } + @Override public boolean visitLeave(DependencyNode node) { parents.pop(); diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/PathRecordingDependencyVisitor.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/PathRecordingDependencyVisitor.java index 119323851..d0576ca90 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/PathRecordingDependencyVisitor.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/PathRecordingDependencyVisitor.java @@ -85,6 +85,7 @@ public List> getPaths() { return paths; } + @Override public boolean visitEnter(DependencyNode node) { boolean accept = filter == null || filter.accept(node, parents); @@ -106,6 +107,7 @@ public boolean visitEnter(DependencyNode node) { return !hasDuplicateNodeInParent; } + @Override public boolean visitLeave(DependencyNode node) { parents.pop(); diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/PostorderNodeListGenerator.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/PostorderNodeListGenerator.java index b8ba0dae5..09dad67cc 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/PostorderNodeListGenerator.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/PostorderNodeListGenerator.java @@ -42,11 +42,7 @@ public boolean visitEnter(DependencyNode node) { visits.push(visited); - if (visited) { - return false; - } - - return true; + return !visited; } @Override diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/PreorderNodeListGenerator.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/PreorderNodeListGenerator.java index f8370b1a3..07841049c 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/PreorderNodeListGenerator.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/PreorderNodeListGenerator.java @@ -21,7 +21,7 @@ import org.eclipse.aether.graph.DependencyNode; /** - * Generates a sequence of dependency nodes from a dependeny graph by traversing the graph in preorder. This visitor + * Generates a sequence of dependency nodes from a dependency graph by traversing the graph in preorder. This visitor * visits each node exactly once regardless how many paths within the dependency graph lead to the node such that the * resulting node sequence is free of duplicates. */ diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/TreeDependencyVisitor.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/TreeDependencyVisitor.java index 94cc382ea..44b3759ca 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/TreeDependencyVisitor.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/TreeDependencyVisitor.java @@ -50,6 +50,7 @@ public TreeDependencyVisitor(DependencyVisitor visitor) { visits = new Stack<>(); } + @Override public boolean visitEnter(DependencyNode node) { boolean visited = visitedNodes.put(node, Boolean.TRUE) != null; @@ -62,6 +63,7 @@ public boolean visitEnter(DependencyNode node) { return visitor.visitEnter(node); } + @Override public boolean visitLeave(DependencyNode node) { Boolean visited = visits.pop(); diff --git a/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java b/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java index 633fb8b9a..4d07c4757 100644 --- a/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java +++ b/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java @@ -81,8 +81,6 @@ public void versionClash() throws RepositoryException { @Test public void versionRangeClashAscOrder() throws RepositoryException { - ConflictResolver resolver = makeDefaultResolver(); - // A -> B -> C[1..2] // \--> C[1..2] DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); @@ -92,7 +90,7 @@ public void versionRangeClashAscOrder() throws RepositoryException { a.setChildren(mutableList(b, c1, c2)); b.setChildren(mutableList(c1, c2)); - DependencyNode ta = versionRangeClash(a, false); + DependencyNode ta = versionRangeClash(a, ConflictResolver.Verbosity.NONE); assertSame(a, ta); assertEquals(2, a.getChildren().size()); @@ -102,9 +100,28 @@ public void versionRangeClashAscOrder() throws RepositoryException { } @Test - public void versionRangeClashAscOrderVerbose() throws RepositoryException { - ConflictResolver resolver = makeDefaultResolver(); + public void versionRangeClashAscOrderStandardVerbose() throws RepositoryException { + // A -> B -> C[1..2] + // \--> C[1..2] + DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); + DependencyNode b = makeDependencyNode("some-group", "b", "1.0"); + DependencyNode c1 = makeDependencyNode("some-group", "c", "1.0"); + DependencyNode c2 = makeDependencyNode("some-group", "c", "2.0"); + a.setChildren(mutableList(b, c1, c2)); + b.setChildren(mutableList(c1, c2)); + + DependencyNode ta = versionRangeClash(a, ConflictResolver.Verbosity.STANDARD); + assertSame(a, ta); + assertEquals(2, a.getChildren().size()); + assertSame(b, a.getChildren().get(0)); + assertEqualsDependencyNode(c2, a.getChildren().get(1)); + assertEquals(1, b.getChildren().size()); + assertEqualsDependencyNode(c2, b.getChildren().get(0)); + } + + @Test + public void versionRangeClashAscOrderFullVerbose() throws RepositoryException { // A -> B -> C[1..2] // \--> C[1..2] DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); @@ -114,7 +131,7 @@ public void versionRangeClashAscOrderVerbose() throws RepositoryException { a.setChildren(mutableList(b, c1, c2)); b.setChildren(mutableList(c1, c2)); - DependencyNode ta = versionRangeClash(a, true); + DependencyNode ta = versionRangeClash(a, ConflictResolver.Verbosity.FULL); assertSame(a, ta); assertEquals(3, a.getChildren().size()); @@ -128,8 +145,6 @@ public void versionRangeClashAscOrderVerbose() throws RepositoryException { @Test public void versionRangeClashDescOrder() throws RepositoryException { - ConflictResolver resolver = makeDefaultResolver(); - // A -> B -> C[1..2] // \--> C[1..2] DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); @@ -139,7 +154,7 @@ public void versionRangeClashDescOrder() throws RepositoryException { a.setChildren(mutableList(b, c2, c1)); b.setChildren(mutableList(c2, c1)); - DependencyNode ta = versionRangeClash(a, false); + DependencyNode ta = versionRangeClash(a, ConflictResolver.Verbosity.NONE); assertSame(a, ta); assertEquals(2, a.getChildren().size()); @@ -149,9 +164,28 @@ public void versionRangeClashDescOrder() throws RepositoryException { } @Test - public void versionRangeClashDescOrderVerbose() throws RepositoryException { - ConflictResolver resolver = makeDefaultResolver(); + public void versionRangeClashDescOrderStandardVerbose() throws RepositoryException { + // A -> B -> C[1..2] + // \--> C[1..2] + DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); + DependencyNode b = makeDependencyNode("some-group", "b", "1.0"); + DependencyNode c1 = makeDependencyNode("some-group", "c", "1.0"); + DependencyNode c2 = makeDependencyNode("some-group", "c", "2.0"); + a.setChildren(mutableList(b, c2, c1)); + b.setChildren(mutableList(c2, c1)); + + DependencyNode ta = versionRangeClash(a, ConflictResolver.Verbosity.STANDARD); + + assertSame(a, ta); + assertEquals(2, a.getChildren().size()); + assertSame(b, a.getChildren().get(0)); + assertEqualsDependencyNode(c2, a.getChildren().get(1)); + assertEquals(1, b.getChildren().size()); + assertEqualsDependencyNode(c2, b.getChildren().get(0)); + } + @Test + public void versionRangeClashDescOrderFullVerbose() throws RepositoryException { // A -> B -> C[1..2] // \--> C[1..2] DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); @@ -161,7 +195,7 @@ public void versionRangeClashDescOrderVerbose() throws RepositoryException { a.setChildren(mutableList(b, c2, c1)); b.setChildren(mutableList(c2, c1)); - DependencyNode ta = versionRangeClash(a, true); + DependencyNode ta = versionRangeClash(a, ConflictResolver.Verbosity.FULL); assertSame(a, ta); assertEquals(3, a.getChildren().size()); @@ -175,8 +209,6 @@ public void versionRangeClashDescOrderVerbose() throws RepositoryException { @Test public void versionRangeClashMixedOrder() throws RepositoryException { - ConflictResolver resolver = makeDefaultResolver(); - // A -> B -> C[1..2] // \--> C[1..2] DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); @@ -186,7 +218,7 @@ public void versionRangeClashMixedOrder() throws RepositoryException { a.setChildren(mutableList(b, c2, c1)); b.setChildren(mutableList(c1, c2)); - DependencyNode ta = versionRangeClash(a, false); + DependencyNode ta = versionRangeClash(a, ConflictResolver.Verbosity.NONE); assertSame(a, ta); assertEquals(2, a.getChildren().size()); @@ -196,9 +228,28 @@ public void versionRangeClashMixedOrder() throws RepositoryException { } @Test - public void versionRangeClashMixedOrderVerbose() throws RepositoryException { - ConflictResolver resolver = makeDefaultResolver(); + public void versionRangeClashMixedOrderStandardVerbose() throws RepositoryException { + // A -> B -> C[1..2] + // \--> C[1..2] + DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); + DependencyNode b = makeDependencyNode("some-group", "b", "1.0"); + DependencyNode c1 = makeDependencyNode("some-group", "c", "1.0"); + DependencyNode c2 = makeDependencyNode("some-group", "c", "2.0"); + a.setChildren(mutableList(b, c2, c1)); + b.setChildren(mutableList(c1, c2)); + + DependencyNode ta = versionRangeClash(a, ConflictResolver.Verbosity.STANDARD); + assertSame(a, ta); + assertEquals(2, a.getChildren().size()); + assertSame(b, a.getChildren().get(0)); + assertEqualsDependencyNode(c2, a.getChildren().get(1)); + assertEquals(1, b.getChildren().size()); + assertEqualsDependencyNode(c2, b.getChildren().get(0)); + } + + @Test + public void versionRangeClashMixedOrderFullVerbose() throws RepositoryException { // A -> B -> C[1..2] // \--> C[1..2] DependencyNode a = makeDependencyNode("some-group", "a", "1.0"); @@ -208,7 +259,7 @@ public void versionRangeClashMixedOrderVerbose() throws RepositoryException { a.setChildren(mutableList(b, c2, c1)); b.setChildren(mutableList(c1, c2)); - DependencyNode ta = versionRangeClash(a, true); + DependencyNode ta = versionRangeClash(a, ConflictResolver.Verbosity.FULL); assertSame(a, ta); assertEquals(3, a.getChildren().size()); @@ -233,7 +284,8 @@ private void assertEqualsDependencyNode(DependencyNode node1, DependencyNode nod /** * Performs a verbose conflict resolution on passed in root. */ - private DependencyNode versionRangeClash(DependencyNode root, boolean verbose) throws RepositoryException { + private DependencyNode versionRangeClash(DependencyNode root, ConflictResolver.Verbosity verbosity) + throws RepositoryException { ConflictResolver resolver = makeDefaultResolver(); System.out.println(); @@ -241,7 +293,7 @@ private DependencyNode versionRangeClash(DependencyNode root, boolean verbose) t root.accept(new DependencyGraphDumper(System.out::println)); // TODO: remove DefaultRepositorySystemSession session = TestUtils.newSession(); - session.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, verbose); + session.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, verbosity); DependencyNode transformedRoot = resolver.transformGraph(root, TestUtils.newTransformationContext(session)); System.out.println(); From 8a15d2105f0670b75ce4e598d0a49f774811d50b Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 22 Mar 2023 20:56:07 +0100 Subject: [PATCH 09/13] Tidy up, comments --- .../graph/transformer/ConflictResolver.java | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java index fe31df529..aa839f6da 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java @@ -78,7 +78,7 @@ public enum Verbosity { /** * Verbosity level to be used in "analyze" resolving use cases (ie. dependency convergence calculations). The * {@link ConflictResolver} in this mode will remove any redundant collected nodes, in turn it will leave one - * with recorded conflicting information. This mode corresponds tp "classic verbose" mode when + * with recorded conflicting information. This mode corresponds to "classic verbose" mode when * {@link #CONFIG_PROP_VERBOSE} was set to {@code true}. Obviously, the resulting dependency tree is not * suitable for artifact resolution unless a filter is employed to exclude the duplicate dependencies. */ @@ -285,7 +285,7 @@ private static void removeLosers(State state) { String winnerId = ArtifactIdUtils.toId(winner.node.getArtifact()); List previousParent = null; ListIterator childIt = null; - ArrayList toRemoveIds = new ArrayList<>(); + HashSet toRemoveIds = new HashSet<>(); for (ConflictItem item : state.items) { if (item == winner) { continue; @@ -297,18 +297,34 @@ private static void removeLosers(State state) { while (childIt.hasNext()) { DependencyNode child = childIt.next(); if (child == item.node) { + // NONE: just remove it and done if (Verbosity.NONE == state.verbosity) { childIt.remove(); break; } + // STANDARD: doing extra bookkeeping to select "which nodes to remove" if (Verbosity.STANDARD == state.verbosity) { String childId = ArtifactIdUtils.toId(child.getArtifact()); + // if two IDs are equal, it means "there is nearest", not conflict per se. + // In that case we do NOT allow this child to be removed (but remove others) + // and this keeps us safe from iteration (and in general, version) ordering + // as we explicitly leave out ID that is "nearest found" state. + // + // This tackles version ranges mostly, where ranges are turned into list of + // several nodes in collector (as many were discovered, ie. from metadata), and + // old code would just "mark" the first hit as conflict, and remove the rest, + // even if rest could contain "more suitable" version, that is not conflicting/diverging. + // This resulted in verbose mode transformed tree, that was misrepresenting things + // for dependency convergence calculations: it represented state like parent node + // depends on "wrong" version (diverge), while "right" version was present (but removed) + // as well, as it was contained in parents version range. if (!Objects.equals(winnerId, childId)) { toRemoveIds.add(childId); } } + // FULL: just record the facts DependencyNode loser = new DefaultDependencyNode(child); loser.setData(NODE_DATA_WINNER, winner.node); loser.setData( From ca0d62e27d1062093444791881826af1a0c4cde8 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 22 Mar 2023 20:58:22 +0100 Subject: [PATCH 10/13] Do all this only if there is reason --- .../eclipse/aether/util/graph/transformer/ConflictResolver.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java index aa839f6da..e2c0b287c 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java @@ -341,7 +341,7 @@ private static void removeLosers(State state) { } // 2nd pass to apply "standard" verbosity: leaving only 1 loser, but with care - if (Verbosity.STANDARD == state.verbosity) { + if (Verbosity.STANDARD == state.verbosity && !toRemoveIds.isEmpty()) { for (ConflictItem item : state.items) { if (item == winner) { continue; From fa9fea0c108fe6f4b1afa21c93796fa6380653be Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 22 Mar 2023 22:40:38 +0100 Subject: [PATCH 11/13] Tidy more --- .../DependencyHierarchyWithRanges.java | 3 +- .../examples/GetDependencyHierarchy.java | 3 +- .../resolver/examples/GetDependencyTree.java | 3 +- .../maven/resolver/examples/util/Booter.java | 3 ++ .../util/ConsoleDependencyGraphDumper.java | 31 ------------------- .../graph/transformer/ConflictResolver.java | 2 +- .../transformer/ConflictResolverTest.java | 6 ++-- 7 files changed, 11 insertions(+), 40 deletions(-) delete mode 100644 maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/ConsoleDependencyGraphDumper.java diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java index b53892449..19820f59f 100644 --- a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java @@ -22,7 +22,6 @@ import java.util.Collections; import org.apache.maven.resolver.examples.util.Booter; -import org.apache.maven.resolver.examples.util.ConsoleDependencyGraphDumper; import org.eclipse.aether.DefaultRepositorySystemSession; import org.eclipse.aether.RepositorySystem; import org.eclipse.aether.artifact.Artifact; @@ -80,6 +79,6 @@ public static void main(String[] args) throws Exception { CollectResult collectResult = system.collectDependencies(session, collectRequest); - collectResult.getRoot().accept(new ConsoleDependencyGraphDumper()); + collectResult.getRoot().accept(Booter.DUMPER_SOUT); } } diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/GetDependencyHierarchy.java b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/GetDependencyHierarchy.java index 3d6441f51..3a4504d74 100644 --- a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/GetDependencyHierarchy.java +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/GetDependencyHierarchy.java @@ -19,7 +19,6 @@ package org.apache.maven.resolver.examples; import org.apache.maven.resolver.examples.util.Booter; -import org.apache.maven.resolver.examples.util.ConsoleDependencyGraphDumper; import org.eclipse.aether.DefaultRepositorySystemSession; import org.eclipse.aether.RepositorySystem; import org.eclipse.aether.artifact.Artifact; @@ -67,6 +66,6 @@ public static void main(String[] args) throws Exception { CollectResult collectResult = system.collectDependencies(session, collectRequest); - collectResult.getRoot().accept(new ConsoleDependencyGraphDumper()); + collectResult.getRoot().accept(Booter.DUMPER_SOUT); } } diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/GetDependencyTree.java b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/GetDependencyTree.java index e47129cb5..77042e40e 100644 --- a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/GetDependencyTree.java +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/GetDependencyTree.java @@ -19,7 +19,6 @@ package org.apache.maven.resolver.examples; import org.apache.maven.resolver.examples.util.Booter; -import org.apache.maven.resolver.examples.util.ConsoleDependencyGraphDumper; import org.eclipse.aether.RepositorySystem; import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.artifact.Artifact; @@ -54,6 +53,6 @@ public static void main(String[] args) throws Exception { CollectResult collectResult = system.collectDependencies(session, collectRequest); - collectResult.getRoot().accept(new ConsoleDependencyGraphDumper()); + collectResult.getRoot().accept(Booter.DUMPER_SOUT); } } diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/Booter.java b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/Booter.java index 93ade9c4f..2744650b6 100644 --- a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/Booter.java +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/Booter.java @@ -28,6 +28,7 @@ import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.repository.LocalRepository; import org.eclipse.aether.repository.RemoteRepository; +import org.eclipse.aether.util.graph.visitor.DependencyGraphDumper; /** * A helper to boot the repository system and a repository system session. @@ -39,6 +40,8 @@ public class Booter { public static final String SISU = "sisu"; + public static final DependencyGraphDumper DUMPER_SOUT = new DependencyGraphDumper(System.out::println); + public static String selectFactory(String[] args) { if (args == null || args.length == 0) { return SERVICE_LOCATOR; diff --git a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/ConsoleDependencyGraphDumper.java b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/ConsoleDependencyGraphDumper.java deleted file mode 100644 index 5e31c765c..000000000 --- a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/ConsoleDependencyGraphDumper.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * 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.resolver.examples.util; - -import org.eclipse.aether.util.graph.visitor.DependencyGraphDumper; - -/** - * A dependency visitor that dumps the graph to the console. - */ -public class ConsoleDependencyGraphDumper extends DependencyGraphDumper { - - public ConsoleDependencyGraphDumper() { - super(System.out::println); - } -} diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java index e2c0b287c..ba3867cb7 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java @@ -88,7 +88,7 @@ public enum Verbosity { * Verbosity level to be used in "analyze" resolving use cases (ie. dependency convergence calculations). The * {@link ConflictResolver} in this mode will not remove any collected node, in turn it will record on all * eliminated nodes the conflicting information. Obviously, the resulting dependency tree is not suitable - * * for artifact resolution unless a filter is employed to exclude the duplicate dependencies. + * for artifact resolution unless a filter is employed to exclude the duplicate dependencies. */ FULL } diff --git a/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java b/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java index 4d07c4757..209de9941 100644 --- a/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java +++ b/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java @@ -281,6 +281,8 @@ private void assertEqualsDependencyNode(DependencyNode node1, DependencyNode nod assertEquals(node1.getDependency().getOptional(), node2.getDependency().getOptional()); } + private static final DependencyGraphDumper DUMPER_SOUT = new DependencyGraphDumper(System.out::println); + /** * Performs a verbose conflict resolution on passed in root. */ @@ -290,7 +292,7 @@ private DependencyNode versionRangeClash(DependencyNode root, ConflictResolver.V System.out.println(); System.out.println("Input node:"); - root.accept(new DependencyGraphDumper(System.out::println)); // TODO: remove + root.accept(DUMPER_SOUT); DefaultRepositorySystemSession session = TestUtils.newSession(); session.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, verbosity); @@ -298,7 +300,7 @@ private DependencyNode versionRangeClash(DependencyNode root, ConflictResolver.V System.out.println(); System.out.println("Transformed node:"); - transformedRoot.accept(new DependencyGraphDumper(System.out::println)); + transformedRoot.accept(DUMPER_SOUT); return transformedRoot; } From c31c1844bb5cb236ccdd06694c0047c38e1fcc45 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 23 Mar 2023 10:01:43 +0100 Subject: [PATCH 12/13] Last fixes and UTs --- .../graph/transformer/ConflictResolver.java | 21 +++++++++------ .../transformer/ConflictResolverTest.java | 27 +++++++++++++++++++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java index ba3867cb7..084f2d666 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java @@ -282,7 +282,7 @@ private boolean gatherConflictItems(DependencyNode node, State state) throws Rep private static void removeLosers(State state) { ConflictItem winner = state.conflictCtx.winner; - String winnerId = ArtifactIdUtils.toId(winner.node.getArtifact()); + String winnerArtifactId = ArtifactIdUtils.toId(winner.node.getArtifact()); List previousParent = null; ListIterator childIt = null; HashSet toRemoveIds = new HashSet<>(); @@ -305,7 +305,7 @@ private static void removeLosers(State state) { // STANDARD: doing extra bookkeeping to select "which nodes to remove" if (Verbosity.STANDARD == state.verbosity) { - String childId = ArtifactIdUtils.toId(child.getArtifact()); + String childArtifactId = ArtifactIdUtils.toId(child.getArtifact()); // if two IDs are equal, it means "there is nearest", not conflict per se. // In that case we do NOT allow this child to be removed (but remove others) // and this keeps us safe from iteration (and in general, version) ordering @@ -319,8 +319,8 @@ private static void removeLosers(State state) { // for dependency convergence calculations: it represented state like parent node // depends on "wrong" version (diverge), while "right" version was present (but removed) // as well, as it was contained in parents version range. - if (!Objects.equals(winnerId, childId)) { - toRemoveIds.add(childId); + if (!Objects.equals(winnerArtifactId, childArtifactId)) { + toRemoveIds.add(childArtifactId); } } @@ -335,6 +335,7 @@ private static void removeLosers(State state) { loser.setScope(item.getScopes().iterator().next()); loser.setChildren(Collections.emptyList()); childIt.set(loser); + item.node = loser; break; } } @@ -342,6 +343,7 @@ private static void removeLosers(State state) { // 2nd pass to apply "standard" verbosity: leaving only 1 loser, but with care if (Verbosity.STANDARD == state.verbosity && !toRemoveIds.isEmpty()) { + previousParent = null; for (ConflictItem item : state.items) { if (item == winner) { continue; @@ -352,9 +354,11 @@ private static void removeLosers(State state) { } while (childIt.hasNext()) { DependencyNode child = childIt.next(); - String childId = ArtifactIdUtils.toId(child.getArtifact()); - if (toRemoveIds.contains(childId)) { - childIt.remove(); + if (child == item.node) { + String childArtifactId = ArtifactIdUtils.toId(child.getArtifact()); + if (toRemoveIds.contains(childArtifactId) && item.parent.size() > 1) { + childIt.remove(); + } break; } } @@ -827,7 +831,8 @@ public static final class ConflictItem { // only for debugging/toString() to help identify the parent node(s) final Artifact artifact; - final DependencyNode node; + // is mutable as removeLosers will mutate it (if Verbosity==STANDARD) + DependencyNode node; int depth; diff --git a/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java b/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java index 209de9941..92f2154cb 100644 --- a/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java +++ b/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java @@ -79,6 +79,33 @@ public void versionClash() throws RepositoryException { assertSame(baz1Node, fooNode.getChildren().get(1)); } + @Test + public void versionClashForkedStandardVerbose() throws RepositoryException { + + // root -> impl1 -> api:1 + // |----> impl2 -> api:2 + DependencyNode root = makeDependencyNode("some-group", "root", "1.0"); + DependencyNode impl1 = makeDependencyNode("some-group", "impl1", "1.0"); + DependencyNode impl2 = makeDependencyNode("some-group", "impl2", "1.0"); + DependencyNode api1 = makeDependencyNode("some-group", "api", "1.1"); + DependencyNode api2 = makeDependencyNode("some-group", "api", "1.0"); + + root.setChildren(mutableList(impl1, impl2)); + impl1.setChildren(mutableList(api1)); + impl2.setChildren(mutableList(api2)); + + DependencyNode transformedNode = versionRangeClash(root, ConflictResolver.Verbosity.STANDARD); + + assertSame(root, transformedNode); + assertEquals(2, root.getChildren().size()); + assertSame(impl1, root.getChildren().get(0)); + assertSame(impl2, root.getChildren().get(1)); + assertEquals(1, impl1.getChildren().size()); + assertSame(api1, impl1.getChildren().get(0)); + assertEquals(1, impl2.getChildren().size()); + assertEqualsDependencyNode(api2, impl2.getChildren().get(0)); + } + @Test public void versionRangeClashAscOrder() throws RepositoryException { // A -> B -> C[1..2] From fe764a2a4746de0546842a9906dccdb3117c63ba Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 23 Mar 2023 10:39:49 +0100 Subject: [PATCH 13/13] Emboss method meaning better and explain --- .../transformer/ConflictResolverTest.java | 54 +++++++++++-------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java b/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java index 92f2154cb..0486e6698 100644 --- a/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java +++ b/maven-resolver-util/src/test/java/org/eclipse/aether/util/graph/transformer/ConflictResolverTest.java @@ -35,6 +35,9 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -103,7 +106,7 @@ public void versionClashForkedStandardVerbose() throws RepositoryException { assertEquals(1, impl1.getChildren().size()); assertSame(api1, impl1.getChildren().get(0)); assertEquals(1, impl2.getChildren().size()); - assertEqualsDependencyNode(api2, impl2.getChildren().get(0)); + assertConflictedButSameAsOriginal(api2, impl2.getChildren().get(0)); } @Test @@ -142,9 +145,9 @@ public void versionRangeClashAscOrderStandardVerbose() throws RepositoryExceptio assertSame(a, ta); assertEquals(2, a.getChildren().size()); assertSame(b, a.getChildren().get(0)); - assertEqualsDependencyNode(c2, a.getChildren().get(1)); + assertSame(c2, a.getChildren().get(1)); assertEquals(1, b.getChildren().size()); - assertEqualsDependencyNode(c2, b.getChildren().get(0)); + assertConflictedButSameAsOriginal(c2, b.getChildren().get(0)); } @Test @@ -163,11 +166,11 @@ public void versionRangeClashAscOrderFullVerbose() throws RepositoryException { assertSame(a, ta); assertEquals(3, a.getChildren().size()); assertSame(b, a.getChildren().get(0)); - assertEqualsDependencyNode(c1, a.getChildren().get(1)); + assertConflictedButSameAsOriginal(c1, a.getChildren().get(1)); assertSame(c2, a.getChildren().get(2)); assertEquals(2, b.getChildren().size()); - assertEqualsDependencyNode(c1, b.getChildren().get(0)); - assertEqualsDependencyNode(c2, b.getChildren().get(1)); + assertConflictedButSameAsOriginal(c1, b.getChildren().get(0)); + assertConflictedButSameAsOriginal(c2, b.getChildren().get(1)); } @Test @@ -206,9 +209,9 @@ public void versionRangeClashDescOrderStandardVerbose() throws RepositoryExcepti assertSame(a, ta); assertEquals(2, a.getChildren().size()); assertSame(b, a.getChildren().get(0)); - assertEqualsDependencyNode(c2, a.getChildren().get(1)); + assertSame(c2, a.getChildren().get(1)); assertEquals(1, b.getChildren().size()); - assertEqualsDependencyNode(c2, b.getChildren().get(0)); + assertConflictedButSameAsOriginal(c2, b.getChildren().get(0)); } @Test @@ -228,10 +231,10 @@ public void versionRangeClashDescOrderFullVerbose() throws RepositoryException { assertEquals(3, a.getChildren().size()); assertSame(b, a.getChildren().get(0)); assertSame(c2, a.getChildren().get(1)); - assertEqualsDependencyNode(c1, a.getChildren().get(2)); + assertConflictedButSameAsOriginal(c1, a.getChildren().get(2)); assertEquals(2, b.getChildren().size()); - assertEqualsDependencyNode(c2, b.getChildren().get(0)); - assertEqualsDependencyNode(c1, b.getChildren().get(1)); + assertConflictedButSameAsOriginal(c2, b.getChildren().get(0)); + assertConflictedButSameAsOriginal(c1, b.getChildren().get(1)); } @Test @@ -270,9 +273,9 @@ public void versionRangeClashMixedOrderStandardVerbose() throws RepositoryExcept assertSame(a, ta); assertEquals(2, a.getChildren().size()); assertSame(b, a.getChildren().get(0)); - assertEqualsDependencyNode(c2, a.getChildren().get(1)); + assertSame(c2, a.getChildren().get(1)); assertEquals(1, b.getChildren().size()); - assertEqualsDependencyNode(c2, b.getChildren().get(0)); + assertConflictedButSameAsOriginal(c2, b.getChildren().get(0)); } @Test @@ -292,20 +295,27 @@ public void versionRangeClashMixedOrderFullVerbose() throws RepositoryException assertEquals(3, a.getChildren().size()); assertSame(b, a.getChildren().get(0)); assertSame(c2, a.getChildren().get(1)); - assertEqualsDependencyNode(c1, a.getChildren().get(2)); + assertConflictedButSameAsOriginal(c1, a.getChildren().get(2)); assertEquals(2, b.getChildren().size()); - assertEqualsDependencyNode(c1, b.getChildren().get(0)); - assertEqualsDependencyNode(c2, b.getChildren().get(1)); + assertConflictedButSameAsOriginal(c1, b.getChildren().get(0)); + assertConflictedButSameAsOriginal(c2, b.getChildren().get(1)); } /** - * Conflict resolution may replace {@link DependencyNode} instances with copies to keep them stateful on different - * levels of graph, hence here we merely assert that node IS what we expect. + * Conflict resolver in case of conflict replaces {@link DependencyNode} instances with copies to keep them + * stateful on different levels of graph and records conflict data. This method assert that two nodes do represent + * same dependency (same GAV, scope, optionality), but that original is not conflicted while current is. */ - private void assertEqualsDependencyNode(DependencyNode node1, DependencyNode node2) { - assertEquals(node1.getDependency().getArtifact(), node2.getDependency().getArtifact()); - assertEquals(node1.getDependency().getScope(), node2.getDependency().getScope()); - assertEquals(node1.getDependency().getOptional(), node2.getDependency().getOptional()); + private void assertConflictedButSameAsOriginal(DependencyNode original, DependencyNode current) { + assertNotSame(original, current); + assertEquals( + original.getDependency().getArtifact(), current.getDependency().getArtifact()); + assertEquals( + original.getDependency().getScope(), current.getDependency().getScope()); + assertEquals( + original.getDependency().getOptional(), current.getDependency().getOptional()); + assertNull(original.getData().get(ConflictResolver.NODE_DATA_WINNER)); + assertNotNull(current.getData().get(ConflictResolver.NODE_DATA_WINNER)); } private static final DependencyGraphDumper DUMPER_SOUT = new DependencyGraphDumper(System.out::println);