diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index e8cce3e82954c0..344c1c0a0e7363 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -2917,7 +2917,7 @@ "bzlTransitiveDigest": "B5uVRXNnUrnGOBQizaLP3KiRFVjh+IMpW7klQTyeTmI=", "recordedFileInputs": { "@@//MODULE.bazel": "eba5503742af5785c2d0d81d88e7407c7f23494b5162c055227435549b8774d1", - "@@//src/test/tools/bzlmod/MODULE.bazel.lock": "d929c96495dfaffb6006d522f8f0f94472cf8f07158e0ad042fdb6defc79190a" + "@@//src/test/tools/bzlmod/MODULE.bazel.lock": "a846e4f8773e2c329a753afb9a85fd4449dbf651024594652b7aceb74edbea44" }, "recordedDirentsInputs": {}, "envVariables": {}, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java index 1f03ae84de249f..3282f26404a093 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java @@ -117,9 +117,10 @@ public void afterCommand() { // lockfile that are still up-to-date and adding the newly resolved extension results. BazelLockFileValue newLockfile = BazelLockFileValue.builder() - .setModuleExtensions(combinedExtensionInfos) .setRegistryFileHashes( ImmutableSortedMap.copyOf(moduleResolutionValue.getRegistryFileHashes())) + .setYankedButAllowedModules(moduleResolutionValue.getYankedButAllowedModules()) + .setModuleExtensions(combinedExtensionInfos) .build(); // Write the new value to the file, but only if needed. This is not just a performance diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java index c3c1154b0638bb..35a1db90b8a96d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java @@ -17,6 +17,7 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.bazel.repository.downloader.Checksum; import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; import com.google.devtools.build.lib.skyframe.SkyFunctions; @@ -43,6 +44,7 @@ static Builder builder() { return new AutoValue_BazelLockFileValue.Builder() .setLockFileVersion(LOCK_FILE_VERSION) .setRegistryFileHashes(ImmutableMap.of()) + .setYankedButAllowedModules(ImmutableSet.of()) .setModuleExtensions(ImmutableMap.of()); } @@ -52,6 +54,9 @@ static Builder builder() { /** Hashes of files retrieved from registries. */ public abstract ImmutableMap> getRegistryFileHashes(); + /** Module versions that are known to be yanked but were explicitly allowed by the user. */ + public abstract ImmutableSet getYankedButAllowedModules(); + /** Mapping the extension id to the module extension data */ public abstract ImmutableMap< ModuleExtensionId, ImmutableMap> @@ -66,6 +71,8 @@ public abstract static class Builder { public abstract Builder setRegistryFileHashes(ImmutableMap> value); + public abstract Builder setYankedButAllowedModules(ImmutableSet value); + public abstract Builder setModuleExtensions( ImmutableMap< ModuleExtensionId, 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 f217bd86a53ec0..4c507d93e329da 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 @@ -68,7 +68,8 @@ public class BazelModuleResolutionFunction implements SkyFunction { private record Result( Selection.Result selectionResult, - ImmutableMap> registryFileHashes) {} + ImmutableMap> registryFileHashes, + ImmutableSet yankedButAllowedModules) {} private static class ModuleResolutionComputeState implements Environment.SkyKeyComputeState { Result discoverAndSelectResult; @@ -142,7 +143,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) return BazelModuleResolutionValue.create( finalDepGraph, state.discoverAndSelectResult.selectionResult.getUnprunedDepGraph(), - ImmutableMap.copyOf(registryFileHashes)); + ImmutableMap.copyOf(registryFileHashes), + state.discoverAndSelectResult.yankedButAllowedModules); } @Nullable @@ -206,12 +208,15 @@ private static Result discoverAndSelect( env.getListener()); } + ImmutableSet yankedButAllowedModules; try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.BZLMOD, "check no yanked versions")) { - checkNoYankedVersions(resolvedDepGraph, yankedVersionValues, allowedYankedVersions); + yankedButAllowedModules = + checkNoYankedVersions(resolvedDepGraph, yankedVersionValues, allowedYankedVersions); } - return new Result(selectionResult, discoveryResult.registryFileHashes()); + return new Result( + selectionResult, discoveryResult.registryFileHashes(), yankedButAllowedModules); } private static void verifyAllOverridesAreOnExistentModules( @@ -305,36 +310,53 @@ public static void checkBazelCompatibility( } } - private static void checkNoYankedVersions( + /** + * Fail if any selected module is yanked and not explicitly allowed. + * + * @return the set of yanked but explicitly allowed modules + */ + private static ImmutableSet checkNoYankedVersions( ImmutableMap depGraph, ImmutableMap yankedVersionValues, Optional> allowedYankedVersions) throws BazelModuleResolutionFunctionException { + ImmutableSet.Builder yankedButAllowedModules = ImmutableSet.builder(); for (InterimModule m : depGraph.values()) { if (m.getRegistry() == null) { // Non-registry modules do not have yanked versions. continue; } - Optional yankedInfo = - YankedVersionsUtil.getYankedInfo( - m.getKey(), - yankedVersionValues.get( - YankedVersionsValue.Key.create(m.getKey(), m.getRegistry().getUrl())), - allowedYankedVersions); - if (yankedInfo.isPresent()) { - throw new BazelModuleResolutionFunctionException( - ExternalDepsException.withMessage( - Code.VERSION_RESOLUTION_ERROR, - "Yanked version detected in your resolved dependency graph: %s, for the reason: " - + "%s.\nYanked versions may contain serious vulnerabilities and should not be " - + "used. To fix this, use a bazel_dep on a newer version of this module. To " - + "continue using this version, allow it using the --allow_yanked_versions " - + "flag or the BZLMOD_ALLOW_YANKED_VERSIONS env variable.", - m.getKey(), - yankedInfo.get()), - Transience.PERSISTENT); + ModuleKey key = m.getKey(); + YankedVersionsValue yankedVersionsValue = + yankedVersionValues.get( + YankedVersionsValue.Key.create(m.getKey(), m.getRegistry().getUrl())); + if (yankedVersionsValue.yankedVersions().isEmpty()) { + // No yanked version information available or no need to check it. + continue; + } + String yankedInfo = yankedVersionsValue.yankedVersions().get().get(key.getVersion()); + if (yankedInfo == null) { + // The version is not yanked. + continue; + } + if (allowedYankedVersions.isEmpty() || allowedYankedVersions.get().contains(key)) { + // The version is yanked but explicitly allowed. + yankedButAllowedModules.add(key); + continue; } + throw new BazelModuleResolutionFunctionException( + ExternalDepsException.withMessage( + Code.VERSION_RESOLUTION_ERROR, + "Yanked version detected in your resolved dependency graph: %s, for the reason: " + + "%s.\nYanked versions may contain serious vulnerabilities and should not be " + + "used. To fix this, use a bazel_dep on a newer version of this module. To " + + "continue using this version, allow it using the --allow_yanked_versions " + + "flag or the BZLMOD_ALLOW_YANKED_VERSIONS env variable.", + m.getKey(), + yankedInfo), + Transience.PERSISTENT); } + return yankedButAllowedModules.build(); } private static ImmutableMap computeFinalDepGraph( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionValue.java index 3cfc7947f928fb..af6a0fd3f98288 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionValue.java @@ -17,6 +17,7 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.bazel.repository.downloader.Checksum; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant; @@ -50,11 +51,15 @@ abstract class BazelModuleResolutionValue implements SkyValue { */ abstract ImmutableMap> getRegistryFileHashes(); + /** Module versions that are known to be yanked but were explicitly allowed by the user. */ + abstract ImmutableSet getYankedButAllowedModules(); + static BazelModuleResolutionValue create( ImmutableMap resolvedDepGraph, ImmutableMap unprunedDepGraph, - ImmutableMap> registryFileHashes) { + ImmutableMap> registryFileHashes, + ImmutableSet yankedButAllowedModules) { return new AutoValue_BazelModuleResolutionValue( - resolvedDepGraph, unprunedDepGraph, registryFileHashes); + resolvedDepGraph, unprunedDepGraph, registryFileHashes, yankedButAllowedModules); } } 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 e5c3ba9639c9d3..f3748976766dfb 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 @@ -21,6 +21,7 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.bazel.bzlmod.Version.ParseException; import com.google.devtools.build.lib.bazel.repository.downloader.Checksum; import com.google.devtools.build.lib.bazel.repository.downloader.Checksum.MissingChecksumException; @@ -44,7 +45,6 @@ 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 @@ -69,6 +69,7 @@ public enum KnownFileHashesMode { private final Map clientEnv; private final Gson gson; private final ImmutableMap> knownFileHashes; + private final ImmutableSet yankedButAllowedModules; private final KnownFileHashesMode knownFileHashesMode; private volatile Optional bazelRegistryJson; private volatile StoredEventHandler bazelRegistryJsonEvents; @@ -80,7 +81,8 @@ public IndexRegistry( DownloadManager downloadManager, Map clientEnv, ImmutableMap> knownFileHashes, - KnownFileHashesMode knownFileHashesMode) { + KnownFileHashesMode knownFileHashesMode, + ImmutableSet yankedButAllowedModules) { this.uri = uri; this.downloadManager = downloadManager; this.clientEnv = clientEnv; @@ -90,6 +92,7 @@ public IndexRegistry( .create(); this.knownFileHashes = knownFileHashes; this.knownFileHashesMode = knownFileHashesMode; + this.yankedButAllowedModules = yankedButAllowedModules; } @Override @@ -436,21 +439,16 @@ public Optional> getYankedVersions( } @Override - public boolean shouldFetchYankedVersions( - ModuleKey selectedModuleKey, Predicate fileHashIsKnown) { + public boolean shouldFetchYankedVersions(ModuleKey selectedModuleKey) { // If the source.json hash is known, this module has been selected before when selection - // succeeded, which means that - // either: + // 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 + // 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)); + // BZLMOD_ALLOW_YANKED_VERSIONS. + // In the first case, we don't fetch yanked versions. + return yankedButAllowedModules.contains(selectedModuleKey) + || !knownFileHashes.containsKey(getSourceJsonUrl(selectedModuleKey)); } /** Represents fields available in {@code metadata.json} for each module. */ 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 93394eb9d3d52b..66fb129cb73c94 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,7 +20,6 @@ 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 { @@ -52,8 +51,7 @@ Optional> getYankedVersions( /** * 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. + * module contained in the post-selection dependency graph. */ - boolean shouldFetchYankedVersions(ModuleKey selectedModuleKey, Predicate fileHashIsKnown); + boolean shouldFetchYankedVersions(ModuleKey selectedModuleKey); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactory.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactory.java index b49d80092d98d0..72cd8a9a84c4ca 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactory.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactory.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.bazel.bzlmod; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions; import com.google.devtools.build.lib.bazel.repository.downloader.Checksum; import java.net.URISyntaxException; @@ -32,6 +33,7 @@ public interface RegistryFactory { Registry createRegistry( String url, ImmutableMap> fileHashes, - RepositoryOptions.LockfileMode lockfileMode) + RepositoryOptions.LockfileMode lockfileMode, + ImmutableSet yankedButAllowedModules) throws URISyntaxException; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java index 16d833bbb1d2d8..0614a76dc0a47b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.bazel.bzlmod; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.bazel.bzlmod.IndexRegistry.KnownFileHashesMode; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; import com.google.devtools.build.lib.bazel.repository.downloader.Checksum; @@ -41,7 +42,8 @@ public RegistryFactoryImpl( public Registry createRegistry( String url, ImmutableMap> knownFileHashes, - LockfileMode lockfileMode) + LockfileMode lockfileMode, + ImmutableSet yankedButAllowedModules) throws URISyntaxException { URI uri = new URI(url); if (uri.getScheme() == null) { @@ -71,6 +73,7 @@ public Registry createRegistry( downloadManager, clientEnvironmentSupplier.get(), knownFileHashes, - knownFileHashesMode); + knownFileHashesMode, + yankedButAllowedModules); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFunction.java index 10a85b8ef54bc7..23284474187b81 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFunction.java @@ -51,7 +51,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) return registryFactory.createRegistry( key.getUrl().replace("%workspace%", workspaceRoot.getPathString()), lockfile.getRegistryFileHashes(), - lockfileMode); + lockfileMode, + lockfile.getYankedButAllowedModules()); } catch (URISyntaxException e) { throw new RegistryException( ExternalDepsException.withCauseAndMessage( 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 a61b15ac40b7c5..2cb2ae725dab7d 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 @@ -46,8 +46,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept return null; } - if (!registry.shouldFetchYankedVersions( - key.getModuleKey(), lockfile.getRegistryFileHashes()::containsKey)) { + if (!registry.shouldFetchYankedVersions(key.getModuleKey())) { return YankedVersionsValue.create(Optional.empty()); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/YankedVersionsUtil.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/YankedVersionsUtil.java index 8cfcd32c37a0b0..ac98fb8fe0dcd0 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/YankedVersionsUtil.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/YankedVersionsUtil.java @@ -59,25 +59,6 @@ static Optional> parseAllowedYankedVersions( return Optional.of(allowedYankedVersionBuilder.build()); } - static Optional getYankedInfo( - ModuleKey key, - YankedVersionsValue yankedVersionsValue, - Optional> allowedYankedVersions) { - return yankedVersionsValue - .yankedVersions() - .flatMap( - yankedVersions -> { - String yankedInfo = yankedVersions.get(key.getVersion()); - if (yankedInfo != null - && allowedYankedVersions.isPresent() - && !allowedYankedVersions.get().contains(key)) { - return Optional.of(yankedInfo); - } else { - return Optional.empty(); - } - }); - } - /** * Parse of a comma-separated list of module version(s) of the form '@' or * 'all' from the string. Returns true if 'all' is present, otherwise returns false. diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java index 545e44e0278141..807c5f04f4c21d 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java @@ -366,7 +366,8 @@ public void setDepGraph(ImmutableMap depGraph) { @Override @Nullable public SkyValue compute(SkyKey skyKey, Environment env) { - return BazelModuleResolutionValue.create(depGraph, ImmutableMap.of(), ImmutableMap.of()); + return BazelModuleResolutionValue.create( + depGraph, ImmutableMap.of(), ImmutableMap.of(), ImmutableSet.of()); } } } 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 4e41b66034aea5..7ebe2524b42d4a 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 @@ -20,6 +20,7 @@ import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; import com.google.devtools.build.lib.bazel.repository.downloader.Checksum; import com.google.devtools.build.lib.events.ExtendedEventHandler; @@ -28,7 +29,6 @@ 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. @@ -102,12 +102,8 @@ public Optional> getYankedVersions( } @Override - public boolean shouldFetchYankedVersions( - ModuleKey selectedModuleKey, Predicate fileHashIsKnown) { - return !fileHashIsKnown.test( - "%s/modules/%s/%s/source.json" - .formatted( - url, selectedModuleKey.getName(), selectedModuleKey.getVersion().toString())); + public boolean shouldFetchYankedVersions(ModuleKey selectedModuleKey) { + return true; } @Override @@ -140,7 +136,8 @@ public FakeRegistry newFakeRegistry(String rootPath) { public Registry createRegistry( String url, ImmutableMap> fileHashes, - LockfileMode lockfileMode) { + LockfileMode lockfileMode, + ImmutableSet yankedButAllowedModules) { return Preconditions.checkNotNull(registries.get(url), "unknown registry url: %s", url); } } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java index b5ad0dffaca4f6..a8ed7b48406e0e 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java @@ -23,6 +23,7 @@ import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import com.google.common.eventbus.Subscribe; import com.google.common.hash.Hashing; @@ -97,7 +98,7 @@ public void testHttpUrl() throws Exception { Registry registry = registryFactory.createRegistry( - server.getUrl() + "/myreg", ImmutableMap.of(), LockfileMode.UPDATE); + server.getUrl() + "/myreg", ImmutableMap.of(), LockfileMode.UPDATE, ImmutableSet.of()); assertThat( registry.getModuleFile( createModuleKey("foo", "1.0"), reporter)) @@ -120,7 +121,7 @@ public void testHttpUrlWithNetrcCreds() throws Exception { "machine [::1] login rinne password rinnepass\n".getBytes(UTF_8))); Registry registry = registryFactory.createRegistry( - server.getUrl() + "/myreg", ImmutableMap.of(), LockfileMode.UPDATE); + server.getUrl() + "/myreg", ImmutableMap.of(), LockfileMode.UPDATE, ImmutableSet.of()); var e = assertThrows( @@ -157,7 +158,8 @@ public void testFileUrl() throws Exception { registryFactory.createRegistry( new File(tempFolder.getRoot(), "fakereg").toURI().toString(), ImmutableMap.of(), - LockfileMode.UPDATE); + LockfileMode.UPDATE, + ImmutableSet.of()); assertThat( registry.getModuleFile( createModuleKey("foo", "1.0"), reporter)) @@ -199,7 +201,8 @@ public void testGetArchiveRepoSpec() throws Exception { server.start(); Registry registry = - registryFactory.createRegistry(server.getUrl(), ImmutableMap.of(), LockfileMode.UPDATE); + registryFactory.createRegistry( + server.getUrl(), ImmutableMap.of(), LockfileMode.UPDATE, ImmutableSet.of()); assertThat( registry.getRepoSpec( createModuleKey("foo", "1.0"), reporter)) @@ -248,7 +251,8 @@ public void testGetLocalPathRepoSpec() throws Exception { server.start(); Registry registry = - registryFactory.createRegistry(server.getUrl(), ImmutableMap.of(), LockfileMode.UPDATE); + registryFactory.createRegistry( + server.getUrl(), ImmutableMap.of(), LockfileMode.UPDATE, ImmutableSet.of()); assertThat( registry.getRepoSpec( createModuleKey("foo", "1.0"), reporter)) @@ -273,7 +277,8 @@ public void testGetRepoInvalidRegistryJsonSpec() throws Exception { "}"); Registry registry = - registryFactory.createRegistry(server.getUrl(), ImmutableMap.of(), LockfileMode.UPDATE); + registryFactory.createRegistry( + server.getUrl(), ImmutableMap.of(), LockfileMode.UPDATE, ImmutableSet.of()); assertThat( registry.getRepoSpec( createModuleKey("foo", "1.0"), reporter)) @@ -307,7 +312,8 @@ public void testGetRepoInvalidModuleJsonSpec() throws Exception { server.start(); Registry registry = - registryFactory.createRegistry(server.getUrl(), ImmutableMap.of(), LockfileMode.UPDATE); + registryFactory.createRegistry( + server.getUrl(), ImmutableMap.of(), LockfileMode.UPDATE, ImmutableSet.of()); assertThrows( IOException.class, () -> @@ -338,7 +344,8 @@ public void testGetYankedVersion() throws Exception { + "}"); server.start(); Registry registry = - registryFactory.createRegistry(server.getUrl(), ImmutableMap.of(), LockfileMode.UPDATE); + registryFactory.createRegistry( + server.getUrl(), ImmutableMap.of(), LockfileMode.UPDATE, ImmutableSet.of()); Optional> yankedVersion = registry.getYankedVersions("red-pill", reporter); assertThat(yankedVersion) @@ -360,7 +367,8 @@ public void testArchiveWithExplicitType() throws Exception { server.start(); Registry registry = - registryFactory.createRegistry(server.getUrl(), ImmutableMap.of(), LockfileMode.UPDATE); + registryFactory.createRegistry( + server.getUrl(), ImmutableMap.of(), LockfileMode.UPDATE, ImmutableSet.of()); assertThat( registry.getRepoSpec( createModuleKey("archive_type", "1.0"), @@ -392,7 +400,8 @@ public void testGetModuleFileChecksums() throws Exception { server.getUrl() + "/myreg/modules/unused/1.0/MODULE.bazel", Optional.of(sha256("unused"))); Registry registry = - registryFactory.createRegistry(server.getUrl() + "/myreg", knownFiles, LockfileMode.UPDATE); + registryFactory.createRegistry( + server.getUrl() + "/myreg", knownFiles, LockfileMode.UPDATE, ImmutableSet.of()); assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter)) .hasValue( ModuleFile.create( @@ -418,7 +427,7 @@ public void testGetModuleFileChecksums() throws Exception { registry = registryFactory.createRegistry( - server.getUrl() + "/myreg", recordedChecksums, LockfileMode.UPDATE); + server.getUrl() + "/myreg", recordedChecksums, LockfileMode.UPDATE, ImmutableSet.of()); // Test that the recorded hashes are used for repo cache hits even when the server content // changes. server.unserve("/myreg/modules/foo/1.0/MODULE.bazel"); @@ -447,7 +456,8 @@ public void testGetModuleFileChecksumMismatch() throws Exception { server.getUrl() + "/myreg/modules/foo/1.0/MODULE.bazel", Optional.of(sha256("original"))); Registry registry = - registryFactory.createRegistry(server.getUrl() + "/myreg", knownFiles, LockfileMode.UPDATE); + registryFactory.createRegistry( + server.getUrl() + "/myreg", knownFiles, LockfileMode.UPDATE, ImmutableSet.of()); var e = assertThrows( IOException.class, @@ -487,7 +497,8 @@ public void testGetRepoSpecChecksum() throws Exception { ImmutableMap.of( server.getUrl() + "/modules/foo/2.0/source.json", Optional.of(sha256("unused"))); Registry registry = - registryFactory.createRegistry(server.getUrl(), knownFiles, LockfileMode.UPDATE); + registryFactory.createRegistry( + server.getUrl(), knownFiles, LockfileMode.UPDATE, ImmutableSet.of()); assertThat(registry.getRepoSpec(createModuleKey("foo", "1.0"), reporter)) .isEqualTo( RepoSpec.builder() @@ -506,7 +517,8 @@ public void testGetRepoSpecChecksum() throws Exception { Optional.of(sha256(sourceJson).toString())); registry = - registryFactory.createRegistry(server.getUrl(), recordedChecksums, LockfileMode.UPDATE); + registryFactory.createRegistry( + server.getUrl(), recordedChecksums, LockfileMode.UPDATE, ImmutableSet.of()); // Test that the recorded hashes are used for repo cache hits even when the server content // changes. server.unserve("/bazel_registry.json"); @@ -549,7 +561,8 @@ public void testGetRepoSpecChecksumMismatch() throws Exception { server.getUrl() + "/modules/foo/1.0/source.json", Optional.of(sha256(sourceJson))); Registry registry = - registryFactory.createRegistry(server.getUrl(), knownFiles, LockfileMode.UPDATE); + registryFactory.createRegistry( + server.getUrl(), knownFiles, LockfileMode.UPDATE, ImmutableSet.of()); var e = assertThrows( IOException.class, () -> registry.getRepoSpec(createModuleKey("foo", "1.0"), reporter)); @@ -592,7 +605,8 @@ public void testBazelRegistryChecksumMismatch() throws Exception { server.getUrl() + "/modules/foo/1.0/source.json", Optional.of(sha256(sourceJson))); Registry registry = - registryFactory.createRegistry(server.getUrl(), knownFiles, LockfileMode.UPDATE); + registryFactory.createRegistry( + server.getUrl(), knownFiles, LockfileMode.UPDATE, ImmutableSet.of()); var e = assertThrows( IOException.class, () -> registry.getRepoSpec(createModuleKey("foo", "1.0"), reporter)); diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryTest.java index db975081459785..c6dc33b675f2a1 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryTest.java @@ -20,6 +20,7 @@ import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache; import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; @@ -44,14 +45,14 @@ public void badSchemes() { URISyntaxException.class, () -> registryFactory.createRegistry( - "/home/www", ImmutableMap.of(), LockfileMode.UPDATE)); + "/home/www", ImmutableMap.of(), LockfileMode.UPDATE, ImmutableSet.of())); assertThat(exception).hasMessageThat().contains("Registry URL has no scheme"); exception = assertThrows( URISyntaxException.class, () -> registryFactory.createRegistry( - "foo://bar", ImmutableMap.of(), LockfileMode.UPDATE)); + "foo://bar", ImmutableMap.of(), LockfileMode.UPDATE, ImmutableSet.of())); assertThat(exception).hasMessageThat().contains("Unrecognized registry URL protocol"); } @@ -66,7 +67,10 @@ public void badPath() { URISyntaxException.class, () -> registryFactory.createRegistry( - "file:c:/path/to/workspace/registry", ImmutableMap.of(), LockfileMode.UPDATE)); + "file:c:/path/to/workspace/registry", + ImmutableMap.of(), + LockfileMode.UPDATE, + ImmutableSet.of())); assertThat(exception).hasMessageThat().contains("Registry URL path is not valid"); } } diff --git a/src/test/py/bazel/bzlmod/bazel_yanked_versions_test.py b/src/test/py/bazel/bzlmod/bazel_yanked_versions_test.py index 352e7b948edd02..51f80ef4212155 100644 --- a/src/test/py/bazel/bzlmod/bazel_yanked_versions_test.py +++ b/src/test/py/bazel/bzlmod/bazel_yanked_versions_test.py @@ -16,7 +16,9 @@ import os import tempfile + from absl.testing import absltest + from src.test.py.bazel import test_base from src.test.py.bazel.bzlmod.test_utils import BazelRegistry @@ -40,6 +42,8 @@ def setUp(self): 'ddd', '1.0', {'yanked1': '1.0', 'yanked2': '1.0'} ).createCcModule( 'eee', '1.0', {'yanked1': '1.0'} + ).createCcModule( + 'fff', '1.0' ).createCcModule( 'yanked1', '1.0' ).createCcModule( @@ -227,6 +231,64 @@ def testAllowedYankedDepsSuccessMix(self): env_add={'BZLMOD_ALLOW_YANKED_VERSIONS': 'yanked2@1.0'}, ) + def testYankedVersionsFetchedIncrementally(self): + self.writeBazelrcFile(allow_yanked_versions=False) + self.ScratchFile( + 'MODULE.bazel', + [ + 'bazel_dep(name = "aaa", version = "1.0")', + ], + ) + self.ScratchFile( + 'BUILD', + [ + 'cc_binary(', + ' name = "main",', + ' srcs = ["main.cc"],', + ' deps = ["@aaa//:lib_aaa"],', + ')', + ], + ) + self.RunBazel(['build', '--nobuild', '//:main']) + + # Yank aaa@1.0 and aaa@1.1. + self.main_registry.addMetadata('aaa', yanked_versions={'1.0': 'already dodgy', '1.1': 'still dodgy'}) + + # Without any changes, both a cold and a warm build still pass. + self.RunBazel(['build', '--nobuild', '//:main']) + self.RunBazel(['shutdown']) + self.RunBazel(['build', '--nobuild', '//:main']) + + # Adding an unrelated dependency should not cause yanked versions to be fetched again. + self.ScratchFile( + 'MODULE.bazel', + [ + 'bazel_dep(name = "aaa", version = "1.0")', + 'bazel_dep(name = "fff", version = "1.0")', + ], + ) + self.RunBazel(['build', '--nobuild', '//:main']) + self.RunBazel(['shutdown']) + self.RunBazel(['build', '--nobuild', '//:main']) + + # If a new version of aaa is selected, yanked versions should be fetched again. + self.ScratchFile( + 'MODULE.bazel', + [ + 'bazel_dep(name = "aaa", version = "1.0")', + 'bazel_dep(name = "fff", version = "1.0")', + # Depends on aaa@1.1. + 'bazel_dep(name = "bbb", version = "1.1")', + ], + ) + exit_code, _, stderr = self.RunBazel(['build', '--nobuild', '//:main'], allow_failure=True) + self.AssertExitCode(exit_code, 48, stderr) + self.assertIn( + 'Yanked version detected in your resolved dependency graph: ' + + 'aaa@1.1, for the reason: still dodgy.', + ''.join(stderr), + ) + if __name__ == '__main__': absltest.main() diff --git a/src/test/tools/bzlmod/MODULE.bazel.lock b/src/test/tools/bzlmod/MODULE.bazel.lock index 4845b95e8d933e..9550cfb447b68e 100644 --- a/src/test/tools/bzlmod/MODULE.bazel.lock +++ b/src/test/tools/bzlmod/MODULE.bazel.lock @@ -57,6 +57,7 @@ "https://bcr.bazel.build/modules/zlib/1.3/MODULE.bazel": "6a9c02f19a24dcedb05572b2381446e27c272cd383aed11d41d99da9e3167a72", "https://bcr.bazel.build/modules/zlib/1.3/source.json": "b6b43d0737af846022636e6e255fd4a96fee0d34f08f3830e6e0bac51465c37c" }, + "yankedButAllowedModules": [], "moduleExtensions": { "@@apple_support~//crosstool:setup.bzl%apple_cc_configure_extension": { "general": {