Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -28,15 +27,15 @@ public class ResolutionResult {

private final Graph<Coordinates> resolution;
private final Set<Conflict> conflicts;
private final Map<Coordinates, Path> paths;
private final Map<Coordinates, ResolvedArtifact> artifacts;

public ResolutionResult(
Graph<Coordinates> resolution,
Set<Conflict> conflicts,
Map<Coordinates, Path> artifactPaths) {
Map<Coordinates, ResolvedArtifact> 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<Coordinates> getResolution() {
Expand All @@ -47,7 +46,7 @@ public Set<Conflict> getConflicts() {
return conflicts;
}

public Map<Coordinates, Path> getPaths() {
return paths;
public Map<Coordinates, ResolvedArtifact> getArtifacts() {
return artifacts;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// 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.
*
* <p>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
* <packaging>pom</packaging>} 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> 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() {
return coordinates;
}

public Optional<Path> 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) {
return true;
}
if (!(o instanceof ResolvedArtifact)) {
return false;
}
ResolvedArtifact that = (ResolvedArtifact) o;
return aggregator == that.aggregator
&& coordinates.equals(that.coordinates)
&& path.equals(that.path);
}

@Override
public int hashCode() {
return Objects.hash(coordinates, path, aggregator);
}

@Override
public String toString() {
return "ResolvedArtifact{"
+ coordinates
+ ", path="
+ path.orElse(null)
+ ", aggregator="
+ aggregator
+ "}";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -99,14 +100,27 @@ private static Set<DependencyInfo> fulfillDependencyInfos(
cacheResults = "1".equals(rjeUnsafeCache) || Boolean.parseBoolean(rjeUnsafeCache);
}

Map<Coordinates, Path> knownPaths = new LinkedHashMap<>();
Set<Coordinates> aggregatingCoordinates = new LinkedHashSet<>();
resolutionResult
.getArtifacts()
.forEach(
(coords, artifact) -> {
artifact.getPath().ifPresent(p -> knownPaths.put(coords, p));
if (artifact.isAggregator()) {
aggregatingCoordinates.add(coords);
}
});

Downloader downloader =
new Downloader(
config.getNetrc(),
request.getLocalCache(resolver.getName()),
request.getRepositories(),
listener,
cacheResults,
resolutionResult.getPaths());
knownPaths,
aggregatingCoordinates);

List<CompletableFuture<Set<DependencyInfo>>> futures = new LinkedList<>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -285,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<Coordinates> aggregatorNodes = new HashSet<>();
for (Map.Entry<Coordinates, List<GradleResolvedArtifact>> entry : artifactsByNode.entrySet()) {
Coordinates coords = entry.getKey();
if (!graph.nodes().contains(coords)) {
Expand All @@ -306,25 +311,37 @@ 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;
}
}
}
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);
}
}

// 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<Coordinates, ResolvedArtifact> artifacts = new HashMap<>();
for (Coordinates node : graph.nodes()) {
artifacts.put(
node, new ResolvedArtifact(node, paths.get(node), aggregatorNodes.contains(node)));
}

return new ResolutionResult(graph, conflicts, artifacts);
}

private String makeDepKey(String group, String artifact, String version) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -274,7 +275,13 @@ public ResolutionResult resolve(ResolutionRequest request) {
getConflicts(request.getDependencies(), resolvedDependencies),
graphNormalizationResult.getConflicts());

return new ResolutionResult(graphNormalizationResult.getNormalizedGraph(), conflicts, Map.of());
Graph<Coordinates> normalizedGraph = graphNormalizationResult.getNormalizedGraph();
Map<Coordinates, ResolvedArtifact> 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<Coordinates> dependencyGraph) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public class Downloader {
private final boolean cacheDownloads;
private final HttpDownloader httpDownloader;
private final Map<Coordinates, Path> knownPaths;
private final Set<Coordinates> aggregatingCoordinates;

public Downloader(
Netrc netrc,
Expand All @@ -63,11 +64,24 @@ public Downloader(
EventListener listener,
boolean cacheDownloads,
Map<Coordinates, Path> knownPaths) {
this(netrc, localRepository, repositories, listener, cacheDownloads, knownPaths, Set.of());
}

public Downloader(
Netrc netrc,
Path localRepository,
Collection<URI> repositories,
EventListener listener,
boolean cacheDownloads,
Map<Coordinates, Path> knownPaths,
Set<Coordinates> 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) {
Expand Down Expand Up @@ -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 `<packaging>pom</packaging>`. 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Coordinates, Path> paths = result.getPaths();
if (paths.containsKey(lowerVersion)) {
Path downloadedArtifact = paths.get(lowerVersion);
Map<Coordinates, ResolvedArtifact> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading