From 265df37cf5316185c3e979fdd3f79a3327a51247 Mon Sep 17 00:00:00 2001 From: Simon Mavi Stewart Date: Tue, 9 Jun 2026 13:02:32 +0100 Subject: [PATCH 1/2] refactor: carry a ResolvedArtifact for every resolution node Replace the `Map` side-channel on `ResolutionResult` with a `Map` that has an entry for every node in the resolved graph. Each `ResolvedArtifact` holds its coordinates and an optional path, so "this node has no known binary" is represented explicitly rather than inferred from an absent map entry. This is a pure refactoring: the Maven and Gradle resolvers populate the new shape from exactly the information they already had, the downloader still receives the same set of known paths, and the lock file, its format, and the resolved-artifact hashes are all unchanged. Co-Authored-By: Claude Opus 4.7 --- .../resolver/ResolutionResult.java | 11 ++-- .../resolver/ResolvedArtifact.java | 66 +++++++++++++++++++ .../resolver/cmd/AbstractMain.java | 8 ++- .../resolver/gradle/GradleResolver.java | 8 ++- .../resolver/maven/MavenResolver.java | 9 ++- .../resolver/ResolverTestBase.java | 7 +- .../resolver/gradle/GradleResolverTest.java | 10 +-- 7 files changed, 103 insertions(+), 16 deletions(-) create mode 100644 private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ResolvedArtifact.java diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ResolutionResult.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ResolutionResult.java index 52bb45637..302b9f560 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ResolutionResult.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ResolutionResult.java @@ -16,7 +16,6 @@ import com.github.bazelbuild.rules_jvm_external.Coordinates; import com.google.common.graph.Graph; -import java.nio.file.Path; import java.util.Map; import java.util.Set; @@ -28,15 +27,15 @@ public class ResolutionResult { private final Graph resolution; private final Set conflicts; - private final Map paths; + private final Map artifacts; public ResolutionResult( Graph resolution, Set conflicts, - Map artifactPaths) { + Map artifacts) { this.resolution = resolution; this.conflicts = Set.copyOf(conflicts); - this.paths = artifactPaths != null ? Map.copyOf(artifactPaths) : Map.of(); + this.artifacts = artifacts != null ? Map.copyOf(artifacts) : Map.of(); } public Graph getResolution() { @@ -47,7 +46,7 @@ public Set getConflicts() { return conflicts; } - public Map getPaths() { - return paths; + public Map getArtifacts() { + return artifacts; } } diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ResolvedArtifact.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ResolvedArtifact.java new file mode 100644 index 000000000..8aa2e1399 --- /dev/null +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ResolvedArtifact.java @@ -0,0 +1,66 @@ +// Copyright 2024 The Bazel Authors. All rights reserved. +// +// Licensed 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. + +package com.github.bazelbuild.rules_jvm_external.resolver; + +import com.github.bazelbuild.rules_jvm_external.Coordinates; +import java.nio.file.Path; +import java.util.Objects; +import java.util.Optional; + +/** + * A node in a {@link ResolutionResult}: a coordinate that was resolved, together with the local + * path to its artifact if one is known. The path is absent when the resolver has not (yet) fetched + * a file for the coordinate. + */ +public final class ResolvedArtifact { + + private final Coordinates coordinates; + private final Optional path; + + public ResolvedArtifact(Coordinates coordinates, Path path) { + this.coordinates = Objects.requireNonNull(coordinates); + this.path = Optional.ofNullable(path); + } + + public Coordinates getCoordinates() { + return coordinates; + } + + public Optional getPath() { + return path; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof ResolvedArtifact)) { + return false; + } + ResolvedArtifact that = (ResolvedArtifact) o; + return coordinates.equals(that.coordinates) && path.equals(that.path); + } + + @Override + public int hashCode() { + return Objects.hash(coordinates, path); + } + + @Override + public String toString() { + return "ResolvedArtifact{" + coordinates + ", path=" + path.orElse(null) + "}"; + } +} diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/AbstractMain.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/AbstractMain.java index d33cacd7b..d7898538e 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/AbstractMain.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/AbstractMain.java @@ -99,6 +99,12 @@ private static Set fulfillDependencyInfos( cacheResults = "1".equals(rjeUnsafeCache) || Boolean.parseBoolean(rjeUnsafeCache); } + Map knownPaths = new LinkedHashMap<>(); + resolutionResult + .getArtifacts() + .forEach( + (coords, artifact) -> artifact.getPath().ifPresent(p -> knownPaths.put(coords, p))); + Downloader downloader = new Downloader( config.getNetrc(), @@ -106,7 +112,7 @@ private static Set fulfillDependencyInfos( request.getRepositories(), listener, cacheResults, - resolutionResult.getPaths()); + knownPaths); List>> futures = new LinkedList<>(); 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 eb9578b03..79e9414e3 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 @@ -19,6 +19,7 @@ import com.github.bazelbuild.rules_jvm_external.resolver.Conflict; import com.github.bazelbuild.rules_jvm_external.resolver.ResolutionRequest; import com.github.bazelbuild.rules_jvm_external.resolver.ResolutionResult; +import com.github.bazelbuild.rules_jvm_external.resolver.ResolvedArtifact; import com.github.bazelbuild.rules_jvm_external.resolver.Resolver; import com.github.bazelbuild.rules_jvm_external.resolver.events.EventListener; import com.github.bazelbuild.rules_jvm_external.resolver.events.LogEvent; @@ -324,7 +325,12 @@ private ResolutionResult parseDependencies( // Only include paths for coordinates that are actually in the final resolved graph paths.keySet().retainAll(graph.nodes()); - return new ResolutionResult(graph, conflicts, paths); + Map artifacts = new HashMap<>(); + for (Coordinates node : graph.nodes()) { + artifacts.put(node, new ResolvedArtifact(node, paths.get(node))); + } + + return new ResolutionResult(graph, conflicts, artifacts); } private String makeDepKey(String group, String artifact, String version) { diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/maven/MavenResolver.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/maven/MavenResolver.java index 82bfe74fe..d5c660451 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/maven/MavenResolver.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/maven/MavenResolver.java @@ -18,6 +18,7 @@ import com.github.bazelbuild.rules_jvm_external.resolver.Conflict; import com.github.bazelbuild.rules_jvm_external.resolver.ResolutionRequest; import com.github.bazelbuild.rules_jvm_external.resolver.ResolutionResult; +import com.github.bazelbuild.rules_jvm_external.resolver.ResolvedArtifact; import com.github.bazelbuild.rules_jvm_external.resolver.Resolver; import com.github.bazelbuild.rules_jvm_external.resolver.events.EventListener; import com.github.bazelbuild.rules_jvm_external.resolver.events.LogEvent; @@ -274,7 +275,13 @@ public ResolutionResult resolve(ResolutionRequest request) { getConflicts(request.getDependencies(), resolvedDependencies), graphNormalizationResult.getConflicts()); - return new ResolutionResult(graphNormalizationResult.getNormalizedGraph(), conflicts, Map.of()); + Graph normalizedGraph = graphNormalizationResult.getNormalizedGraph(); + Map artifacts = new HashMap<>(); + for (Coordinates node : normalizedGraph.nodes()) { + artifacts.put(node, new ResolvedArtifact(node, null)); + } + + return new ResolutionResult(normalizedGraph, conflicts, artifacts); } private GraphNormalizationResult makeVersionsConsistent(Graph dependencyGraph) { 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 db55e004e..52498d2f6 100644 --- a/tests/com/github/bazelbuild/rules_jvm_external/resolver/ResolverTestBase.java +++ b/tests/com/github/bazelbuild/rules_jvm_external/resolver/ResolverTestBase.java @@ -877,9 +877,10 @@ public void shouldRespectForceVersionWhenResolvingConflicts() throws IOException // Validate that the downloaded artifact has the correct SHA for the forced version. // This catches bugs where the resolver fetches artifacts via detached configurations // that bypass force_version, resulting in the wrong file being downloaded. - Map paths = result.getPaths(); - if (paths.containsKey(lowerVersion)) { - Path downloadedArtifact = paths.get(lowerVersion); + Map artifacts = result.getArtifacts(); + ResolvedArtifact lower = artifacts.get(lowerVersion); + if (lower != null && lower.getPath().isPresent()) { + Path downloadedArtifact = lower.getPath().get(); Path expectedArtifact = repo.resolve(lowerVersion.toRepoPath()); // Compare SHA256 of downloaded file vs expected file in repo diff --git a/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolverTest.java b/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolverTest.java index b249b185e..a5cef5709 100644 --- a/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolverTest.java +++ b/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolverTest.java @@ -22,6 +22,7 @@ import com.github.bazelbuild.rules_jvm_external.Coordinates; import com.github.bazelbuild.rules_jvm_external.resolver.MavenRepo; import com.github.bazelbuild.rules_jvm_external.resolver.ResolutionResult; +import com.github.bazelbuild.rules_jvm_external.resolver.ResolvedArtifact; import com.github.bazelbuild.rules_jvm_external.resolver.Resolver; import com.github.bazelbuild.rules_jvm_external.resolver.ResolverTestBase; import com.github.bazelbuild.rules_jvm_external.resolver.cmd.ResolverConfig; @@ -284,10 +285,11 @@ public void shouldRecordCorrectShaForResolvedVersionNotConflictingVersion() { assertEquals("Should resolve to exactly one version", 1, conflictedNodes.size()); assertTrue("Should resolve to higher version", conflictedNodes.contains(higherVersion)); - // Verify paths map contains only the resolved (higher) version, not the lower version - Map paths = result.getPaths(); - assertTrue("Paths should contain resolved version", paths.containsKey(higherVersion)); + // Verify the resolved artifacts contain only the resolved (higher) version, not the lower one + Map artifacts = result.getArtifacts(); + assertTrue("Artifacts should contain resolved version", artifacts.containsKey(higherVersion)); assertFalse( - "Paths should not contain conflicting lower version", paths.containsKey(lowerVersion)); + "Artifacts should not contain conflicting lower version", + artifacts.containsKey(lowerVersion)); } } From 8cfae04a871488d326876e16aae9085242dccfd6 Mon Sep 17 00:00:00 2001 From: Simon Mavi Stewart Date: Tue, 9 Jun 2026 16:25:53 +0100 Subject: [PATCH 2/2] fix: render binary-less GMM umbrella coordinates as aggregators A Gradle Module Metadata "umbrella" coordinate (and likewise a `pom` aggregator or an `available-at` redirect) publishes a POM but no binary of its own. The Gradle resolver's path fallback used to associate such a node's POM with it as if the POM were the module's JAR, so the downloader hashed the POM in the JAR's place and the lock file recorded a bogus artifact hash. Decide "no binary" upstream in the resolver instead: a node whose only file is a POM is recorded as an aggregator on its ResolvedArtifact, and any stale POM path is dropped so it is never handed to the downloader. The downloader honours the explicit aggregator signal by returning a binary-less result -- keeping the POM's repositories so the lock file can still locate the coordinate -- rather than throwing UriNotFoundException. An empty path alone cannot carry this meaning because Maven reports empty paths for everything it still has to fetch, so the signal must be explicit. Builds on "refactor: carry a ResolvedArtifact for every resolution node". Co-Authored-By: Claude Opus 4.7 --- .../resolver/ResolvedArtifact.java | 38 ++++++++- .../resolver/cmd/AbstractMain.java | 12 ++- .../resolver/gradle/GradleResolver.java | 19 ++++- .../resolver/remote/Downloader.java | 22 +++++ .../rules_jvm_external/resolver/gradle/BUILD | 2 + .../resolver/gradle/GradleResolverTest.java | 80 +++++++++++++++++++ .../resolver/maven/DownloaderTest.java | 25 ++++++ 7 files changed, 189 insertions(+), 9 deletions(-) diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ResolvedArtifact.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ResolvedArtifact.java index 8aa2e1399..b6fd1ca38 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ResolvedArtifact.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ResolvedArtifact.java @@ -23,15 +23,31 @@ * A node in a {@link ResolutionResult}: a coordinate that was resolved, together with the local * path to its artifact if one is known. The path is absent when the resolver has not (yet) fetched * a file for the coordinate. + * + *

An empty path is ambiguous on its own: for some resolvers it simply means "the downloader + * still has to fetch this", while for others it means "this coordinate has no binary at all". The + * {@link #isAggregator()} flag disambiguates the second case: when {@code true}, the resolver has + * positively determined that the coordinate ships no binary of its own and exists only to pull in + * other artifacts (a Gradle module-metadata umbrella, an {@code available-at} redirect, or a {@code + * pom} aggregator). Such a node should be rendered as an exports-only + * wrapper rather than having a file downloaded and hashed for it. This is distinct from the + * graph-surgery notion of an "aggregating dependency" in {@code GradleResolver} (which removes the + * node entirely); an aggregator here remains in the graph as a wrapper. */ public final class ResolvedArtifact { private final Coordinates coordinates; private final Optional path; + private final boolean aggregator; public ResolvedArtifact(Coordinates coordinates, Path path) { + this(coordinates, path, false); + } + + public ResolvedArtifact(Coordinates coordinates, Path path, boolean aggregator) { this.coordinates = Objects.requireNonNull(coordinates); this.path = Optional.ofNullable(path); + this.aggregator = aggregator; } public Coordinates getCoordinates() { @@ -42,6 +58,14 @@ public Optional getPath() { return path; } + /** + * Whether the resolver determined this coordinate has no binary of its own and exists only to + * aggregate or redirect to other artifacts. See the class javadoc for details. + */ + public boolean isAggregator() { + return aggregator; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -51,16 +75,24 @@ public boolean equals(Object o) { return false; } ResolvedArtifact that = (ResolvedArtifact) o; - return coordinates.equals(that.coordinates) && path.equals(that.path); + return aggregator == that.aggregator + && coordinates.equals(that.coordinates) + && path.equals(that.path); } @Override public int hashCode() { - return Objects.hash(coordinates, path); + return Objects.hash(coordinates, path, aggregator); } @Override public String toString() { - return "ResolvedArtifact{" + coordinates + ", path=" + path.orElse(null) + "}"; + return "ResolvedArtifact{" + + coordinates + + ", path=" + + path.orElse(null) + + ", aggregator=" + + aggregator + + "}"; } } diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/AbstractMain.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/AbstractMain.java index d7898538e..56ee3ddb5 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/AbstractMain.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/cmd/AbstractMain.java @@ -46,6 +46,7 @@ import java.nio.file.Path; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -100,10 +101,16 @@ private static Set fulfillDependencyInfos( } Map knownPaths = new LinkedHashMap<>(); + Set aggregatingCoordinates = new LinkedHashSet<>(); resolutionResult .getArtifacts() .forEach( - (coords, artifact) -> artifact.getPath().ifPresent(p -> knownPaths.put(coords, p))); + (coords, artifact) -> { + artifact.getPath().ifPresent(p -> knownPaths.put(coords, p)); + if (artifact.isAggregator()) { + aggregatingCoordinates.add(coords); + } + }); Downloader downloader = new Downloader( @@ -112,7 +119,8 @@ private static Set fulfillDependencyInfos( request.getRepositories(), listener, cacheResults, - knownPaths); + knownPaths, + aggregatingCoordinates); List>> futures = new LinkedList<>(); 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 79e9414e3..d19a19591 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 @@ -286,7 +286,11 @@ private ResolutionResult parseDependencies( // Collapse aggregating dependencies (dependencies with only classified artifacts) collapseAggregatingDependencies(graph, paths, artifactsByNode); - // Populate paths for all nodes in the final graph from collected artifacts + // Populate paths for all nodes in the final graph from collected artifacts. Nodes that + // resolved but have no binary of their own (only a POM) are recorded as aggregators so the + // downloader and lock file render them as exports-only wrappers instead of treating the POM + // as the module's binary. + Set aggregatorNodes = new HashSet<>(); for (Map.Entry> entry : artifactsByNode.entrySet()) { Coordinates coords = entry.getKey(); if (!graph.nodes().contains(coords)) { @@ -307,11 +311,12 @@ private ResolutionResult parseDependencies( break; } } - // Fallback: any existing file (including pom) + // Fallback: any existing real binary. Never a POM: a node whose only file is a POM is an + // umbrella/redirect coordinate with no binary of its own, not a module whose binary is a POM. if (bestFile == null) { for (GradleResolvedArtifact artifact : entry.getValue()) { File file = artifact.getFile(); - if (file != null && file.exists()) { + if (file != null && file.exists() && !file.getName().endsWith(".pom")) { bestFile = file; break; } @@ -319,6 +324,11 @@ private ResolutionResult parseDependencies( } if (bestFile != null) { paths.put(coords, bestFile.toPath()); + } else { + // No real binary for this node. Drop any POM path an earlier pass associated with it so + // we never hand a POM to the downloader as though it were the module's binary. + paths.remove(coords); + aggregatorNodes.add(coords); } } @@ -327,7 +337,8 @@ private ResolutionResult parseDependencies( Map artifacts = new HashMap<>(); for (Coordinates node : graph.nodes()) { - artifacts.put(node, new ResolvedArtifact(node, paths.get(node))); + artifacts.put( + node, new ResolvedArtifact(node, paths.get(node), aggregatorNodes.contains(node))); } return new ResolutionResult(graph, conflicts, artifacts); diff --git a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/remote/Downloader.java b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/remote/Downloader.java index c5aa48efa..0b09fef65 100644 --- a/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/remote/Downloader.java +++ b/private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/remote/Downloader.java @@ -55,6 +55,7 @@ public class Downloader { private final boolean cacheDownloads; private final HttpDownloader httpDownloader; private final Map knownPaths; + private final Set aggregatingCoordinates; public Downloader( Netrc netrc, @@ -63,11 +64,24 @@ public Downloader( EventListener listener, boolean cacheDownloads, Map knownPaths) { + this(netrc, localRepository, repositories, listener, cacheDownloads, knownPaths, Set.of()); + } + + public Downloader( + Netrc netrc, + Path localRepository, + Collection repositories, + EventListener listener, + boolean cacheDownloads, + Map knownPaths, + Set aggregatingCoordinates) { this.localRepository = localRepository; this.repos = Set.copyOf(repositories); this.cacheDownloads = cacheDownloads; this.httpDownloader = new HttpDownloader(netrc, listener); this.knownPaths = knownPaths != null ? Map.copyOf(knownPaths) : Map.of(); + this.aggregatingCoordinates = + aggregatingCoordinates != null ? Set.copyOf(aggregatingCoordinates) : Set.of(); } public DownloadResult download(Coordinates coords) { @@ -116,6 +130,14 @@ public DownloadResult download(Coordinates coords) { } } + if (aggregatingCoordinates.contains(coords)) { + // The resolver positively determined this coordinate has no binary of its own (for example + // a Gradle module-metadata umbrella that redirects to a platform sibling), even though its + // POM does not declare `pom`. Render it as a binary-less aggregating + // result, capturing the repositories that hold its POM so the lock file can still locate it. + return new DownloadResult(coords, pomResult.getRepositories(), null, null); + } + throw new UriNotFoundException("Unable to download from any repo: " + coords); } diff --git a/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/BUILD b/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/BUILD index ba998bfc8..9c06a6c59 100644 --- a/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/BUILD +++ b/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/BUILD @@ -38,6 +38,8 @@ java_test( "//private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/gradle", "//private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/gradle/models", "//private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/netrc", + "//private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/remote", + "//private/tools/java/com/github/bazelbuild/rules_jvm_external/resolver/ui", "//tests/com/github/bazelbuild/rules_jvm_external/resolver", artifact( "junit:junit", diff --git a/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolverTest.java b/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolverTest.java index a5cef5709..2664284ae 100644 --- a/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolverTest.java +++ b/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/GradleResolverTest.java @@ -28,6 +28,9 @@ import com.github.bazelbuild.rules_jvm_external.resolver.cmd.ResolverConfig; import com.github.bazelbuild.rules_jvm_external.resolver.events.EventListener; import com.github.bazelbuild.rules_jvm_external.resolver.netrc.Netrc; +import com.github.bazelbuild.rules_jvm_external.resolver.remote.DownloadResult; +import com.github.bazelbuild.rules_jvm_external.resolver.remote.Downloader; +import com.github.bazelbuild.rules_jvm_external.resolver.ui.NullListener; import com.google.common.graph.Graph; import com.google.devtools.build.runfiles.AutoBazelRepository; import com.google.devtools.build.runfiles.Runfiles; @@ -137,6 +140,83 @@ public void resolvesJvmButNotAndroidVariant() throws IOException, XMLStreamExcep assertEquals(Set.of(baseCoordinates, jvmCoordinates), resolved.nodes()); } + @Test + public void gmmUmbrellaWithoutBinaryResolvesAsAggregator() + throws IOException, XMLStreamException { + // A real Gradle Module Metadata "umbrella" publishes a POM and `.module` metadata but no + // binary for the umbrella coordinate itself: it redirects to a sibling (here, the JVM + // variant). The resolver must record the umbrella as an aggregator with no binary, rather + // than smuggling its POM in as though it were the module JAR. + Coordinates baseCoordinates = new Coordinates("com.example:sample:1.0"); + Coordinates jvmCoordinates = new Coordinates("com.example:sample-jvm:1.0"); + MavenRepo mavenRepo = MavenRepo.create(); + GradleModuleMetadataHelper moduleMetadataHelper = new GradleModuleMetadataHelper(mavenRepo); + + Runfiles runfiles = + Runfiles.preload().withSourceRepository(AutoBazelRepository_GradleResolverTest.NAME); + Path baseMetadataPath = + Paths.get( + runfiles.rlocation( + "rules_jvm_external/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/fixtures/simpleJvmVariant/sample-1.0.module")); + moduleMetadataHelper.addToMavenRepo(baseCoordinates, Files.readString(baseMetadataPath)); + + Path jvmMetadataPath = + Paths.get( + runfiles.rlocation( + "rules_jvm_external/tests/com/github/bazelbuild/rules_jvm_external/resolver/gradle/fixtures/simpleJvmVariant/sample-jvm-1.0.module")); + moduleMetadataHelper.addToMavenRepo(jvmCoordinates, Files.readString(jvmMetadataPath)); + + // Remove the umbrella JAR so the umbrella coordinate has only a POM (+ module metadata), + // matching the real-world shape. + Files.delete(mavenRepo.getPath().resolve(baseCoordinates.toRepoPath())); + + ResolutionResult result = + resolver.resolve(prepareRequestFor(mavenRepo.getPath().toUri(), baseCoordinates)); + assertEquals(Set.of(baseCoordinates, jvmCoordinates), result.getResolution().nodes()); + + // The umbrella is an aggregator with no binary; the JVM sibling carries the real JAR. + ResolvedArtifact base = result.getArtifacts().get(baseCoordinates); + assertTrue("Umbrella should be an aggregator", base.isAggregator()); + assertTrue("Umbrella should have no binary path", base.getPath().isEmpty()); + + ResolvedArtifact jvm = result.getArtifacts().get(jvmCoordinates); + assertFalse("JVM sibling should not be an aggregator", jvm.isAggregator()); + assertTrue("JVM sibling should have a binary path", jvm.getPath().isPresent()); + + // Feed the resolver's findings to a Downloader the way AbstractMain does, and confirm the + // umbrella downloads to a binary-less result while the sibling downloads its real JAR. + Map knownPaths = + result.getArtifacts().values().stream() + .filter(artifact -> artifact.getPath().isPresent()) + .collect( + Collectors.toMap( + ResolvedArtifact::getCoordinates, artifact -> artifact.getPath().get())); + Set aggregatingCoordinates = + result.getArtifacts().values().stream() + .filter(ResolvedArtifact::isAggregator) + .map(ResolvedArtifact::getCoordinates) + .collect(Collectors.toSet()); + + Path localRepo = Files.createTempDirectory("local"); + Downloader downloader = + new Downloader( + Netrc.fromUserHome(), + localRepo, + Set.of(mavenRepo.getPath().toUri()), + new NullListener(), + false, + knownPaths, + aggregatingCoordinates); + + DownloadResult baseDownload = downloader.download(baseCoordinates); + assertTrue(baseDownload.getPath().isEmpty()); + assertTrue(baseDownload.getSha256().isEmpty()); + + DownloadResult jvmDownload = downloader.download(jvmCoordinates); + assertTrue(jvmDownload.getPath().isPresent()); + assertTrue(jvmDownload.getSha256().isPresent()); + } + @Test public void throwsAnExceptionIfASingleDependencyWasNotResolved() throws IOException { Coordinates validCoordinates = new Coordinates("com.example:sample:1.0"); diff --git a/tests/com/github/bazelbuild/rules_jvm_external/resolver/maven/DownloaderTest.java b/tests/com/github/bazelbuild/rules_jvm_external/resolver/maven/DownloaderTest.java index 79aecf731..8c2ed838b 100644 --- a/tests/com/github/bazelbuild/rules_jvm_external/resolver/maven/DownloaderTest.java +++ b/tests/com/github/bazelbuild/rules_jvm_external/resolver/maven/DownloaderTest.java @@ -30,6 +30,31 @@ import org.junit.Test; public class DownloaderTest { + @Test + public void downloaderTreatsAggregatingCoordinateAsNoBinary() throws IOException { + // A coordinate the resolver marked as an aggregator (no binary of its own) downloads to a + // binary-less result, even though its POM does not declare `pom`. + Coordinates coords = new Coordinates("com.example:sample:1.0"); + + // Write only the POM (no JAR) for the coordinate. + Path repo = MavenRepo.create().add(coords.setExtension("pom")).getPath(); + Path localRepo = Files.createTempDirectory("local"); + + DownloadResult downloadResult = + new Downloader( + Netrc.fromUserHome(), + localRepo, + Set.of(repo.toUri()), + new NullListener(), + false, + Map.of(), + Set.of(coords)) + .download(coords); + + assertTrue(downloadResult.getPath().isEmpty()); + assertTrue(downloadResult.getSha256().isEmpty()); + } + @Test public void downloaderHandleUndeclaredCharacterEntityInPOM() throws IOException { Coordinates coords = new Coordinates("com.example:characterentity:1.0");