diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java index 9d863d9f677596..f217bd86a53ec0 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java @@ -174,7 +174,7 @@ private static Result discoverAndSelect( var yankedVersionsKeys = resolvedDepGraph.values().stream() .filter(m -> m.getRegistry() != null) - .map(m -> YankedVersionsValue.Key.create(m.getName(), m.getRegistry().getUrl())) + .map(m -> YankedVersionsValue.Key.create(m.getKey(), m.getRegistry().getUrl())) .collect(toImmutableSet()); SkyframeLookupResult yankedVersionsResult = env.getValuesAndExceptions(yankedVersionsKeys); if (env.valuesMissing()) { @@ -319,7 +319,7 @@ private static void checkNoYankedVersions( YankedVersionsUtil.getYankedInfo( m.getKey(), yankedVersionValues.get( - YankedVersionsValue.Key.create(m.getName(), m.getRegistry().getUrl())), + YankedVersionsValue.Key.create(m.getKey(), m.getRegistry().getUrl())), allowedYankedVersions); if (yankedInfo.isPresent()) { throw new BazelModuleResolutionFunctionException( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java index 1424320d0be29a..e76ba5c80a8fb8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java @@ -44,6 +44,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Optional; +import java.util.function.Predicate; /** * Represents a Bazel module registry that serves a list of module metadata from a static HTTP @@ -254,13 +255,7 @@ private T parseJson(String jsonString, String url, Class klass) throws IO @Override public RepoSpec getRepoSpec(ModuleKey key, ExtendedEventHandler eventHandler) throws IOException, InterruptedException { - String jsonUrl = - constructUrl( - uri.toString(), - "modules", - key.getName(), - key.getVersion().toString(), - SOURCE_JSON_FILENAME); + String jsonUrl = getSourceJsonUrl(key); Optional jsonString = grabJsonFile(jsonUrl, eventHandler, /* useChecksum= */ true); if (jsonString.isEmpty()) { throw new FileNotFoundException( @@ -292,7 +287,11 @@ public RepoSpec getRepoSpec(ModuleKey key, ExtendedEventHandler eventHandler) } } - @SuppressWarnings("OptionalAssignedToNull") + private String getSourceJsonUrl(ModuleKey key) { + return constructUrl( + getUrl(), "modules", key.getName(), key.getVersion().toString(), SOURCE_JSON_FILENAME); + } + private Optional getBazelRegistryJson(ExtendedEventHandler eventHandler) throws IOException, InterruptedException { if (bazelRegistryJson == null) { @@ -440,6 +439,24 @@ public Optional> getYankedVersions( } } + @Override + public boolean shouldFetchYankedVersions( + ModuleKey selectedModuleKey, Predicate fileHashIsKnown) { + // If the source.json hash is known, this module has been selected before when selection + // succeeded, which means that + // either: + // * it wasn't yanked at that point in time and any successful selection since then has not seen + // a higher module + // version, or + // * it was yanked at that point in time, but explicitly allowed via + // BZLMOD_ALLOW_YANKED_VERSIONS. + // In both cases, we don't fetch yanked versions. + // TODO: Should we store whether we are in the second case and refetch yanked versions if the + // environment variable + // changes? + return !fileHashIsKnown.test(getSourceJsonUrl(selectedModuleKey)); + } + /** Represents fields available in {@code metadata.json} for each module. */ static class MetadataJson { // There are other attributes in the metadata.json file, but for now, we only care about diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Registry.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Registry.java index 619149d7621a36..93394eb9d3d52b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Registry.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Registry.java @@ -20,6 +20,7 @@ import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; import java.util.Optional; +import java.util.function.Predicate; /** A database where module metadata is stored. */ public interface Registry extends SkyValue { @@ -48,4 +49,11 @@ RepoSpec getRepoSpec(ModuleKey key, ExtendedEventHandler eventHandler) Optional> getYankedVersions( String moduleName, ExtendedEventHandler eventHandler) throws IOException, InterruptedException; + + /** + * Returns whether the (mutable) yanked versions should be fetched from the registry for the given + * module contained in the post-selection dependency graph, given the knowledge of certain + * registry file hashes contained in the lockfile. + */ + boolean shouldFetchYankedVersions(ModuleKey selectedModuleKey, Predicate fileHashIsKnown); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/YankedVersionsFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/YankedVersionsFunction.java index 3680413410d092..6420600709c0e2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/YankedVersionsFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/YankedVersionsFunction.java @@ -37,17 +37,27 @@ public class YankedVersionsFunction implements SkyFunction { public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { var key = (YankedVersionsValue.Key) skyKey.argument(); + BazelLockFileValue lockfile = (BazelLockFileValue) env.getValue(BazelLockFileValue.KEY); + if (lockfile == null) { + return null; + } Registry registry = (Registry) env.getValue(RegistryKey.create(key.getRegistryUrl())); if (registry == null) { return null; } + if (!registry.shouldFetchYankedVersions( + key.getModuleKey(), lockfile.getRegistryFileHashes()::containsKey)) { + return YankedVersionsValue.create(Optional.empty()); + } + try (SilentCloseable c = Profiler.instance() .profile( - ProfilerTask.BZLMOD, () -> "getting yanked versions: " + key.getModuleName())) { + ProfilerTask.BZLMOD, + () -> "getting yanked versions: " + key.getModuleKey().getName())) { return YankedVersionsValue.create( - registry.getYankedVersions(key.getModuleName(), env.getListener())); + registry.getYankedVersions(key.getModuleKey().getName(), env.getListener())); } catch (IOException e) { env.getListener() .handle( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/YankedVersionsValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/YankedVersionsValue.java index d3dd5f4ab52532..0071ca54c76a1d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/YankedVersionsValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/YankedVersionsValue.java @@ -39,13 +39,13 @@ public static YankedVersionsValue create(Optional> abstract static class Key implements SkyKey { private static final SkyKeyInterner interner = SkyKey.newInterner(); - abstract String getModuleName(); + abstract ModuleKey getModuleKey(); abstract String getRegistryUrl(); @AutoCodec.Instantiator - static Key create(String moduleName, String registryUrl) { - return interner.intern(new AutoValue_YankedVersionsValue_Key(moduleName, registryUrl)); + static Key create(ModuleKey moduleKey, String registryUrl) { + return interner.intern(new AutoValue_YankedVersionsValue_Key(moduleKey, registryUrl)); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/FakeRegistry.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/FakeRegistry.java index 5a775e4748adb3..f1087f0882346b 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/FakeRegistry.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/FakeRegistry.java @@ -28,6 +28,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.function.Predicate; /** * Fake implementation of {@link Registry}, where modules can be freely added and stored in memory. @@ -99,6 +100,15 @@ public Optional> getYankedVersions( return Optional.ofNullable(yankedVersionMap.get(moduleName)); } + @Override + public boolean shouldFetchYankedVersions( + ModuleKey selectedModuleKey, Predicate fileHashIsKnown) { + return !fileHashIsKnown.test( + "%s/modules/%s/%s/source.json" + .formatted( + url, selectedModuleKey.getName(), selectedModuleKey.getVersion().toString())); + } + @Override public boolean equals(Object other) { return other instanceof FakeRegistry fakeRegistry