From 4f85017c9be976443518faf11b281f651ccb02f4 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Mon, 13 May 2024 09:18:57 -0700 Subject: [PATCH] Migrate to an incremental format for `MODULE.bazel.lock` The old lockfile fields and all code related to it as well as the workarounds for local path inclusions are removed. The distribution archive lockfile needs to be checked in temporarily as the main `MODULE.bazel.lock` file does not contain the hashes for the registry files. Fixes #19621 Fixes #20369 RELNOTES: The format for MODULE.bazel.lock is now less likely to result in merge conflicts and is updated incrementally, with only new files downloaded from registries and existing ones taken from the repository cache (if configured). Closes #22154. PiperOrigin-RevId: 633233519 Change-Id: Ie2c3042e4141a36e472b2c25cbd67be4aad096a1 --- .bazelci/presubmit.yml | 1 + MODULE.bazel | 3 + distdir.bzl | 6 +- extensions.bzl | 12 +- repositories.bzl | 4 +- .../lib/bazel/BazelRepositoryModule.java | 4 +- .../devtools/build/lib/bazel/bzlmod/BUILD | 29 +- .../bazel/bzlmod/BazelDepGraphFunction.java | 135 +---- .../bazel/bzlmod/BazelLockFileFunction.java | 54 +- .../lib/bazel/bzlmod/BazelLockFileModule.java | 81 +-- .../lib/bazel/bzlmod/BazelLockFileValue.java | 94 +-- .../bazel/bzlmod/BazelModTidyFunction.java | 10 +- .../lib/bazel/bzlmod/BazelModTidyValue.java | 32 +- .../bzlmod/BazelModuleResolutionEvent.java | 76 --- .../bzlmod/BazelModuleResolutionFunction.java | 129 +++-- .../bzlmod/BazelModuleResolutionValue.java | 14 +- .../bazel/bzlmod/BzlmodFlagsAndEnvVars.java | 93 --- .../build/lib/bazel/bzlmod/Discovery.java | 8 + .../bazel/bzlmod/ExternalDepsException.java | 4 + .../lib/bazel/bzlmod/GsonTypeAdapterUtil.java | 100 +--- .../build/lib/bazel/bzlmod/IndexRegistry.java | 93 ++- .../lib/bazel/bzlmod/ModuleFileFunction.java | 4 + .../build/lib/bazel/bzlmod/Registry.java | 6 + .../lib/bazel/bzlmod/RegistryFactory.java | 7 +- .../lib/bazel/bzlmod/RegistryFactoryImpl.java | 24 +- .../lib/bazel/bzlmod/RegistryFunction.java | 14 +- .../bzlmod/SingleExtensionEvalFunction.java | 9 +- .../bazel/bzlmod/YankedVersionsFunction.java | 7 +- .../lib/bazel/bzlmod/YankedVersionsUtil.java | 19 - .../lib/bazel/bzlmod/YankedVersionsValue.java | 3 + .../build/lib/bazel/commands/ModCommand.java | 89 +-- .../bazel/repository/downloader/Checksum.java | 8 + .../downloader/DownloadManager.java | 10 +- .../lib/skyframe/BzlmodRepoRuleFunction.java | 34 +- .../build/lib/skyframe/SkyframeExecutor.java | 1 + .../skyframe/packages/BazelPackageLoader.java | 7 +- src/main/protobuf/failure_details.proto | 1 + .../build/lib/analysis/util/AnalysisMock.java | 4 +- .../devtools/build/lib/bazel/bzlmod/BUILD | 2 +- .../bzlmod/BazelDepGraphFunctionTest.java | 7 +- .../bzlmod/BazelLockFileFunctionTest.java | 548 ------------------ .../BazelModuleResolutionFunctionTest.java | 4 +- .../bzlmod/BzlmodRepoRuleFunctionTest.java | 4 +- .../build/lib/bazel/bzlmod/DiscoveryTest.java | 6 +- .../build/lib/bazel/bzlmod/FakeRegistry.java | 14 +- .../lib/bazel/bzlmod/IndexRegistryTest.java | 69 ++- .../bzlmod/ModuleExtensionResolutionTest.java | 4 +- .../bazel/bzlmod/ModuleFileFunctionTest.java | 6 +- .../lib/bazel/bzlmod/RegistryFactoryTest.java | 26 +- .../repository/RepositoryDelegatorTest.java | 4 +- .../google/devtools/build/lib/testutil/BUILD | 2 +- .../py/bazel/bzlmod/bazel_lockfile_test.py | 132 ++--- .../bzlmod/bazel_yanked_versions_test.py | 103 ++++ src/test/shell/bazel/remote_helpers.sh | 8 + .../shell/bazel/starlark_repository_test.sh | 3 - src/test/tools/bzlmod/BUILD | 1 - src/tools/bzlmod/blazel_utils.bzl | 21 + src/tools/bzlmod/utils.bzl | 182 +++++- 58 files changed, 815 insertions(+), 1560 deletions(-) delete mode 100644 src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionEvent.java delete mode 100644 src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodFlagsAndEnvVars.java delete mode 100644 src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunctionTest.java create mode 100644 src/tools/bzlmod/blazel_utils.bzl diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 96262f88df2b0d..78e9995cea73e8 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -428,6 +428,7 @@ tasks: - "-//src/test/py/bazel:bazel_yanked_versions_test" - "-//src/test/py/bazel:bzlmod_query_test" - "-//src/test/py/bazel:mod_command_test" + - "-//src/test/shell/bazel:starlark_repository_test" - "-//src/test/shell/bazel:verify_workspace" # Flaky on rbe_ubuntu2004 # https://github.com/bazelbuild/continuous-integration/issues/1631 diff --git a/MODULE.bazel b/MODULE.bazel index 6f08b7adc58537..c420597a6c4f2e 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -33,6 +33,9 @@ bazel_dep(name = "rules_testing", version = "0.6.0") bazel_dep(name = "googletest", version = "1.14.0", repo_name = "com_google_googletest") bazel_dep(name = "with_cfg.bzl", version = "0.2.4") +# TODO(fmeum): Remove the dependency on buildozer after Bazel is built with 7.2.0. +bazel_dep(name = "buildozer", version = "7.1.1.1") + # TODO(pcloudy): Add remoteapis and googleapis as Bazel modules in the BCR. bazel_dep(name = "remoteapis", version = "") bazel_dep(name = "googleapis", version = "") diff --git a/distdir.bzl b/distdir.bzl index e872dc4d253b1e..cbc02a7b9f3a72 100644 --- a/distdir.bzl +++ b/distdir.bzl @@ -13,7 +13,7 @@ # limitations under the License. """Defines a repository rule that generates an archive consisting of the specified files to fetch""" -load("//src/tools/bzlmod:utils.bzl", "parse_http_artifacts") +load("//src/tools/bzlmod:utils.bzl", "parse_http_artifacts", "parse_registry_files") _BUILD = """ load("@rules_pkg//pkg:tar.bzl", "pkg_tar") @@ -87,10 +87,11 @@ def _repo_cache_tar_impl(ctx): """ lockfile_path = ctx.path(ctx.attr.lockfile) http_artifacts = parse_http_artifacts(ctx, lockfile_path, ctx.attr.repos) + registry_files = parse_registry_files(ctx, lockfile_path, ctx.attr.module_files) archive_files = [] readme_content = "This directory contains repository cache artifacts for the following URLs:\n\n" - for artifact in http_artifacts: + for artifact in http_artifacts + registry_files: url = artifact["url"] if "integrity" in artifact: # ./tempfile could be a hard link if --experimental_repository_cache_hardlinks is used, @@ -122,6 +123,7 @@ _repo_cache_tar_attrs = { "lockfile": attr.label(default = Label("//:MODULE.bazel.lock")), "dirname": attr.string(default = "repository_cache"), "repos": attr.string_list(), + "module_files": attr.label_list(), } repo_cache_tar = repository_rule( diff --git a/extensions.bzl b/extensions.bzl index c5cfdfbb4cc7dc..d531b97f61c2a2 100644 --- a/extensions.bzl +++ b/extensions.bzl @@ -29,7 +29,17 @@ def _bazel_build_deps(_ctx): _ctx.path(Label("//:MODULE.bazel")) # Make sure the `bootstrap_repo_cache` repo is updated when MODULE.bazel changes. embedded_jdk_repositories() debian_deps() - repo_cache_tar(name = "bootstrap_repo_cache", repos = DIST_ARCHIVE_REPOS, dirname = "derived/repository_cache") + repo_cache_tar( + name = "bootstrap_repo_cache", + repos = DIST_ARCHIVE_REPOS, + dirname = "derived/repository_cache", + module_files = [ + "//:MODULE.bazel", + "//third_party/googleapis:MODULE.bazel", + "//third_party/remoteapis:MODULE.bazel", + "//src:MODULE.tools", + ], + ) BAZEL_TOOLS_DEPS_REPOS = parse_bazel_module_repos(_ctx, _ctx.path(Label("//src/test/tools/bzlmod:MODULE.bazel.lock"))) repo_cache_tar(name = "bazel_tools_repo_cache", repos = BAZEL_TOOLS_DEPS_REPOS, lockfile = "//src/test/tools/bzlmod:MODULE.bazel.lock") distdir_tar(name = "workspace_repo_cache", dist_deps = WORKSPACE_REPOS) diff --git a/repositories.bzl b/repositories.bzl index 911ab621d8cb50..71be37187708b4 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -24,10 +24,12 @@ load("//src/tools/bzlmod:utils.bzl", "get_canonical_repo_name") # ################################################################################## DIST_ARCHIVE_REPOS = [get_canonical_repo_name(repo) for repo in [ + # keep sorted "abseil-cpp", "apple_support", "bazel_skylib", "blake3", + "buildozer", "c-ares", "com_github_grpc_grpc", "com_google_protobuf", @@ -35,10 +37,10 @@ DIST_ARCHIVE_REPOS = [get_canonical_repo_name(repo) for repo in [ "platforms", "rules_cc", "rules_go", + "rules_graalvm", "rules_java", "rules_jvm_external", "rules_kotlin", - "rules_graalvm", "rules_license", "rules_pkg", "rules_proto", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index 8378eedc6e6368..58ce1c66eef484 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -270,8 +270,8 @@ SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(directories.getWorkspace .addSkyFunction( SkyFunctions.REGISTRY, new RegistryFunction( - new RegistryFactoryImpl( - directories.getWorkspace(), downloadManager, clientEnvironmentSupplier))) + new RegistryFactoryImpl(downloadManager, clientEnvironmentSupplier), + directories.getWorkspace())) .addSkyFunction(SkyFunctions.REPO_SPEC, new RepoSpecFunction()) .addSkyFunction(SkyFunctions.YANKED_VERSIONS, new YankedVersionsFunction()) .addSkyFunction( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD index be364dfffe00ba..9f930c4f4fe9d3 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD @@ -79,12 +79,13 @@ java_library( ], deps = [ ":common", + ":yanked_versions_value", + "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options", "//src/main/java/com/google/devtools/build/lib/bazel/repository/cache", "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/util:os", - "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", "//third_party:gson", @@ -101,10 +102,11 @@ java_library( "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_extension", "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options", - "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader", "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/skyframe", "//third_party:flogger", "//third_party:guava", "//third_party:jsr305", @@ -119,9 +121,7 @@ java_library( "BazelDepGraphValue.java", "BazelFetchAllValue.java", "BazelLockFileValue.java", - "BazelModuleResolutionEvent.java", "BazelModuleResolutionValue.java", - "BzlmodFlagsAndEnvVars.java", "CompiledModuleFile.java", "GitOverride.java", "InterimModule.java", @@ -188,7 +188,6 @@ java_library( "BazelFetchAllFunction.java", "BazelLockFileFunction.java", "BazelModuleResolutionFunction.java", - "BzlmodFlagsAndEnvVars.java", "DelegateTypeAdapterFactory.java", "Discovery.java", "GsonTypeAdapterUtil.java", @@ -298,12 +297,10 @@ java_library( deps = [ ":resolution", ":root_module_file_fixup", - "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options", "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", - "//src/main/java/net/starlark/java/eval", "//third_party:auto_value", "//third_party:guava", ], @@ -316,20 +313,15 @@ java_library( ], deps = [ ":common", - ":exception", ":resolution", - ":resolution_impl", ":root_module_file_fixup", ":tidy", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function", - "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value", "//src/main/java/com/google/devtools/build/lib/vfs", - "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", "//src/main/java/net/starlark/java/eval", - "//src/main/protobuf:failure_details_java_proto", "//third_party:guava", "//third_party:jsr305", ], @@ -401,3 +393,16 @@ java_library( "//third_party:guava", ], ) + +java_library( + name = "yanked_versions_value", + srcs = ["YankedVersionsValue.java"], + deps = [ + ":common", + "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", + "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//third_party:auto_value", + "//third_party:guava", + ], +) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java index a8aaeb2df53f75..5770b09e1d3d74 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java @@ -15,40 +15,31 @@ package com.google.devtools.build.lib.bazel.bzlmod; -import static com.google.common.base.Strings.nullToEmpty; import static com.google.common.collect.ImmutableBiMap.toImmutableBiMap; import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static java.util.stream.Collectors.counting; import static java.util.stream.Collectors.groupingBy; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.BiMap; import com.google.common.collect.HashBiMap; import com.google.common.collect.ImmutableBiMap; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableTable; -import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue; -import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.packages.LabelConverter; import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code; -import com.google.devtools.build.lib.skyframe.ClientEnvironmentFunction; -import com.google.devtools.build.lib.skyframe.ClientEnvironmentValue; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; -import java.util.Map; import java.util.Map.Entry; import javax.annotation.Nullable; @@ -65,59 +56,12 @@ public BazelDepGraphFunction() {} @Nullable public SkyValue compute(SkyKey skyKey, Environment env) throws BazelDepGraphFunctionException, InterruptedException { - RootModuleFileValue root = - (RootModuleFileValue) env.getValue(ModuleFileValue.KEY_FOR_ROOT_MODULE); - if (root == null) { + BazelModuleResolutionValue selectionResult = + (BazelModuleResolutionValue) env.getValue(BazelModuleResolutionValue.KEY); + if (env.valuesMissing()) { return null; } - LockfileMode lockfileMode = BazelLockFileFunction.LOCKFILE_MODE.get(env); - - ImmutableMap localOverrideHashes = null; - ImmutableMap depGraph = null; - BzlmodFlagsAndEnvVars flags = null; - BazelLockFileValue lockfile = null; - - // If the module has not changed (has the same contents and flags as the lockfile), - // read the dependency graph from the lock file, else run resolution and update lockfile - if (!lockfileMode.equals(LockfileMode.OFF)) { - lockfile = (BazelLockFileValue) env.getValue(BazelLockFileValue.KEY); - if (lockfile == null) { - return null; - } - flags = getFlagsAndEnvVars(env); - if (flags == null) { // unable to read environment variables - return null; - } - localOverrideHashes = getLocalOverridesHashes(root.getOverrides(), env); - if (localOverrideHashes == null) { // still reading an override "module" - return null; - } - - if (root.getModuleFileHash().equals(lockfile.getModuleFileHash()) - && flags.equals(lockfile.getFlags()) - && localOverrideHashes.equals(lockfile.getLocalOverrideHashes())) { - depGraph = lockfile.getModuleDepGraph(); - } else if (lockfileMode.equals(LockfileMode.ERROR)) { - ImmutableList diffLockfile = - lockfile.getModuleAndFlagsDiff(root.getModuleFileHash(), localOverrideHashes, flags); - throw new BazelDepGraphFunctionException( - ExternalDepsException.withMessage( - Code.BAD_MODULE, - "MODULE.bazel.lock is no longer up-to-date because: %s. " - + "Please run `bazel mod deps --lockfile_mode=update` to update your lockfile.", - String.join(", ", diffLockfile)), - Transience.PERSISTENT); - } - } - - if (depGraph == null) { - BazelModuleResolutionValue selectionResult = - (BazelModuleResolutionValue) env.getValue(BazelModuleResolutionValue.KEY); - if (env.valuesMissing()) { - return null; - } - depGraph = selectionResult.getResolvedDepGraph(); - } + var depGraph = selectionResult.getResolvedDepGraph(); ImmutableBiMap canonicalRepoNameLookup = computeCanonicalRepoNameLookup(depGraph); @@ -131,20 +75,6 @@ public SkyValue compute(SkyKey skyKey, Environment env) ImmutableBiMap extensionUniqueNames = calculateUniqueNameForUsedExtensionId(extensionUsagesById); - if (lockfileMode.equals(LockfileMode.UPDATE)) { - BazelLockFileValue resolutionOnlyLockfile = - BazelLockFileValue.builder() - .setModuleFileHash(root.getModuleFileHash()) - .setFlags(flags) - .setLocalOverrideHashes(localOverrideHashes) - .setModuleDepGraph(depGraph) - .build(); - env.getListener() - .post( - BazelModuleResolutionEvent.create( - lockfile, resolutionOnlyLockfile, extensionUsagesById)); - } - return BazelDepGraphValue.create( depGraph, canonicalRepoNameLookup, @@ -153,63 +83,6 @@ public SkyValue compute(SkyKey skyKey, Environment env) extensionUniqueNames.inverse()); } - @Nullable - @VisibleForTesting - static ImmutableMap getLocalOverridesHashes( - Map overrides, Environment env) throws InterruptedException { - ImmutableMap.Builder localOverrideHashes = new ImmutableMap.Builder<>(); - for (Entry entry : overrides.entrySet()) { - if (entry.getValue() instanceof LocalPathOverride) { - ModuleFileValue moduleValue = - (ModuleFileValue) - env.getValue( - ModuleFileValue.key( - ModuleKey.create(entry.getKey(), Version.EMPTY), entry.getValue())); - if (moduleValue == null) { - return null; - } - localOverrideHashes.put(entry.getKey(), moduleValue.getModuleFileHash()); - } - } - return localOverrideHashes.buildOrThrow(); - } - - @VisibleForTesting - @Nullable - static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws InterruptedException { - ClientEnvironmentValue allowedYankedVersionsFromEnv = - (ClientEnvironmentValue) - env.getValue( - ClientEnvironmentFunction.key( - YankedVersionsUtil.BZLMOD_ALLOWED_YANKED_VERSIONS_ENV)); - if (allowedYankedVersionsFromEnv == null) { - return null; - } - - ImmutableMap moduleOverrides = - ModuleFileFunction.MODULE_OVERRIDES.get(env).entrySet().stream() - .collect( - toImmutableMap(Entry::getKey, e -> ((LocalPathOverride) e.getValue()).getPath())); - - ImmutableList yankedVersions = - ImmutableList.copyOf(YankedVersionsUtil.ALLOWED_YANKED_VERSIONS.get(env)); - Boolean ignoreDevDeps = ModuleFileFunction.IGNORE_DEV_DEPS.get(env); - String compatabilityMode = - BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE.get(env).name(); - String directDepsMode = BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES.get(env).name(); - - String envYanked = allowedYankedVersionsFromEnv.getValue(); - - return BzlmodFlagsAndEnvVars.create( - ModuleFileFunction.REGISTRIES.get(env), - moduleOverrides, - yankedVersions, - nullToEmpty(envYanked), - ignoreDevDeps, - directDepsMode, - compatabilityMode); - } - private static ImmutableTable getExtensionUsagesById( ImmutableMap depGraph, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java index 20206b8d30ab69..f34b03ba70c062 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java @@ -17,9 +17,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; import com.google.devtools.build.lib.cmdline.LabelConstants; @@ -54,20 +51,7 @@ public class BazelLockFileFunction implements SkyFunction { private final Path rootDirectory; - private static final BzlmodFlagsAndEnvVars EMPTY_FLAGS = - BzlmodFlagsAndEnvVars.create( - ImmutableSet.of(), ImmutableMap.of(), ImmutableList.of(), "", false, "", ""); - - private static final BazelLockFileValue EMPTY_LOCKFILE = - BazelLockFileValue.builder() - .setLockFileVersion(BazelLockFileValue.LOCK_FILE_VERSION) - .setModuleFileHash("") - .setFlags(EMPTY_FLAGS) - .setLocalOverrideHashes(ImmutableMap.of()) - .setModuleDepGraph(ImmutableMap.of()) - .setModuleExtensions(ImmutableMap.of()) - .setRegistryFileHashes(ImmutableMap.of()) - .build(); + private static final BazelLockFileValue EMPTY_LOCKFILE = BazelLockFileValue.builder().build(); public BazelLockFileFunction(Path rootDirectory) { this.rootDirectory = rootDirectory; @@ -86,11 +70,11 @@ public SkyValue compute(SkyKey skyKey, Environment env) } try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.BZLMOD, "parse lockfile")) { - return getLockfileValue(lockfilePath, rootDirectory); + return getLockfileValue(lockfilePath, LOCKFILE_MODE.get(env)); } catch (IOException | JsonSyntaxException | NullPointerException e) { throw new BazelLockfileFunctionException( ExternalDepsException.withMessage( - Code.BAD_MODULE, + Code.BAD_LOCKFILE, "Failed to read and parse the MODULE.bazel.lock file with error: %s." + " Try deleting it and rerun the build.", e.getMessage()), @@ -98,31 +82,31 @@ public SkyValue compute(SkyKey skyKey, Environment env) } } - public static BazelLockFileValue getLockfileValue(RootedPath lockfilePath, Path rootDirectory) - throws IOException { - BazelLockFileValue bazelLockFileValue; + public static BazelLockFileValue getLockfileValue( + RootedPath lockfilePath, LockfileMode lockfileMode) + throws IOException, BazelLockfileFunctionException { try { String json = FileSystemUtils.readContent(lockfilePath.asPath(), UTF_8); Matcher matcher = LOCKFILE_VERSION_PATTERN.matcher(json); int version = matcher.find() ? Integer.parseInt(matcher.group(1)) : -1; if (version == BazelLockFileValue.LOCK_FILE_VERSION) { - bazelLockFileValue = - GsonTypeAdapterUtil.createLockFileGson( - lockfilePath - .asPath() - .getParentDirectory() - .getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME), - rootDirectory) - .fromJson(json, BazelLockFileValue.class); + return GsonTypeAdapterUtil.LOCKFILE_GSON.fromJson(json, BazelLockFileValue.class); } else { - // This is an old version, needs to be updated - // Keep old version to recognize the problem in error mode - bazelLockFileValue = EMPTY_LOCKFILE.toBuilder().setLockFileVersion(version).build(); + // This is an old version, its information can't be used. + if (lockfileMode == LockfileMode.ERROR) { + throw new BazelLockfileFunctionException( + ExternalDepsException.withMessage( + Code.BAD_LOCKFILE, + "The version of MODULE.bazel.lock is not supported by this version of Bazel." + + " Please run `bazel mod deps --lockfile_mode=update` to update your" + + " lockfile."), + Transience.PERSISTENT); + } + return EMPTY_LOCKFILE; } } catch (FileNotFoundException e) { - bazelLockFileValue = EMPTY_LOCKFILE; + return EMPTY_LOCKFILE; } - return bazelLockFileValue; } 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 c4bf267a990237..24114f8e34183a 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 @@ -14,27 +14,26 @@ package com.google.devtools.build.lib.bazel.bzlmod; +import static com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction.LOCKFILE_MODE; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedMap; -import com.google.common.eventbus.Subscribe; import com.google.common.flogger.GoogleLogger; -import com.google.devtools.build.lib.bazel.repository.RepositoryOptions; 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.cmdline.LabelConstants; import com.google.devtools.build.lib.runtime.BlazeModule; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; +import com.google.devtools.build.skyframe.MemoizingEvaluator; import java.io.IOException; import java.util.HashMap; import java.util.Map; -import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import javax.annotation.Nullable; @@ -46,8 +45,6 @@ public class BazelLockFileModule extends BlazeModule { private SkyframeExecutor executor; private Path workspaceRoot; - private boolean enabled; - @Nullable private BazelModuleResolutionEvent moduleResolutionEvent; private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); @@ -55,42 +52,40 @@ public class BazelLockFileModule extends BlazeModule { public void beforeCommand(CommandEnvironment env) { executor = env.getSkyframeExecutor(); workspaceRoot = env.getWorkspace(); - - enabled = - env.getOptions().getOptions(RepositoryOptions.class).lockfileMode == LockfileMode.UPDATE; - moduleResolutionEvent = null; - env.getEventBus().register(this); } @Override public void afterCommand() { - if (!enabled || moduleResolutionEvent == null) { - // Command does not use Bazel modules or the lockfile mode is not update. - // Since Skyframe caches events, they are replayed even when nothing has changed. - return; - } - - BazelDepGraphValue depGraphValue; + MemoizingEvaluator evaluator = executor.getEvaluator(); BazelModuleResolutionValue moduleResolutionValue; + BazelDepGraphValue depGraphValue; + BazelLockFileValue oldLockfile; try { - depGraphValue = - (BazelDepGraphValue) executor.getEvaluator().getExistingValue(BazelDepGraphValue.KEY); + PrecomputedValue lockfileModeValue = + (PrecomputedValue) evaluator.getExistingValue(LOCKFILE_MODE.getKey()); + if (lockfileModeValue == null) { + // No command run on this server has triggered module resolution yet. + return; + } + LockfileMode lockfileMode = (LockfileMode) lockfileModeValue.get(); + // Check the Skyframe value instead of the option since some commands (e.g. shutdown) don't + // propagate the options to Skyframe, but we can only operate on Skyframe values that were + // generated in UPDATE mode. + if (lockfileMode != LockfileMode.UPDATE) { + return; + } moduleResolutionValue = - (BazelModuleResolutionValue) - executor.getEvaluator().getExistingValue(BazelModuleResolutionValue.KEY); + (BazelModuleResolutionValue) evaluator.getExistingValue(BazelModuleResolutionValue.KEY); + depGraphValue = (BazelDepGraphValue) evaluator.getExistingValue(BazelDepGraphValue.KEY); + oldLockfile = (BazelLockFileValue) evaluator.getExistingValue(BazelLockFileValue.KEY); } catch (InterruptedException e) { // Not thrown in Bazel. throw new IllegalStateException(e); } - - BazelLockFileValue oldLockfile = moduleResolutionEvent.getOnDiskLockfileValue(); - ImmutableMap> fileHashes; - if (moduleResolutionValue == null) { - // BazelDepGraphFunction took the dep graph from the lockfile and didn't cause evaluation of - // BazelModuleResolutionFunction. The file hashes in the lockfile are still up-to-date. - fileHashes = oldLockfile.getRegistryFileHashes(); - } else { - fileHashes = ImmutableSortedMap.copyOf(moduleResolutionValue.getRegistryFileHashes()); + if (moduleResolutionValue == null || depGraphValue == null || oldLockfile == null) { + // An error during the actual build prevented the evaluation of these values and has already + // been reported at this point. + return; } // All nodes corresponding to module extensions that have been evaluated in the current build @@ -121,8 +116,10 @@ public void afterCommand() { // Create an updated version of the lockfile, keeping only the extension results from the old // lockfile that are still up-to-date and adding the newly resolved extension results. BazelLockFileValue newLockfile = - moduleResolutionEvent.getResolutionOnlyLockfileValue().toBuilder() - .setRegistryFileHashes(fileHashes) + BazelLockFileValue.builder() + .setRegistryFileHashes( + ImmutableSortedMap.copyOf(moduleResolutionValue.getRegistryFileHashes())) + .setSelectedYankedVersions(moduleResolutionValue.getSelectedYankedVersions()) .setModuleExtensions(combinedExtensionInfos) .build(); @@ -248,31 +245,17 @@ private boolean shouldKeepExtension( * @param workspaceRoot Root of the workspace where the lockfile is located * @param updatedLockfile The updated lockfile data to save */ - public static void updateLockfile(Path workspaceRoot, BazelLockFileValue updatedLockfile) { + private static void updateLockfile(Path workspaceRoot, BazelLockFileValue updatedLockfile) { RootedPath lockfilePath = RootedPath.toRootedPath(Root.fromPath(workspaceRoot), LabelConstants.MODULE_LOCKFILE_NAME); try { FileSystemUtils.writeContent( lockfilePath.asPath(), UTF_8, - GsonTypeAdapterUtil.createLockFileGson( - lockfilePath - .asPath() - .getParentDirectory() - .getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME), - workspaceRoot) - .toJson(updatedLockfile) - + "\n"); + GsonTypeAdapterUtil.LOCKFILE_GSON.toJson(updatedLockfile) + "\n"); } catch (IOException e) { logger.atSevere().withCause(e).log( "Error while updating MODULE.bazel.lock file: %s", e.getMessage()); } } - - @Subscribe - public void bazelModuleResolved(BazelModuleResolutionEvent moduleResolutionEvent) { - // Latest event wins, which is relevant in the case of `bazel mod tidy`, where a new event is - // sent after the command has modified the module file. - this.moduleResolutionEvent = moduleResolutionEvent; - } } 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 1593779740a83b..441dc285eb21a2 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 @@ -15,10 +15,7 @@ package com.google.devtools.build.lib.bazel.bzlmod; -import static com.google.devtools.build.lib.bazel.bzlmod.InterimModule.toModule; - import com.google.auto.value.AutoValue; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.bazel.repository.downloader.Checksum; import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; @@ -27,7 +24,6 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.ryanharter.auto.value.gson.GenerateTypeAdapter; -import java.util.Map; import java.util.Optional; /** @@ -39,35 +35,30 @@ @GenerateTypeAdapter public abstract class BazelLockFileValue implements SkyValue, Postable { - public static final int LOCK_FILE_VERSION = 9; + public static final int LOCK_FILE_VERSION = 10; @SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_LOCK_FILE; static Builder builder() { return new AutoValue_BazelLockFileValue.Builder() .setLockFileVersion(LOCK_FILE_VERSION) - .setModuleExtensions(ImmutableMap.of()) - .setRegistryFileHashes(ImmutableMap.of()); + .setRegistryFileHashes(ImmutableMap.of()) + .setSelectedYankedVersions(ImmutableMap.of()) + .setModuleExtensions(ImmutableMap.of()); } /** Current version of the lock file */ public abstract int getLockFileVersion(); - /** Hash of the Module file */ - public abstract String getModuleFileHash(); - - /** Command line flags and environment variables that can affect the resolution */ - public abstract BzlmodFlagsAndEnvVars getFlags(); - - /** Module hash of each local path override in the root module file */ - public abstract ImmutableMap getLocalOverrideHashes(); - - /** The post-selection dep graph retrieved from the lock file. */ - public abstract ImmutableMap getModuleDepGraph(); - /** Hashes of files retrieved from registries. */ public abstract ImmutableMap> getRegistryFileHashes(); + /** + * Selected module versions that are known to be yanked (and hence must have been explicitly + * allowed by the user). + */ + public abstract ImmutableMap getSelectedYankedVersions(); + /** Mapping the extension id to the module extension data */ public abstract ImmutableMap< ModuleExtensionId, ImmutableMap> @@ -80,16 +71,10 @@ static Builder builder() { public abstract static class Builder { public abstract Builder setLockFileVersion(int value); - public abstract Builder setModuleFileHash(String value); - - public abstract Builder setFlags(BzlmodFlagsAndEnvVars value); - - public abstract Builder setLocalOverrideHashes(ImmutableMap value); - - public abstract Builder setModuleDepGraph(ImmutableMap value); - public abstract Builder setRegistryFileHashes(ImmutableMap> value); + public abstract Builder setSelectedYankedVersions(ImmutableMap value); + public abstract Builder setModuleExtensions( ImmutableMap< ModuleExtensionId, @@ -98,59 +83,4 @@ public abstract Builder setModuleExtensions( public abstract BazelLockFileValue build(); } - - /** Returns the difference between the lockfile and the current module & flags */ - public ImmutableList getModuleAndFlagsDiff( - String moduleFileHash, - ImmutableMap localOverrideHashes, - BzlmodFlagsAndEnvVars flags) { - ImmutableList.Builder moduleDiff = new ImmutableList.Builder<>(); - if (getLockFileVersion() != BazelLockFileValue.LOCK_FILE_VERSION) { - return moduleDiff - .add("the version of the lockfile is not compatible with the current Bazel") - .build(); - } - if (!moduleFileHash.equals(getModuleFileHash())) { - moduleDiff.add("the root MODULE.bazel has been modified"); - } - moduleDiff.addAll(getFlags().getDiffFlags(flags)); - - for (Map.Entry entry : localOverrideHashes.entrySet()) { - String currentValue = entry.getValue(); - String lockfileValue = getLocalOverrideHashes().get(entry.getKey()); - // If the lockfile value is null, the module hash would be different anyway - if (lockfileValue != null && !currentValue.equals(lockfileValue)) { - moduleDiff.add( - "The MODULE.bazel file has changed for the overriden module: " + entry.getKey()); - } - } - return moduleDiff.build(); - } - - /** - * Returns a new BazelLockFileValue in which all information about the root module has been - * replaced by the given value. - * - *

This operation is shallow: If the new root module has different dependencies, the dep graph - * will not be updated. - */ - public BazelLockFileValue withShallowlyReplacedRootModule( - ModuleFileValue.RootModuleFileValue value) { - ImmutableMap.Builder newDepGraph = ImmutableMap.builder(); - newDepGraph.putAll(getModuleDepGraph()); - newDepGraph.put( - ModuleKey.ROOT, - toModule( - value - .getModule() - .withDepSpecsTransformed( - InterimModule.applyOverrides( - value.getOverrides(), value.getModule().getName())), - /* override= */ null, - /* remoteRepoSpec= */ null)); - return toBuilder() - .setModuleFileHash(value.getModuleFileHash()) - .setModuleDepGraph(newDepGraph.buildKeepingLast()) - .build(); - } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java index 89cd8b4d708a1d..6545dce9fb2586 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java @@ -16,10 +16,6 @@ package com.google.devtools.build.lib.bazel.bzlmod; import static com.google.common.collect.ImmutableSet.toImmutableSet; -import static com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction.LOCKFILE_MODE; -import static com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction.IGNORE_DEV_DEPS; -import static com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction.MODULE_OVERRIDES; -import static com.google.devtools.build.lib.skyframe.PrecomputedValue.STARLARK_SEMANTICS; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -114,10 +110,6 @@ public SkyValue compute(SkyKey skyKey, Environment env) return BazelModTidyValue.create( fixups.build(), buildozer.asPath(), - rootModuleFileValue.getIncludeLabelToCompiledModuleFile(), - MODULE_OVERRIDES.get(env), - IGNORE_DEV_DEPS.get(env), - LOCKFILE_MODE.get(env), - STARLARK_SEMANTICS.get(env)); + rootModuleFileValue.getIncludeLabelToCompiledModuleFile()); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java index 492d47c0eefa81..d476c9481c6f00 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java @@ -18,15 +18,12 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import java.util.List; -import java.util.Map; -import net.starlark.java.eval.StarlarkSemantics; /** All Skyframe information required for the {@code bazel mod tidy} command. */ @AutoValue @@ -42,36 +39,11 @@ public abstract class BazelModTidyValue implements SkyValue { public abstract ImmutableMap includeLabelToCompiledModuleFile(); - /** The value of {@link ModuleFileFunction#MODULE_OVERRIDES}. */ - public abstract ImmutableMap moduleOverrides(); - - /** The value of {@link ModuleFileFunction#IGNORE_DEV_DEPS}. */ - public abstract boolean ignoreDevDeps(); - - /** The value of {@link BazelLockFileFunction#LOCKFILE_MODE}. */ - public abstract LockfileMode lockfileMode(); - - /** - * The value of {@link - * com.google.devtools.build.lib.skyframe.PrecomputedValue#STARLARK_SEMANTICS}. - */ - public abstract StarlarkSemantics starlarkSemantics(); - static BazelModTidyValue create( List fixups, Path buildozer, - ImmutableMap includeLabelToCompiledModuleFile, - Map moduleOverrides, - boolean ignoreDevDeps, - LockfileMode lockfileMode, - StarlarkSemantics starlarkSemantics) { + ImmutableMap includeLabelToCompiledModuleFile) { return new AutoValue_BazelModTidyValue( - ImmutableList.copyOf(fixups), - buildozer, - includeLabelToCompiledModuleFile, - ImmutableMap.copyOf(moduleOverrides), - ignoreDevDeps, - lockfileMode, - starlarkSemantics); + ImmutableList.copyOf(fixups), buildozer, includeLabelToCompiledModuleFile); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionEvent.java deleted file mode 100644 index 867eec90cac0b2..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionEvent.java +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright 2023 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.google.devtools.build.lib.bazel.bzlmod; - -import com.google.common.collect.ImmutableTable; -import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; - -/** - * After resolving bazel module this event is sent from {@link BazelDepGraphFunction} holding the - * lockfile value with module updates, and the module extension usages. It will be received in - * {@link BazelLockFileModule} to be used to update the lockfile content - * - *

Instances of this class are retained in Skyframe nodes and subject to frequent {@link - * java.util.Set}-based deduplication. As such, it must have a cheap implementation of {@link - * #hashCode()} and {@link #equals(Object)}. It currently uses reference equality since the logic - * that creates instances of this class already ensures that there is only one instance per build. - */ -public final class BazelModuleResolutionEvent implements Postable { - - private final BazelLockFileValue onDiskLockfileValue; - private final BazelLockFileValue resolutionOnlyLockfileValue; - private final ImmutableTable - extensionUsagesById; - - private BazelModuleResolutionEvent( - BazelLockFileValue onDiskLockfileValue, - BazelLockFileValue resolutionOnlyLockfileValue, - ImmutableTable extensionUsagesById) { - this.onDiskLockfileValue = onDiskLockfileValue; - this.resolutionOnlyLockfileValue = resolutionOnlyLockfileValue; - this.extensionUsagesById = extensionUsagesById; - } - - public static BazelModuleResolutionEvent create( - BazelLockFileValue onDiskLockfileValue, - BazelLockFileValue resolutionOnlyLockfileValue, - ImmutableTable extensionUsagesById) { - return new BazelModuleResolutionEvent( - onDiskLockfileValue, resolutionOnlyLockfileValue, extensionUsagesById); - } - - /** Returns the contents of the lockfile as it existed on disk before the current build. */ - public BazelLockFileValue getOnDiskLockfileValue() { - return onDiskLockfileValue; - } - - /** - * Returns the result of Bazel module resolution in the form of a lockfile without any information - * about module extension results. - */ - public BazelLockFileValue getResolutionOnlyLockfileValue() { - return resolutionOnlyLockfileValue; - } - - public ImmutableTable - getExtensionUsagesById() { - return extensionUsagesById; - } - - @Override - public boolean storeForReplay() { - return true; - } -} 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 4e3fb5d60f8bfb..c320a2d241f5de 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 @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.bazel.bzlmod; -import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.devtools.build.lib.bazel.bzlmod.YankedVersionsUtil.BZLMOD_ALLOWED_YANKED_VERSIONS_ENV; @@ -47,6 +46,7 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.SkyframeLookupResult; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; @@ -68,7 +68,8 @@ public class BazelModuleResolutionFunction implements SkyFunction { private record Result( Selection.Result selectionResult, - ImmutableMap> registryFileHashes) {} + ImmutableMap> registryFileHashes, + ImmutableMap selectedYankedVersions) {} 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.selectedYankedVersions); } @Nullable @@ -171,21 +173,12 @@ private static Result discoverAndSelect( } ImmutableMap resolvedDepGraph = selectionResult.getResolvedDepGraph(); - var yankedVersionsKeys = - resolvedDepGraph.values().stream() - .filter(m -> m.getRegistry() != null) - .map(m -> YankedVersionsValue.Key.create(m.getName(), m.getRegistry().getUrl())) - .collect(toImmutableSet()); - SkyframeLookupResult yankedVersionsResult = env.getValuesAndExceptions(yankedVersionsKeys); - if (env.valuesMissing()) { - return null; + ImmutableMap yankedVersionsValues; + try (SilentCloseable c = + Profiler.instance().profile(ProfilerTask.BZLMOD, "collect yanked versions")) { + yankedVersionsValues = collectYankedVersionsValues(env, resolvedDepGraph.values()); } - var yankedVersionValues = - yankedVersionsKeys.stream() - .collect( - toImmutableMap( - key -> key, key -> (YankedVersionsValue) yankedVersionsResult.get(key))); - if (yankedVersionValues.values().stream().anyMatch(Objects::isNull)) { + if (yankedVersionsValues == null) { return null; } @@ -206,12 +199,46 @@ private static Result discoverAndSelect( env.getListener()); } - try (SilentCloseable c = - Profiler.instance().profile(ProfilerTask.BZLMOD, "check no yanked versions")) { - checkNoYankedVersions(resolvedDepGraph, yankedVersionValues, allowedYankedVersions); - } + var selectedYankedVersions = checkNoYankedVersions(yankedVersionsValues, allowedYankedVersions); + return new Result( + selectionResult, discoveryResult.registryFileHashes(), selectedYankedVersions); + } - return new Result(selectionResult, discoveryResult.registryFileHashes()); + @Nullable + private static ImmutableMap collectYankedVersionsValues( + Environment env, ImmutableCollection modules) throws InterruptedException { + ImmutableMap.Builder yankedVersionsValues = + ImmutableMap.builder(); + Map yankedVersionsKeys = new HashMap<>(); + for (InterimModule m : modules) { + if (m.getRegistry() == null) { + // Modules with a non-registry override are never yanked. + yankedVersionsValues.put(m.getKey(), YankedVersionsValue.NONE_YANKED); + continue; + } + var lockfileYankedVersionsValue = + m.getRegistry().tryGetYankedVersionsFromLockfile(m.getKey()); + if (lockfileYankedVersionsValue.isPresent()) { + yankedVersionsValues.put(m.getKey(), lockfileYankedVersionsValue.get()); + } else { + // We need to download the list of yanked versions from the registry. + yankedVersionsKeys.put( + m.getKey(), YankedVersionsValue.Key.create(m.getName(), m.getRegistry().getUrl())); + } + } + SkyframeLookupResult yankedVersionsResult = + env.getValuesAndExceptions(yankedVersionsKeys.values()); + if (env.valuesMissing()) { + return null; + } + for (var entry : yankedVersionsKeys.entrySet()) { + var yankedVersionsValue = (YankedVersionsValue) yankedVersionsResult.get(entry.getValue()); + if (yankedVersionsValue == null) { + return null; + } + yankedVersionsValues.put(entry.getKey(), yankedVersionsValue); + } + return yankedVersionsValues.buildOrThrow(); } private static void verifyAllOverridesAreOnExistentModules( @@ -304,36 +331,46 @@ public static void checkBazelCompatibility( } } - private static void checkNoYankedVersions( - ImmutableMap depGraph, - ImmutableMap yankedVersionValues, + /** + * Fail if any selected module is yanked and not explicitly allowed. + * + * @return the yanked info for each yanked but explicitly allowed module + */ + private static ImmutableMap checkNoYankedVersions( + ImmutableMap yankedVersionValues, Optional> allowedYankedVersions) throws BazelModuleResolutionFunctionException { - for (InterimModule m : depGraph.values()) { - if (m.getRegistry() == null) { - // Non-registry modules do not have yanked versions. + ImmutableMap.Builder selectedYankedVersions = ImmutableMap.builder(); + for (var entry : yankedVersionValues.entrySet()) { + ModuleKey key = entry.getKey(); + YankedVersionsValue yankedVersionsValue = entry.getValue(); + if (yankedVersionsValue.yankedVersions().isEmpty()) { + // No yanked version information available for this module. continue; } - Optional yankedInfo = - YankedVersionsUtil.getYankedInfo( - m.getKey(), - yankedVersionValues.get( - YankedVersionsValue.Key.create(m.getName(), 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); + String yankedInfo = yankedVersionsValue.yankedVersions().get().get(key.getVersion()); + if (yankedInfo == null) { + // The selected version is not yanked. + continue; + } + if (allowedYankedVersions.isEmpty() || allowedYankedVersions.get().contains(key)) { + // The selected version is yanked but explicitly allowed. + selectedYankedVersions.put(key, yankedInfo); + 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.", + key, + yankedInfo), + Transience.PERSISTENT); } + return selectedYankedVersions.buildOrThrow(); } 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 60971030d549b0..5b1d45e5abd450 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 @@ -30,9 +30,6 @@ */ @AutoValue abstract class BazelModuleResolutionValue implements SkyValue { - /* TODO(andreisolo): Also load the modules overridden by {@code single_version_override} or - NonRegistryOverride if we need to detect changes in the dependency graph caused by them. - */ @SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_MODULE_RESOLUTION; @@ -53,11 +50,18 @@ abstract class BazelModuleResolutionValue implements SkyValue { */ abstract ImmutableMap> getRegistryFileHashes(); + /** + * Selected module versions that are known to be yanked (and hence must have been explicitly + * allowed by the user). + */ + abstract ImmutableMap getSelectedYankedVersions(); + static BazelModuleResolutionValue create( ImmutableMap resolvedDepGraph, ImmutableMap unprunedDepGraph, - ImmutableMap> registryFileHashes) { + ImmutableMap> registryFileHashes, + ImmutableMap selectedYankedVersions) { return new AutoValue_BazelModuleResolutionValue( - resolvedDepGraph, unprunedDepGraph, registryFileHashes); + resolvedDepGraph, unprunedDepGraph, registryFileHashes, selectedYankedVersions); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodFlagsAndEnvVars.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodFlagsAndEnvVars.java deleted file mode 100644 index d425bc4518042d..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodFlagsAndEnvVars.java +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright 2023 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.google.devtools.build.lib.bazel.bzlmod; - -import com.google.auto.value.AutoValue; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.ryanharter.auto.value.gson.GenerateTypeAdapter; - -/** Stores the values of flags and environment variables that affect the resolution */ -@AutoValue -@GenerateTypeAdapter -abstract class BzlmodFlagsAndEnvVars { - - public static BzlmodFlagsAndEnvVars create( - ImmutableSet registries, - ImmutableMap moduleOverrides, - ImmutableList yankedVersions, - String envVarYankedVersions, - boolean ignoreDevDeps, - String directDepsMode, - String compatabilityMode) { - return new AutoValue_BzlmodFlagsAndEnvVars( - registries, - moduleOverrides, - yankedVersions, - envVarYankedVersions, - ignoreDevDeps, - directDepsMode, - compatabilityMode); - } - - /** Registries provided via command line */ - public abstract ImmutableSet cmdRegistries(); - - /** ModulesOverride provided via command line */ - public abstract ImmutableMap cmdModuleOverrides(); - - /** Allowed yanked version in the dependency graph */ - public abstract ImmutableList allowedYankedVersions(); - - /** Allowed yanked version in the dependency graph from environment variables */ - public abstract String envVarAllowedYankedVersions(); - - /** Whether to ignore things declared as dev dependencies or not */ - public abstract boolean ignoreDevDependency(); - - /** Error level of direct dependencies check */ - public abstract String directDependenciesMode(); - - /** Error level of bazel compatability check */ - public abstract String compatibilityMode(); - - public ImmutableList getDiffFlags(BzlmodFlagsAndEnvVars flags) { - ImmutableList.Builder diffFlags = new ImmutableList.Builder<>(); - if (!flags.cmdRegistries().equals(cmdRegistries())) { - diffFlags.add("the value of --registry flag has been modified"); - } - if (!flags.cmdModuleOverrides().equals(cmdModuleOverrides())) { - diffFlags.add("the value of --override_module flag has been modified"); - } - if (!flags.allowedYankedVersions().equals(allowedYankedVersions())) { - diffFlags.add("the value of --allow_yanked_versions flag has been modified"); - } - if (!flags.envVarAllowedYankedVersions().equals(envVarAllowedYankedVersions())) { - diffFlags.add( - "the value of BZLMOD_ALLOW_YANKED_VERSIONS environment variable has been modified"); - } - if (flags.ignoreDevDependency() != ignoreDevDependency()) { - diffFlags.add("the value of --ignore_dev_dependency flag has been modified"); - } - if (!flags.directDependenciesMode().equals(directDependenciesMode())) { - diffFlags.add("the value of --check_direct_dependencies flag has been modified"); - } - if (!flags.compatibilityMode().equals(compatibilityMode())) { - diffFlags.add("the value of --check_bazel_compatibility flag has been modified"); - } - return diffFlags.build(); - } -} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java index 6cc5eaebeda813..f42c2eec30ae56 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java @@ -91,6 +91,14 @@ public static Result run(Environment env, RootModuleFileValue root) moduleFileValue = (ModuleFileValue) result.getOrThrow(skyKey, ExternalDepsException.class); } catch (ExternalDepsException e) { + if (e.getDetailedExitCode().getFailureDetail() == null + || e.getDetailedExitCode().getFailureDetail().getExternalDeps().getCode() + != FailureDetails.ExternalDeps.Code.BAD_MODULE) { + // This is not due to a bad module, so don't print a dependency chain. This covers cases + // such as a parse error in the lockfile or an I/O exception during registry access, + // which aren't related to any particular module dep. + throw e; + } // Trace back a dependency chain to the root module. There can be multiple paths to the // failing module, but any of those is useful for debugging. List depChain = new ArrayList<>(); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ExternalDepsException.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ExternalDepsException.java index a6ae28b7132155..0d6a862d491700 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ExternalDepsException.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ExternalDepsException.java @@ -51,6 +51,10 @@ public static ExternalDepsException withCauseAndMessage( String.format(format, args) + ": " + cause.getMessage(), cause, code); } + public static ExternalDepsException withCause(ExternalDeps.Code code, Throwable cause) { + return new ExternalDepsException(cause.getMessage(), cause, code); + } + @Override public DetailedExitCode getDetailedExitCode() { return detailedExitCode; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java index 912b455287352e..440e39a8edd502 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java @@ -20,7 +20,6 @@ import static com.google.devtools.build.lib.bazel.bzlmod.DelegateTypeAdapterFactory.IMMUTABLE_MAP; import static com.google.devtools.build.lib.bazel.bzlmod.DelegateTypeAdapterFactory.IMMUTABLE_SET; -import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableTable; @@ -32,7 +31,6 @@ import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.rules.repository.RepoRecordedInput; -import com.google.devtools.build.lib.vfs.Path; import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.google.gson.JsonParseException; @@ -49,7 +47,6 @@ import java.util.Base64; import java.util.Optional; import javax.annotation.Nullable; -import net.starlark.java.syntax.Location; /** * Utility class to hold type adapters and helper methods to get gson registered with type adapters @@ -366,87 +363,6 @@ public ImmutableTable read(JsonReader jsonReader) } }; - /** - * A variant of {@link Location} that converts the absolute path to the root module file to a - * constant and back. - */ - // protected only for @AutoValue - @GenerateTypeAdapter - @AutoValue - protected abstract static class RootModuleFileEscapingLocation { - // This marker string is neither a valid absolute path nor a valid URL and thus cannot conflict - // with any real module file location. - private static final String ROOT_MODULE_FILE_LABEL = "@@//:MODULE.bazel"; - - public abstract String file(); - - public abstract int line(); - - public abstract int column(); - - public Location toLocation(String moduleFilePath, String workspaceRoot) { - String file; - if (file().equals(ROOT_MODULE_FILE_LABEL)) { - file = moduleFilePath; - } else { - file = file().replace("%workspace%", workspaceRoot); - } - return Location.fromFileLineColumn(file, line(), column()); - } - - public static RootModuleFileEscapingLocation fromLocation( - Location location, String moduleFilePath, String workspaceRoot) { - String file; - if (location.file().equals(moduleFilePath)) { - file = ROOT_MODULE_FILE_LABEL; - } else { - file = location.file().replace(workspaceRoot, "%workspace%"); - } - return new AutoValue_GsonTypeAdapterUtil_RootModuleFileEscapingLocation( - file, location.line(), location.column()); - } - } - - private static final class LocationTypeAdapterFactory implements TypeAdapterFactory { - - private final String moduleFilePath; - private final String workspaceRoot; - - public LocationTypeAdapterFactory(Path moduleFilePath, Path workspaceRoot) { - this.moduleFilePath = moduleFilePath.getPathString(); - this.workspaceRoot = workspaceRoot.getPathString(); - } - - @Nullable - @Override - @SuppressWarnings("unchecked") - public TypeAdapter create(Gson gson, TypeToken typeToken) { - if (typeToken.getRawType() != Location.class) { - return null; - } - TypeAdapter relativizedLocationTypeAdapter = - gson.getAdapter(RootModuleFileEscapingLocation.class); - return (TypeAdapter) - new TypeAdapter() { - - @Override - public void write(JsonWriter jsonWriter, Location location) throws IOException { - relativizedLocationTypeAdapter.write( - jsonWriter, - RootModuleFileEscapingLocation.fromLocation( - location, moduleFilePath, workspaceRoot)); - } - - @Override - public Location read(JsonReader jsonReader) throws IOException { - return relativizedLocationTypeAdapter - .read(jsonReader) - .toLocation(moduleFilePath, workspaceRoot); - } - }; - } - } - private static final TypeAdapter REPO_RECORDED_INPUT_FILE_TYPE_ADAPTER = new TypeAdapter<>() { @Override @@ -528,17 +444,13 @@ public Optional read(JsonReader jsonReader) throws IOException { } } - public static Gson createLockFileGson(Path moduleFilePath, Path workspaceRoot) { - return newGsonBuilder() - .setPrettyPrinting() - .registerTypeAdapterFactory(new LocationTypeAdapterFactory(moduleFilePath, workspaceRoot)) - .registerTypeAdapterFactory(new OptionalChecksumTypeAdapterFactory()) - .create(); - } + public static final Gson LOCKFILE_GSON = + newGsonBuilder() + .setPrettyPrinting() + .registerTypeAdapterFactory(new OptionalChecksumTypeAdapterFactory()) + .create(); - public static Gson createSingleExtensionUsagesValueHashGson() { - return newGsonBuilder().create(); - } + public static final Gson SINGLE_EXTENSION_USAGES_VALUE_GSON = newGsonBuilder().create(); private static GsonBuilder newGsonBuilder() { return new GsonBuilder() 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 578e78a69d9d1d..9de56c594550a1 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 @@ -17,11 +17,13 @@ import static java.nio.charset.StandardCharsets.UTF_8; +import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; 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; import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.events.StoredEventHandler; @@ -57,17 +59,16 @@ public class IndexRegistry implements Registry { */ public enum KnownFileHashesMode { IGNORE, - USE_AND_UPDATE; + USE_AND_UPDATE, + ENFORCE } - /** The unresolved version of the url. Ex: has %workspace% placeholder */ - private final String unresolvedUri; - private final URI uri; private final DownloadManager downloadManager; private final Map clientEnv; private final Gson gson; private final ImmutableMap> knownFileHashes; + private final ImmutableMap previouslySelectedYankedVersions; private final KnownFileHashesMode knownFileHashesMode; private volatile Optional bazelRegistryJson; private volatile StoredEventHandler bazelRegistryJsonEvents; @@ -76,13 +77,12 @@ public enum KnownFileHashesMode { public IndexRegistry( URI uri, - String unresolvedUri, DownloadManager downloadManager, Map clientEnv, ImmutableMap> knownFileHashes, - KnownFileHashesMode knownFileHashesMode) { + KnownFileHashesMode knownFileHashesMode, + ImmutableMap previouslySelectedYankedVersions) { this.uri = uri; - this.unresolvedUri = unresolvedUri; this.downloadManager = downloadManager; this.clientEnv = clientEnv; this.gson = @@ -91,11 +91,12 @@ public IndexRegistry( .create(); this.knownFileHashes = knownFileHashes; this.knownFileHashesMode = knownFileHashesMode; + this.previouslySelectedYankedVersions = previouslySelectedYankedVersions; } @Override public String getUrl() { - return unresolvedUri; + return uri.toString(); } private String constructUrl(String base, String... segments) { @@ -127,6 +128,14 @@ private Optional doGrabFile( if (knownFileHashesMode != KnownFileHashesMode.IGNORE && useChecksum) { Optional knownChecksum = knownFileHashes.get(url); if (knownChecksum == null) { + if (knownFileHashesMode == KnownFileHashesMode.ENFORCE) { + throw new MissingChecksumException( + String.format( + "Missing checksum for registry file %s not permitted with --lockfile_mode=error." + + " Please run `bazel mod deps --lockfile_mode=update` to update your" + + " lockfile.", + url)); + } // This is a new file, download without providing a checksum. checksum = Optional.empty(); } else if (knownChecksum.isEmpty()) { @@ -140,10 +149,17 @@ private Optional doGrabFile( } else { checksum = Optional.empty(); } + if (knownFileHashesMode == KnownFileHashesMode.ENFORCE) { + Preconditions.checkState( + checksum.isPresent(), + "Cannot fetch a file without a checksum in ENFORCE mode. This is a bug in Bazel, please " + + "report at https://github.com/bazelbuild/bazel/issues/new/choose."); + } try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.BZLMOD, () -> "download file: " + url)) { return Optional.of( - downloadManager.downloadAndReadOneUrl(new URL(url), eventHandler, clientEnv, checksum)); + downloadManager.downloadAndReadOneUrlForBzlmod( + new URL(url), eventHandler, clientEnv, checksum)); } catch (FileNotFoundException e) { return Optional.empty(); } catch (IOException e) { @@ -158,7 +174,7 @@ public Optional getModuleFile(ModuleKey key, ExtendedEventHandler ev throws IOException, InterruptedException { String url = constructUrl( - uri.toString(), "modules", key.getName(), key.getVersion().toString(), "MODULE.bazel"); + getUrl(), "modules", key.getName(), key.getVersion().toString(), "MODULE.bazel"); Optional maybeContent = grabFile(url, eventHandler, /* useChecksum= */ true); return maybeContent.map(content -> ModuleFile.create(content, url)); } @@ -237,17 +253,12 @@ 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( - String.format("Module %s's %s not found in registry %s", key, SOURCE_JSON_FILENAME, uri)); + String.format( + "Module %s's %s not found in registry %s", key, SOURCE_JSON_FILENAME, getUrl())); } SourceJson sourceJson = parseJson(jsonString.get(), jsonUrl, SourceJson.class); switch (sourceJson.type) { @@ -275,7 +286,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) { @@ -284,7 +299,7 @@ private Optional getBazelRegistryJson(ExtendedEventHandler ev var storedEventHandler = new StoredEventHandler(); bazelRegistryJson = grabJson( - constructUrl(uri.toString(), "bazel_registry.json"), + constructUrl(getUrl(), "bazel_registry.json"), BazelRegistryJson.class, storedEventHandler, /* useChecksum= */ true); @@ -361,7 +376,7 @@ private RepoSpec createArchiveRepoSpec( for (Map.Entry entry : sourceJson.patches.entrySet()) { remotePatches.put( constructUrl( - unresolvedUri, + getUrl(), "modules", key.getName(), key.getVersion().toString(), @@ -399,7 +414,7 @@ public Optional> getYankedVersions( throws IOException, InterruptedException { Optional metadataJson = grabJson( - constructUrl(uri.toString(), "modules", moduleName, "metadata.json"), + constructUrl(getUrl(), "modules", moduleName, "metadata.json"), MetadataJson.class, eventHandler, // metadata.json is not immutable @@ -423,6 +438,40 @@ public Optional> getYankedVersions( } } + @Override + public Optional tryGetYankedVersionsFromLockfile( + ModuleKey selectedModuleKey) { + String yankedInfo = previouslySelectedYankedVersions.get(selectedModuleKey); + if (yankedInfo != null) { + // The module version was selected when the lockfile was created, but known to be yanked + // (hence, it was explicitly allowed by the user). We reuse the yanked info from the lockfile. + // Rationale: A module that was yanked in the past should remain yanked in the future. The + // yanked info may have been updated since then, but by not fetching it, we avoid network + // access if the set of yanked versions has not changed, but the set allowed versions has. + return Optional.of( + YankedVersionsValue.create( + Optional.of(ImmutableMap.of(selectedModuleKey.getVersion(), yankedInfo)))); + } + if (knownFileHashes.containsKey(getSourceJsonUrl(selectedModuleKey))) { + // If the source.json hash is recorded in the lockfile, we know that the module was selected + // when the lockfile was created. Since it does not appear in the list of selected yanked + // versions recorded in the lockfile, it must not have been yanked at that time. We do not + // refresh yanked versions information. + // Rationale: This ensures that builds with --lockfile_mode=update or error are reproducible + // and do not fail due to changes in the set of yanked versions. Furthermore, it avoids + // refetching yanked versions for all modules every time the user modifies or adds a + // dependency. If the selected version for a module changes, yanked version information is + // always refreshed. + return Optional.of(YankedVersionsValue.NONE_YANKED); + } + // The lockfile does not contain sufficient information to determine the "yanked" status of the + // module - network access to the registry is required. + // Note that this point can't (and must not) be reached with --lockfile_mode=error: The lockfile + // records the source.json hashes of all selected modules and the result of selection is fully + // determined by the lockfile. + return Optional.empty(); + } + /** 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/ModuleFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java index 20c3281155a252..60f09174bd7d2f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.bazel.bzlmod.CompiledModuleFile.IncludeStatement; import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.NonRootModuleFileValue; import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue; +import com.google.devtools.build.lib.bazel.repository.downloader.Checksum.MissingChecksumException; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; @@ -617,6 +618,9 @@ private GetModuleFileResult getModuleFile( continue; } return new GetModuleFileResult(moduleFile.get(), registry, downloadEventHandler); + } catch (MissingChecksumException e) { + throw new ModuleFileFunctionException( + ExternalDepsException.withCause(Code.BAD_LOCKFILE, e)); } catch (IOException e) { throw errorf( Code.ERROR_ACCESSING_REGISTRY, e, "Error accessing registry %s", registry.getUrl()); 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..77420ee02c74c9 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 @@ -48,4 +48,10 @@ RepoSpec getRepoSpec(ModuleKey key, ExtendedEventHandler eventHandler) Optional> getYankedVersions( String moduleName, ExtendedEventHandler eventHandler) throws IOException, InterruptedException; + + /** + * Returns the yanked versions information, limited to the given selected module version, purely + * based on the lockfile (if possible). + */ + Optional tryGetYankedVersionsFromLockfile(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 e24b3452fe8e07..d910c3d2d75c33 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.devtools.build.lib.bazel.repository.RepositoryOptions; import com.google.devtools.build.lib.bazel.repository.downloader.Checksum; import java.net.URISyntaxException; import java.util.Optional; @@ -28,6 +29,10 @@ public interface RegistryFactory { * *

Outside of tests, only {@link RegistryFunction} should call this method. */ - Registry createRegistry(String url, ImmutableMap> fileHashes) + Registry createRegistry( + String url, + RepositoryOptions.LockfileMode lockfileMode, + ImmutableMap> fileHashes, + ImmutableMap previouslySelectedYankedVersions) 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 06efd4050cbd0d..f202b650db17ec 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 @@ -17,9 +17,9 @@ import com.google.common.collect.ImmutableMap; 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; import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; -import com.google.devtools.build.lib.vfs.Path; import java.net.URI; import java.net.URISyntaxException; import java.util.Map; @@ -28,24 +28,23 @@ /** Prod implementation of {@link RegistryFactory}. */ public class RegistryFactoryImpl implements RegistryFactory { - private final Path workspacePath; private final DownloadManager downloadManager; private final Supplier> clientEnvironmentSupplier; public RegistryFactoryImpl( - Path workspacePath, - DownloadManager downloadManager, - Supplier> clientEnvironmentSupplier) { - this.workspacePath = workspacePath; + DownloadManager downloadManager, Supplier> clientEnvironmentSupplier) { this.downloadManager = downloadManager; this.clientEnvironmentSupplier = clientEnvironmentSupplier; } @Override public Registry createRegistry( - String unresolvedUrl, ImmutableMap> knownFileHashes) + String url, + LockfileMode lockfileMode, + ImmutableMap> knownFileHashes, + ImmutableMap previouslySelectedYankedVersions) throws URISyntaxException { - URI uri = new URI(unresolvedUrl.replace("%workspace%", workspacePath.getPathString())); + URI uri = new URI(url); if (uri.getScheme() == null) { throw new URISyntaxException( uri.toString(), @@ -60,17 +59,20 @@ public Registry createRegistry( } var knownFileHashesMode = switch (uri.getScheme()) { - case "http", "https" -> KnownFileHashesMode.USE_AND_UPDATE; + case "http", "https" -> + lockfileMode == LockfileMode.ERROR + ? KnownFileHashesMode.ENFORCE + : KnownFileHashesMode.USE_AND_UPDATE; case "file" -> KnownFileHashesMode.IGNORE; default -> throw new URISyntaxException(uri.toString(), "Unrecognized registry URL protocol"); }; return new IndexRegistry( uri, - unresolvedUrl, downloadManager, clientEnvironmentSupplier.get(), knownFileHashes, - knownFileHashesMode); + knownFileHashesMode, + previouslySelectedYankedVersions); } } 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 0c6c12a7ac84bc..121027c658e2aa 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 @@ -15,7 +15,9 @@ package com.google.devtools.build.lib.bazel.bzlmod; +import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; import com.google.devtools.build.lib.server.FailureDetails; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; @@ -26,15 +28,19 @@ /** A simple SkyFunction that creates a {@link Registry} with a given URL. */ public class RegistryFunction implements SkyFunction { private final RegistryFactory registryFactory; + private final Path workspaceRoot; - public RegistryFunction(RegistryFactory registryFactory) { + public RegistryFunction(RegistryFactory registryFactory, Path workspaceRoot) { this.registryFactory = registryFactory; + this.workspaceRoot = workspaceRoot; } @Override @Nullable public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException, RegistryException { + LockfileMode lockfileMode = BazelLockFileFunction.LOCKFILE_MODE.get(env); + BazelLockFileValue lockfile = (BazelLockFileValue) env.getValue(BazelLockFileValue.KEY); if (lockfile == null) { return null; @@ -42,7 +48,11 @@ public SkyValue compute(SkyKey skyKey, Environment env) RegistryKey key = (RegistryKey) skyKey.argument(); try { - return registryFactory.createRegistry(key.getUrl(), lockfile.getRegistryFileHashes()); + return registryFactory.createRegistry( + key.getUrl().replace("%workspace%", workspaceRoot.getPathString()), + lockfileMode, + lockfile.getRegistryFileHashes(), + lockfile.getSelectedYankedVersions()); } catch (URISyntaxException e) { throw new RegistryException( ExternalDepsException.withCauseAndMessage( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java index 2194abe3816148..ccd9ec0a1b3b9a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java @@ -199,7 +199,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) if (lockedExtension == null && extensionShouldHaveBeenLocked) { throw new SingleExtensionEvalFunctionException( ExternalDepsException.withMessage( - Code.BAD_MODULE, + Code.BAD_LOCKFILE, "The module extension '%s'%s does not exist in the lockfile", extensionId, extension.getEvalFactors().isEmpty() @@ -224,8 +224,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) .setBzlTransitiveDigest(extension.getBzlTransitiveDigest()) .setUsagesDigest( SingleExtensionUsagesValue.hashForEvaluation( - GsonTypeAdapterUtil.createSingleExtensionUsagesValueHashGson(), - usagesValue)) + GsonTypeAdapterUtil.SINGLE_EXTENSION_USAGES_VALUE_GSON, usagesValue)) .setRecordedFileInputs(moduleExtensionResult.getRecordedFileInputs()) .setRecordedDirentsInputs(moduleExtensionResult.getRecordedDirentsInputs()) .setEnvVariables(extension.getEnvVars()) @@ -281,7 +280,7 @@ private SingleExtensionValue tryGettingValueFromLockFile( // relevant for the evaluation of the extension. if (!Arrays.equals( SingleExtensionUsagesValue.hashForEvaluation( - GsonTypeAdapterUtil.createSingleExtensionUsagesValueHashGson(), usagesValue), + GsonTypeAdapterUtil.SINGLE_EXTENSION_USAGES_VALUE_GSON, usagesValue), lockedExtension.getUsagesDigest())) { diffRecorder.record("The usages of the extension '" + extensionId + "' have changed"); } @@ -316,7 +315,7 @@ private SingleExtensionValue tryGettingValueFromLockFile( if (lockfileMode.equals(LockfileMode.ERROR)) { throw new SingleExtensionEvalFunctionException( ExternalDepsException.withMessage( - Code.BAD_MODULE, + Code.BAD_LOCKFILE, "MODULE.bazel.lock is no longer up-to-date because: %s. " + "Please run `bazel mod deps --lockfile_mode=update` to update your lockfile.", diffRecorder.getRecordedDiffMessages()), 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 89cd60a5da3bdd..57a2344f6657fd 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 @@ -27,8 +27,8 @@ import javax.annotation.Nullable; /** - * A simple SkyFunction that computes a {@link RepoSpec} for the given {@link InterimModule} by - * fetching required information from its {@link Registry}. + * A simple SkyFunction that fetches the yanked versions for a given module from its {@link + * Registry}. */ public class YankedVersionsFunction implements SkyFunction { @@ -53,7 +53,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept .handle( Event.warn( String.format( - "Could not read metadata file for module %s: %s", key, e.getMessage()))); + "Could not read metadata file for module %s from registry %s: %s", + key.getModuleName(), key.getRegistryUrl(), e.getMessage()))); // This is failing open: If we can't read the metadata file, we allow yanked modules to be // fetched. 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/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..93362ed1acc4af 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 @@ -27,6 +27,9 @@ @AutoValue public abstract class YankedVersionsValue implements SkyValue { + /** A value representing a module without yanked versions. */ + public static final YankedVersionsValue NONE_YANKED = create(Optional.of(ImmutableMap.of())); + public abstract Optional> yankedVersions(); public static YankedVersionsValue create(Optional> yankedVersions) { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java index 36320c5ecefaf5..f82f882854e43b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java @@ -28,21 +28,14 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; -import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.analysis.NoBuildEvent; import com.google.devtools.build.lib.analysis.NoBuildRequestFinishedEvent; import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue; import com.google.devtools.build.lib.bazel.bzlmod.BazelModTidyValue; import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue; import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule; -import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleResolutionEvent; import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleValue; -import com.google.devtools.build.lib.bazel.bzlmod.CompiledModuleFile; -import com.google.devtools.build.lib.bazel.bzlmod.ExternalDepsException; import com.google.devtools.build.lib.bazel.bzlmod.ModuleExtensionId; -import com.google.devtools.build.lib.bazel.bzlmod.ModuleFile; -import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction; -import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue; import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey; import com.google.devtools.build.lib.bazel.bzlmod.RootModuleFileFixup; import com.google.devtools.build.lib.bazel.bzlmod.modcommand.ExtensionArg; @@ -54,7 +47,6 @@ import com.google.devtools.build.lib.bazel.bzlmod.modcommand.ModOptions.ModSubcommandConverter; import com.google.devtools.build.lib.bazel.bzlmod.modcommand.ModuleArg; import com.google.devtools.build.lib.bazel.bzlmod.modcommand.ModuleArg.ModuleArgConverter; -import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.cmdline.RepositoryName; @@ -79,11 +71,8 @@ import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.InterruptedFailureDetails; import com.google.devtools.build.lib.util.MaybeCompleteSet; -import com.google.devtools.build.lib.vfs.FileSystemUtils; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.EvaluationContext; import com.google.devtools.build.skyframe.EvaluationResult; -import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.common.options.OptionsParsingException; @@ -193,7 +182,6 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe @Nullable BazelModuleInspectorValue moduleInspector; @Nullable BazelModTidyValue modTidyValue; ImmutableList repoMappingValues; - TidyEventRecorder tidyEventRecorder = new TidyEventRecorder(); SkyframeExecutor skyframeExecutor = env.getSkyframeExecutor(); LoadingPhaseThreadsOption threadsOption = options.getOptions(LoadingPhaseThreadsOption.class); @@ -212,7 +200,6 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe keys.addAll(repoMappingKeys); } else if (subcommand.equals(ModSubcommand.TIDY)) { keys.add(BazelModTidyValue.KEY); - env.getEventBus().register(tidyEventRecorder); } else { keys.add(BazelDepGraphValue.KEY, BazelModuleInspectorValue.KEY); } @@ -279,7 +266,7 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe return reportAndCreateFailureResult( env, "the 'tidy' command doesn't take extra arguments", Code.TOO_MANY_ARGUMENTS); } - return runTidy(env, modTidyValue, tidyEventRecorder); + return runTidy(env, modTidyValue); } // Extract and check the --base_module argument first to use it when parsing the other args. @@ -566,17 +553,7 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe return BlazeCommandResult.success(); } - private static class TidyEventRecorder { - @Nullable BazelModuleResolutionEvent bazelModuleResolutionEvent; - - @Subscribe - public void bazelModuleResolved(BazelModuleResolutionEvent event) { - bazelModuleResolutionEvent = event; - } - } - - private BlazeCommandResult runTidy( - CommandEnvironment env, BazelModTidyValue modTidyValue, TidyEventRecorder eventRecorder) { + private BlazeCommandResult runTidy(CommandEnvironment env, BazelModTidyValue modTidyValue) { CommandBuilder buildozerCommand = new CommandBuilder() .setWorkingDir(env.getWorkspace()) @@ -612,68 +589,6 @@ private BlazeCommandResult runTidy( env.getReporter().handle(Event.info(fixupEvent.getSuccessMessage())); } - if (modTidyValue.lockfileMode().equals(LockfileMode.UPDATE)) { - // We cannot safely rerun Skyframe evaluation here to pick up the updated module file. - // Instead, we construct a new BazelModuleResolutionEvent with the updated module file - // contents to be picked up by BazelLockFileModule. Since changing use_repos doesn't affect - // module resolution or module extension evaluation, we can reuse the existing lockfile - // information except for the root module file value. - RootedPath moduleFilePath = ModuleFileFunction.getModuleFilePath(env.getWorkspace()); - byte[] moduleFileContents; - try { - moduleFileContents = FileSystemUtils.readContent(moduleFilePath.asPath()); - } catch (IOException e) { - return reportAndCreateFailureResult( - env, - "Unexpected error while reading module file after running buildozer: " + e.getMessage(), - Code.BUILDOZER_FAILED); - } - CompiledModuleFile compiledModuleFile; - try { - compiledModuleFile = - CompiledModuleFile.parseAndCompile( - ModuleFile.create(moduleFileContents, moduleFilePath.asPath().getPathString()), - ModuleKey.ROOT, - modTidyValue.starlarkSemantics(), - env.getRuntime().getRuleClassProvider().getBazelStarlarkEnvironment(), - env.getReporter()); - } catch (ExternalDepsException e) { - return reportAndCreateFailureResult( - env, - "Unexpected error while compiling module file after running buildozer: " - + e.getMessage(), - Code.BUILDOZER_FAILED); - } - ModuleFileValue.RootModuleFileValue newRootModuleFileValue; - try { - newRootModuleFileValue = - ModuleFileFunction.evaluateRootModuleFile( - compiledModuleFile, - modTidyValue.includeLabelToCompiledModuleFile(), - ModuleFileFunction.getBuiltinModules( - env.getDirectories().getEmbeddedBinariesRoot()), - modTidyValue.moduleOverrides(), - modTidyValue.ignoreDevDeps(), - modTidyValue.starlarkSemantics(), - env.getReporter()); - } catch (SkyFunctionException | InterruptedException e) { - return reportAndCreateFailureResult( - env, - "Unexpected error parsing module file after running buildozer: " + e.getMessage(), - Code.BUILDOZER_FAILED); - } - // BazelModuleResolutionEvent is cached by Skyframe and thus always emitted. - BazelModuleResolutionEvent updatedModuleResolutionEvent = - BazelModuleResolutionEvent.create( - eventRecorder.bazelModuleResolutionEvent.getOnDiskLockfileValue(), - eventRecorder - .bazelModuleResolutionEvent - .getResolutionOnlyLockfileValue() - .withShallowlyReplacedRootModule(newRootModuleFileValue), - eventRecorder.bazelModuleResolutionEvent.getExtensionUsagesById()); - env.getReporter().post(updatedModuleResolutionEvent); - } - return BlazeCommandResult.success(); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Checksum.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Checksum.java index 8a5ea43b114bb5..8a75b5f91c483f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Checksum.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Checksum.java @@ -17,6 +17,7 @@ import com.google.common.base.Ascii; import com.google.common.hash.HashCode; import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache.KeyType; +import java.io.IOException; import java.util.Base64; /** The content checksum for an HTTP download, which knows its own type. */ @@ -32,6 +33,13 @@ private InvalidChecksumException(String msg) { } } + /** Exception thrown to indicate that a checksum is missing. */ + public static final class MissingChecksumException extends IOException { + public MissingChecksumException(String message) { + super(message); + } + } + private final KeyType keyType; private final HashCode hashCode; private final boolean useSubresourceIntegrity; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java index 9737437cec0cf9..e67aaf8adeade4 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java @@ -375,6 +375,9 @@ private Path downloadInExecutor( /** * Downloads the contents of one URL and reads it into a byte array. * + *

This is only meant to be used for Bzlmod registry downloads as it ignores the value of + * --repository_disable_download. + * *

If the checksum and path to the repository cache is specified, attempt to load the file from * the {@link RepositoryCache}. If it doesn't exist, proceed to download the file and load it into * the cache prior to returning the value. @@ -388,7 +391,7 @@ private Path downloadInExecutor( * @throws IOException if download was attempted and ended up failing * @throws InterruptedException if this thread is being cast into oblivion */ - public byte[] downloadAndReadOneUrl( + public byte[] downloadAndReadOneUrlForBzlmod( URL originalUrl, ExtendedEventHandler eventHandler, Map clientEnv, @@ -439,11 +442,6 @@ public byte[] downloadAndReadOneUrl( authHeaders = rewriter.updateAuthHeaders(rewrittenUrlMappings, authHeaders, netrcCreds); } - if (disableDownload) { - throw new IOException( - String.format("Failed to download %s: download is disabled.", originalUrl)); - } - if (rewrittenUrls.isEmpty()) { throw new IOException(getRewriterBlockedAllUrlsMessage(ImmutableList.of(originalUrl))); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java index 850bf379914f9b..97bcf7770a0a48 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.skyframe; -import static com.google.common.collect.ImmutableMap.toImmutableMap; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -22,7 +21,6 @@ import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.bazel.bzlmod.ArchiveRepoSpecBuilder; -import com.google.devtools.build.lib.bazel.bzlmod.AttributeValues; import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue; import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleCreator; import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleValue; @@ -51,7 +49,6 @@ import com.google.devtools.build.skyframe.SkyFunctionException.Transience; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; -import java.util.Map; import java.util.Map.Entry; import java.util.Optional; import java.util.Set; @@ -193,7 +190,7 @@ private BzlmodRepoRuleValue createRuleFromSpec( var attributes = ImmutableMap.builder() - .putAll(resolveRemotePatchesUrl(repoSpec).attributes()) + .putAll(repoSpec.attributes().attributes()) .put("name", repositoryName.getName()) .buildOrThrow(); try { @@ -217,35 +214,6 @@ private BzlmodRepoRuleValue createRuleFromSpec( } } - /* Resolves repo specs containing remote patches that are stored with %workspace% place holder*/ - @SuppressWarnings("unchecked") - private AttributeValues resolveRemotePatchesUrl(RepoSpec repoSpec) { - if (repoSpec - .getRuleClass() - .equals(ArchiveRepoSpecBuilder.HTTP_ARCHIVE_PATH + "%http_archive")) { - return AttributeValues.create( - repoSpec.attributes().attributes().entrySet().stream() - .collect( - toImmutableMap( - Map.Entry::getKey, - e -> { - if (e.getKey().equals("remote_patches")) { - Map remotePatches = (Map) e.getValue(); - return remotePatches.keySet().stream() - .collect( - toImmutableMap( - key -> - key.replace( - "%workspace%", - directories.getWorkspace().getPathString()), - remotePatches::get)); - } - return e.getValue(); - }))); - } - return repoSpec.attributes(); - } - // Starlark rules loaded from bazel_tools that may define Bazel module repositories and thus must // be loaded without relying on any other modules. private static final Set BOOTSTRAP_RULE_CLASSES = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index a27685dc05ee31..0f12a465d215b4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -271,6 +271,7 @@ import java.util.concurrent.locks.Lock; import java.util.function.Consumer; import java.util.function.Supplier; +import java.util.stream.Stream; import javax.annotation.Nullable; import javax.annotation.concurrent.GuardedBy; import net.starlark.java.eval.StarlarkSemantics; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java index 344048abe5413c..36e72039fdcb95 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java @@ -139,8 +139,7 @@ public BazelPackageLoader buildImpl() { HttpDownloader httpDownloader = new HttpDownloader(); DownloadManager downloadManager = new DownloadManager(repositoryCache, httpDownloader); RegistryFactory registryFactory = - new RegistryFactoryImpl( - directories.getWorkspace(), downloadManager, Suppliers.ofInstance(ImmutableMap.of())); + new RegistryFactoryImpl(downloadManager, Suppliers.ofInstance(ImmutableMap.of())); // Allow tests to override the following functions to use fake registry or custom built-in // modules @@ -160,7 +159,9 @@ public BazelPackageLoader buildImpl() { } if (!this.extraSkyFunctions.containsKey(SkyFunctions.REGISTRY)) { addExtraSkyFunctions( - ImmutableMap.of(SkyFunctions.REGISTRY, new RegistryFunction(registryFactory))); + ImmutableMap.of( + SkyFunctions.REGISTRY, + new RegistryFunction(registryFactory, directories.getWorkspace()))); } addExtraSkyFunctions( diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index 98419ca1b9497f..5a0f7a9664793f 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto @@ -1300,6 +1300,7 @@ message ExternalDeps { INVALID_REGISTRY_URL = 4 [(metadata) = { exit_code: 48 }]; ERROR_ACCESSING_REGISTRY = 5 [(metadata) = { exit_code: 32 }]; INVALID_EXTENSION_IMPORT = 6 [(metadata) = { exit_code: 48 }]; + BAD_LOCKFILE = 7 [(metadata) = { exit_code: 48 }]; } Code code = 1; diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java index 9121c7f5a062fa..1d142e19e2cdb7 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java @@ -181,7 +181,9 @@ public ImmutableMap getSkyFunctions(BlazeDirectori SkyFunctions.SINGLE_EXTENSION_EVAL, new SingleExtensionEvalFunction(directories, ImmutableMap::of, downloadManager)) .put(SkyFunctions.SINGLE_EXTENSION_USAGES, new SingleExtensionUsagesFunction()) - .put(SkyFunctions.REGISTRY, new RegistryFunction(FakeRegistry.DEFAULT_FACTORY)) + .put( + SkyFunctions.REGISTRY, + new RegistryFunction(FakeRegistry.DEFAULT_FACTORY, directories.getWorkspace())) .put(SkyFunctions.REPO_SPEC, new RepoSpecFunction()) .put(SkyFunctions.YANKED_VERSIONS, new YankedVersionsFunction()) .put( diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD index 09fe37dc762c5f..378fff3ced078c 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD @@ -33,7 +33,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info", "//src/main/java/com/google/devtools/build/lib/analysis:server_directories", "//src/main/java/com/google/devtools/build/lib/authandtls", - "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:bazel_lockfile_module", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:exception", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection", @@ -117,6 +116,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:registry", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution_impl", + "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options", "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/events", 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 a91d9024aab2eb..f8852f22cfc8d6 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 @@ -129,7 +129,9 @@ public void setup() throws Exception { .put(SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(rootDirectory)) .put(SkyFunctions.BAZEL_DEP_GRAPH, new BazelDepGraphFunction()) .put(SkyFunctions.BAZEL_MODULE_RESOLUTION, resolutionFunctionMock) - .put(SkyFunctions.REGISTRY, new RegistryFunction(new FakeRegistry.Factory())) + .put( + SkyFunctions.REGISTRY, + new RegistryFunction(new FakeRegistry.Factory(), directories.getWorkspace())) .put(SkyFunctions.REPO_SPEC, new RepoSpecFunction()) .put(SkyFunctions.YANKED_VERSIONS, new YankedVersionsFunction()) .put( @@ -365,7 +367,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(), ImmutableMap.of()); } } } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunctionTest.java deleted file mode 100644 index f4dc8815ef01cd..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunctionTest.java +++ /dev/null @@ -1,548 +0,0 @@ -// Copyright 2023 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.google.devtools.build.lib.bazel.bzlmod; - -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.fail; - -import com.google.auto.value.AutoValue; -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.devtools.build.lib.actions.FileValue; -import com.google.devtools.build.lib.analysis.BlazeDirectories; -import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; -import com.google.devtools.build.lib.analysis.ServerDirectories; -import com.google.devtools.build.lib.analysis.util.AnalysisMock; -import com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction.BazelLockfileFunctionException; -import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue; -import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode; -import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode; -import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; -import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryModule; -import com.google.devtools.build.lib.clock.BlazeClock; -import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; -import com.google.devtools.build.lib.pkgcache.PathPackageLocator; -import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction; -import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule; -import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction; -import com.google.devtools.build.lib.rules.repository.RepositoryFunction; -import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants; -import com.google.devtools.build.lib.skyframe.BzlmodRepoRuleFunction; -import com.google.devtools.build.lib.skyframe.ClientEnvironmentFunction; -import com.google.devtools.build.lib.skyframe.ExternalFilesHelper; -import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; -import com.google.devtools.build.lib.skyframe.FileFunction; -import com.google.devtools.build.lib.skyframe.FileStateFunction; -import com.google.devtools.build.lib.skyframe.PrecomputedFunction; -import com.google.devtools.build.lib.skyframe.PrecomputedValue; -import com.google.devtools.build.lib.skyframe.SkyFunctions; -import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryBootstrap; -import com.google.devtools.build.lib.testutil.FoundationTestCase; -import com.google.devtools.build.lib.testutil.TestRuleClassProvider; -import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; -import com.google.devtools.build.lib.vfs.FileStateKey; -import com.google.devtools.build.lib.vfs.Root; -import com.google.devtools.build.lib.vfs.SyscallCache; -import com.google.devtools.build.skyframe.EvaluationContext; -import com.google.devtools.build.skyframe.EvaluationResult; -import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator; -import com.google.devtools.build.skyframe.MemoizingEvaluator; -import com.google.devtools.build.skyframe.RecordingDifferencer; -import com.google.devtools.build.skyframe.SequencedRecordingDifferencer; -import com.google.devtools.build.skyframe.SkyFunction; -import com.google.devtools.build.skyframe.SkyFunctionName; -import com.google.devtools.build.skyframe.SkyKey; -import com.google.devtools.build.skyframe.SkyValue; -import com.google.gson.JsonObject; -import com.google.gson.JsonParser; -import java.util.Optional; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; -import java.util.regex.Pattern; -import javax.annotation.Nullable; -import net.starlark.java.eval.StarlarkSemantics; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for {@link BazelLockFileFunction}. */ -@RunWith(JUnit4.class) -public class BazelLockFileFunctionTest extends FoundationTestCase { - - private MemoizingEvaluator evaluator; - private RecordingDifferencer differencer; - private EvaluationContext evaluationContext; - private static SkyFunctionName updateLockfileFunction; - - @Before - public void setup() throws Exception { - differencer = new SequencedRecordingDifferencer(); - evaluationContext = - EvaluationContext.newBuilder().setParallelism(8).setEventHandler(reporter).build(); - - AtomicReference packageLocator = - new AtomicReference<>( - new PathPackageLocator( - outputBase, - ImmutableList.of(Root.fromPath(rootDirectory)), - BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY)); - BlazeDirectories directories = - new BlazeDirectories( - new ServerDirectories(rootDirectory, outputBase, rootDirectory), - rootDirectory, - /* defaultSystemJavabase= */ null, - AnalysisMock.get().getProductName()); - ExternalFilesHelper externalFilesHelper = - ExternalFilesHelper.createForTesting( - packageLocator, - ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS, - directories); - RepositoryFunction localRepositoryFunction = new LocalRepositoryFunction(); - ImmutableMap repositoryHandlers = - ImmutableMap.of(LocalRepositoryRule.NAME, localRepositoryFunction); - - ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder(); - TestRuleClassProvider.addStandardRules(builder); - builder - .clearWorkspaceFileSuffixForTesting() - .addStarlarkBootstrap(new RepositoryBootstrap(new StarlarkRepositoryModule())); - ConfiguredRuleClassProvider ruleClassProvider = builder.build(); - - updateLockfileFunction = SkyFunctionName.createHermetic("LockfileWrite"); - - evaluator = - new InMemoryMemoizingEvaluator( - ImmutableMap.builder() - .put(FileValue.FILE, new FileFunction(packageLocator, directories)) - .put( - FileStateKey.FILE_STATE, - new FileStateFunction( - Suppliers.ofInstance( - new TimestampGranularityMonitor(BlazeClock.instance())), - SyscallCache.NO_CACHE, - externalFilesHelper)) - .put( - SkyFunctions.REPOSITORY_DIRECTORY, - new RepositoryDelegatorFunction( - repositoryHandlers, - null, - new AtomicBoolean(true), - ImmutableMap::of, - directories, - BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)) - .put( - BzlmodRepoRuleValue.BZLMOD_REPO_RULE, - new BzlmodRepoRuleFunction(ruleClassProvider, directories)) - .put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction()) - .put( - SkyFunctions.MODULE_FILE, - new ModuleFileFunction( - ruleClassProvider.getBazelStarlarkEnvironment(), - rootDirectory, - ImmutableMap.of())) - .put(SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(rootDirectory)) - .put(SkyFunctions.REGISTRY, new RegistryFunction(new FakeRegistry.Factory())) - .put(SkyFunctions.REPO_SPEC, new RepoSpecFunction()) - .put(SkyFunctions.YANKED_VERSIONS, new YankedVersionsFunction()) - .put( - SkyFunctions.MODULE_EXTENSION_REPO_MAPPING_ENTRIES, - new ModuleExtensionRepoMappingEntriesFunction()) - .put( - SkyFunctions.CLIENT_ENVIRONMENT_VARIABLE, - new ClientEnvironmentFunction( - new AtomicReference<>(ImmutableMap.of("BZLMOD_ALLOW_YANKED_VERSIONS", "")))) - .put( - updateLockfileFunction, - new SkyFunction() { - @Nullable - @Override - public SkyValue compute(SkyKey skyKey, Environment env) - throws BazelLockfileFunctionException, InterruptedException { - - UpdateLockFileKey key = (UpdateLockFileKey) skyKey; - BzlmodFlagsAndEnvVars flags = BazelDepGraphFunction.getFlagsAndEnvVars(env); - if (flags == null) { - return null; - } - - ImmutableMap localOverrideHashes = - BazelDepGraphFunction.getLocalOverridesHashes(key.overrides(), env); - if (localOverrideHashes == null) { - return null; - } - BazelLockFileModule.updateLockfile( - rootDirectory, - BazelLockFileValue.builder() - .setModuleFileHash(key.moduleHash()) - .setFlags(flags) - .setLocalOverrideHashes(localOverrideHashes) - .setModuleDepGraph(key.depGraph()) - .setRegistryFileHashes(ImmutableMap.of()) - .build()); - - return new SkyValue() {}; - } - }) - .buildOrThrow(), - differencer); - - PrecomputedValue.STARLARK_SEMANTICS.set( - differencer, - StarlarkSemantics.builder().setBool(BuildLanguageOptions.ENABLE_BZLMOD, true).build()); - ModuleFileFunction.REGISTRIES.set(differencer, ImmutableSet.of()); - ModuleFileFunction.IGNORE_DEV_DEPS.set(differencer, true); - ModuleFileFunction.MODULE_OVERRIDES.set(differencer, ImmutableMap.of()); - YankedVersionsUtil.ALLOWED_YANKED_VERSIONS.set(differencer, ImmutableList.of()); - BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE.set( - differencer, BazelCompatibilityMode.ERROR); - BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES.set( - differencer, CheckDirectDepsMode.ERROR); - BazelLockFileFunction.LOCKFILE_MODE.set(differencer, LockfileMode.UPDATE); - RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set(differencer, ImmutableMap.of()); - RepositoryDelegatorFunction.FORCE_FETCH.set( - differencer, RepositoryDelegatorFunction.FORCE_FETCH_DISABLED); - RepositoryDelegatorFunction.VENDOR_DIRECTORY.set(differencer, Optional.empty()); - } - - @Test - public void simpleModule() throws Exception { - scratch.overwriteFile( - rootDirectory.getRelative("MODULE.bazel").getPathString(), - "module(name='my_root', version='1.0')", - "bazel_dep(name = 'dep_1', version = '1.0')", - "bazel_dep(name = 'dep_2', version = '2.0')"); - - EvaluationResult rootResult = - evaluator.evaluate( - ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); - if (rootResult.hasError()) { - fail(rootResult.getError().toString()); - } - RootModuleFileValue rootValue = rootResult.get(ModuleFileValue.KEY_FOR_ROOT_MODULE); - - ImmutableMap depGraph = - ImmutableMap.of(ModuleKey.ROOT, InterimModule.toModule(rootValue.getModule(), null, null)); - - UpdateLockFileKey key = - UpdateLockFileKey.create("moduleHash", depGraph, rootValue.getOverrides()); - EvaluationResult result = - evaluator.evaluate(ImmutableList.of(key), evaluationContext); - if (result.hasError()) { - fail(result.getError().toString()); - } - - result = evaluator.evaluate(ImmutableList.of(BazelLockFileValue.KEY), evaluationContext); - if (result.hasError()) { - fail(result.getError().toString()); - } - - BazelLockFileValue value = result.get(BazelLockFileValue.KEY); - assertThat(value.getModuleDepGraph()).isEqualTo(depGraph); - } - - @Test - public void moduleWithFlags() throws Exception { - // Test having --override_module, --ignore_dev_dependency, --check_bazel_compatibility - // --check_direct_dependencies & --registry - scratch.overwriteFile( - rootDirectory.getRelative("MODULE.bazel").getPathString(), - "module(name='my_root', version='1.0')"); - - EvaluationResult rootResult = - evaluator.evaluate( - ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); - if (rootResult.hasError()) { - fail(rootResult.getError().toString()); - } - RootModuleFileValue rootValue = rootResult.get(ModuleFileValue.KEY_FOR_ROOT_MODULE); - - ImmutableMap depGraph = - ImmutableMap.of(ModuleKey.ROOT, InterimModule.toModule(rootValue.getModule(), null, null)); - - ImmutableList yankedVersions = ImmutableList.of("2.4", "2.3"); - LocalPathOverride override = LocalPathOverride.create("override_path"); - ImmutableSet registries = ImmutableSet.of("registry1", "registry2"); - ImmutableMap moduleOverride = ImmutableMap.of("my_dep_1", override.getPath()); - - ModuleFileFunction.IGNORE_DEV_DEPS.set(differencer, true); - ModuleFileFunction.REGISTRIES.set(differencer, registries); - ModuleFileFunction.MODULE_OVERRIDES.set(differencer, ImmutableMap.of("my_dep_1", override)); - YankedVersionsUtil.ALLOWED_YANKED_VERSIONS.set(differencer, yankedVersions); - BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES.set( - differencer, CheckDirectDepsMode.WARNING); - BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE.set( - differencer, BazelCompatibilityMode.WARNING); - - UpdateLockFileKey key = - UpdateLockFileKey.create("moduleHash", depGraph, rootValue.getOverrides()); - EvaluationResult result = - evaluator.evaluate(ImmutableList.of(key), evaluationContext); - if (result.hasError()) { - fail(result.getError().toString()); - } - - result = evaluator.evaluate(ImmutableList.of(BazelLockFileValue.KEY), evaluationContext); - if (result.hasError()) { - fail(result.getError().toString()); - } - - BazelLockFileValue value = result.get(BazelLockFileValue.KEY); - assertThat(value.getModuleDepGraph()).isEqualTo(depGraph); - assertThat(value.getFlags().ignoreDevDependency()).isTrue(); - assertThat(value.getFlags().cmdRegistries()).isEqualTo(registries); - assertThat(value.getFlags().cmdModuleOverrides()).isEqualTo(moduleOverride); - assertThat(value.getFlags().allowedYankedVersions()).isEqualTo(yankedVersions); - assertThat(value.getFlags().directDependenciesMode()) - .isEqualTo(CheckDirectDepsMode.WARNING.toString()); - assertThat(value.getFlags().compatibilityMode()) - .isEqualTo(BazelCompatibilityMode.WARNING.toString()); - } - - @Test - public void moduleWithLocalOverrides() throws Exception { - scratch.overwriteFile( - rootDirectory.getRelative("MODULE.bazel").getPathString(), - "module(name='root',version='0.1')", - "local_path_override(module_name='ss',path='code_for_ss')"); - scratch.file( - rootDirectory.getRelative("code_for_ss/MODULE.bazel").getPathString(), - "module(name='ss',version='1.0')"); - scratch.file(rootDirectory.getRelative("code_for_ss/WORKSPACE").getPathString()); - - EvaluationResult rootResult = - evaluator.evaluate( - ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); - if (rootResult.hasError()) { - fail(rootResult.getError().toString()); - } - RootModuleFileValue rootValue = rootResult.get(ModuleFileValue.KEY_FOR_ROOT_MODULE); - - ImmutableMap depGraph = - ImmutableMap.of(ModuleKey.ROOT, InterimModule.toModule(rootValue.getModule(), null, null)); - - UpdateLockFileKey key = - UpdateLockFileKey.create("moduleHash", depGraph, rootValue.getOverrides()); - EvaluationResult result = - evaluator.evaluate(ImmutableList.of(key), evaluationContext); - if (result.hasError()) { - fail(result.getError().toString()); - } - - result = evaluator.evaluate(ImmutableList.of(BazelLockFileValue.KEY), evaluationContext); - if (result.hasError()) { - fail(result.getError().toString()); - } - - BazelLockFileValue value = result.get(BazelLockFileValue.KEY); - assertThat(value.getModuleDepGraph()).isEqualTo(depGraph); - assertThat(value.getLocalOverrideHashes()).isNotEmpty(); - } - - @Test - public void fullModule() throws Exception { - scratch.overwriteFile( - rootDirectory.getRelative("MODULE.bazel").getPathString(), - "module(name='my_root', version='1.0')", - "register_toolchains('//my:toolchain', '//my:toolchain2')", - "ext1 = use_extension('//:defs.bzl','ext_1')", - "use_repo(ext1,'myrepo')", - "ext2 = use_extension('@ext//:defs.bzl','ext_2')", - "ext2.tag(file='@myrepo//:Hello1.txt')", - "ext2.tag(file='@myrepo//:Hello2.txt')", - "use_repo(ext2,'ext_repo')"); - - EvaluationResult rootResult = - evaluator.evaluate( - ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); - if (rootResult.hasError()) { - fail(rootResult.getError().toString()); - } - RootModuleFileValue rootValue = rootResult.get(ModuleFileValue.KEY_FOR_ROOT_MODULE); - - ImmutableMap depGraph = - ImmutableMap.of(ModuleKey.ROOT, InterimModule.toModule(rootValue.getModule(), null, null)); - - UpdateLockFileKey key = - UpdateLockFileKey.create("moduleHash", depGraph, rootValue.getOverrides()); - EvaluationResult result = - evaluator.evaluate(ImmutableList.of(key), evaluationContext); - if (result.hasError()) { - fail(result.getError().toString()); - } - - result = evaluator.evaluate(ImmutableList.of(BazelLockFileValue.KEY), evaluationContext); - if (result.hasError()) { - fail(result.getError().toString()); - } - - BazelLockFileValue value = result.get(BazelLockFileValue.KEY); - assertThat(value.getModuleDepGraph()).isEqualTo(depGraph); - } - - @Test - public void invalidLockfileEmptyFile() throws Exception { - scratch.overwriteFile( - rootDirectory.getRelative("MODULE.bazel").getPathString(), - "module(name='my_root', version='1.0')"); - - EvaluationResult rootResult = - evaluator.evaluate( - ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); - if (rootResult.hasError()) { - fail(rootResult.getError().toString()); - } - RootModuleFileValue rootValue = rootResult.get(ModuleFileValue.KEY_FOR_ROOT_MODULE); - - ImmutableMap depGraph = - ImmutableMap.of(ModuleKey.ROOT, InterimModule.toModule(rootValue.getModule(), null, null)); - - UpdateLockFileKey key = - UpdateLockFileKey.create("moduleHash", depGraph, rootValue.getOverrides()); - EvaluationResult result = - evaluator.evaluate(ImmutableList.of(key), evaluationContext); - if (result.hasError()) { - fail(result.getError().toString()); - } - - scratch.overwriteFile( - rootDirectory.getRelative("MODULE.bazel.lock").getPathString(), - "{\"lockFileVersion\": " + BazelLockFileValue.LOCK_FILE_VERSION + "}"); - - result = evaluator.evaluate(ImmutableList.of(BazelLockFileValue.KEY), evaluationContext); - if (!result.hasError()) { - fail("expected error about missing field in the lockfile, but succeeded"); - } - assertThat(result.getError().toString()) - .contains( - "Failed to read and parse the MODULE.bazel.lock file with error: " - + "java.lang.IllegalStateException: Missing required properties: moduleFileHash " - + "flags localOverrideHashes moduleDepGraph. Try deleting it and rerun the build."); - } - - @Test - public void invalidLockfileNullFlag() throws Exception { - scratch.overwriteFile( - rootDirectory.getRelative("MODULE.bazel").getPathString(), - "module(name='my_root', version='1.0')"); - - EvaluationResult rootResult = - evaluator.evaluate( - ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); - if (rootResult.hasError()) { - fail(rootResult.getError().toString()); - } - RootModuleFileValue rootValue = rootResult.get(ModuleFileValue.KEY_FOR_ROOT_MODULE); - - ImmutableMap depGraph = - ImmutableMap.of(ModuleKey.ROOT, InterimModule.toModule(rootValue.getModule(), null, null)); - - UpdateLockFileKey key = - UpdateLockFileKey.create("moduleHash", depGraph, rootValue.getOverrides()); - EvaluationResult result = - evaluator.evaluate(ImmutableList.of(key), evaluationContext); - if (result.hasError()) { - fail(result.getError().toString()); - } - - JsonObject jsonObject = - (JsonObject) JsonParser.parseString(scratch.readFile("MODULE.bazel.lock")); - jsonObject.get("flags").getAsJsonObject().remove("directDependenciesMode"); - scratch.overwriteFile( - rootDirectory.getRelative("MODULE.bazel.lock").getPathString(), jsonObject.toString()); - - result = evaluator.evaluate(ImmutableList.of(BazelLockFileValue.KEY), evaluationContext); - if (!result.hasError()) { - fail("expected error about missing field in the lockfile, but succeeded"); - } - assertThat(result.getError().toString()) - .contains( - "Failed to read and parse the MODULE.bazel.lock file with error: Null" - + " directDependenciesMode. Try deleting it and rerun the build."); - } - - @Test - public void invalidLockfileMalformed() throws Exception { - scratch.overwriteFile( - rootDirectory.getRelative("MODULE.bazel").getPathString(), - "module(name='my_root', version='1.0')"); - - EvaluationResult rootResult = - evaluator.evaluate( - ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); - if (rootResult.hasError()) { - fail(rootResult.getError().toString()); - } - RootModuleFileValue rootValue = rootResult.get(ModuleFileValue.KEY_FOR_ROOT_MODULE); - - ImmutableMap depGraph = - ImmutableMap.of(ModuleKey.ROOT, InterimModule.toModule(rootValue.getModule(), null, null)); - - UpdateLockFileKey key = - UpdateLockFileKey.create("moduleHash", depGraph, rootValue.getOverrides()); - EvaluationResult result = - evaluator.evaluate(ImmutableList.of(key), evaluationContext); - if (result.hasError()) { - fail(result.getError().toString()); - } - - JsonObject jsonObject = - (JsonObject) JsonParser.parseString(scratch.readFile("MODULE.bazel.lock")); - jsonObject.get("flags").getAsJsonObject().addProperty("allowedYankedVersions", "string!"); - scratch.overwriteFile( - rootDirectory.getRelative("MODULE.bazel.lock").getPathString(), jsonObject.toString()); - - result = evaluator.evaluate(ImmutableList.of(BazelLockFileValue.KEY), evaluationContext); - if (!result.hasError()) { - fail("expected error about invalid field value in the lockfile, but succeeded"); - } - Pattern expectedExceptionMessage = - Pattern.compile( - Pattern.quote( - "Failed to read and parse the MODULE.bazel.lock file with error:" - + " java.lang.IllegalStateException: Expected BEGIN_ARRAY but was STRING at" - + " line 1 column 129 path $.flags.allowedYankedVersions") - + ".*" - + Pattern.quote("Try deleting it and rerun the build."), - Pattern.DOTALL); - assertThat(result.getError().toString()).containsMatch(expectedExceptionMessage); - } - - @AutoValue - abstract static class UpdateLockFileKey implements SkyKey { - - abstract String moduleHash(); - - abstract ImmutableMap depGraph(); - - abstract ImmutableMap overrides(); - - static UpdateLockFileKey create( - String moduleHash, - ImmutableMap depGraph, - ImmutableMap overrides) { - return new AutoValue_BazelLockFileFunctionTest_UpdateLockFileKey( - moduleHash, depGraph, overrides); - } - - @Override - public SkyFunctionName functionName() { - return updateLockfileFunction; - } - } -} diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java index 2c4a7b3099c6d6..38ee7f4d338e29 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java @@ -146,7 +146,9 @@ public void setup() throws Exception { .put(SkyFunctions.BAZEL_DEP_GRAPH, new BazelDepGraphFunction()) .put(SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(rootDirectory)) .put(SkyFunctions.BAZEL_MODULE_RESOLUTION, new BazelModuleResolutionFunction()) - .put(SkyFunctions.REGISTRY, new RegistryFunction(registryFactory)) + .put( + SkyFunctions.REGISTRY, + new RegistryFunction(registryFactory, directories.getWorkspace())) .put(SkyFunctions.REPO_SPEC, new RepoSpecFunction()) .put(SkyFunctions.YANKED_VERSIONS, new YankedVersionsFunction()) .put( diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleFunctionTest.java index 444ba83ce719d1..9c98532eec437b 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleFunctionTest.java @@ -134,7 +134,9 @@ public void setup() throws Exception { ruleClassProvider.getBazelStarlarkEnvironment(), workspaceRoot, ImmutableMap.of())) - .put(SkyFunctions.REGISTRY, new RegistryFunction(registryFactory)) + .put( + SkyFunctions.REGISTRY, + new RegistryFunction(registryFactory, directories.getWorkspace())) .put(SkyFunctions.REPO_SPEC, new RepoSpecFunction()) .put(SkyFunctions.YANKED_VERSIONS, new YankedVersionsFunction()) .put( diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java index 81e4685a404dbe..15a5fb4e2b572c 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.bazel.bzlmod.BzlmodTestUtil.InterimModuleBuilder; import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue; +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.bazel.repository.starlark.StarlarkRepositoryModule; import com.google.devtools.build.lib.clock.BlazeClock; @@ -204,7 +205,9 @@ private void setUpWithBuiltinModules(ImmutableMap b .put( BzlmodRepoRuleValue.BZLMOD_REPO_RULE, new BzlmodRepoRuleFunction(ruleClassProvider, directories)) - .put(SkyFunctions.REGISTRY, new RegistryFunction(registryFactory)) + .put( + SkyFunctions.REGISTRY, + new RegistryFunction(registryFactory, directories.getWorkspace())) .put(SkyFunctions.REPO_SPEC, new RepoSpecFunction()) .put(SkyFunctions.YANKED_VERSIONS, new YankedVersionsFunction()) .put( @@ -231,6 +234,7 @@ private void setUpWithBuiltinModules(ImmutableMap b ModuleFileFunction.IGNORE_DEV_DEPS.set(differencer, false); ModuleFileFunction.MODULE_OVERRIDES.set(differencer, ImmutableMap.of()); YankedVersionsUtil.ALLOWED_YANKED_VERSIONS.set(differencer, ImmutableList.of()); + BazelLockFileFunction.LOCKFILE_MODE.set(differencer, LockfileMode.UPDATE); } @Test 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 1b31b80f82087c..9fe40a91905fd4 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.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; import com.google.errorprone.annotations.CanIgnoreReturnValue; @@ -86,7 +87,7 @@ public RepoSpec getRepoSpec(ModuleKey key, ExtendedEventHandler eventHandler) { "%s/modules/%s/%s/source.json" .formatted(url, key.getName(), key.getVersion().toString()), Optional.of( - GsonTypeAdapterUtil.createSingleExtensionUsagesValueHashGson() + GsonTypeAdapterUtil.SINGLE_EXTENSION_USAGES_VALUE_GSON .toJson(repoSpec) .getBytes(UTF_8)))); return repoSpec; @@ -98,6 +99,12 @@ public Optional> getYankedVersions( return Optional.ofNullable(yankedVersionMap.get(moduleName)); } + @Override + public Optional tryGetYankedVersionsFromLockfile( + ModuleKey selectedModuleKey) { + return Optional.empty(); + } + @Override public boolean equals(Object other) { return other instanceof FakeRegistry @@ -126,7 +133,10 @@ public FakeRegistry newFakeRegistry(String rootPath) { @Override public Registry createRegistry( - String url, ImmutableMap> fileHashes) { + String url, + LockfileMode lockfileMode, + ImmutableMap> fileHashes, + ImmutableMap previouslySelectedYankedVersions) { 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 f44daa4d364be1..e76f499ff020b1 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 @@ -32,12 +32,12 @@ import com.google.devtools.build.lib.authandtls.Netrc; import com.google.devtools.build.lib.authandtls.NetrcCredentials; import com.google.devtools.build.lib.authandtls.NetrcParser; +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.Checksum; import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader; import com.google.devtools.build.lib.testutil.FoundationTestCase; -import com.google.devtools.build.lib.vfs.Path; import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; @@ -85,12 +85,10 @@ public ImmutableMap> getRecordedHashes() { public void setUp() throws Exception { eventRecorder = new EventRecorder(); eventBus.register(eventRecorder); - Path workspaceRoot = scratch.dir("/ws"); repositoryCache = new RepositoryCache(); downloadManager = new DownloadManager(repositoryCache, new HttpDownloader()); registryFactory = - new RegistryFactoryImpl( - workspaceRoot, downloadManager, Suppliers.ofInstance(ImmutableMap.of())); + new RegistryFactoryImpl(downloadManager, Suppliers.ofInstance(ImmutableMap.of())); } @Test @@ -99,7 +97,8 @@ public void testHttpUrl() throws Exception { server.start(); Registry registry = - registryFactory.createRegistry(server.getUrl() + "/myreg", ImmutableMap.of()); + registryFactory.createRegistry( + server.getUrl() + "/myreg", LockfileMode.UPDATE, ImmutableMap.of(), ImmutableMap.of()); assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter)) .hasValue( ModuleFile.create( @@ -116,7 +115,8 @@ public void testHttpUrlWithNetrcCreds() throws Exception { new ByteArrayInputStream( "machine [::1] login rinne password rinnepass\n".getBytes(UTF_8))); Registry registry = - registryFactory.createRegistry(server.getUrl() + "/myreg", ImmutableMap.of()); + registryFactory.createRegistry( + server.getUrl() + "/myreg", LockfileMode.UPDATE, ImmutableMap.of(), ImmutableMap.of()); var e = assertThrows( @@ -146,7 +146,10 @@ public void testFileUrl() throws Exception { Registry registry = registryFactory.createRegistry( - new File(tempFolder.getRoot(), "fakereg").toURI().toString(), ImmutableMap.of()); + new File(tempFolder.getRoot(), "fakereg").toURI().toString(), + LockfileMode.UPDATE, + ImmutableMap.of(), + ImmutableMap.of()); assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter)) .hasValue(ModuleFile.create("lol".getBytes(UTF_8), file.toURI().toString())); assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter)).isEmpty(); @@ -182,7 +185,9 @@ public void testGetArchiveRepoSpec() throws Exception { "}"); server.start(); - Registry registry = registryFactory.createRegistry(server.getUrl(), ImmutableMap.of()); + Registry registry = + registryFactory.createRegistry( + server.getUrl(), LockfileMode.UPDATE, ImmutableMap.of(), ImmutableMap.of()); assertThat(registry.getRepoSpec(createModuleKey("foo", "1.0"), reporter)) .isEqualTo( new ArchiveRepoSpecBuilder() @@ -226,7 +231,9 @@ public void testGetLocalPathRepoSpec() throws Exception { "}"); server.start(); - Registry registry = registryFactory.createRegistry(server.getUrl(), ImmutableMap.of()); + Registry registry = + registryFactory.createRegistry( + server.getUrl(), LockfileMode.UPDATE, ImmutableMap.of(), ImmutableMap.of()); assertThat(registry.getRepoSpec(createModuleKey("foo", "1.0"), reporter)) .isEqualTo( RepoSpec.builder() @@ -248,7 +255,9 @@ public void testGetRepoInvalidRegistryJsonSpec() throws Exception { " \"strip_prefix\": \"pref\"", "}"); - Registry registry = registryFactory.createRegistry(server.getUrl(), ImmutableMap.of()); + Registry registry = + registryFactory.createRegistry( + server.getUrl(), LockfileMode.UPDATE, ImmutableMap.of(), ImmutableMap.of()); assertThat(registry.getRepoSpec(createModuleKey("foo", "1.0"), reporter)) .isEqualTo( new ArchiveRepoSpecBuilder() @@ -279,7 +288,9 @@ public void testGetRepoInvalidModuleJsonSpec() throws Exception { "}"); server.start(); - Registry registry = registryFactory.createRegistry(server.getUrl(), ImmutableMap.of()); + Registry registry = + registryFactory.createRegistry( + server.getUrl(), LockfileMode.UPDATE, ImmutableMap.of(), ImmutableMap.of()); assertThrows( IOException.class, () -> registry.getRepoSpec(createModuleKey("foo", "1.0"), reporter)); } @@ -306,7 +317,9 @@ public void testGetYankedVersion() throws Exception { + " }\n" + "}"); server.start(); - Registry registry = registryFactory.createRegistry(server.getUrl(), ImmutableMap.of()); + Registry registry = + registryFactory.createRegistry( + server.getUrl(), LockfileMode.UPDATE, ImmutableMap.of(), ImmutableMap.of()); Optional> yankedVersion = registry.getYankedVersions("red-pill", reporter); assertThat(yankedVersion) @@ -327,7 +340,9 @@ public void testArchiveWithExplicitType() throws Exception { "}"); server.start(); - Registry registry = registryFactory.createRegistry(server.getUrl(), ImmutableMap.of()); + Registry registry = + registryFactory.createRegistry( + server.getUrl(), LockfileMode.UPDATE, ImmutableMap.of(), ImmutableMap.of()); assertThat(registry.getRepoSpec(createModuleKey("archive_type", "1.0"), reporter)) .isEqualTo( new ArchiveRepoSpecBuilder() @@ -354,7 +369,9 @@ public void testGetModuleFileChecksums() throws Exception { Optional.of(sha256("old")), server.getUrl() + "/myreg/modules/unused/1.0/MODULE.bazel", Optional.of(sha256("unused"))); - Registry registry = registryFactory.createRegistry(server.getUrl() + "/myreg", knownFiles); + Registry registry = + registryFactory.createRegistry( + server.getUrl() + "/myreg", LockfileMode.UPDATE, knownFiles, ImmutableMap.of()); assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter)) .hasValue( ModuleFile.create( @@ -378,7 +395,9 @@ public void testGetModuleFileChecksums() throws Exception { Optional.empty()) .inOrder(); - registry = registryFactory.createRegistry(server.getUrl() + "/myreg", recordedChecksums); + registry = + registryFactory.createRegistry( + server.getUrl() + "/myreg", LockfileMode.UPDATE, recordedChecksums, ImmutableMap.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"); @@ -406,7 +425,9 @@ public void testGetModuleFileChecksumMismatch() throws Exception { ImmutableMap.of( server.getUrl() + "/myreg/modules/foo/1.0/MODULE.bazel", Optional.of(sha256("original"))); - Registry registry = registryFactory.createRegistry(server.getUrl() + "/myreg", knownFiles); + Registry registry = + registryFactory.createRegistry( + server.getUrl() + "/myreg", LockfileMode.UPDATE, knownFiles, ImmutableMap.of()); var e = assertThrows( IOException.class, @@ -445,7 +466,9 @@ public void testGetRepoSpecChecksum() throws Exception { var knownFiles = ImmutableMap.of( server.getUrl() + "/modules/foo/2.0/source.json", Optional.of(sha256("unused"))); - Registry registry = registryFactory.createRegistry(server.getUrl(), knownFiles); + Registry registry = + registryFactory.createRegistry( + server.getUrl(), LockfileMode.UPDATE, knownFiles, ImmutableMap.of()); assertThat(registry.getRepoSpec(createModuleKey("foo", "1.0"), reporter)) .isEqualTo( RepoSpec.builder() @@ -463,7 +486,9 @@ public void testGetRepoSpecChecksum() throws Exception { server.getUrl() + "/modules/foo/1.0/source.json", Optional.of(sha256(sourceJson).toString())); - registry = registryFactory.createRegistry(server.getUrl(), recordedChecksums); + registry = + registryFactory.createRegistry( + server.getUrl(), LockfileMode.UPDATE, recordedChecksums, ImmutableMap.of()); // Test that the recorded hashes are used for repo cache hits even when the server content // changes. server.unserve("/bazel_registry.json"); @@ -505,7 +530,9 @@ public void testGetRepoSpecChecksumMismatch() throws Exception { Optional.of(sha256(registryJson)), server.getUrl() + "/modules/foo/1.0/source.json", Optional.of(sha256(sourceJson))); - Registry registry = registryFactory.createRegistry(server.getUrl(), knownFiles); + Registry registry = + registryFactory.createRegistry( + server.getUrl(), LockfileMode.UPDATE, knownFiles, ImmutableMap.of()); var e = assertThrows( IOException.class, () -> registry.getRepoSpec(createModuleKey("foo", "1.0"), reporter)); @@ -547,7 +574,9 @@ public void testBazelRegistryChecksumMismatch() throws Exception { Optional.of(sha256(registryJson)), server.getUrl() + "/modules/foo/1.0/source.json", Optional.of(sha256(sourceJson))); - Registry registry = registryFactory.createRegistry(server.getUrl(), knownFiles); + Registry registry = + registryFactory.createRegistry( + server.getUrl(), LockfileMode.UPDATE, knownFiles, ImmutableMap.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/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java index abacd9ee3006fe..1d38f1bbed43df 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java @@ -268,7 +268,9 @@ public void setup() throws Exception { .put( SkyFunctions.SINGLE_EXTENSION_EVAL, new SingleExtensionEvalFunction(directories, ImmutableMap::of, downloadManager)) - .put(SkyFunctions.REGISTRY, new RegistryFunction(registryFactory)) + .put( + SkyFunctions.REGISTRY, + new RegistryFunction(registryFactory, directories.getWorkspace())) .put(SkyFunctions.REPO_SPEC, new RepoSpecFunction()) .put(SkyFunctions.YANKED_VERSIONS, new YankedVersionsFunction()) .put( diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java index e7af7cab62166f..7908509f7e2e56 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.bazel.bzlmod.BzlmodTestUtil.InterimModuleBuilder; import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue; +import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryModule; import com.google.devtools.build.lib.clock.BlazeClock; import com.google.devtools.build.lib.cmdline.RepositoryName; @@ -177,7 +178,9 @@ private void setUpWithBuiltinModules(ImmutableMap b .put( BzlmodRepoRuleValue.BZLMOD_REPO_RULE, new BzlmodRepoRuleFunction(ruleClassProvider, directories)) - .put(SkyFunctions.REGISTRY, new RegistryFunction(registryFactory)) + .put( + SkyFunctions.REGISTRY, + new RegistryFunction(registryFactory, directories.getWorkspace())) .put(SkyFunctions.REPO_SPEC, new RepoSpecFunction()) .put(SkyFunctions.YANKED_VERSIONS, new YankedVersionsFunction()) .put( @@ -204,6 +207,7 @@ private void setUpWithBuiltinModules(ImmutableMap b ModuleFileFunction.IGNORE_DEV_DEPS.set(differencer, false); ModuleFileFunction.MODULE_OVERRIDES.set(differencer, ImmutableMap.of()); YankedVersionsUtil.ALLOWED_YANKED_VERSIONS.set(differencer, ImmutableList.of()); + BazelLockFileFunction.LOCKFILE_MODE.set(differencer, LockfileMode.UPDATE); } @Test 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 c56ad709836890..cc963f233e5e3e 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,11 +20,10 @@ import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; +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; import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader; -import com.google.devtools.build.lib.testutil.FoundationTestCase; -import com.google.devtools.build.lib.vfs.Path; import java.net.URISyntaxException; import org.junit.Test; import org.junit.runner.RunWith; @@ -32,34 +31,34 @@ /** Tests for {@link RegistryFactory}. */ @RunWith(JUnit4.class) -public class RegistryFactoryTest extends FoundationTestCase { +public class RegistryFactoryTest { @Test - public void badSchemes() throws Exception { - Path workspaceRoot = scratch.dir("/ws"); + public void badSchemes() { RegistryFactory registryFactory = new RegistryFactoryImpl( - workspaceRoot, new DownloadManager(new RepositoryCache(), new HttpDownloader()), Suppliers.ofInstance(ImmutableMap.of())); Throwable exception = assertThrows( URISyntaxException.class, - () -> registryFactory.createRegistry("/home/www", ImmutableMap.of())); + () -> + registryFactory.createRegistry( + "/home/www", LockfileMode.UPDATE, ImmutableMap.of(), ImmutableMap.of())); assertThat(exception).hasMessageThat().contains("Registry URL has no scheme"); exception = assertThrows( URISyntaxException.class, - () -> registryFactory.createRegistry("foo://bar", ImmutableMap.of())); + () -> + registryFactory.createRegistry( + "foo://bar", LockfileMode.UPDATE, ImmutableMap.of(), ImmutableMap.of())); assertThat(exception).hasMessageThat().contains("Unrecognized registry URL protocol"); } @Test - public void badPath() throws Exception { - Path workspaceRoot = scratch.dir("/ws"); + public void badPath() { RegistryFactory registryFactory = new RegistryFactoryImpl( - workspaceRoot, new DownloadManager(new RepositoryCache(), new HttpDownloader()), Suppliers.ofInstance(ImmutableMap.of())); Throwable exception = @@ -67,7 +66,10 @@ public void badPath() throws Exception { URISyntaxException.class, () -> registryFactory.createRegistry( - "file:c:/path/to/workspace/registry", ImmutableMap.of())); + "file:c:/path/to/workspace/registry", + LockfileMode.UPDATE, + ImmutableMap.of(), + ImmutableMap.of())); assertThat(exception).hasMessageThat().contains("Registry URL path is not valid"); } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java index b6f888e61eccdd..bc176a5b776d94 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java @@ -264,7 +264,9 @@ public void setupDelegator() throws Exception { .put( BzlmodRepoRuleValue.BZLMOD_REPO_RULE, new BzlmodRepoRuleFunction(ruleClassProvider, directories)) - .put(SkyFunctions.REGISTRY, new RegistryFunction(registryFactory)) + .put( + SkyFunctions.REGISTRY, + new RegistryFunction(registryFactory, directories.getWorkspace())) .put(SkyFunctions.REPO_SPEC, new RepoSpecFunction()) .put(SkyFunctions.YANKED_VERSIONS, new YankedVersionsFunction()) .put( diff --git a/src/test/java/com/google/devtools/build/lib/testutil/BUILD b/src/test/java/com/google/devtools/build/lib/testutil/BUILD index 9a9b4f834ca164..f5d1477482c697 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/BUILD +++ b/src/test/java/com/google/devtools/build/lib/testutil/BUILD @@ -1,9 +1,9 @@ # Description: # A grab-bag of testing utilities. -load("//src/tools/bzlmod:utils.bzl", "get_canonical_repo_name") load("@bazel_skylib//rules:write_file.bzl", "write_file") load("@rules_java//java:defs.bzl", "java_library", "java_test") +load("//src/tools/bzlmod:blazel_utils.bzl", "get_canonical_repo_name") package( default_applicable_licenses = ["//:license"], diff --git a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py index 9e6ae20949b998..d96e3735a382f3 100644 --- a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py +++ b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py @@ -64,6 +64,32 @@ def tearDown(self): self.main_registry.stop() test_base.TestBase.tearDown(self) + def testInvalidLockfile(self): + self.ScratchFile('BUILD', ['filegroup(name = "hello")']) + self.RunBazel(['build', '//:all']) + + with open(self.Path('MODULE.bazel.lock'), 'r') as f: + lockfile = json.loads(f.read().strip()) + lockfile['registryFileHashes'] = 'I am a string' + + with open(self.Path('MODULE.bazel.lock'), 'w') as f: + f.write(json.dumps(lockfile)) + + exit_code, _, stderr = self.RunBazel( + ['build', '//:all'], allow_failure=True + ) + stderr = '\n'.join(stderr) + self.AssertExitCode(exit_code, 48, stderr) + self.assertRegex( + stderr, + ( + 'ERROR: Error computing the main repository mapping: Failed to read' + ' and parse the MODULE\\.bazel\\.lock file with error:' + ' java\\.lang\\.IllegalStateException: Expected BEGIN_OBJECT but' + ' was STRING .*\\. Try deleting it and rerun the build.' + ), + ) + def testChangeModuleInRegistryWithoutLockfile(self): # Add module 'sss' to the registry with dep on 'aaa' self.main_registry.createCcModule('sss', '1.3', {'aaa': '1.1'}) @@ -218,18 +244,18 @@ def testLockfileErrorMode(self): [ 'build', '--nobuild', - '--check_direct_dependencies=warning', '//:all', ], ) - # Run with updated module and a different flag - self.ScratchFile('MODULE.bazel', ['module(name="lala")']) + # Run with updated module + self.ScratchFile( + 'MODULE.bazel', ['bazel_dep(name="does_not_exist",version="0")'] + ) exit_code, _, stderr = self.RunBazel( [ 'build', '--nobuild', - '--check_direct_dependencies=error', '--lockfile_mode=error', '//:all', ], @@ -239,58 +265,16 @@ def testLockfileErrorMode(self): self.assertIn( ( 'ERROR: Error computing the main repository mapping:' - ' MODULE.bazel.lock is no longer up-to-date because: the root' - ' MODULE.bazel has been modified, the value of' - ' --check_direct_dependencies flag has been modified. Please run' + ' Missing checksum for registry file {}'.format( + self.main_registry.getURL() + ) + + '/modules/does_not_exist/0/MODULE.bazel not permitted with' + ' --lockfile_mode=error. Please run' ' `bazel mod deps --lockfile_mode=update` to update your lockfile.' ), stderr, ) - def testLocalOverrideWithErrorMode(self): - self.ScratchFile( - 'MODULE.bazel', - [ - 'module(name="lala")', - 'bazel_dep(name="bar")', - 'local_path_override(module_name="bar",path="bar")', - ], - ) - self.ScratchFile('BUILD', ['filegroup(name = "hello")']) - self.ScratchFile('bar/MODULE.bazel', ['module(name="bar")']) - self.ScratchFile('bar/BUILD', ['filegroup(name = "hello from bar")']) - self.RunBazel( - [ - 'build', - '--nobuild', - '//:all', - ], - ) - - # Run with updated module and a different flag - self.ScratchFile( - 'bar/MODULE.bazel', - [ - 'module(name="bar")', - 'bazel_dep(name="hmmm")', - ], - ) - exit_code, _, stderr = self.RunBazel( - ['build', '--nobuild', '--lockfile_mode=error', '//:all'], - allow_failure=True, - ) - self.AssertExitCode(exit_code, 48, stderr) - self.assertIn( - ( - 'ERROR: Error computing the main repository mapping:' - ' MODULE.bazel.lock is no longer up-to-date because: The' - ' MODULE.bazel file has changed for the overriden module: bar.' - ' Please run `bazel mod deps --lockfile_mode=update` to update your' - ' lockfile.' - ), - stderr, - ) - def testModuleExtension(self): self.ScratchFile( 'MODULE.bazel', @@ -493,7 +477,6 @@ def testModuleExtensionsInDifferentBuilds(self): with open(self.Path('MODULE.bazel.lock'), 'r') as f: lockfile = json.loads(f.read().strip()) - self.assertGreater(len(lockfile['moduleDepGraph']), 0) ext_keys = list(lockfile['moduleExtensions'].keys()) self.assertIn('//:extension.bzl%extA', ext_keys) self.assertIn('//:extension.bzl%extB', ext_keys) @@ -863,10 +846,10 @@ def testOldVersion(self): self.AssertExitCode(exit_code, 48, stderr) self.assertIn( ( - 'ERROR: Error computing the main repository mapping:' - ' MODULE.bazel.lock is no longer up-to-date because: the version of' - ' the lockfile is not compatible with the current Bazel. Please run' - ' `bazel mod deps --lockfile_mode=update` to update your lockfile.' + 'ERROR: Error computing the main repository mapping: The version' + ' of MODULE.bazel.lock is not supported by this version of Bazel.' + ' Please run `bazel mod deps --lockfile_mode=update` to update your' + ' lockfile.' ), stderr, ) @@ -1335,9 +1318,10 @@ def testExtensionEvaluationOnlyRerunOnRelevantUsagesChanges(self): '', 'def _ext_1_impl(ctx):', ' print("Ext 1 is being evaluated")', - ' num_tags = len([', - ' tag for mod in ctx.modules for tag in mod.tags.tag', - ' ])', + ( + ' num_tags = len([tag for mod in ctx.modules for tag in' + ' mod.tags.tag])' + ), ' repo_rule(name="dep", value="Ext 1 saw %s tags" % num_tags)', '', 'ext_1 = module_extension(', @@ -1347,9 +1331,10 @@ def testExtensionEvaluationOnlyRerunOnRelevantUsagesChanges(self): '', 'def _ext_2_impl(ctx):', ' print("Ext 2 is being evaluated")', - ' num_tags = len([', - ' tag for mod in ctx.modules for tag in mod.tags.tag', - ' ])', + ( + ' num_tags = len([tag for mod in ctx.modules for tag in' + ' mod.tags.tag])' + ), ' repo_rule(name="dep", value="Ext 2 saw %s tags" % num_tags)', '', 'ext_2 = module_extension(', @@ -1359,9 +1344,10 @@ def testExtensionEvaluationOnlyRerunOnRelevantUsagesChanges(self): '', 'def _ext_3_impl(ctx):', ' print("Ext 3 is being evaluated")', - ' num_tags = len([', - ' tag for mod in ctx.modules for tag in mod.tags.tag', - ' ])', + ( + ' num_tags = len([tag for mod in ctx.modules for tag in' + ' mod.tags.tag])' + ), ' repo_rule(name="dep", value="Ext 3 saw %s tags" % num_tags)', '', 'ext_3 = module_extension(', @@ -1536,19 +1522,9 @@ def testLockfileWithNoUserSpecificPath(self): ['build', '--registry=file:///%workspace%/registry', '//:lala'] ) - with open('MODULE.bazel.lock', 'r') as json_file: - lockfile = json.load(json_file) - ss_dep = lockfile['moduleDepGraph']['ss@1.3-1'] - remote_patches = ss_dep['repoSpec']['attributes']['remote_patches'] - ext_usage_location = ss_dep['extensionUsages'][0]['proxies'][0][ - 'location' - ]['file'] - - self.assertNotIn(self.my_registry.getURL(), ext_usage_location) - self.assertIn('%workspace%', ext_usage_location) - for key in remote_patches.keys(): - self.assertNotIn(self.my_registry.getURL(), key) - self.assertIn('%workspace%', key) + with open('MODULE.bazel.lock', 'r') as f: + self.assertNotIn(self.my_registry.getURL(), f.read()) + finally: self.my_registry.stop() 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..3cf795726b621c 100644 --- a/src/test/py/bazel/bzlmod/bazel_yanked_versions_test.py +++ b/src/test/py/bazel/bzlmod/bazel_yanked_versions_test.py @@ -16,6 +16,7 @@ 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 +41,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( @@ -199,6 +202,42 @@ def testAllowedYankedDepsByEnvVar(self): ''.join(stderr), ) + def testAllowedYankedDepsByEnvVarErrorMode(self): + self.writeBazelrcFile(allow_yanked_versions=False) + self.ScratchFile( + 'MODULE.bazel', + [ + 'bazel_dep(name = "ddd", version = "1.0")', + ], + ) + self.ScratchFile( + 'BUILD', + [ + 'cc_binary(', + ' name = "main",', + ' srcs = ["main.cc"],', + ' deps = ["@ddd//:lib_ddd"],', + ')', + ], + ) + self.RunBazel( + ['build', '--nobuild', '//:main'], + env_add={'BZLMOD_ALLOW_YANKED_VERSIONS': 'yanked1@1.0,yanked2@1.0'}, + ) + + # Test changing the env var, the build should fail again. + exit_code, _, stderr = self.RunBazel( + ['build', '--nobuild', '--lockfile_mode=error', '//:main'], + env_add={'BZLMOD_ALLOW_YANKED_VERSIONS': 'yanked2@1.0'}, + allow_failure=True, + ) + self.AssertExitCode(exit_code, 48, stderr) + self.assertIn( + 'Yanked version detected in your resolved dependency graph: ' + + 'yanked1@1.0, for the reason: dodgy.', + ''.join(stderr), + ) + def testAllowedYankedDepsSuccessMix(self): self.writeBazelrcFile(allow_yanked_versions=False) self.ScratchFile( @@ -227,6 +266,70 @@ 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/shell/bazel/remote_helpers.sh b/src/test/shell/bazel/remote_helpers.sh index e8181d210ff7db..14bbb63f36ac67 100755 --- a/src/test/shell/bazel/remote_helpers.sh +++ b/src/test/shell/bazel/remote_helpers.sh @@ -294,7 +294,15 @@ function setup_credential_helper() { cat > "${TEST_TMPDIR}/credhelper" <<'EOF' #!/usr/bin/env python3 +import json import os +import sys + +# Neither count nor add headers to requests to the BCR. +uri = json.load(sys.stdin)["uri"] +if uri.startswith("https://bcr.bazel.build/"): + print("{}") + sys.exit(0) path = os.path.join(os.environ["TEST_TMPDIR"], "credhelper.callcount") fd = os.open(path, os.O_WRONLY|os.O_CREAT|os.O_APPEND) diff --git a/src/test/shell/bazel/starlark_repository_test.sh b/src/test/shell/bazel/starlark_repository_test.sh index 5742bb5ad35629..a53803f14d8486 100755 --- a/src/test/shell/bazel/starlark_repository_test.sh +++ b/src/test/shell/bazel/starlark_repository_test.sh @@ -2327,9 +2327,6 @@ genrule( ) EOF - # Ensure that all Bzlmod-related downloads have happened before disabling - # downloads. - bazel mod deps || fail "Failed to cache Bazel modules" bazel build --repository_disable_download //:it > "${TEST_log}" 2>&1 \ && fail "Expected failure" || : expect_log "Failed to download repository @.*: download is disabled" diff --git a/src/test/tools/bzlmod/BUILD b/src/test/tools/bzlmod/BUILD index ccaa79a657c45e..a0bbf6bc15bbbb 100644 --- a/src/test/tools/bzlmod/BUILD +++ b/src/test/tools/bzlmod/BUILD @@ -23,6 +23,5 @@ sh_test( "//src:bazel", "//src/test/shell/bazel:test-deps", ], - tags = ["requires-network"], deps = ["@bazel_tools//tools/bash/runfiles"], ) diff --git a/src/tools/bzlmod/blazel_utils.bzl b/src/tools/bzlmod/blazel_utils.bzl new file mode 100644 index 00000000000000..9cd5a2723afb33 --- /dev/null +++ b/src/tools/bzlmod/blazel_utils.bzl @@ -0,0 +1,21 @@ +# 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. + +"""Helper functions for Bzlmod build that are safe to use both internally and externally.""" + +def get_canonical_repo_name(apparent_repo_name): + """Returns the canonical repo name for the given apparent repo name seen by the module this bzl file belongs to.""" + if not apparent_repo_name.startswith("@"): + apparent_repo_name = "@" + apparent_repo_name + return Label(apparent_repo_name).workspace_name diff --git a/src/tools/bzlmod/utils.bzl b/src/tools/bzlmod/utils.bzl index c9bfcdd270eed9..c8b22a93bd26db 100644 --- a/src/tools/bzlmod/utils.bzl +++ b/src/tools/bzlmod/utils.bzl @@ -14,11 +14,10 @@ """Helper functions for Bzlmod build""" -def get_canonical_repo_name(apparent_repo_name): - """Returns the canonical repo name for the given apparent repo name seen by the module this bzl file belongs to.""" - if not apparent_repo_name.startswith("@"): - apparent_repo_name = "@" + apparent_repo_name - return Label(apparent_repo_name).workspace_name +load("@buildozer//:buildozer.bzl", "BUILDOZER_LABEL") +load(":blazel_utils.bzl", _get_canonical_repo_name = "get_canonical_repo_name") + +get_canonical_repo_name = _get_canonical_repo_name def extract_url(attributes): """Extracts the url from the given attributes. @@ -52,26 +51,56 @@ def parse_http_artifacts(ctx, lockfile_path, required_repos): lockfile = json.decode(ctx.read(lockfile_path)) http_artifacts = [] found_repos = [] - for _, module in lockfile["moduleDepGraph"].items(): - if "repoSpec" in module and module["repoSpec"]["ruleClassName"] == "http_archive": - repo_spec = module["repoSpec"] - attributes = repo_spec["attributes"] - repo_name = _module_repo_name(module) + if "moduleDepGraph" in lockfile: + # TODO: Remove this branch after Bazel is built with 7.2.0. + for _, module in lockfile["moduleDepGraph"].items(): + if "repoSpec" in module and module["repoSpec"]["ruleClassName"] == "http_archive": + repo_spec = module["repoSpec"] + attributes = repo_spec["attributes"] + repo_name = _module_repo_name(module) + if repo_name not in required_repos: + continue + found_repos.append(repo_name) + + http_artifacts.append({ + "integrity": attributes["integrity"], + "url": extract_url(attributes), + }) + if "remote_patches" in attributes: + for patch, integrity in attributes["remote_patches"].items(): + http_artifacts.append({ + "integrity": integrity, + "url": patch, + }) + else: + for url, sha256 in lockfile["registryFileHashes"].items(): + if not url.endswith("/source.json"): + continue + segments = url.split("/") + module = { + "name": segments[-3], + "version": segments[-2], + } + repo_name = _module_repo_name(module) if repo_name not in required_repos: continue found_repos.append(repo_name) + ctx.delete("./tempfile") + ctx.download(url, "./tempfile", executable = False, sha256 = sha256) + source_json = json.decode(ctx.read("./tempfile")) + http_artifacts.append({ - "integrity": attributes["integrity"], - "url": extract_url(attributes), + "integrity": source_json["integrity"], + "url": source_json["url"], }) - if "remote_patches" in attributes: - for patch, integrity in attributes["remote_patches"].items(): - http_artifacts.append({ - "integrity": integrity, - "url": patch, - }) + + for patch, integrity in source_json.get("patches", {}).items(): + http_artifacts.append({ + "integrity": integrity, + "url": url.rsplit("/", 1)[0] + "/patches/" + patch, + }) for extension_id, extension_entry in lockfile["moduleExtensions"].items(): if extension_id.startswith("@@"): @@ -109,6 +138,83 @@ def parse_http_artifacts(ctx, lockfile_path, required_repos): return http_artifacts +BCR_URL_SCHEME = "https://bcr.bazel.build/modules/{name}/{version}/{file}" + +def parse_registry_files(ctx, lockfile_path, module_files): + """Parses the registry files referenced by the given lockfile and returns them in http_file form. + + Args: + ctx: the repository / module extension ctx object. + lockfile_path: The path of the lockfile to extract the registry files from. + module_files: The paths of non-registry module files to use during fake module resolution. + + Returns: + A list of http artifacts in the form of + [{"sha256": , "url": }, ...] + """ + lockfile = json.decode(ctx.read(lockfile_path)) + registry_file_hashes = lockfile.get("registryFileHashes", {}) + if registry_file_hashes: + return [ + {"sha256": sha256, "url": url} + for url, sha256 in registry_file_hashes.items() + ] + + # TODO: Remove the following code after Bazel is built with 7.2.0. + registry_files = ["https://bcr.bazel.build/bazel_registry.json"] + + # 1. Collect all source.json files of selected module versions. + for module in lockfile["moduleDepGraph"].values(): + if module["version"]: + registry_files.append(BCR_URL_SCHEME.format( + name = module["name"], + version = module["version"], + file = "source.json", + )) + + # 2. Download registry files to compute their hashes. + registry_file_artifacts = [] + downloads = { + url: ctx.download(url, "./tempdir/{}".format(i), executable = False, block = False) + for i, url in enumerate(registry_files) + } + for url, download in downloads.items(): + hash = download.wait() + registry_file_artifacts.append({"url": url, "sha256": hash.sha256}) + + # 3. Perform module resolution in Starlark to get the MODULE.bazel file URLs + # of all module versions relevant during resolution. The lockfile only + # contains the selected module versions. + module_file_stack = [ctx.path(module_file) for module_file in module_files] + seen_deps = {} + for _ in range(1000000): + if not module_file_stack: + break + bazel_deps = _extract_bazel_deps(ctx, module_file_stack.pop()) + downloads = {} + for dep in bazel_deps: + if dep in seen_deps: + continue + url = BCR_URL_SCHEME.format( + name = dep.name, + version = dep.version, + file = "MODULE.bazel", + ) + path = ctx.path("./tempdir/modules/{name}/{version}/MODULE.bazel".format( + name = dep.name, + version = dep.version, + )) + module_file_stack.append(path) + seen_deps[dep] = None + downloads[url] = ctx.download(url, path, executable = False, block = False) + + for url, download in downloads.items(): + hash = download.wait() + registry_file_artifacts.append({"url": url, "sha256": hash.sha256}) + + ctx.delete("./tempdir") + return registry_file_artifacts + def parse_bazel_module_repos(ctx, lockfile_path): """Parse repo names of http_archive backed Bazel modules from the given lockfile. @@ -122,13 +228,17 @@ def parse_bazel_module_repos(ctx, lockfile_path): lockfile = json.decode(ctx.read(lockfile_path)) repos = [] - for _, module in lockfile["moduleDepGraph"].items(): - if "repoSpec" in module and module["repoSpec"]["ruleClassName"] == "http_archive": - repo_spec = module["repoSpec"] - attributes = repo_spec["attributes"] - repo_name = _module_repo_name(module) - repos.append(repo_name) - return repos + for url in lockfile["registryFileHashes"].keys(): + if not url.endswith("/source.json"): + continue + segments = url.split("/") + module = { + "name": segments[-3], + "version": segments[-2], + } + repo_name = _module_repo_name(module) + repos.append(repo_name) + return {repo: None for repo in repos}.keys() # Keep in sync with ModuleKey. _WELL_KNOWN_MODULES = ["bazel_tools", "local_config_platform", "platforms"] @@ -143,3 +253,25 @@ def _module_repo_name(module): return "{}~".format(module_name) return "{}~{}".format(module_name, module["version"]) + +def _extract_bazel_deps(ctx, module_file): + buildozer = ctx.path(BUILDOZER_LABEL) + temp_path = "tempdir/buildozer/MODULE.bazel" + ctx.delete(temp_path) + ctx.symlink(module_file, temp_path) + result = ctx.execute([buildozer, "print name version dev_dependency", temp_path + ":%bazel_dep"]) + if result.return_code != 0: + fail("Failed to extract bazel_dep from {}:\n{}".format(module_file, result.stderr)) + deps = [] + for line in result.stdout.splitlines(): + if " " in line: + # The dep doesn't have a version specified, which is only valid in + # the root module. Ignore it. + continue + if line.endswith(" True"): + # The dep is a dev_dependency, ignore it. + continue + name, version, _ = line.split(" ") + deps.append(struct(name = name, version = version)) + + return deps