Skip to content

Commit

Permalink
Merge pull request quarkusio#45106 from aloubyansky/graph-conflict-re…
Browse files Browse the repository at this point in the history
…solution

More efficient dependency tree conflict resolver
  • Loading branch information
gsmet authored Dec 16, 2024
2 parents 3a0d44f + e0f2509 commit 109cb5d
Show file tree
Hide file tree
Showing 5 changed files with 262 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@
import java.util.HashSet;
import java.util.Set;

import org.eclipse.aether.util.artifact.JavaScopes;

import io.quarkus.bootstrap.model.ApplicationModel;
import io.quarkus.bootstrap.resolver.TsArtifact;
import io.quarkus.bootstrap.resolver.TsDependency;
import io.quarkus.bootstrap.resolver.TsQuarkusExt;
import io.quarkus.maven.dependency.ArtifactCoords;
import io.quarkus.maven.dependency.ArtifactDependency;
import io.quarkus.maven.dependency.Dependency;
import io.quarkus.maven.dependency.DependencyFlags;
import io.quarkus.maven.dependency.GACTV;

public class OptionalDepsTest extends BootstrapFromOriginalJarTestBase {

Expand Down Expand Up @@ -70,17 +72,73 @@ protected TsArtifact composeApplication() {
@Override
protected void assertAppModel(ApplicationModel model) throws Exception {
final Set<Dependency> expected = new HashSet<>();
expected.add(new ArtifactDependency(new GACTV("io.quarkus.bootstrap.test", "ext-a-deployment", "1"), "compile",

expected.add(new ArtifactDependency(
ArtifactCoords.jar(TsArtifact.DEFAULT_GROUP_ID, "ext-a", TsArtifact.DEFAULT_VERSION),
JavaScopes.COMPILE,
DependencyFlags.DIRECT,
DependencyFlags.OPTIONAL,
DependencyFlags.RUNTIME_EXTENSION_ARTIFACT,
DependencyFlags.TOP_LEVEL_RUNTIME_EXTENSION_ARTIFACT,
DependencyFlags.RUNTIME_CP,
DependencyFlags.DEPLOYMENT_CP));

expected.add(new ArtifactDependency(
ArtifactCoords.jar(TsArtifact.DEFAULT_GROUP_ID, "ext-a-dep", TsArtifact.DEFAULT_VERSION),
JavaScopes.COMPILE,
DependencyFlags.OPTIONAL,
DependencyFlags.RUNTIME_CP,
DependencyFlags.DEPLOYMENT_CP));

expected.add(new ArtifactDependency(
ArtifactCoords.jar(TsArtifact.DEFAULT_GROUP_ID, "ext-a-deployment", TsArtifact.DEFAULT_VERSION),
JavaScopes.COMPILE,
DependencyFlags.OPTIONAL,
DependencyFlags.DEPLOYMENT_CP));

expected.add(new ArtifactDependency(
ArtifactCoords.jar(TsArtifact.DEFAULT_GROUP_ID, "app-optional-dep", TsArtifact.DEFAULT_VERSION),
JavaScopes.COMPILE,
DependencyFlags.OPTIONAL,
DependencyFlags.DIRECT,
DependencyFlags.RUNTIME_CP,
DependencyFlags.DEPLOYMENT_CP));
expected.add(new ArtifactDependency(new GACTV("io.quarkus.bootstrap.test", "ext-b-deployment-dep", "1"), "compile",

expected.add(new ArtifactDependency(
ArtifactCoords.jar(TsArtifact.DEFAULT_GROUP_ID, "ext-b", TsArtifact.DEFAULT_VERSION),
JavaScopes.COMPILE,
DependencyFlags.OPTIONAL,
DependencyFlags.RUNTIME_EXTENSION_ARTIFACT,
DependencyFlags.TOP_LEVEL_RUNTIME_EXTENSION_ARTIFACT,
DependencyFlags.RUNTIME_CP,
DependencyFlags.DEPLOYMENT_CP));

expected.add(new ArtifactDependency(ArtifactCoords.jar(
TsArtifact.DEFAULT_GROUP_ID, "ext-b-deployment", TsArtifact.DEFAULT_VERSION),
JavaScopes.COMPILE,
DependencyFlags.OPTIONAL,
DependencyFlags.DEPLOYMENT_CP));
expected.add(new ArtifactDependency(new GACTV("io.quarkus.bootstrap.test", "ext-b-deployment", "1"), "compile",

expected.add(new ArtifactDependency(
ArtifactCoords.jar(TsArtifact.DEFAULT_GROUP_ID, "ext-b-deployment-dep", TsArtifact.DEFAULT_VERSION),
JavaScopes.COMPILE,
DependencyFlags.OPTIONAL,
DependencyFlags.DEPLOYMENT_CP));
expected.add(new ArtifactDependency(new GACTV("io.quarkus.bootstrap.test", "ext-d-deployment", "1"), "compile",

expected.add(new ArtifactDependency(
ArtifactCoords.jar(TsArtifact.DEFAULT_GROUP_ID, "ext-d", TsArtifact.DEFAULT_VERSION),
JavaScopes.COMPILE,
DependencyFlags.DIRECT,
DependencyFlags.RUNTIME_EXTENSION_ARTIFACT,
DependencyFlags.TOP_LEVEL_RUNTIME_EXTENSION_ARTIFACT,
DependencyFlags.RUNTIME_CP,
DependencyFlags.DEPLOYMENT_CP));
assertEquals(expected, getDeploymentOnlyDeps(model));

expected.add(new ArtifactDependency(
ArtifactCoords.jar(TsArtifact.DEFAULT_GROUP_ID, "ext-d-deployment", TsArtifact.DEFAULT_VERSION),
JavaScopes.COMPILE,
DependencyFlags.DEPLOYMENT_CP));

assertEquals(expected, getDependenciesWithFlag(model, DependencyFlags.DEPLOYMENT_CP));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package io.quarkus.bootstrap.resolver.maven;

import static io.quarkus.bootstrap.util.DependencyUtils.getKey;
import static io.quarkus.bootstrap.util.DependencyUtils.hasWinner;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.eclipse.aether.collection.DependencyGraphTransformationContext;
import org.eclipse.aether.graph.DefaultDependencyNode;
import org.eclipse.aether.graph.Dependency;
import org.eclipse.aether.graph.DependencyNode;
import org.eclipse.aether.util.artifact.JavaScopes;
import org.eclipse.aether.util.graph.transformer.ConflictResolver;

import io.quarkus.maven.dependency.ArtifactKey;

/**
* Dependency tree conflict resolver.
* <p>
* The idea is to have a more efficient implementation than the
* {@link org.eclipse.aether.util.graph.transformer.ConflictIdSorter#transformGraph(DependencyNode, DependencyGraphTransformationContext)}
* for the use-cases the Quarkus deployment dependency resolver is designed for.
* <p>
* Specifically, this conflict resolver does not properly handle version ranges, that are not expected to be present in the
* phase it used.
*/
class DependencyTreeConflictResolver {

/**
* Resolves dependency version conflicts in the given dependency tree.
*
* @param root the root of the dependency tree
*/
static void resolveConflicts(DependencyNode root) {
new DependencyTreeConflictResolver(root).run();
}

final OrderedDependencyVisitor visitor;

private DependencyTreeConflictResolver(DependencyNode root) {
visitor = new OrderedDependencyVisitor(root);
}

private void run() {
visitor.next();// skip the root
final Map<ArtifactKey, VisitedDependency> visited = new HashMap<>();
while (visitor.hasNext()) {
var node = visitor.next();
if (!hasWinner(node)) {
visited.compute(getKey(node.getArtifact()), this::resolveConflict);
}
}
}

private VisitedDependency resolveConflict(ArtifactKey key, VisitedDependency prev) {
if (prev == null) {
return new VisitedDependency(visitor);
}
prev.resolveConflict(visitor);
return prev;
}

private static class VisitedDependency {
final DependencyNode node;
final int subtreeIndex;

private VisitedDependency(OrderedDependencyVisitor visitor) {
this.node = visitor.getCurrent();
this.subtreeIndex = visitor.getSubtreeIndex();
}

private void resolveConflict(OrderedDependencyVisitor visitor) {
var otherNode = visitor.getCurrent();
if (subtreeIndex != visitor.getSubtreeIndex()) {
final Dependency currentDep = node.getDependency();
final Dependency otherDep = otherNode.getDependency();
if (!currentDep.getScope().equals(otherDep.getScope())
&& getScopePriority(currentDep.getScope()) > getScopePriority(otherDep.getScope())) {
node.setScope(otherDep.getScope());
}
if (currentDep.isOptional() && !otherDep.isOptional()) {
node.setOptional(false);
}
}
otherNode.setChildren(List.of());
otherNode.setData(ConflictResolver.NODE_DATA_WINNER, new DefaultDependencyNode(node.getDependency()));
}
}

private static int getScopePriority(String scope) {
return switch (scope) {
case JavaScopes.COMPILE -> 0;
case JavaScopes.RUNTIME -> 1;
case JavaScopes.PROVIDED -> 2;
case JavaScopes.TEST -> 3;
default -> 4;
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,9 @@
import java.util.function.BiConsumer;

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.CollectRequest;
import org.eclipse.aether.collection.DependencyCollectionException;
import org.eclipse.aether.collection.DependencyGraphTransformationContext;
import org.eclipse.aether.collection.DependencySelector;
import org.eclipse.aether.graph.DefaultDependencyNode;
import org.eclipse.aether.graph.Dependency;
Expand All @@ -43,7 +40,6 @@
import org.eclipse.aether.util.artifact.JavaScopes;
import org.eclipse.aether.util.graph.manager.DependencyManagerUtils;
import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
import org.eclipse.aether.util.graph.transformer.ConflictIdSorter;
import org.eclipse.aether.util.graph.transformer.ConflictResolver;
import org.jboss.logging.Logger;

Expand Down Expand Up @@ -251,7 +247,7 @@ public void resolve(CollectRequest collectRtDepsRequest) throws AppModelResolver
if (!runtimeModelOnly) {
injectDeploymentDeps();
}
root = normalize(resolver.getSession(), root);
DependencyTreeConflictResolver.resolveConflicts(root);
populateModelBuilder(root);

// clear the reloadable flags
Expand Down Expand Up @@ -464,18 +460,6 @@ private void clearReloadableFlag(ResolvedDependencyBuilder dep) {
}
}

private static DependencyNode normalize(RepositorySystemSession session, DependencyNode root)
throws AppModelResolverException {
final DependencyGraphTransformationContext context = new SimpleDependencyGraphTransformationContext(session);
try {
// resolves version conflicts
root = new ConflictIdSorter().transformGraph(root, context);
return session.getDependencyGraphTransformer().transformGraph(root, context);
} catch (RepositoryException e) {
throw new AppModelResolverException("Failed to resolve dependency graph conflicts", e);
}
}

/**
* Resolves a project's runtime dependencies. This is the first step in the Quarkus application model resolution.
* These dependencies do not include Quarkus conditional dependencies.
Expand Down Expand Up @@ -977,6 +961,8 @@ private void collectDeploymentDeps() {
+ "or the artifact does not have any dependencies while at least a dependency on the runtime artifact "
+ info.runtimeArtifact + " is expected");
}
ensureScopeAndOptionality(deploymentNode, runtimeNode.getDependency().getScope(),
runtimeNode.getDependency().isOptional());

replaceRuntimeExtensionNodes(deploymentNode);
if (!presentInTargetGraph) {
Expand Down Expand Up @@ -1058,9 +1044,13 @@ void activate() {
return;
}
activated = true;
final AppDep parent = conditionalDep.parent;
final DependencyNode originalNode = collectDependencies(conditionalDep.node.getArtifact(),
conditionalDep.parent.ext.exclusions,
conditionalDep.parent.node.getRepositories());
parent.ext.exclusions,
parent.node.getRepositories());
ensureScopeAndOptionality(originalNode, parent.ext.runtimeNode.getDependency().getScope(),
parent.ext.runtimeNode.getDependency().isOptional());

final DefaultDependencyNode rtNode = (DefaultDependencyNode) conditionalDep.node;
rtNode.setRepositories(originalNode.getRepositories());
// if this node has conditional dependencies on its own, they may have been activated by this time
Expand All @@ -1077,10 +1067,10 @@ void activate() {
visitRuntimeDeps();
conditionalDep.setFlags(
(byte) (COLLECT_DEPLOYMENT_INJECTION_POINTS | (collectReloadableModules ? COLLECT_RELOADABLE_MODULES : 0)));
if (conditionalDep.parent.resolvedDep != null) {
conditionalDep.parent.resolvedDep.addDependency(conditionalDep.resolvedDep.getArtifactCoords());
if (parent.resolvedDep != null) {
parent.resolvedDep.addDependency(conditionalDep.resolvedDep.getArtifactCoords());
}
conditionalDep.parent.ext.runtimeNode.getChildren().add(rtNode);
parent.ext.runtimeNode.getChildren().add(rtNode);
}

private void visitRuntimeDeps() {
Expand All @@ -1103,6 +1093,30 @@ boolean isSatisfied() {
}
}

/**
* Makes sure the node's dependency scope and optionality (including its children) match the expected values.
*
* @param node dependency node
* @param scope expected scope
* @param optional expected optionality
*/
private static void ensureScopeAndOptionality(DependencyNode node, String scope, boolean optional) {
var dep = node.getDependency();
if (optional == dep.isOptional() && scope.equals(dep.getScope())) {
return;
}
var visitor = new OrderedDependencyVisitor(node);
while (visitor.hasNext()) {
dep = visitor.next().getDependency();
if (optional != dep.isOptional()) {
visitor.getCurrent().setOptional(optional);
}
if (!scope.equals(dep.getScope())) {
visitor.getCurrent().setScope(scope);
}
}
}

private static boolean isSameKey(Artifact a1, Artifact a2) {
return a2.getArtifactId().equals(a1.getArtifactId())
&& a2.getGroupId().equals(a1.getGroupId())
Expand Down
Loading

0 comments on commit 109cb5d

Please sign in to comment.