diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java index 985106a0b..2c6fa43af 100644 --- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java +++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java @@ -22,6 +22,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -164,6 +165,14 @@ public DependencyNode transformGraph(DependencyNode node, DependencyGraphTransfo throw new RepositoryException("conflict groups have not been identified"); } + Map> cyclicPredecessors = new HashMap<>(); + for (Collection cycle : conflictIdCycles) { + for (String conflictId : cycle) { + Collection predecessors = cyclicPredecessors.computeIfAbsent(conflictId, k -> new HashSet<>()); + predecessors.addAll(cycle); + } + } + State state = new State( ConflictResolver.getVerbosity(context.getSession()), ConfigUtils.getBoolean( @@ -174,9 +183,9 @@ public DependencyNode transformGraph(DependencyNode node, DependencyGraphTransfo scopeSelector.getInstance(node, context), scopeDeriver.getInstance(node, context), optionalitySelector.getInstance(node, context), - conflictIds); - - state.build(node); + cyclicPredecessors, + conflictIds, + node); // loop over topographically sorted conflictIds for (String conflictId : sortedConflictIds) { @@ -277,6 +286,11 @@ private static class State { */ private final ConflictResolver.OptionalitySelector optionalitySelector; + /** + * The conflict predecessors by conflictId. + */ + private final Map> cyclicPredecessors; + /** * The node to conflictId mapping from {@link ConflictMarker}. */ @@ -293,6 +307,11 @@ private static class State { */ private final Map resolvedIds; + /** + * The root {@link Path}. + */ + private final Path root; + @SuppressWarnings("checkstyle:ParameterNumber") private State( ConflictResolver.Verbosity verbosity, @@ -301,16 +320,21 @@ private State( ConflictResolver.ScopeSelector scopeSelector, ConflictResolver.ScopeDeriver scopeDeriver, ConflictResolver.OptionalitySelector optionalitySelector, - Map conflictIds) { + Map> cyclicPredecessors, + Map conflictIds, + DependencyNode node) + throws RepositoryException { this.verbosity = verbosity; this.showCyclesInStandardVerbosity = showCyclesInStandardVerbosity; this.versionSelector = versionSelector; this.scopeSelector = scopeSelector; this.scopeDeriver = scopeDeriver; this.optionalitySelector = optionalitySelector; + this.cyclicPredecessors = cyclicPredecessors; this.conflictIds = conflictIds; this.partitions = new HashMap<>(); this.resolvedIds = new HashMap<>(); + this.root = build(node); } /** @@ -318,9 +342,10 @@ private State( * tree. As a side effect, {@link #partitions} are being filled up as well, that combined with topo * sorted conflictIds can serve as a starting to point to walk the graph. */ - private void build(DependencyNode node) throws RepositoryException { + private Path build(DependencyNode node) throws RepositoryException { Path root = new Path(this, node, null, false); gatherCRNodes(root); + return root; } /** @@ -332,9 +357,7 @@ private void gatherCRNodes(Path node) throws RepositoryException { // add children; we will get back those really added (not causing cycles) List added = node.addChildren(children); for (Path child : added) { - if (!child.cycle) { - gatherCRNodes(child); - } + gatherCRNodes(child); } } } @@ -615,13 +638,16 @@ private void moveOutOfScope() { private List addChildren(List children) throws RepositoryException { ArrayList added = new ArrayList<>(children.size()); for (DependencyNode child : children) { - String childConflictId = this.state.conflictIds.get(child); - boolean cycle = this.state.partitions.getOrDefault(childConflictId, Collections.emptyList()).stream() - .anyMatch(p -> p.dn.equals(child)); + boolean cycle = this.state + .cyclicPredecessors + .getOrDefault(this.state.conflictIds.get(child), Collections.emptySet()) + .contains(this.conflictId); Path c = new Path(this.state, child, this, cycle); this.children.add(c); c.derive(0, false); - added.add(c); + if (c.parent != null && !c.parent.cycle) { + added.add(c); + } } return added; } 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 9d8fca505..2f1b80130 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 @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Stream; import org.eclipse.aether.RepositoryException; @@ -28,10 +29,12 @@ import org.eclipse.aether.graph.DefaultDependencyNode; import org.eclipse.aether.graph.Dependency; import org.eclipse.aether.graph.DependencyNode; +import org.eclipse.aether.graph.DependencyVisitor; import org.eclipse.aether.internal.test.util.DependencyGraphParser; 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.eclipse.aether.util.graph.visitor.TreeDependencyVisitor; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -483,6 +486,8 @@ void winnerCycleRemoved(ConflictResolver conflictResolver) throws RepositoryExce System.out.println("CR=" + conflictResolver.getClass().getSimpleName() + "; verbosity=" + verbosity.name()); if (!cyclesLeftInPlace) { transformed.accept(DUMPER_SOUT); + } else { + transformed.accept(new TreeDependencyVisitor(DUMPER_SOUT)); } assertSame(transformed, root); @@ -522,13 +527,167 @@ void winnerCycleRemoved(ConflictResolver conflictResolver) throws RepositoryExce } } + @ParameterizedTest + @MethodSource("conflictResolverSource") + void subtreeRepeatedSameInstance(ConflictResolver conflictResolver) throws RepositoryException { + // root -> d1 ----> st1* -> st2 -> st3 + // |---> st1* --> st2 --> st3 + // |-----> st3 + // Note: st1* are TWO DIFFERENT instances, but st2->st3 are same instances on different paths + DependencyNode root = makeDependencyNode("some-group", "root", "1.0"); + DependencyNode d1 = makeDependencyNode("some-group", "d1", "1.0"); + DependencyNode st1a = makeDependencyNode("some-group", "st1", "1.0"); + DependencyNode st1b = makeDependencyNode("some-group", "st1", "1.0"); + DependencyNode st2 = makeDependencyNode("some-group", "st2", "1.0"); + DependencyNode st3 = makeDependencyNode("some-group", "st3", "1.0"); + + root.setChildren(mutableList(d1, st1b)); + d1.setChildren(mutableList(st1a)); + st1a.setChildren(mutableList(st2)); + st2.setChildren(mutableList(st3)); + st1b.setChildren(mutableList(st2, st3)); + + System.out.println(); + System.out.println("Input node:"); + root.accept(DUMPER_SOUT); + + DependencyNode transformedRoot = transform(conflictResolver, root); + + System.out.println(); + System.out.println("Transformed node:"); + transformedRoot.accept(DUMPER_SOUT); + + assertSame(transformedRoot, root); + AtomicInteger occurrenceOfSt3 = new AtomicInteger(0); + root.accept(new DependencyVisitor() { + @Override + public boolean visitEnter(DependencyNode node) { + if (node.getArtifact() != null + && node.getArtifact().getArtifactId().equals("st3")) { + occurrenceOfSt3.incrementAndGet(); + } + return true; + } + + @Override + public boolean visitLeave(DependencyNode node) { + return true; + } + }); + assertEquals(1, occurrenceOfSt3.get()); // st3 must be present only once in tree + } + + @ParameterizedTest + @MethodSource("conflictResolverSource") + void subtreeRepeatedSameInstanceWithRealCycle(ConflictResolver conflictResolver) throws RepositoryException { + // root -> d1 ----> st1* -> st2 -> st3 + // |---> st1* --> st2 --> st3* -> st2 + // |-----> st3 + // Note: st1* are TWO DIFFERENT instances, but st2->st3 are same instances on different paths + // Note: st3* are TWO DIFFERENT instances + DependencyNode root = makeDependencyNode("some-group", "root", "1.0"); + DependencyNode d1 = makeDependencyNode("some-group", "d1", "1.0"); + DependencyNode st1a = makeDependencyNode("some-group", "st1", "1.0"); + DependencyNode st1b = makeDependencyNode("some-group", "st1", "1.0"); + DependencyNode st2 = makeDependencyNode("some-group", "st2", "1.0"); + DependencyNode st3 = makeDependencyNode("some-group", "st3", "1.0"); + DependencyNode st3a = makeDependencyNode("some-group", "st3", "1.0"); + + root.setChildren(mutableList(d1, st1b)); + d1.setChildren(mutableList(st1a)); + st1a.setChildren(mutableList(st2)); + st2.setChildren(mutableList(st3a)); + st1b.setChildren(mutableList(st2, st3)); + st3a.setChildren(mutableList(st2)); + + System.out.println(); + System.out.println("Input node:"); + root.accept(new TreeDependencyVisitor(DUMPER_SOUT)); // we have a cycle, so must wrap it + + DependencyNode transformedRoot = transform(conflictResolver, root); + + System.out.println(); + System.out.println("Transformed node:"); + // transformedRoot.accept(DUMPER_SOUT); + + assertSame(transformedRoot, root); + AtomicInteger occurrenceOfSt3 = new AtomicInteger(0); + root.accept(new DependencyVisitor() { + @Override + public boolean visitEnter(DependencyNode node) { + if (node.getArtifact() != null + && node.getArtifact().getArtifactId().equals("st3")) { + occurrenceOfSt3.incrementAndGet(); + } + return true; + } + + @Override + public boolean visitLeave(DependencyNode node) { + return true; + } + }); + assertEquals(1, occurrenceOfSt3.get()); // st3 must be present only once in tree + } + + @ParameterizedTest + @MethodSource("conflictResolverSource") + void nettyBoringSslExample(ConflictResolver conflictResolver) throws RepositoryException { + // root -> nbc1 ---> nbc2 + // |---> nbc2 ---> nbc1 + DependencyNode root = makeDependencyNode("some-group", "root", "1.0"); + DependencyNode nbc1 = makeDependencyNode("some-group", "nbc", "1.0", "arch1", "compile"); + DependencyNode nbc2 = makeDependencyNode("some-group", "nbc", "1.0", "arch2", "compile"); + + root.setChildren(mutableList(nbc1, nbc2)); + nbc1.setChildren(mutableList(nbc2)); + nbc2.setChildren(mutableList(nbc1)); + + System.out.println(); + System.out.println("Input node:"); + root.accept(new TreeDependencyVisitor(DUMPER_SOUT)); // we have a cycle, so must wrap it + + DependencyNode transformedRoot = transform(conflictResolver, root); + + System.out.println(); + System.out.println("Transformed node:"); + transformedRoot.accept(DUMPER_SOUT); + + assertSame(transformedRoot, root); + AtomicInteger occurrence = new AtomicInteger(0); + root.accept(new DependencyVisitor() { + @Override + public boolean visitEnter(DependencyNode node) { + if (node.getArtifact() != null + && node.getArtifact().getArtifactId().equals("nbc")) { + occurrence.incrementAndGet(); + } + return true; + } + + @Override + public boolean visitLeave(DependencyNode node) { + return true; + } + }); + assertEquals(2, occurrence.get()); // st3 must be present only once in tree + } + private static DependencyNode makeDependencyNode(String groupId, String artifactId, String version) { return makeDependencyNode(groupId, artifactId, version, "compile"); } private static DependencyNode makeDependencyNode(String groupId, String artifactId, String version, String scope) { - DefaultDependencyNode node = new DefaultDependencyNode( - new Dependency(new DefaultArtifact(groupId + ':' + artifactId + ':' + version), scope)); + return makeDependencyNode(groupId, artifactId, version, null, "compile"); + } + + private static DependencyNode makeDependencyNode( + String groupId, String artifactId, String version, String classifier, String scope) { + DefaultDependencyNode node = (classifier != null && !classifier.isEmpty()) + ? new DefaultDependencyNode(new Dependency( + new DefaultArtifact(groupId + ':' + artifactId + ":jar:" + classifier + ":" + version), scope)) + : new DefaultDependencyNode( + new Dependency(new DefaultArtifact(groupId + ':' + artifactId + ':' + version), scope)); node.setVersion(new TestVersion(version)); node.setVersionConstraint(new TestVersionConstraint(node.getVersion())); return node;