From 267d2883374ebea3143a8caa55580b0847a16814 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 24 Mar 2023 13:52:39 +0100 Subject: [PATCH] [MRESOLVER-345] Conflict resolution in verbose mode is sensitive to version ordering (#271) Our two collector implementations produce slightly different dirty trees when in transitive dependencies ranges are in use. The new BF produces (w/ and w/o skipper) produces this dirty tree: ``` 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 ``` The "old" DF produces this one: ``` menforcer426:aid:ext:1 compile +- menforcer426:bid:ext:1 compile | +- menforcer426:cid:ext:1 compile | +- menforcer426:cid:ext:2 compile | \- menforcer426:cid:ext:3 compile +- menforcer426:cid:ext:1 compile +- menforcer426:cid:ext:2 compile \- menforcer426:cid:ext:3 compile ``` Spot the difference: the two dirty trees are "semantically" (or content-wise?) equal/same, except for the artifact ordering, where there was a version range (collector in this case discovers available versions that "fits" range and created nodes for them, one for each version that lies within version constraint). DF collector relies and provides "metadata based" ordering (as metadata contained the versions), while BF explicitly sorts them in descending order (for internal optimization purposes). Point is, both dirty trees are ok. But, Conflict resolver in verbose mode for two inputs above produces different outputs. For DF with "divergence found", while for BF "no divergence found" (correctly). Overall changes in this PR: * most are test and test related resources * cosmetic changes like javadoc typos, adding override etc * added reusable `DependencyGraphDumper` * key changes are in `ConflictResolver` covered by `ConflictResolverTest` UT changes * overall fix is to make `ConflictResolver` insensitive to input dirty tree version ordering, make sure output is same for "semantically" (or content-wise?) same inputs. How tested this: * built/installed this PR * built maven-3.9.x with this resolver * ran maven IT suite -- OK * ran https://github.com/apache/maven-enforcer/pull/259 w/ built maven (so fixed resolver is used). This PR contains only one reproducer IT that fails with any released Maven version -- OK w/ maven built as above. Just realized how enforcer ITs are good source of inspiration for resolver use cases, so many if not all of new tests are actually inspired by enforcer ITs. --- https://issues.apache.org/jira/browse/MRESOLVER-345 --- .../resolver/examples/AllResolverDemos.java | 1 + .../DependencyHierarchyWithRanges.java | 84 ++++++ .../examples/GetDependencyHierarchy.java | 3 +- .../resolver/examples/GetDependencyTree.java | 3 +- .../resolver/examples/resolver/Resolver.java | 15 +- .../maven/resolver/examples/util/Booter.java | 3 + .../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 +++ ...ependencyCollectorDelegateTestSupport.java | 17 ++ .../collect/bf/BfDependencyCollectorTest.java | 11 +- .../collect/df/DfDependencyCollectorTest.java | 11 +- ...nsitiveDepsUseRangesDirtyTreeResult_BF.txt | 8 + ...nsitiveDepsUseRangesDirtyTreeResult_DF.txt | 8 + ...transitiveDepsUseRangesDirtyTree_aid_1.ini | 3 + ...transitiveDepsUseRangesDirtyTree_bid_1.ini | 2 + ...transitiveDepsUseRangesDirtyTree_cid_1.ini | 1 + ...transitiveDepsUseRangesDirtyTree_cid_2.ini | 1 + ...transitiveDepsUseRangesDirtyTree_cid_3.ini | 1 + .../graph/transformer/ConflictResolver.java | 164 ++++++++--- .../AbstractDepthFirstNodeListGenerator.java | 2 + .../visitor/CloningDependencyVisitor.java | 2 + .../graph/visitor/DependencyGraphDumper.java | 45 +-- .../visitor/FilteringDependencyVisitor.java | 4 +- .../PathRecordingDependencyVisitor.java | 2 + .../visitor/PostorderNodeListGenerator.java | 6 +- .../visitor/PreorderNodeListGenerator.java | 2 +- .../graph/visitor/TreeDependencyVisitor.java | 2 + .../transformer/ConflictResolverTest.java | 265 ++++++++++++++++++ 32 files changed, 780 insertions(+), 75 deletions(-) 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 create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult_BF.txt create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult_DF.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 create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_1.ini create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_2.ini create mode 100644 maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_3.ini rename maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/ConsoleDependencyGraphDumper.java => maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java (75%) 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..19820f59f --- /dev/null +++ b/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/DependencyHierarchyWithRanges.java @@ -0,0 +1,84 @@ +/* + * 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.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/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(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/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-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/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 + + 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..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 @@ -497,6 +497,23 @@ public void testDependencyManagement_DefaultDependencyManager() throws Dependenc assertEqualSubtree(expectedTree, toDependencyResult(result.getRoot(), "compile", null)); } + @Test + public void testTransitiveDepsUseRangesDirtyTree() throws DependencyCollectionException, IOException { + // 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)); + + CollectResult result = collector.collectDependencies(session, request); + 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 353030fdd..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 @@ -59,10 +59,13 @@ 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()); + } + + @Override + protected String getTransitiveDepsUseRangesDirtyTreeResource() { + return "transitiveDepsUseRangesDirtyTreeResult_BF.txt"; } 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..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 @@ -28,9 +28,12 @@ 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()); + } + + @Override + protected String getTransitiveDepsUseRangesDirtyTreeResource() { + return "transitiveDepsUseRangesDirtyTreeResult_DF.txt"; } } diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult_BF.txt b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult_BF.txt new file mode 100644 index 000000000..b1711a4bd --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTreeResult_BF.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/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-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/transitiveDepsUseRangesDirtyTree_cid_1.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_1.ini new file mode 100644 index 000000000..61a252c23 --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_1.ini @@ -0,0 +1 @@ +[dependencies] diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_2.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_2.ini new file mode 100644 index 000000000..61a252c23 --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_2.ini @@ -0,0 +1 @@ +[dependencies] diff --git a/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_3.ini b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_3.ini new file mode 100644 index 000000000..61a252c23 --- /dev/null +++ b/maven-resolver-impl/src/test/resources/artifact-descriptions/transitiveDepsUseRangesDirtyTree_cid_3.ini @@ -0,0 +1 @@ +[dependencies] 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..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 @@ -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 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. + */ + 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 winnerArtifactId = ArtifactIdUtils.toId(winner.node.getArtifact()); List previousParent = null; ListIterator childIt = null; - boolean conflictVisualized = false; + HashSet toRemoveIds = new HashSet<>(); for (ConflictItem item : state.items) { if (item == winner) { continue; @@ -244,30 +293,78 @@ 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 { + // 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 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 + // 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(winnerArtifactId, childArtifactId)) { + toRemoveIds.add(childArtifactId); + } } + + // FULL: just record the facts + 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); + item.node = loser; break; } } } + + // 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; + } + if (item.parent != previousParent) { + childIt = item.parent.listIterator(); + previousParent = item.parent; + } + while (childIt.hasNext()) { + DependencyNode child = childIt.next(); + if (child == item.node) { + String childArtifactId = ArtifactIdUtils.toId(child.getArtifact()); + if (toRemoveIds.contains(childArtifactId) && item.parent.size() > 1) { + 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 +458,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 +555,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); @@ -734,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/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-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/ConsoleDependencyGraphDumper.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java similarity index 75% rename from maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/ConsoleDependencyGraphDumper.java rename to maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java index e94ae2c32..60a12727f 100644 --- a/maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/util/ConsoleDependencyGraphDumper.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/visitor/DependencyGraphDumper.java @@ -16,12 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.maven.resolver.examples.util; +package org.eclipse.aether.util.graph.visitor; -import java.io.PrintStream; 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; @@ -31,25 +31,27 @@ 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 the console. + * 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 ConsoleDependencyGraphDumper implements DependencyVisitor { +public class DependencyGraphDumper implements DependencyVisitor { - private final PrintStream out; + private final Consumer consumer; private final List childInfos = new ArrayList<>(); - public ConsoleDependencyGraphDumper() { - this(null); - } - - public ConsoleDependencyGraphDumper(PrintStream out) { - this.out = (out != null) ? out : System.out; + public DependencyGraphDumper(Consumer consumer) { + this.consumer = requireNonNull(consumer); } + @Override public boolean visitEnter(DependencyNode node) { - out.println(formatIndentation() + formatNode(node)); + consumer.accept(formatIndentation() + formatNode(node)); childInfos.add(new ChildInfo(node.getChildren().size())); return true; } @@ -84,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 01b288000..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 @@ -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,9 +31,13 @@ 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; +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; @@ -77,6 +82,266 @@ 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()); + assertConflictedButSameAsOriginal(api2, impl2.getChildren().get(0)); + } + + @Test + public void versionRangeClashAscOrder() 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.NONE); + + 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 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)); + assertSame(c2, a.getChildren().get(1)); + assertEquals(1, b.getChildren().size()); + assertConflictedButSameAsOriginal(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"); + 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.FULL); + + assertSame(a, ta); + assertEquals(3, a.getChildren().size()); + assertSame(b, a.getChildren().get(0)); + assertConflictedButSameAsOriginal(c1, a.getChildren().get(1)); + assertSame(c2, a.getChildren().get(2)); + assertEquals(2, b.getChildren().size()); + assertConflictedButSameAsOriginal(c1, b.getChildren().get(0)); + assertConflictedButSameAsOriginal(c2, b.getChildren().get(1)); + } + + @Test + public void versionRangeClashDescOrder() 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.NONE); + + 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 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)); + assertSame(c2, a.getChildren().get(1)); + assertEquals(1, b.getChildren().size()); + assertConflictedButSameAsOriginal(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"); + 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.FULL); + + assertSame(a, ta); + assertEquals(3, a.getChildren().size()); + assertSame(b, a.getChildren().get(0)); + assertSame(c2, a.getChildren().get(1)); + assertConflictedButSameAsOriginal(c1, a.getChildren().get(2)); + assertEquals(2, b.getChildren().size()); + assertConflictedButSameAsOriginal(c2, b.getChildren().get(0)); + assertConflictedButSameAsOriginal(c1, b.getChildren().get(1)); + } + + @Test + public void versionRangeClashMixedOrder() 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.NONE); + + 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 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)); + assertSame(c2, a.getChildren().get(1)); + assertEquals(1, b.getChildren().size()); + assertConflictedButSameAsOriginal(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"); + 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.FULL); + + assertSame(a, ta); + assertEquals(3, a.getChildren().size()); + assertSame(b, a.getChildren().get(0)); + assertSame(c2, a.getChildren().get(1)); + assertConflictedButSameAsOriginal(c1, a.getChildren().get(2)); + assertEquals(2, b.getChildren().size()); + assertConflictedButSameAsOriginal(c1, b.getChildren().get(0)); + assertConflictedButSameAsOriginal(c2, b.getChildren().get(1)); + } + + /** + * 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 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); + + /** + * Performs a verbose conflict resolution on passed in root. + */ + private DependencyNode versionRangeClash(DependencyNode root, ConflictResolver.Verbosity verbosity) + throws RepositoryException { + ConflictResolver resolver = makeDefaultResolver(); + + System.out.println(); + System.out.println("Input node:"); + root.accept(DUMPER_SOUT); + + DefaultRepositorySystemSession session = TestUtils.newSession(); + session.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, verbosity); + DependencyNode transformedRoot = resolver.transformGraph(root, TestUtils.newTransformationContext(session)); + + System.out.println(); + System.out.println("Transformed node:"); + transformedRoot.accept(DUMPER_SOUT); + + return transformedRoot; + } + @Test public void derivedScopeChange() throws RepositoryException { ConflictResolver resolver = makeDefaultResolver();