From e837306d278843cfeb2d8e1581c4c593f990c708 Mon Sep 17 00:00:00 2001 From: Cai Date: Fri, 26 Nov 2021 01:10:15 -0800 Subject: [PATCH 1/3] skip & reconcile --- .../impl/collect/ConflictWinnerFinder.java | 255 ++++++++++++ .../internal/impl/collect/DataPool.java | 12 +- .../collect/DefaultDependencyCollector.java | 96 ++++- .../collect/DependencyResolveReconciler.java | 370 ++++++++++++++++++ .../internal/impl/collect/NodeStack.java | 12 + ...aultDependencyCollectorReconcilerTest.java | 95 +++++ .../DefaultDependencyCollectorTest.java | 24 +- .../DependencyResolveReconcilerTest.java | 157 ++++++++ 8 files changed, 992 insertions(+), 29 deletions(-) create mode 100644 maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java create mode 100644 maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java create mode 100644 maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorReconcilerTest.java create mode 100644 maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconcilerTest.java diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java new file mode 100644 index 000000000..36c269cc7 --- /dev/null +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java @@ -0,0 +1,255 @@ +package org.eclipse.aether.internal.impl.collect; + +/* + * 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. + */ + +import com.google.common.collect.LinkedHashMultimap; +import com.google.common.collect.Multimap; +import org.eclipse.aether.DefaultRepositorySystemSession; +import org.eclipse.aether.RepositoryException; +import org.eclipse.aether.RepositorySystemSession; +import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.collection.CollectResult; +import org.eclipse.aether.collection.DependencyCollectionException; +import org.eclipse.aether.collection.DependencyGraphTransformer; +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.transformer.ConflictResolver; +import org.eclipse.aether.util.graph.visitor.CloningDependencyVisitor; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.Collection; +import java.util.LinkedHashSet; +import java.util.Objects; + +import static org.eclipse.aether.internal.impl.collect.DependencyResolveReconciler.ga; +import static org.eclipse.aether.internal.impl.collect.DependencyResolveReconciler.gav; + +/** + * Find the conflict winner by transforming a cloned dependency graph + */ +public class ConflictWinnerFinder +{ + + private boolean verbose; + + private static final Logger LOGGER = LoggerFactory.getLogger( ConflictWinnerFinder.class ); + + public ConflictWinnerFinder( boolean verbose ) + { + this.verbose = verbose; + } + + /** + * Find artifacts that conflicts with the winner. Only artifacts resolved before winner are considered as + * conflicts. + * + * @return + */ + public Collection getVersionConflicts( RepositorySystemSession session, CollectResult res ) + throws DependencyCollectionException + { + LinkedHashSet conflicts = new LinkedHashSet<>(); + Multimap conflictMap = resolveConflictWinners( session, res ); + if ( conflictMap == null ) + { + return conflicts; + } + + for ( String key : conflictMap.keySet() ) + { + Collection col = conflictMap.get( key ); + if ( col != null && col.size() > 1 ) + { + Conflict[] array = col.toArray( new Conflict[0] ); + + //find winner + Artifact winner = null; + int index = 0; + for ( int i = 0; i < array.length; i++ ) + { + Conflict cur = array[i]; + if ( cur.conflictedWith == null ) + { + winner = cur.artifact; + index = i; + break; + } + } + + //find conflicts: resolved before winner && version not equals + for ( int i = 0; i < array.length; i++ ) + { + if ( i < index ) + { + if ( verbose ) + { + LOGGER.info( "Found dependency: {} that conflicts with: {} ", array[i], winner ); + } + conflicts.add( array[i] ); + } + } + } + } + + return conflicts; + } + + /** + * Use the ConflictResolver to find out all conflict winners based on a cloned dependency graph + * + * @param session + * @param res + * @return + * @throws DependencyCollectionException + */ + private Multimap resolveConflictWinners( RepositorySystemSession session, CollectResult res ) + throws DependencyCollectionException + { + long start = System.nanoTime(); + + DefaultRepositorySystemSession cloned = new DefaultRepositorySystemSession( session ); + // set as verbose so that the winner will be recorded in the DependencyNode.getData + cloned.setConfigProperty( ConflictResolver.CONFIG_PROP_VERBOSE, true ); + DependencyGraphTransformer transformer = session.getDependencyGraphTransformer(); + if ( transformer == null ) + { + return null; + } + + //clone graph + CloningDependencyVisitor vis = new CloningDependencyVisitor(); + DependencyNode root = res.getRoot(); + root.accept( vis ); + root = vis.getRootNode(); + + //this part copied from DefaultDependencyCollector + try + { + DefaultDependencyGraphTransformationContext context = + new DefaultDependencyGraphTransformationContext( cloned ); + root = transformer.transformGraph( root, context ); + } + catch ( RepositoryException e ) + { + res.addException( e ); + } + + if ( !res.getExceptions().isEmpty() ) + { + throw new DependencyCollectionException( res ); + } + + DependencyConflictWinnersVisitor conflicts = new DependencyConflictWinnersVisitor(); + root.accept( conflicts ); + + if ( verbose ) + { + LOGGER.info( "Finished to resolve conflict winners in : {} ", ( System.nanoTime() - start ) ); + } + return conflicts.conflictMap; + } + + + //find out all conflict winners + static class DependencyConflictWinnersVisitor + implements DependencyVisitor + { + /** + * The version conflicts of dependencies, conflicts were put to the map following the resolve sequence. ga -> + * conflict + */ + Multimap conflictMap = LinkedHashMultimap.create(); + + public boolean visitEnter( DependencyNode node ) + { + Artifact a = node.getArtifact(); + Conflict conflict = new Conflict( a ); + conflictMap.put( ga( a ), conflict ); + DependencyNode winner = (DependencyNode) node.getData().get( ConflictResolver.NODE_DATA_WINNER ); + if ( winner != null ) + { + if ( !ArtifactIdUtils.equalsId( a, winner.getArtifact() ) ) + { + conflict.conflictedWith = winner.getArtifact(); + } + } + return true; + } + + + public boolean visitLeave( DependencyNode node ) + { + return true; + } + } + + static class Conflict + { + Artifact artifact; + Artifact conflictedWith; + String gav; + String ga; + String version; + private final int hashCode; + + Conflict( Artifact artifact ) + { + this.artifact = artifact; + this.version = artifact.getVersion(); + this.ga = ga( artifact ); + this.gav = gav( artifact ); + + hashCode = Objects.hash( this.artifact ); + } + + @Override + public boolean equals( Object obj ) + { + if ( obj == this ) + { + return true; + } + else if ( !( obj instanceof Conflict ) ) + { + return false; + } + Conflict that = (Conflict) obj; + return Objects.equals( artifact, that.artifact ); + } + + @Override + public int hashCode() + { + return hashCode; + } + + @Override + public String toString() + { + return "{" + + "artifact=" + + artifact + + ( conflictedWith == null ? "" : ", conflicted with: " + conflictedWith ) + + '}'; + } + } +} diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java index 4a145558d..039f95b5b 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java @@ -362,17 +362,17 @@ public int hashCode() static final class GraphKey { - private final Artifact artifact; + final Artifact artifact; - private final List repositories; + final List repositories; - private final DependencySelector selector; + final DependencySelector selector; - private final DependencyManager manager; + final DependencyManager manager; - private final DependencyTraverser traverser; + final DependencyTraverser traverser; - private final VersionFilter filter; + final VersionFilter filter; private final int hashCode; diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java index a46499dfd..e8d4b0bd2 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java @@ -80,6 +80,17 @@ public class DefaultDependencyCollector implements DependencyCollector, Service { + /** + * The key in the repository session's {@link org.eclipse.aether.RepositorySystemSession#getConfigProperties() + * configuration properties} used to store a {@link Boolean} flag controlling the resolver's skip & reconcile mode. + */ + public static final String CONFIG_PROP_RESOLVER_MODE = "resolver.mode"; + + /** + * The key in the repository session's {@link org.eclipse.aether.RepositorySystemSession#getConfigProperties() + * configuration properties} used to store a {@link Boolean} flag controlling the resolver's verbose mode. + */ + public static final String CONFIG_PROP_RESOLVER_VERBOSE_MODE = "resolver.verbose"; private static final String CONFIG_PROP_MAX_EXCEPTIONS = "aether.dependencyCollector.maxExceptions"; @@ -97,6 +108,17 @@ public class DefaultDependencyCollector private VersionRangeResolver versionRangeResolver; + /** + * control the log related with DependencyResolveSkipper + */ + private boolean verbose; + + /** + * control the resolve mode that whether enable skip & reconcile + */ + private boolean skipMode; + + public DefaultDependencyCollector() { // enables default constructor @@ -147,6 +169,9 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle requireNonNull( request, "request cannot be null" ); session = optimizeSession( session ); + verbose = ConfigUtils.getBoolean( session, false, CONFIG_PROP_RESOLVER_VERBOSE_MODE ); + skipMode = ConfigUtils.getBoolean( session, true, CONFIG_PROP_RESOLVER_MODE ); + RequestTrace trace = RequestTrace.newChild( request.getTrace(), request ); CollectResult result = new CollectResult( request ); @@ -252,7 +277,8 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle DefaultVersionFilterContext versionContext = new DefaultVersionFilterContext( session ); - Args args = new Args( session, trace, pool, nodes, context, versionContext, request ); + Args args = new Args( session, trace, pool, nodes, new DependencyResolveReconciler( skipMode, verbose ), + context, versionContext, request ); Results results = new Results( result, session ); process( args, results, dependencies, repositories, @@ -261,6 +287,25 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle depTraverser != null ? depTraverser.deriveChildTraverser( context ) : null, verFilter != null ? verFilter.deriveChildFilter( context ) : null ); + //reconcile the skipped nodes + Collection reconcileNodes = + args.reconciler.getNodesToReconcile( session, result ); + for ( DependencyResolveReconciler.DependencyResolveSkip skip : reconcileNodes ) + { + Args newArgs = + new Args( session, trace, pool, new NodeStack(), args.reconciler, context, versionContext, + request ); + DataPool.GraphKey key = skip.graphKey; + List parents = skip.parentPathsOfCurrentNode; + for ( DependencyNode parent : parents ) + { + newArgs.nodes.push( parent ); + } + newArgs.nodes.push( skip.node ); + process( newArgs, results, skip.dependencies, key.repositories, key.selector, key.manager, + key.traverser, key.filter ); + } + errorPath = results.errorPath; } @@ -499,21 +544,39 @@ private void doRecurse( Args args, Results results, List repos Object key = args.pool.toKey( d.getArtifact(), childRepos, childSelector, childManager, childTraverser, childFilter ); - List children = args.pool.getChildren( key ); - if ( children == null ) + List parents = args.nodes.getParentNodes(); + int depth = parents.size() + 1; //the depth if pushed the child to stack + List cachedChildren = args.pool.getChildren( key ); + DependencyResolveReconciler.CacheResult result = args.reconciler.findCache( child, cachedChildren, depth ); + if ( result != null ) { - args.pool.putChildren( key, child.getChildren() ); - - args.nodes.push( child ); - - process( args, results, descriptorResult.getDependencies(), childRepos, childSelector, childManager, - childTraverser, childFilter ); - - args.nodes.pop(); + if ( result.candidateWithSameKey ) + { + child.setChildren( result.dependencyNodes ); + } + else if ( result.candidateWithLowerDepth ) + { + //No need to set the children as the result can be ignored (won't be picked up) + args.reconciler.addSkip( child, key, descriptorResult.getDependencies(), parents, + result.parentPathsOfCandidateLowerDepth ); + if ( verbose ) + { + LOGGER.info( "Skipped resolving artifact {} of depth {}", child.getArtifact(), depth ); + } + } } else { - child.setChildren( children ); + args.nodes.push( child ); + if ( verbose ) + { + LOGGER.info( "Resolving artifact {} of depth {}", child.getArtifact(), depth ); + } + process( args, results, descriptorResult.getDependencies(), childRepos, childSelector, childManager, + childTraverser, childFilter ); + args.pool.putChildren( key, child.getChildren() ); + args.reconciler.cacheChildrenWithDepth( child, parents ); + args.nodes.pop(); } } @@ -706,9 +769,13 @@ static class Args final CollectRequest request; + final DependencyResolveReconciler reconciler; + + @SuppressWarnings( "checkstyle:parameternumber" ) Args( RepositorySystemSession session, RequestTrace trace, DataPool pool, NodeStack nodes, - DefaultDependencyCollectionContext collectionContext, DefaultVersionFilterContext versionContext, - CollectRequest request ) + DependencyResolveReconciler reconciler, + DefaultDependencyCollectionContext collectionContext, DefaultVersionFilterContext versionContext, + CollectRequest request ) { this.session = session; this.request = request; @@ -717,6 +784,7 @@ static class Args this.trace = trace; this.pool = pool; this.nodes = nodes; + this.reconciler = reconciler; this.collectionContext = collectionContext; this.versionContext = versionContext; } diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java new file mode 100644 index 000000000..d06e4482b --- /dev/null +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java @@ -0,0 +1,370 @@ +package org.eclipse.aether.internal.impl.collect; + +/* + * 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. + */ + +import com.google.common.collect.LinkedHashMultimap; +import com.google.common.collect.Multimap; +import org.eclipse.aether.RepositorySystemSession; +import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.collection.CollectResult; +import org.eclipse.aether.collection.DependencyCollectionException; +import org.eclipse.aether.graph.Dependency; +import org.eclipse.aether.graph.DependencyNode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +/** + * Skip resolve if depth is deeper, then reconcile the result by resolving the version conflicts. + */ +public class DependencyResolveReconciler +{ + /** + * Cache maven dependency resolve result by depth gav -> result + */ + private Map nodesWithDepth = new HashMap<>( 256 ); + /** + * Track the nodes that have been skipped to resolve for later reconciling + */ + private LinkedHashSet skippedNodes = new LinkedHashSet<>(); + + private static final Logger LOGGER = LoggerFactory.getLogger( DependencyResolveReconciler.class ); + private boolean skipMode; + private boolean verbose; + + public DependencyResolveReconciler( boolean skip, boolean verbose ) + { + this.skipMode = skip; + this.verbose = verbose; + + if ( this.verbose ) + { + LOGGER.info( "Skip & reconcile mode is {} ", this.skipMode ? "on" : "off" ); + } + } + + public void cacheChildrenWithDepth( DependencyNode node, List parents ) + { + if ( !this.skipMode ) + { + return; + } + + int depth = parents.size() + 1; + Artifact artifact = node.getArtifact(); + List children = node.getChildren(); + nodesWithDepth.put( gav( artifact ), new DependencyResolveResult( children, parents, depth ) ); + } + + + public CacheResult findCache( DependencyNode current, List cacheByGraphKey, int depth ) + { + CacheResult cache = new CacheResult(); + //non skip mode + if ( !skipMode && cacheByGraphKey != null ) + { + cache.dependencyNodes = cacheByGraphKey; + cache.candidateWithSameKey = true; + return cache; + } + + //skip mode + DependencyResolveResult result = nodesWithDepth.get( gav( current.getArtifact() ) ); + if ( result != null ) + { + if ( result.depth <= depth ) + { + cache.dependencyNodes = result.children; + cache.candidateWithLowerDepth = true; + cache.parentPathsOfCandidateLowerDepth = result.parentPaths; + return cache; + } + } + + return null; + } + + /** + * Record the nodes has been skipped + * + * @param current current dependency node + * @param key the graph key + * @param dependencies children of current dependency node + * @param parents parents of current dependency node + * @param skippedBy parents of the node that the skipped node is reusing + */ + public void addSkip( DependencyNode current, Object key, List dependencies, + List parents, List skippedBy ) + { + if ( skipMode && dependencies != null && dependencies.size() > 0 ) + { + DependencyResolveSkip skip = + new DependencyResolveSkip( current, (DataPool.GraphKey) key, dependencies, parents, skippedBy, + parents.size() + 1 ); + skippedNodes.add( skip ); + } + } + + /** + * Find all skipped Nodes that need to be reconciled. + * + * @return + */ + public Collection getNodesToReconcile( RepositorySystemSession session, + CollectResult result ) + throws DependencyCollectionException + { + if ( !this.skipMode ) + { + return Collections.emptyList(); + } + + ConflictWinnerFinder winnerFinder = new ConflictWinnerFinder( verbose ); + Collection conflicts = winnerFinder.getVersionConflicts( session, result ); + if ( conflicts == null || conflicts.isEmpty() ) + { + return Collections.emptyList(); + } + + //find all nodes that contain conflict nodes. + Set conflictedNodes = new HashSet<>(); + conflictedNodes.addAll( findNodeHasConflictsInParentPaths( conflicts ) ); + + //Evict node from cache if the cached node is based on parent paths contains conflicting nodes + for ( String gav : conflictedNodes ) + { + nodesWithDepth.remove( gav ); + } + + //get the least set of nodes need to reconcile + Set filteredSkips = filterSkippedNodes( conflictedNodes ); + if ( filteredSkips.size() == 0 ) + { + return Collections.emptyList(); + } + + if ( verbose ) + { + LOGGER.info( "Skipped resolving {} nodes, and reconciled {} nodes to solve {} dependency conflicts.", + skippedNodes.size(), filteredSkips.size(), conflicts.size() ); + } + return filteredSkips; + } + + + private LinkedHashSet filterSkippedNodes( Set conflictedNodes ) + { + LinkedHashSet skips = new LinkedHashSet<>(); + for ( DependencyResolveSkip skip : skippedNodes ) + { + //SKIP: skipped node's GAV differs with the one evicted from cache + if ( !conflictedNodes.contains( gav( skip.node.getArtifact() ) ) ) + { + continue; + } + + //SKIP: the node is skipped as expected when the parent path includes all segments of skipped path + boolean selfContained = true; + for ( DependencyNode parentNode : skip.parentPathsOfCandidateLowerDepth ) + { + if ( !skip.parentPathsOfCurrentNode.contains( parentNode ) ) + { + selfContained = false; + break; + } + } + + if ( selfContained ) + { + continue; + } + + skips.add( skip ); + } + + //group nodes by artifact + Multimap reconcileNodes = LinkedHashMultimap.create(); + for ( DependencyResolveSkip skip : skips ) + { + reconcileNodes.put( skip.node.getArtifact(), skip ); + } + + //only reconcile the node with lowest depth + LinkedHashSet filteredSkips = new LinkedHashSet<>(); + for ( Artifact key : reconcileNodes.keySet() ) + { + Collection col = reconcileNodes.get( key ); + List list = new ArrayList<>(); + list.addAll( col ); + Collections.sort( list, new Comparator() + { + @Override + public int compare( DependencyResolveSkip o1, DependencyResolveSkip o2 ) + { + return o1.depth - o2.depth; + } + } ); + + if ( verbose ) + { + LOGGER.info( "Reconcile: {}", list.get( 0 ) ); + } + filteredSkips.add( list.get( 0 ) ); + } + return filteredSkips; + } + + + private List findNodeHasConflictsInParentPaths( Collection conflicts ) + { + Set keys = nodesWithDepth.keySet(); + Set conflictIds = new HashSet<>(); + for ( ConflictWinnerFinder.Conflict c : conflicts ) + { + conflictIds.add( c.gav ); + } + + List results = new ArrayList<>(); + for ( String key : keys ) + { + List parents = nodesWithDepth.get( key ).parentPaths; + for ( DependencyNode p : parents ) + { + if ( p.getArtifact() != null ) + { + if ( conflictIds.contains( gav( p.getArtifact() ) ) ) + { + results.add( key ); + break; + } + } + } + } + + return results; + } + + static String ga( Artifact artifact ) + { + return artifact.getGroupId() + ":" + artifact.getArtifactId(); + } + + static String gav( Artifact artifact ) + { + return ga( artifact ) + ":" + artifact.getVersion(); + } + + + static class DependencyResolveSkip + { + int depth; + DependencyNode node; + DataPool.GraphKey graphKey; + List dependencies; + List parentPathsOfCurrentNode; + List parentPathsOfCandidateLowerDepth; + private final int hashCode; + + DependencyResolveSkip( DependencyNode node, DataPool.GraphKey graphKey, List dependencies, + List parents, List skippedBy, int depth ) + { + this.node = node; + this.graphKey = graphKey; + this.dependencies = dependencies; + this.parentPathsOfCurrentNode = parents; + this.parentPathsOfCandidateLowerDepth = skippedBy; + this.depth = depth; + hashCode = Objects.hash( this.node ); + } + + @Override + public boolean equals( Object o ) + { + if ( this == o ) + { + return true; + } + if ( o == null || getClass() != o.getClass() ) + { + return false; + } + DependencyResolveSkip that = (DependencyResolveSkip) o; + return Objects.equals( node, that.node ); + } + + @Override + public int hashCode() + { + return hashCode; + } + + @Override + public String toString() + { + return "{" + + "node=" + + node.getArtifact() + + ", parentPathsOfCandidate=" + + parentPathsOfCandidateLowerDepth + + ", parentPathsOfCurrentNode=" + + parentPathsOfCurrentNode + + ", depth=" + + depth + + '}'; + } + } + + + static final class DependencyResolveResult + { + private final List children; + private final List parentPaths; + private int depth; + + DependencyResolveResult( List children, List parents, int depth ) + { + this.children = children; + this.parentPaths = parents; + this.depth = depth; + } + } + + + static class CacheResult + { + List dependencyNodes; + List parentPathsOfCandidateLowerDepth; + boolean candidateWithSameKey; + boolean candidateWithLowerDepth; + } + + +} diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/NodeStack.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/NodeStack.java index 668dbc4b0..6b3f6544d 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/NodeStack.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/NodeStack.java @@ -19,7 +19,9 @@ * under the License. */ +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import org.eclipse.aether.artifact.Artifact; import org.eclipse.aether.graph.DependencyNode; @@ -66,6 +68,16 @@ public void pop() size--; } + public List getParentNodes() + { + List parents = new ArrayList<>(); + for ( int i = size - 1; i >= 0; i-- ) + { + parents.add( nodes[i] ); + } + return parents; + } + public int find( Artifact artifact ) { for ( int i = size - 1; i >= 0; i-- ) diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorReconcilerTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorReconcilerTest.java new file mode 100644 index 000000000..c9be5a5b0 --- /dev/null +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorReconcilerTest.java @@ -0,0 +1,95 @@ +package org.eclipse.aether.internal.impl.collect; +/* + * 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. + */ + +import org.eclipse.aether.artifact.DefaultArtifact; +import org.eclipse.aether.collection.CollectRequest; +import org.eclipse.aether.collection.CollectResult; +import org.eclipse.aether.collection.DependencyCollectionException; +import org.eclipse.aether.graph.Dependency; +import org.eclipse.aether.graph.Exclusion; +import org.eclipse.aether.internal.test.util.DependencyGraphParser; +import org.eclipse.aether.util.graph.manager.TransitiveDependencyManager; +import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector; +import org.junit.Ignore; +import org.junit.Test; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.assertEquals; + +public class DefaultDependencyCollectorReconcilerTest extends DefaultDependencyCollectorTest +{ + + @Override + public void setup() + { + super.setupCollector( true ); + } + + @Override + @Ignore + public void testCyclicDependencies() throws Exception + { + //do nothing + } + + private Dependency newDep( String coords, String scope, Collection exclusions ) + { + Dependency d = new Dependency( new DefaultArtifact( coords ), scope ); + return d.setExclusions( exclusions ); + } + + + @Test + /** + * Skip won't break exclusions. + */ + public void testSkipAndReconcileWithDifferentExclusion() throws DependencyCollectionException + { + collector.setArtifactDescriptorReader( newReader( "managed/" ) ); + parser = new DependencyGraphParser( "artifact-descriptions/managed/" ); + session.setDependencyManager( new TransitiveDependencyManager() ); + + ExclusionDependencySelector exclSel1 = new ExclusionDependencySelector(); + session.setDependencySelector( exclSel1 ); + + Dependency root1 = newDep( "gid:root:ext:ver", "compile", + Collections.singleton( new Exclusion( "gid", "transitive-1", "", "ext" ) ) ); + Dependency root2 = newDep( "gid:root:ext:ver", "compile", + Collections.singleton( new Exclusion( "gid", "transitive-2", "", "ext" ) ) ); + List dependencies = Arrays.asList( root1, root2 ); + CollectRequest request = new CollectRequest( dependencies, null, Arrays.asList( repository ) ); + request.addManagedDependency( newDep( "gid:direct:ext:managed-by-dominant-request" ) ); + request.addManagedDependency( newDep( "gid:transitive-1:ext:managed-by-root" ) ); + CollectResult result = collector.collectDependencies( session, request ); + assertEquals( 0, result.getExceptions().size() ); + assertEquals( 2, result.getRoot().getChildren().size() ); + assertEquals( root1, dep( result.getRoot(), 0 ) ); + //transitive-1 not excluded + assertEquals( 1, path( result.getRoot(), 0 ).getChildren().size() ); + //has transitive-2 excluded + assertEquals( 0, path( result.getRoot(), 1 ).getChildren().size() ); + assertEquals( root2, dep( result.getRoot(), 1 ) ); + } + +} diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java index 5d4c2aa01..a7932cc02 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java @@ -78,33 +78,39 @@ public class DefaultDependencyCollectorTest { - private DefaultDependencyCollector collector; + protected DefaultDependencyCollector collector; - private DefaultRepositorySystemSession session; + protected DefaultRepositorySystemSession session; - private DependencyGraphParser parser; + protected DependencyGraphParser parser; - private RemoteRepository repository; + protected RemoteRepository repository; - private IniArtifactDescriptorReader newReader( String prefix ) + protected IniArtifactDescriptorReader newReader( String prefix ) { return new IniArtifactDescriptorReader( "artifact-descriptions/" + prefix ); } - private Dependency newDep( String coords ) + protected Dependency newDep( String coords ) { return newDep( coords, "" ); } - private Dependency newDep( String coords, String scope ) + protected Dependency newDep( String coords, String scope ) { return new Dependency( new DefaultArtifact( coords ), scope ); } @Before public void setup() + { + setupCollector(false); + } + + public void setupCollector(boolean skipAndReconcile) { session = TestUtils.newSession(); + session.setConfigProperty(DefaultDependencyCollector.CONFIG_PROP_RESOLVER_MODE, skipAndReconcile); collector = new DefaultDependencyCollector(); collector.setArtifactDescriptorReader( newReader( "" ) ); @@ -154,12 +160,12 @@ private static void assertEqualSubtree( DependencyNode expected, DependencyNode parents.removeLast(); } - private Dependency dep( DependencyNode root, int... coords ) + protected Dependency dep( DependencyNode root, int... coords ) { return path( root, coords ).getDependency(); } - private DependencyNode path( DependencyNode root, int... coords ) + protected DependencyNode path( DependencyNode root, int... coords ) { try { diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconcilerTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconcilerTest.java new file mode 100644 index 000000000..79060bd45 --- /dev/null +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconcilerTest.java @@ -0,0 +1,157 @@ +package org.eclipse.aether.internal.impl.collect; +/* + * 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. + */ + +import org.eclipse.aether.DefaultRepositorySystemSession; +import org.eclipse.aether.RepositoryException; +import org.eclipse.aether.artifact.DefaultArtifact; +import org.eclipse.aether.collection.CollectRequest; +import org.eclipse.aether.collection.CollectResult; +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.internal.impl.StubRemoteRepositoryManager; +import org.eclipse.aether.internal.impl.StubVersionRangeResolver; +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.transformer.*; +import org.junit.Before; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +import static org.junit.Assert.assertEquals; + +/** + * Test with skip & reconcile mode + */ +public class DependencyResolveReconcilerTest +{ + protected DefaultDependencyCollector collector; + + protected DefaultRepositorySystemSession session; + + protected Dependency newDep( String coords ) + { + return newDep( coords, "" ); + } + + protected Dependency newDep( String coords, String scope ) + { + return new Dependency( new DefaultArtifact( coords ), scope ); + } + + protected DependencyGraphTransformer newTransformer() + { + return new ConflictResolver( new NearestVersionSelector(), new JavaScopeSelector(), + new SimpleOptionalitySelector(), new JavaScopeDeriver() ); + } + + private static DependencyNode makeDependencyNode( String groupId, String artifactId, String version ) + { + return makeDependencyNode( groupId, artifactId, version, "compile" ); + } + + private static List mutableList( DependencyNode... nodes ) + { + return new ArrayList<>( Arrays.asList( nodes ) ); + } + + private static DependencyNode makeDependencyNode( String groupId, String artifactId, String version, String scope ) + { + DefaultDependencyNode node = new DefaultDependencyNode( + new Dependency( new DefaultArtifact( groupId + ':' + artifactId + ':' + version ), scope ) + ); + node.setVersion( new TestVersion( version ) ); + node.setVersionConstraint( new TestVersionConstraint( node.getVersion() ) ); + return node; + } + + @Before + public void setup() + { + session = TestUtils.newSession(); + session.setDependencyGraphTransformer( newTransformer() ); + + collector = new DefaultDependencyCollector(); + collector.setVersionRangeResolver( new StubVersionRangeResolver() ); + collector.setRemoteRepositoryManager( new StubRemoteRepositoryManager() ); + } + + + @Test + public void testReconcile() throws RepositoryException + { + // A -> B -> C 3.0 -> D -> Z => 1) D of C3.0 is resolved + // |--> E -> F --> G -> D -> Z => 2) The D here is skipped as the depth is deeper and reuse the result in step 1 + // |--> C 2.0 -> H => maven picks C 2.0, so the D of C3.0 in step 1 is no longer valid, need reconcile the skipped D node in step 2 + DependencyNode aNode = makeDependencyNode( "some-group", "A", "1.0" ); + DependencyNode bNode = makeDependencyNode( "some-group", "B", "1.0" ); + DependencyNode c3Node = makeDependencyNode( "some-group", "C", "3.0" ); + DependencyNode dNode = makeDependencyNode( "some-group", "D", "1.0" ); + DependencyNode eNode = makeDependencyNode( "some-group", "E", "1.0" ); + DependencyNode fNode = makeDependencyNode( "some-group", "F", "1.0" ); + DependencyNode c2Node = makeDependencyNode( "some-group", "C", "2.0" ); + DependencyNode gNode = makeDependencyNode( "some-group", "G", "1.0" ); + DependencyNode hNode = makeDependencyNode( "some-group", "H", "1.0" ); + DependencyNode zNode = makeDependencyNode( "some-group", "Z", "1.0" ); + aNode.setChildren( mutableList( bNode, eNode, c2Node ) ); + bNode.setChildren( mutableList( c3Node ) ); + c3Node.setChildren( mutableList( dNode ) ); + dNode.setChildren( mutableList( zNode ) ); + eNode.setChildren( mutableList( fNode ) ); + fNode.setChildren( mutableList( gNode ) ); + gNode.setChildren( mutableList( dNode ) ); + c2Node.setChildren( mutableList( hNode ) ); + + CollectRequest request = new CollectRequest(); + request.addDependency( new Dependency( aNode.getArtifact(), "compile" ) ); + CollectResult result = new CollectResult( request ); + result.setRoot( aNode ); + + //follow the resolve sequence + DependencyResolveReconciler reconciler = new DependencyResolveReconciler( true, true ); + reconciler.cacheChildrenWithDepth( aNode, new ArrayList<>() ); + reconciler.cacheChildrenWithDepth( bNode, mutableList( aNode ) ); + reconciler.cacheChildrenWithDepth( c3Node, mutableList( aNode, bNode ) ); + reconciler.cacheChildrenWithDepth( dNode, mutableList( aNode, bNode, c3Node ) ); + reconciler.cacheChildrenWithDepth( eNode, mutableList( aNode ) ); + reconciler.cacheChildrenWithDepth( fNode, mutableList( aNode, eNode ) ); + reconciler.cacheChildrenWithDepth( gNode, mutableList( aNode, eNode, fNode ) ); + reconciler.addSkip( dNode, new DataPool.GraphKey( dNode.getArtifact(), null, null, null, null, null ), + Arrays.asList( newDep( "some-group:Z:ext:1.0" ) ), mutableList( aNode, eNode, fNode, gNode ), + mutableList( aNode, bNode, c3Node ) ); + reconciler.cacheChildrenWithDepth( c2Node, mutableList( aNode ) ); + + Collection skips = + reconciler.getNodesToReconcile( session, result ); + assertEquals( skips.size(), 1 ); + + DependencyResolveReconciler.DependencyResolveSkip skip = + skips.toArray( new DependencyResolveReconciler.DependencyResolveSkip[0] )[0]; + assertEquals( skip.depth, 5 ); + assertEquals( skip.node.getArtifact().getArtifactId(), "D" ); + assertEquals( skip.parentPathsOfCandidateLowerDepth, mutableList( aNode, bNode, c3Node ) ); + } +} From 27e30e0fffe85b937947843a2171906d547e54e4 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sat, 27 Nov 2021 20:12:40 +0100 Subject: [PATCH 2/3] Proposed changes --- .../impl/collect/ConflictWinnerFinder.java | 42 +++-- .../collect/DefaultDependencyCollector.java | 150 +++++++++++------- .../collect/DependencyResolveReconciler.java | 79 +++------ .../DefaultDependencyCollectorTest.java | 2 +- .../DependencyResolveReconcilerTest.java | 2 +- 5 files changed, 130 insertions(+), 145 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java index 36c269cc7..f5a0cfeef 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java @@ -19,8 +19,6 @@ * under the License. */ -import com.google.common.collect.LinkedHashMultimap; -import com.google.common.collect.Multimap; import org.eclipse.aether.DefaultRepositorySystemSession; import org.eclipse.aether.RepositoryException; import org.eclipse.aether.RepositorySystemSession; @@ -36,7 +34,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.LinkedHashSet; import java.util.Objects; @@ -49,26 +49,19 @@ public class ConflictWinnerFinder { - private boolean verbose; - private static final Logger LOGGER = LoggerFactory.getLogger( ConflictWinnerFinder.class ); - public ConflictWinnerFinder( boolean verbose ) - { - this.verbose = verbose; - } - /** * Find artifacts that conflicts with the winner. Only artifacts resolved before winner are considered as * conflicts. * * @return */ - public Collection getVersionConflicts( RepositorySystemSession session, CollectResult res ) + Collection getVersionConflicts( RepositorySystemSession session, CollectResult result ) throws DependencyCollectionException { LinkedHashSet conflicts = new LinkedHashSet<>(); - Multimap conflictMap = resolveConflictWinners( session, res ); + HashMap> conflictMap = resolveConflictWinners( session, result ); if ( conflictMap == null ) { return conflicts; @@ -77,7 +70,7 @@ public Collection getVersionConflicts( RepositorySystemSession session for ( String key : conflictMap.keySet() ) { Collection col = conflictMap.get( key ); - if ( col != null && col.size() > 1 ) + if ( col != null && col.size() > 1 ) // more than ONE { Conflict[] array = col.toArray( new Conflict[0] ); @@ -100,9 +93,9 @@ public Collection getVersionConflicts( RepositorySystemSession session { if ( i < index ) { - if ( verbose ) + if ( LOGGER.isDebugEnabled() ) { - LOGGER.info( "Found dependency: {} that conflicts with: {} ", array[i], winner ); + LOGGER.debug( "Found dependency: {} that conflicts with: {} ", array[i], winner ); } conflicts.add( array[i] ); } @@ -117,11 +110,12 @@ public Collection getVersionConflicts( RepositorySystemSession session * Use the ConflictResolver to find out all conflict winners based on a cloned dependency graph * * @param session - * @param res + * @param result * @return * @throws DependencyCollectionException */ - private Multimap resolveConflictWinners( RepositorySystemSession session, CollectResult res ) + private HashMap> resolveConflictWinners( RepositorySystemSession session, + CollectResult result ) throws DependencyCollectionException { long start = System.nanoTime(); @@ -137,7 +131,7 @@ private Multimap resolveConflictWinners( RepositorySystemSessi //clone graph CloningDependencyVisitor vis = new CloningDependencyVisitor(); - DependencyNode root = res.getRoot(); + DependencyNode root = result.getRoot(); root.accept( vis ); root = vis.getRootNode(); @@ -150,20 +144,20 @@ private Multimap resolveConflictWinners( RepositorySystemSessi } catch ( RepositoryException e ) { - res.addException( e ); + result.addException( e ); } - if ( !res.getExceptions().isEmpty() ) + if ( !result.getExceptions().isEmpty() ) { - throw new DependencyCollectionException( res ); + throw new DependencyCollectionException( result ); } DependencyConflictWinnersVisitor conflicts = new DependencyConflictWinnersVisitor(); root.accept( conflicts ); - if ( verbose ) + if ( LOGGER.isDebugEnabled() ) { - LOGGER.info( "Finished to resolve conflict winners in : {} ", ( System.nanoTime() - start ) ); + LOGGER.debug( "Finished to resolve conflict winners in : {} ", ( System.nanoTime() - start ) ); } return conflicts.conflictMap; } @@ -177,13 +171,13 @@ static class DependencyConflictWinnersVisitor * The version conflicts of dependencies, conflicts were put to the map following the resolve sequence. ga -> * conflict */ - Multimap conflictMap = LinkedHashMultimap.create(); + final HashMap> conflictMap = new HashMap<>(); public boolean visitEnter( DependencyNode node ) { Artifact a = node.getArtifact(); Conflict conflict = new Conflict( a ); - conflictMap.put( ga( a ), conflict ); + conflictMap.computeIfAbsent( ga( a ), k -> new ArrayList<>() ).add( conflict ); DependencyNode winner = (DependencyNode) node.getData().get( ConflictResolver.NODE_DATA_WINNER ); if ( winner != null ) { diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java index e8d4b0bd2..d960a4c30 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java @@ -83,14 +83,17 @@ public class DefaultDependencyCollector /** * The key in the repository session's {@link org.eclipse.aether.RepositorySystemSession#getConfigProperties() * configuration properties} used to store a {@link Boolean} flag controlling the resolver's skip & reconcile mode. + * + * @since 1.7.3 */ - public static final String CONFIG_PROP_RESOLVER_MODE = "resolver.mode"; + public static final String CONFIG_PROP_USE_SKIP_RECONCILE = "aether.dependencyCollector.useSkipReconcile"; /** - * The key in the repository session's {@link org.eclipse.aether.RepositorySystemSession#getConfigProperties() - * configuration properties} used to store a {@link Boolean} flag controlling the resolver's verbose mode. + * The default value for {@link #CONFIG_PROP_USE_SKIP_RECONCILE}, {@code true}. + * + * @since 1.7.3 */ - public static final String CONFIG_PROP_RESOLVER_VERBOSE_MODE = "resolver.verbose"; + public static final boolean CONFIG_PROP_USE_SKIP_RECONCILE_DEFAULT = true; private static final String CONFIG_PROP_MAX_EXCEPTIONS = "aether.dependencyCollector.maxExceptions"; @@ -108,17 +111,6 @@ public class DefaultDependencyCollector private VersionRangeResolver versionRangeResolver; - /** - * control the log related with DependencyResolveSkipper - */ - private boolean verbose; - - /** - * control the resolve mode that whether enable skip & reconcile - */ - private boolean skipMode; - - public DefaultDependencyCollector() { // enables default constructor @@ -169,8 +161,13 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle requireNonNull( request, "request cannot be null" ); session = optimizeSession( session ); - verbose = ConfigUtils.getBoolean( session, false, CONFIG_PROP_RESOLVER_VERBOSE_MODE ); - skipMode = ConfigUtils.getBoolean( session, true, CONFIG_PROP_RESOLVER_MODE ); + boolean useSkipReconcile = ConfigUtils.getBoolean( + session, CONFIG_PROP_USE_SKIP_RECONCILE_DEFAULT, CONFIG_PROP_USE_SKIP_RECONCILE + ); + if ( useSkipReconcile ) + { + LOGGER.debug( "Collector skip & reconcile enabled." ); + } RequestTrace trace = RequestTrace.newChild( request.getTrace(), request ); @@ -277,7 +274,9 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle DefaultVersionFilterContext versionContext = new DefaultVersionFilterContext( session ); - Args args = new Args( session, trace, pool, nodes, new DependencyResolveReconciler( skipMode, verbose ), + DependencyResolveReconciler dependencyResolveReconciler = + useSkipReconcile ? new DependencyResolveReconciler() : null; + Args args = new Args( session, trace, pool, nodes, dependencyResolveReconciler, context, versionContext, request ); Results results = new Results( result, session ); @@ -287,23 +286,26 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle depTraverser != null ? depTraverser.deriveChildTraverser( context ) : null, verFilter != null ? verFilter.deriveChildFilter( context ) : null ); - //reconcile the skipped nodes - Collection reconcileNodes = - args.reconciler.getNodesToReconcile( session, result ); - for ( DependencyResolveReconciler.DependencyResolveSkip skip : reconcileNodes ) + if ( args.reconciler != null ) { - Args newArgs = - new Args( session, trace, pool, new NodeStack(), args.reconciler, context, versionContext, - request ); - DataPool.GraphKey key = skip.graphKey; - List parents = skip.parentPathsOfCurrentNode; - for ( DependencyNode parent : parents ) + //reconcile the skipped nodes + Collection reconcileNodes = + args.reconciler.getNodesToReconcile( session, result ); + for ( DependencyResolveReconciler.DependencyResolveSkip skip : reconcileNodes ) { - newArgs.nodes.push( parent ); + Args newArgs = + new Args( session, trace, pool, new NodeStack(), args.reconciler, context, versionContext, + request ); + DataPool.GraphKey key = skip.graphKey; + List parents = skip.parentPathsOfCurrentNode; + for ( DependencyNode parent : parents ) + { + newArgs.nodes.push( parent ); + } + newArgs.nodes.push( skip.node ); + process( newArgs, results, skip.dependencies, key.repositories, key.selector, key.manager, + key.traverser, key.filter ); } - newArgs.nodes.push( skip.node ); - process( newArgs, results, skip.dependencies, key.repositories, key.selector, key.manager, - key.traverser, key.filter ); } errorPath = results.errorPath; @@ -536,47 +538,70 @@ private void doRecurse( Args args, Results results, List repos VersionFilter childFilter = verFilter != null ? verFilter.deriveChildFilter( context ) : null; final List childRepos = - args.ignoreRepos - ? repositories - : remoteRepositoryManager.aggregateRepositories( args.session, repositories, - descriptorResult.getRepositories(), true ); + args.ignoreRepos + ? repositories + : remoteRepositoryManager.aggregateRepositories( args.session, repositories, + descriptorResult.getRepositories(), true ); - Object key = - args.pool.toKey( d.getArtifact(), childRepos, childSelector, childManager, childTraverser, childFilter ); + Object key = args.pool.toKey( + d.getArtifact(), childRepos, childSelector, childManager, childTraverser, childFilter + ); - List parents = args.nodes.getParentNodes(); - int depth = parents.size() + 1; //the depth if pushed the child to stack - List cachedChildren = args.pool.getChildren( key ); - DependencyResolveReconciler.CacheResult result = args.reconciler.findCache( child, cachedChildren, depth ); - if ( result != null ) + if ( args.reconciler == null ) { - if ( result.candidateWithSameKey ) + List children = args.pool.getChildren( key ); + if ( children == null ) { - child.setChildren( result.dependencyNodes ); + args.pool.putChildren( key, child.getChildren() ); + + args.nodes.push( child ); + + process( args, results, descriptorResult.getDependencies(), childRepos, childSelector, childManager, + childTraverser, childFilter ); + + args.nodes.pop(); } - else if ( result.candidateWithLowerDepth ) + else { - //No need to set the children as the result can be ignored (won't be picked up) - args.reconciler.addSkip( child, key, descriptorResult.getDependencies(), parents, - result.parentPathsOfCandidateLowerDepth ); - if ( verbose ) - { - LOGGER.info( "Skipped resolving artifact {} of depth {}", child.getArtifact(), depth ); - } + child.setChildren( children ); } } else { - args.nodes.push( child ); - if ( verbose ) + List parents = args.nodes.getParentNodes(); + int depth = parents.size() + 1; //the depth if pushed the child to stack + List cachedChildren = args.pool.getChildren( key ); + DependencyResolveReconciler.CacheResult result = args.reconciler.findCache( child, cachedChildren, depth ); + if ( result != null ) { - LOGGER.info( "Resolving artifact {} of depth {}", child.getArtifact(), depth ); + if ( result.candidateWithSameKey ) + { + child.setChildren( result.dependencyNodes ); + } + else if ( result.candidateWithLowerDepth ) + { + //No need to set the children as the result can be ignored (won't be picked up) + args.reconciler.addSkip( child, key, descriptorResult.getDependencies(), parents, + result.parentPathsOfCandidateLowerDepth ); + if ( LOGGER.isDebugEnabled() ) + { + LOGGER.debug( "Skipped resolving artifact {} of depth {}", child.getArtifact(), depth ); + } + } + } + else + { + args.nodes.push( child ); + if ( LOGGER.isDebugEnabled() ) + { + LOGGER.debug( "Resolving artifact {} of depth {}", child.getArtifact(), depth ); + } + process( args, results, descriptorResult.getDependencies(), childRepos, childSelector, childManager, + childTraverser, childFilter ); + args.pool.putChildren( key, child.getChildren() ); + args.reconciler.cacheChildrenWithDepth( child, parents ); + args.nodes.pop(); } - process( args, results, descriptorResult.getDependencies(), childRepos, childSelector, childManager, - childTraverser, childFilter ); - args.pool.putChildren( key, child.getChildren() ); - args.reconciler.cacheChildrenWithDepth( child, parents ); - args.nodes.pop(); } } @@ -769,6 +794,9 @@ static class Args final CollectRequest request; + /** + * Nullable: is {@code null} if {@link #CONFIG_PROP_USE_SKIP_RECONCILE} evaluates to {@code false}. + */ final DependencyResolveReconciler reconciler; @SuppressWarnings( "checkstyle:parameternumber" ) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java index d06e4482b..35da8e6a5 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java @@ -19,8 +19,6 @@ * under the License. */ -import com.google.common.collect.LinkedHashMultimap; -import com.google.common.collect.Multimap; import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.artifact.Artifact; import org.eclipse.aether.collection.CollectResult; @@ -38,7 +36,6 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Set; @@ -47,37 +44,27 @@ */ public class DependencyResolveReconciler { + + private static final Logger LOGGER = LoggerFactory.getLogger( DependencyResolveReconciler.class ); + /** * Cache maven dependency resolve result by depth gav -> result */ - private Map nodesWithDepth = new HashMap<>( 256 ); + private final HashMap nodesWithDepth; + /** * Track the nodes that have been skipped to resolve for later reconciling */ - private LinkedHashSet skippedNodes = new LinkedHashSet<>(); - - private static final Logger LOGGER = LoggerFactory.getLogger( DependencyResolveReconciler.class ); - private boolean skipMode; - private boolean verbose; + private final LinkedHashSet skippedNodes; - public DependencyResolveReconciler( boolean skip, boolean verbose ) + public DependencyResolveReconciler() { - this.skipMode = skip; - this.verbose = verbose; - - if ( this.verbose ) - { - LOGGER.info( "Skip & reconcile mode is {} ", this.skipMode ? "on" : "off" ); - } + this.nodesWithDepth = new HashMap<>( 256 ); + this.skippedNodes = new LinkedHashSet<>(); } public void cacheChildrenWithDepth( DependencyNode node, List parents ) { - if ( !this.skipMode ) - { - return; - } - int depth = parents.size() + 1; Artifact artifact = node.getArtifact(); List children = node.getChildren(); @@ -87,21 +74,12 @@ public void cacheChildrenWithDepth( DependencyNode node, List pa public CacheResult findCache( DependencyNode current, List cacheByGraphKey, int depth ) { - CacheResult cache = new CacheResult(); - //non skip mode - if ( !skipMode && cacheByGraphKey != null ) - { - cache.dependencyNodes = cacheByGraphKey; - cache.candidateWithSameKey = true; - return cache; - } - - //skip mode DependencyResolveResult result = nodesWithDepth.get( gav( current.getArtifact() ) ); if ( result != null ) { if ( result.depth <= depth ) { + CacheResult cache = new CacheResult(); cache.dependencyNodes = result.children; cache.candidateWithLowerDepth = true; cache.parentPathsOfCandidateLowerDepth = result.parentPaths; @@ -124,7 +102,7 @@ public CacheResult findCache( DependencyNode current, List cache public void addSkip( DependencyNode current, Object key, List dependencies, List parents, List skippedBy ) { - if ( skipMode && dependencies != null && dependencies.size() > 0 ) + if ( dependencies != null && dependencies.size() > 0 ) { DependencyResolveSkip skip = new DependencyResolveSkip( current, (DataPool.GraphKey) key, dependencies, parents, skippedBy, @@ -142,12 +120,7 @@ public Collection getNodesToReconcile( RepositorySystemSe CollectResult result ) throws DependencyCollectionException { - if ( !this.skipMode ) - { - return Collections.emptyList(); - } - - ConflictWinnerFinder winnerFinder = new ConflictWinnerFinder( verbose ); + ConflictWinnerFinder winnerFinder = new ConflictWinnerFinder(); Collection conflicts = winnerFinder.getVersionConflicts( session, result ); if ( conflicts == null || conflicts.isEmpty() ) { @@ -155,8 +128,7 @@ public Collection getNodesToReconcile( RepositorySystemSe } //find all nodes that contain conflict nodes. - Set conflictedNodes = new HashSet<>(); - conflictedNodes.addAll( findNodeHasConflictsInParentPaths( conflicts ) ); + HashSet conflictedNodes = new HashSet<>( findNodeHasConflictsInParentPaths( conflicts ) ); //Evict node from cache if the cached node is based on parent paths contains conflicting nodes for ( String gav : conflictedNodes ) @@ -171,9 +143,9 @@ public Collection getNodesToReconcile( RepositorySystemSe return Collections.emptyList(); } - if ( verbose ) + if ( LOGGER.isDebugEnabled() ) { - LOGGER.info( "Skipped resolving {} nodes, and reconciled {} nodes to solve {} dependency conflicts.", + LOGGER.debug( "Skipped resolving {} nodes, and reconciled {} nodes to solve {} dependency conflicts.", skippedNodes.size(), filteredSkips.size(), conflicts.size() ); } return filteredSkips; @@ -211,10 +183,10 @@ private LinkedHashSet filterSkippedNodes( Set con } //group nodes by artifact - Multimap reconcileNodes = LinkedHashMultimap.create(); + HashMap> reconcileNodes = new HashMap<>(); for ( DependencyResolveSkip skip : skips ) { - reconcileNodes.put( skip.node.getArtifact(), skip ); + reconcileNodes.computeIfAbsent( skip.node.getArtifact(), k -> new ArrayList<>() ).add( skip ); } //only reconcile the node with lowest depth @@ -222,20 +194,11 @@ private LinkedHashSet filterSkippedNodes( Set con for ( Artifact key : reconcileNodes.keySet() ) { Collection col = reconcileNodes.get( key ); - List list = new ArrayList<>(); - list.addAll( col ); - Collections.sort( list, new Comparator() - { - @Override - public int compare( DependencyResolveSkip o1, DependencyResolveSkip o2 ) - { - return o1.depth - o2.depth; - } - } ); - - if ( verbose ) + List list = new ArrayList<>( col ); + list.sort( Comparator.comparingInt( o -> o.depth ) ); + if ( LOGGER.isDebugEnabled() ) { - LOGGER.info( "Reconcile: {}", list.get( 0 ) ); + LOGGER.debug( "Reconcile: {}", list.get( 0 ) ); } filteredSkips.add( list.get( 0 ) ); } diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java index a7932cc02..9145ee83a 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java @@ -110,7 +110,7 @@ public void setup() public void setupCollector(boolean skipAndReconcile) { session = TestUtils.newSession(); - session.setConfigProperty(DefaultDependencyCollector.CONFIG_PROP_RESOLVER_MODE, skipAndReconcile); + session.setConfigProperty(DefaultDependencyCollector.CONFIG_PROP_USE_SKIP_RECONCILE, skipAndReconcile); collector = new DefaultDependencyCollector(); collector.setArtifactDescriptorReader( newReader( "" ) ); diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconcilerTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconcilerTest.java index 79060bd45..c3de72d06 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconcilerTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconcilerTest.java @@ -131,7 +131,7 @@ public void testReconcile() throws RepositoryException result.setRoot( aNode ); //follow the resolve sequence - DependencyResolveReconciler reconciler = new DependencyResolveReconciler( true, true ); + DependencyResolveReconciler reconciler = new DependencyResolveReconciler(); reconciler.cacheChildrenWithDepth( aNode, new ArrayList<>() ); reconciler.cacheChildrenWithDepth( bNode, mutableList( aNode ) ); reconciler.cacheChildrenWithDepth( c3Node, mutableList( aNode, bNode ) ); From 836b94b65a87e32a2b8fb5298b62b8ed238608ca Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 28 Nov 2021 22:28:37 +0100 Subject: [PATCH 3/3] Remove guards around logger --- .../internal/impl/collect/ConflictWinnerFinder.java | 10 ++-------- .../impl/collect/DefaultDependencyCollector.java | 10 ++-------- .../impl/collect/DependencyResolveReconciler.java | 12 +++--------- 3 files changed, 7 insertions(+), 25 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java index f5a0cfeef..bcdd022f6 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java @@ -93,10 +93,7 @@ Collection getVersionConflicts( RepositorySystemSession session, Colle { if ( i < index ) { - if ( LOGGER.isDebugEnabled() ) - { - LOGGER.debug( "Found dependency: {} that conflicts with: {} ", array[i], winner ); - } + LOGGER.debug( "Found dependency: {} that conflicts with: {} ", array[i], winner ); conflicts.add( array[i] ); } } @@ -155,10 +152,7 @@ private HashMap> resolveConflictWinners( RepositoryS DependencyConflictWinnersVisitor conflicts = new DependencyConflictWinnersVisitor(); root.accept( conflicts ); - if ( LOGGER.isDebugEnabled() ) - { - LOGGER.debug( "Finished to resolve conflict winners in : {} ", ( System.nanoTime() - start ) ); - } + LOGGER.debug( "Finished to resolve conflict winners in : {} ", ( System.nanoTime() - start ) ); return conflicts.conflictMap; } diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java index d960a4c30..8645d66e1 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java @@ -583,19 +583,13 @@ else if ( result.candidateWithLowerDepth ) //No need to set the children as the result can be ignored (won't be picked up) args.reconciler.addSkip( child, key, descriptorResult.getDependencies(), parents, result.parentPathsOfCandidateLowerDepth ); - if ( LOGGER.isDebugEnabled() ) - { - LOGGER.debug( "Skipped resolving artifact {} of depth {}", child.getArtifact(), depth ); - } + LOGGER.debug( "Skipped resolving artifact {} of depth {}", child.getArtifact(), depth ); } } else { args.nodes.push( child ); - if ( LOGGER.isDebugEnabled() ) - { - LOGGER.debug( "Resolving artifact {} of depth {}", child.getArtifact(), depth ); - } + LOGGER.debug( "Resolving artifact {} of depth {}", child.getArtifact(), depth ); process( args, results, descriptorResult.getDependencies(), childRepos, childSelector, childManager, childTraverser, childFilter ); args.pool.putChildren( key, child.getChildren() ); diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java index 35da8e6a5..a930109e5 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java @@ -143,11 +143,8 @@ public Collection getNodesToReconcile( RepositorySystemSe return Collections.emptyList(); } - if ( LOGGER.isDebugEnabled() ) - { - LOGGER.debug( "Skipped resolving {} nodes, and reconciled {} nodes to solve {} dependency conflicts.", - skippedNodes.size(), filteredSkips.size(), conflicts.size() ); - } + LOGGER.debug( "Skipped resolving {} nodes, and reconciled {} nodes to solve {} dependency conflicts.", + skippedNodes.size(), filteredSkips.size(), conflicts.size() ); return filteredSkips; } @@ -196,10 +193,7 @@ private LinkedHashSet filterSkippedNodes( Set con Collection col = reconcileNodes.get( key ); List list = new ArrayList<>( col ); list.sort( Comparator.comparingInt( o -> o.depth ) ); - if ( LOGGER.isDebugEnabled() ) - { - LOGGER.debug( "Reconcile: {}", list.get( 0 ) ); - } + LOGGER.debug( "Reconcile: {}", list.get( 0 ) ); filteredSkips.add( list.get( 0 ) ); } return filteredSkips;