Skip to content

Commit 652ae3d

Browse files
authored
Bug: Reproducer and fix for #1583 (#1585)
The new "path" CR in certain cases (cycle involving two different instances of `DependencyNode` as one was managed into same as other + parent of the cycle node has multiple/mixed children of cycling and non cycling + parent of the cycle head was the sole winner) left cycles in place even at verbosity levels of `NONE` and `STANDARD` when cycles should be removed. This is fixed. Another issue was that "path" CR was "too eager" by applying winner scope/optional to all nodes, while it should have apply it only to winner, as losers are marked as such (and possibly eliminated). Changes: * fixed the bug, also cleaned up `push(int)` method as it had some redundancy and misplaced comments * fixed the eager application of winner scope/optional -- only winner have those applied * dropped some copy-pasta constant remnants (they are all in super class) * improved `Verbosity` javadoc re loops * added UT that reproduces the issue covering all CRs and verbosity levels * added demo with this exact case; demos also run as part of `run-its`, so is an extra safety belt Fixes #1583
1 parent d0daebd commit 652ae3d

File tree

6 files changed

+234
-117
lines changed

6 files changed

+234
-117
lines changed

maven-resolver-demos/maven-resolver-demo-snippets/src/main/java/org/apache/maven/resolver/examples/GetDependencyHierarchyWithConflictsStrategies.java

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,16 +54,36 @@ public static void main(String[] args) throws Exception {
5454
System.out.println("------------------------------------------------------------");
5555
System.out.println(GetDependencyHierarchyWithConflictsStrategies.class.getSimpleName());
5656

57-
runItWithStrategy(args, ConfigurableVersionSelector.NEAREST_SELECTION_STRATEGY);
58-
runItWithStrategy(args, ConfigurableVersionSelector.HIGHEST_SELECTION_STRATEGY);
57+
CollectRequest collectRequest;
58+
59+
// okttp cleanly shows difference between nearest/closest
60+
collectRequest = new CollectRequest();
61+
collectRequest.setRootArtifact(new DefaultArtifact("demo:demo:1.0"));
62+
collectRequest.setDependencies(
63+
List.of(new Dependency(new DefaultArtifact("com.squareup.okhttp3:okhttp:jar:4.12.0"), "compile")));
64+
runItWithStrategy(args, ConfigurableVersionSelector.NEAREST_SELECTION_STRATEGY, collectRequest);
65+
runItWithStrategy(args, ConfigurableVersionSelector.HIGHEST_SELECTION_STRATEGY, collectRequest);
66+
67+
// MENFORCER-408 inspired
68+
collectRequest = new CollectRequest();
69+
collectRequest.setRootArtifact(new DefaultArtifact("demo:demo:1.0"));
70+
collectRequest.setDependencies(List.of(
71+
new Dependency(new DefaultArtifact("org.seleniumhq.selenium:selenium-java:jar:3.0.1"), "test")));
72+
collectRequest.setManagedDependencies(List.of(
73+
new Dependency(new DefaultArtifact("org.seleniumhq.selenium:selenium-java:jar:3.0.1"), "test"),
74+
new Dependency(new DefaultArtifact("org.seleniumhq.selenium:selenium-remote-driver:jar:3.0.1"), "test"),
75+
new Dependency(new DefaultArtifact("com.codeborne:phantomjsdriver:jar:1.3.0"), "test")));
76+
runItWithStrategy(args, ConfigurableVersionSelector.NEAREST_SELECTION_STRATEGY, collectRequest);
77+
runItWithStrategy(args, ConfigurableVersionSelector.HIGHEST_SELECTION_STRATEGY, collectRequest);
5978
}
6079

61-
private static void runItWithStrategy(String[] args, String selectionStrategy) throws Exception {
80+
private static void runItWithStrategy(String[] args, String selectionStrategy, CollectRequest collectRequest)
81+
throws Exception {
6282
System.out.println();
6383
System.out.println(selectionStrategy);
6484
try (RepositorySystem system = Booter.newRepositorySystem(Booter.selectFactory(args))) {
6585
SessionBuilder sessionBuilder = Booter.newRepositorySystemSession(system);
66-
sessionBuilder.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, ConflictResolver.Verbosity.FULL);
86+
sessionBuilder.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, ConflictResolver.Verbosity.STANDARD);
6787
sessionBuilder.setConfigProperty(DependencyManagerUtils.CONFIG_PROP_VERBOSE, true);
6888
sessionBuilder.setConfigProperty(
6989
ConfigurableVersionSelector.CONFIG_PROP_SELECTION_STRATEGY, selectionStrategy);
@@ -81,10 +101,6 @@ private static void runItWithStrategy(String[] args, String selectionStrategy) t
81101
.setTransferListener(null)
82102
.build()) {
83103

84-
CollectRequest collectRequest = new CollectRequest();
85-
collectRequest.setRootArtifact(new DefaultArtifact("demo:demo:1.0"));
86-
collectRequest.setDependencies(List.of(
87-
new Dependency(new DefaultArtifact("com.squareup.okhttp3:okhttp:jar:4.12.0"), "compile")));
88104
collectRequest.setRepositories(Booter.newRepositories(system, session));
89105

90106
CollectResult result = system.collectDependencies(session, collectRequest);

maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ClassicConflictResolver.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -806,16 +806,6 @@ private static final class ConflictItem extends ConflictResolver.ConflictItem {
806806
// bit field of OPTIONAL_FALSE and OPTIONAL_TRUE
807807
int optionalities;
808808

809-
/**
810-
* Bit flag indicating whether one or more paths consider the dependency non-optional.
811-
*/
812-
public static final int OPTIONAL_FALSE = 0x01;
813-
814-
/**
815-
* Bit flag indicating whether one or more paths consider the dependency optional.
816-
*/
817-
public static final int OPTIONAL_TRUE = 0x02;
818-
819809
private ConflictItem(DependencyNode parent, DependencyNode node, String scope, boolean optional) {
820810
if (parent != null) {
821811
this.parent = parent.getChildren();

maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -151,26 +151,31 @@ public class ConflictResolver implements DependencyGraphTransformer {
151151
*/
152152
public enum Verbosity {
153153
/**
154-
* Verbosity level to be used in all "common" resolving use cases (ie. dependencies to build class path). The
154+
* Verbosity level to be used in all "common" resolving use cases (ie dependencies to build class path). The
155155
* {@link ConflictResolver} in this mode will trim down the graph to the barest minimum: will not leave
156156
* any conflicting node in place, hence no conflicts will be present in transformed graph either.
157157
*/
158158
NONE,
159159

160160
/**
161-
* Verbosity level to be used in "analyze" resolving use cases (ie. dependency convergence calculations). The
162-
* {@link ConflictResolver} in this mode will remove any redundant collected nodes, in turn it will leave one
163-
* with recorded conflicting information. This mode corresponds to "classic verbose" mode when
161+
* Verbosity level to be used in "analyze" resolving use cases (ie dependency convergence calculations). The
162+
* {@link ConflictResolver} in this mode will remove any redundant collected nodes and cycles, in turn it will
163+
* leave one with recorded conflicting information. This mode corresponds to "classic verbose" mode when
164164
* {@link #CONFIG_PROP_VERBOSE} was set to {@code true}. Obviously, the resulting dependency tree is not
165165
* suitable for artifact resolution unless a filter is employed to exclude the duplicate dependencies.
166166
*/
167167
STANDARD,
168168

169169
/**
170-
* Verbosity level to be used in "analyze" resolving use cases (ie. dependency convergence calculations). The
171-
* {@link ConflictResolver} in this mode will not remove any collected node, in turn it will record on all
172-
* eliminated nodes the conflicting information. Obviously, the resulting dependency tree is not suitable
173-
* for artifact resolution unless a filter is employed to exclude the duplicate dependencies.
170+
* Verbosity level to be used in "analyze" resolving use cases (ie dependency convergence calculations). The
171+
* {@link ConflictResolver} in this mode will not remove any collected node nor cycle, in turn it will record
172+
* on all eliminated nodes the conflicting information. Obviously, the resulting dependency tree is not suitable
173+
* for artifact resolution unless a filter is employed to exclude the duplicate dependencies and possible cycles.
174+
* Because of left in cycles, user of this verbosity level should ensure that graph post-processing does not
175+
* contain elements that would explode on them. In other words, session should be modified with proper
176+
* graph transformers.
177+
*
178+
* @see RepositorySystemSession#getDependencyGraphTransformer()
174179
*/
175180
FULL
176181
}

0 commit comments

Comments
 (0)