Handle relocations in the gradle resolver (#1485)
diff --git a/MODULE.bazel b/MODULE.bazel index bf68b1a..fad546c 100644 --- a/MODULE.bazel +++ b/MODULE.bazel
@@ -741,6 +741,8 @@ "com.almworks.sqlite4java:libsqlite4java-linux-i386:1.0.392", # https://github.com/bazel-contrib/rules_jvm_external/issues/1471 "androidx.fragment:fragment-ktx:1.6.1", + # https://github.com/bazel-contrib/rules_jvm_external/issues/250 + "org.slf4j:slf4j-log4j12:2.0.0", ], generate_compat_repositories = True, lock_file = "//tests/custom_maven_install:regression_testing_gradle_install.json",
diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/gradle/BUILD.bazel b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/gradle/BUILD.bazel index a1fc937..de36652 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/gradle/BUILD.bazel +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/gradle/BUILD.bazel
@@ -29,6 +29,14 @@ repository_name = "rules_jvm_external_deps", ), artifact( + "org.apache.maven:maven-model", + repository_name = "rules_jvm_external_deps", + ), + artifact( + "org.codehaus.plexus:plexus-utils", + repository_name = "rules_jvm_external_deps", + ), + artifact( "com.github.jknack:handlebars", repository_name = "rules_jvm_external_deps", ),
diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolver.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolver.java index b839ac1..02e73d5 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolver.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolver.java
@@ -39,6 +39,9 @@ import com.google.common.hash.Hashing; import com.google.devtools.build.runfiles.AutoBazelRepository; import com.google.devtools.build.runfiles.Runfiles; +import java.io.BufferedInputStream; +import java.io.File; +import java.io.FileInputStream; import java.io.IOException; import java.net.MalformedURLException; import java.net.URI; @@ -55,6 +58,11 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import org.apache.maven.model.Model; +import org.apache.maven.model.Relocation; +import org.apache.maven.model.io.xpp3.MavenXpp3Reader; +import org.codehaus.plexus.util.ReaderFactory; +import org.codehaus.plexus.util.xml.pull.XmlPullParserException; /** The implementation for the Gradle resolver */ @AutoBazelRepository @@ -155,6 +163,9 @@ MutableGraph<Coordinates> graph = GraphBuilder.directed().allowsSelfLoops(true).build(); Set<Conflict> conflicts = new HashSet<>(); + Map<Coordinates, String> coordinateHashes = new HashMap<>(); + // Track artifacts per node so we can inspect POM files for relocation later + Map<Coordinates, List<GradleResolvedArtifact>> artifactsByNode = new HashMap<>(); List<GradleResolvedDependency> implementationDependencies = resolved.getResolvedDependencies(); List<GradleUnresolvedDependency> unresolvedDependencies = resolved.getUnresolvedDependencies(); if (implementationDependencies == null) { @@ -172,10 +183,11 @@ artifact.getClassifier(), artifact.getExtension()); String extension = gradleCoordinates.getExtension(); - if (extension != null && extension.equals("pom")) { - extension = null; - } String classifier = gradleCoordinates.getClassifier(); + // POM artifacts should not generate their own node; coerce to JAR for node identity + if ("pom".equals(extension)) { + extension = null; // Coordinates() will default to jar + } Coordinates coordinates = new Coordinates( gradleCoordinates.getGroupId(), @@ -183,7 +195,10 @@ extension, classifier, gradleCoordinates.getVersion()); - addDependency(graph, coordinates, dependency, conflicts, requestedDeps, visited); + // Track artifact for this node + artifactsByNode.computeIfAbsent(coordinates, k -> new ArrayList<>()).add(artifact); + addDependency( + graph, coordinates, dependency, conflicts, requestedDeps, visited, artifactsByNode); // if there's a conflict and the conflicting version isn't one that's actually requested // then it's an actual conflict we want to report if (dependency.isConflict() && !isRequestedDep(requestedDeps, dependency)) { @@ -240,6 +255,10 @@ if (!unresolvedRequestedDeps.isEmpty()) { throw new GradleDependencyResolutionException(unresolvedRequestedDeps); } + + // After building the graph, contract relocation stubs (keep aggregating POMs) + collapseRelocations(graph, coordinateHashes, conflicts, artifactsByNode); + return new ResolutionResult(graph, conflicts); } @@ -269,7 +288,8 @@ GradleResolvedDependency parentInfo, Set<Conflict> conflicts, List<GradleDependency> requestedDeps, - Set<Coordinates> visited) { + Set<Coordinates> visited, + Map<Coordinates, List<GradleResolvedArtifact>> artifactsByNode) { if (visited.contains(parent)) { return; } @@ -287,8 +307,9 @@ childArtifact.getClassifier(), childArtifact.getExtension()); String extension = childArtifact.getExtension(); - if (extension != null && extension.equals("pom")) { - extension = null; + // POM artifacts should not generate their own node; coerce to JAR for node identity + if ("pom".equals(extension)) { + extension = null; // Coordinates() will default to jar } Coordinates child = new Coordinates( @@ -297,6 +318,8 @@ extension, childCoordinates.getClassifier(), childCoordinates.getVersion()); + // Track artifact for child node + artifactsByNode.computeIfAbsent(child, k -> new ArrayList<>()).add(childArtifact); graph.addNode(child); graph.putEdge(parent, child); // if there's a conflict and the conflicting version isn't one that's actually requested @@ -329,12 +352,129 @@ childInfo, conflicts, requestedDeps, - visited); // recursively traverse the graph + visited, + artifactsByNode); // recursively traverse the graph } } } } + private void collapseRelocations( + MutableGraph<Coordinates> graph, + Map<Coordinates, String> coordinateHashes, + Set<Conflict> conflicts, + Map<Coordinates, List<GradleResolvedArtifact>> artifactsByNode) { + List<Coordinates> toRemove = new ArrayList<>(); + + for (Coordinates node : graph.nodes()) { + List<GradleResolvedArtifact> artifacts = artifactsByNode.get(node); + if (artifacts == null || artifacts.isEmpty()) { + continue; + } + + File pomFile = null; + for (GradleResolvedArtifact a : artifacts) { + File f = a.getFile(); + if (f != null && f.getName().endsWith(".pom")) { + pomFile = f; + break; + } + } + if (pomFile == null) { + continue; // no POM attached => cannot determine relocation + } + + // Check for relocation in the POM + Coordinates target = readRelocationTarget(pomFile, node); + if (target == null) { + continue; // aggregator or normal module, keep as-is + } + + // Find the target node in the graph by matching G:A:V (ignore classifier/extension) + Coordinates targetNode = null; + for (Coordinates candidate : graph.nodes()) { + if (candidate.getGroupId().equals(target.getGroupId()) + && candidate.getArtifactId().equals(target.getArtifactId()) + && candidate.getVersion().equals(target.getVersion())) { + // Prefer non-POM node if possible + if (targetNode == null) { + targetNode = candidate; + } else if (!"pom".equals(candidate.getExtension()) + && "pom".equals(targetNode.getExtension())) { + targetNode = candidate; + } + } + } + + if (targetNode == null) { + // Could not find the relocation target in the graph; be conservative and skip + continue; + } + + // Rewire all predecessors of node to point to targetNode + for (Coordinates pred : new HashSet<>(graph.predecessors(node))) { + if (!pred.equals(targetNode)) { + graph.putEdge(pred, targetNode); + } + } + + // Update conflicts that reference this node + Set<Conflict> toAdd = new HashSet<>(); + Set<Conflict> toDrop = new HashSet<>(); + for (Conflict c : conflicts) { + if (c.getResolved().equals(node)) { + toDrop.add(c); + toAdd.add(new Conflict(targetNode, c.getRequested())); + } else if (c.getRequested().equals(node)) { + toDrop.add(c); + toAdd.add(new Conflict(c.getResolved(), targetNode)); + } + } + conflicts.removeAll(toDrop); + conflicts.addAll(toAdd); + + // Remove any hash for the POM node + coordinateHashes.remove(node); + + toRemove.add(node); + } + + // Remove after iteration to avoid concurrent modification + for (Coordinates n : toRemove) { + graph.removeNode(n); + } + } + + private Coordinates readRelocationTarget(File pomFile, Coordinates fallback) { + try (FileInputStream fis = new FileInputStream(pomFile); + BufferedInputStream bis = new BufferedInputStream(fis)) { + MavenXpp3Reader reader = new MavenXpp3Reader(); + Model model = reader.read(ReaderFactory.newXmlReader(bis)); + if (model.getDistributionManagement() != null) { + Relocation relocation = model.getDistributionManagement().getRelocation(); + if (relocation != null) { + String g = + relocation.getGroupId() != null ? relocation.getGroupId() : fallback.getGroupId(); + String a = + relocation.getArtifactId() != null + ? relocation.getArtifactId() + : fallback.getArtifactId(); + String v = + relocation.getVersion() != null ? relocation.getVersion() : fallback.getVersion(); + return new Coordinates(g, a, fallback.getExtension(), fallback.getClassifier(), v); + } + } + } catch (IOException | XmlPullParserException e) { + // If parsing fails, treat as no relocation + if (isVerbose()) { + eventListener.onEvent( + new LogEvent( + "gradle", "Failed to parse POM for relocation: " + pomFile, e.getMessage())); + } + } + return null; + } + private Repository createRepository(URI uri) { Netrc.Credential credential = netrc.getCredential(uri.getHost()); if (credential == null) {
diff --git a/tests/com/github/bazelbuild/rules_jvm_external/resolver/MavenRepo.java b/tests/com/github/bazelbuild/rules_jvm_external/resolver/MavenRepo.java index c40dbca..0958892 100644 --- a/tests/com/github/bazelbuild/rules_jvm_external/resolver/MavenRepo.java +++ b/tests/com/github/bazelbuild/rules_jvm_external/resolver/MavenRepo.java
@@ -25,7 +25,9 @@ import java.nio.file.Path; import java.nio.file.Paths; import org.apache.maven.model.Dependency; +import org.apache.maven.model.DistributionManagement; import org.apache.maven.model.Model; +import org.apache.maven.model.Relocation; import org.apache.maven.model.io.xpp3.MavenXpp3Writer; public class MavenRepo { @@ -124,6 +126,33 @@ writePomFile(model); } + /** + * Add a POM that declares a relocation from the 'from' coordinates to the 'to' coordinates. This + * writes only the POM (no artifact file), which mirrors real-world relocation stubs. + */ + public MavenRepo addRelocation(Coordinates from, Coordinates to) { + try { + Model model = new Model(); + model.setModelVersion("4.0.0"); + model.setGroupId(from.getGroupId()); + model.setArtifactId(from.getArtifactId()); + model.setVersion(from.getVersion()); + // packaging defaults to jar; relocation is specified under distributionManagement + DistributionManagement dm = new DistributionManagement(); + Relocation relocation = new Relocation(); + relocation.setGroupId(to.getGroupId()); + relocation.setArtifactId(to.getArtifactId()); + relocation.setVersion(to.getVersion()); + dm.setRelocation(relocation); + model.setDistributionManagement(dm); + + writePomFile(model); + return this; + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + private void writeFile(Coordinates coords) throws IOException { Path output = root.resolve(coords.toRepoPath()); // We don't read the contents, it just needs to exist
diff --git a/tests/com/github/bazelbuild/rules_jvm_external/resolver/ResolverTestBase.java b/tests/com/github/bazelbuild/rules_jvm_external/resolver/ResolverTestBase.java index 6981d58..27953ab 100644 --- a/tests/com/github/bazelbuild/rules_jvm_external/resolver/ResolverTestBase.java +++ b/tests/com/github/bazelbuild/rules_jvm_external/resolver/ResolverTestBase.java
@@ -703,6 +703,46 @@ resolved.nodes()); } + @Test + public void shouldContractRelocatedPomButKeepAggregatorPom() { + // Relocated artifact: old:relocated -> new:target + Coordinates relocated = new Coordinates("com.example:relocated:1.0.0"); + Coordinates target = new Coordinates("com.example:target:1.0.0"); + + // Aggregator POM with two children should remain in the graph + Coordinates aggregator = new Coordinates("com.example:aggregator:9.9.9"); + Model aggregatorModel = createModel(aggregator); + aggregatorModel.setPackaging("pom"); + + Coordinates childA = new Coordinates("com.example:childA:1.2.3"); + Coordinates childB = new Coordinates("com.example:childB:4.5.6"); + + Path repo = + MavenRepo.create() + // target and its jar + .add(target) + // relocated POM that points to target + .addRelocation(relocated, target) + // aggregator POM and its children + .add(childA) + .add(childB) + .add(aggregatorModel, childA, childB) + .getPath(); + + // Ask to resolve both the relocated coordinate and the aggregator coordinate + Graph<Coordinates> graph = + resolver.resolve(prepareRequestFor(repo.toUri(), relocated, aggregator)).getResolution(); + + // The relocated coordinate should be contracted to the target, so the graph should contain + // the target instead of the relocated origin. + assertTrue(graph.nodes().contains(target)); + assertFalse(graph.nodes().contains(relocated)); + + // The aggregator POM should stay as a node with edges to both children + assertTrue(graph.nodes().contains(aggregator)); + assertEquals(Set.of(childA, childB), graph.successors(aggregator)); + } + protected Model createModel(Coordinates coords) { Model model = new Model(); model.setModelVersion("4.0.0");
diff --git a/tests/custom_maven_install/regression_testing_gradle_install.json b/tests/custom_maven_install/regression_testing_gradle_install.json index 69fbe45..29e297f 100644 --- a/tests/custom_maven_install/regression_testing_gradle_install.json +++ b/tests/custom_maven_install/regression_testing_gradle_install.json
@@ -1,7 +1,7 @@ { "__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL", - "__INPUT_ARTIFACTS_HASH": -246340730, - "__RESOLVED_ARTIFACTS_HASH": 489340253, + "__INPUT_ARTIFACTS_HASH": 1541018928, + "__RESOLVED_ARTIFACTS_HASH": 128343746, "artifacts": { "androidx.activity:activity-ktx:aar": { "shasums": { @@ -393,6 +393,12 @@ }, "version": "1.0.0" }, + "ch.qos.reload4j:reload4j": { + "shasums": { + "jar": "fa07aa7adedf2a65eb09007443bb60b51c725b1b5b88e481e00c529ff2d3b5b5" + }, + "version": "1.2.19" + }, "com.almworks.sqlite4java:libsqlite4java-linux-i386:so": { "shasums": { "jar": "3c93ee3f997e957715fd08b263948d460da000a6e0bb904ae525e790f39429eb" @@ -482,6 +488,18 @@ "jar": "ace2a10dc8e2d5fd34925ecac03e4988b2c0f851650c94b8cef49ba1bd111478" }, "version": "13.0" + }, + "org.slf4j:slf4j-api": { + "shasums": { + "jar": "a223e6df91b84f19d49c5ebc5f5f97c7f4438419f84a52fa05e1cfc6eed38aa9" + }, + "version": "2.0.0" + }, + "org.slf4j:slf4j-reload4j": { + "shasums": { + "jar": "7a663d4dc18e49f0ea1cf8dc9362e6f216ad3ca2db8dd49760f76060a03236aa" + }, + "version": "2.0.0" } }, "conflict_resolution": { @@ -928,6 +946,10 @@ "org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm": [ "org.jetbrains.kotlin:kotlin-stdlib-common", "org.jetbrains.kotlinx:kotlinx-coroutines-bom" + ], + "org.slf4j:slf4j-reload4j": [ + "ch.qos.reload4j:reload4j", + "org.slf4j:slf4j-api" ] }, "packages": { @@ -950,6 +972,22 @@ "androidx.lifecycle:lifecycle-common": [ "androidx.lifecycle" ], + "ch.qos.reload4j:reload4j": [ + "org.apache.log4j", + "org.apache.log4j.chainsaw", + "org.apache.log4j.config", + "org.apache.log4j.helpers", + "org.apache.log4j.jdbc", + "org.apache.log4j.net", + "org.apache.log4j.or", + "org.apache.log4j.or.jms", + "org.apache.log4j.or.sax", + "org.apache.log4j.pattern", + "org.apache.log4j.rewrite", + "org.apache.log4j.spi", + "org.apache.log4j.varia", + "org.apache.log4j.xml" + ], "com.almworks.sqlite4java:sqlite4java": [ "com.almworks.sqlite4java", "javolution.util.stripped" @@ -1044,10 +1082,20 @@ "org.jetbrains:annotations": [ "org.intellij.lang.annotations", "org.jetbrains.annotations" + ], + "org.slf4j:slf4j-api": [ + "org.slf4j", + "org.slf4j.event", + "org.slf4j.helpers", + "org.slf4j.spi" + ], + "org.slf4j:slf4j-reload4j": [ + "org.slf4j.reload4j" ] }, "repositories": { "https://repo1.maven.org/maven2/": [ + "ch.qos.reload4j:reload4j", "com.almworks.sqlite4java:libsqlite4java-linux-i386:so", "com.almworks.sqlite4java:sqlite4java", "com.google.guava:listenablefuture", @@ -1062,7 +1110,9 @@ "org.jetbrains.kotlinx:kotlinx-coroutines-bom", "org.jetbrains.kotlinx:kotlinx-coroutines-core", "org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm", - "org.jetbrains:annotations" + "org.jetbrains:annotations", + "org.slf4j:slf4j-api", + "org.slf4j:slf4j-reload4j" ], "https://maven.google.com/": [ "androidx.activity:activity-ktx:aar", @@ -1140,6 +1190,11 @@ "kotlinx.coroutines.internal.MainDispatcherFactory": [ "kotlinx.coroutines.android.AndroidDispatcherFactory" ] + }, + "org.slf4j:slf4j-reload4j": { + "org.slf4j.spi.SLF4JServiceProvider": [ + "org.slf4j.reload4j.Reload4jServiceProvider" + ] } }, "skipped": [