From e730201e6bf8d6c1c80433b5b42305c3167a8660 Mon Sep 17 00:00:00 2001 From: Salma Samy Date: Tue, 13 Feb 2024 01:00:07 +0200 Subject: [PATCH] [7.1.0] Reproducible extension (#21306) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Allow module extension to declare it is reproducible and opt-out from the lockfile - Update bazelrc command from build to common to keep the same StarlarkSemantics for other commands - Update fetch option documentation --------- Co-authored-by: Xùdōng Yáng --- .bazelrc | 2 +- site/en/external/overview.md | 12 ++ .../lib/bazel/bzlmod/BazelLockFileModule.java | 12 +- .../bazel/bzlmod/LockFileModuleExtension.java | 5 + .../bazel/bzlmod/ModuleExtensionContext.java | 17 ++- .../bazel/bzlmod/ModuleExtensionMetadata.java | 24 ++- .../bzlmod/SingleExtensionEvalFunction.java | 60 ++++---- .../build/lib/pkgcache/PackageOptions.java | 14 +- .../py/bazel/bzlmod/bazel_lockfile_test.py | 139 ++++++++++++++++-- 9 files changed, 228 insertions(+), 57 deletions(-) diff --git a/.bazelrc b/.bazelrc index 6960254fea25fd..45cd93df85cab5 100644 --- a/.bazelrc +++ b/.bazelrc @@ -61,7 +61,7 @@ build --java_language_version=11 build --tool_java_language_version=11 # Fail if a glob doesn't match anything (https://github.com/bazelbuild/bazel/issues/8195) -build --incompatible_disallow_empty_glob +common --incompatible_disallow_empty_glob # Manually enable cc toolchain resolution before it is flipped. https://github.com/bazelbuild/bazel/issues/7260 build --incompatible_enable_cc_toolchain_resolution diff --git a/site/en/external/overview.md b/site/en/external/overview.md index bdda1cf5f9bddc..3ea4a449430dc5 100644 --- a/site/en/external/overview.md +++ b/site/en/external/overview.md @@ -94,6 +94,18 @@ Normally, Bazel only fetches a repo when it needs something from the repo, and the repo hasn't already been fetched. If the repo has already been fetched before, Bazel only re-fetches it if its definition has changed. +The `fetch` command can be used to initiate a pre-fetch for a repository, +target, or all necessary repositories to perform any build. This capability +enables offline builds using the `--nofetch` option. + +The `--fetch` option serves to manage network access. Its default value is true. +However, when set to false (`--nofetch`), the command will utilize any cached +version of the dependency, and if none exists, the command will result in +failure. + +See [fetch options](/reference/command-line-reference#fetch-options) for more +information about controlling fetch. + ### Directory layout {:#directory-layout} After being fetched, the repo can be found in the subdirectory `external` in the 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 50fbd82feb5a07..567a01bbea2c17 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 @@ -121,6 +121,11 @@ public void afterCommand() throws AbruptExitException { // Add the new resolved extensions for (var event : extensionResolutionEventsMap.values()) { + LockFileModuleExtension extension = event.getModuleExtension(); + if (!extension.shouldLockExtesnsion()) { + continue; + } + var oldExtensionEntries = updatedExtensionMap.get(event.getExtensionId()); ImmutableMap extensionEntries; if (oldExtensionEntries != null) { @@ -128,11 +133,11 @@ public void afterCommand() throws AbruptExitException { extensionEntries = new ImmutableMap.Builder() .putAll(oldExtensionEntries) - .put(event.getExtensionFactors(), event.getModuleExtension()) + .put(event.getExtensionFactors(), extension) .buildKeepingLast(); } else { // new extension - extensionEntries = ImmutableMap.of(event.getExtensionFactors(), event.getModuleExtension()); + extensionEntries = ImmutableMap.of(event.getExtensionFactors(), extension); } updatedExtensionMap.put(event.getExtensionId(), extensionEntries); } @@ -164,12 +169,13 @@ private boolean shouldKeepExtension( // If there is a new event for this extension, compare it with the existing ones ModuleExtensionResolutionEvent extEvent = extensionResolutionEventsMap.get(extensionId); if (extEvent != null) { + boolean doNotLockExtension = !extEvent.getModuleExtension().shouldLockExtesnsion(); boolean dependencyOnOsChanged = lockedExtensionKey.getOs().isEmpty() != extEvent.getExtensionFactors().getOs().isEmpty(); boolean dependencyOnArchChanged = lockedExtensionKey.getArch().isEmpty() != extEvent.getExtensionFactors().getArch().isEmpty(); - if (dependencyOnOsChanged || dependencyOnArchChanged) { + if (doNotLockExtension || dependencyOnOsChanged || dependencyOnArchChanged) { return false; } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java index fea4715f7de5e8..88cdd5c2e7888d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java @@ -54,6 +54,11 @@ public static Builder builder() { public abstract Builder toBuilder(); + public boolean shouldLockExtesnsion() { + return getModuleExtensionMetadata().isEmpty() + || !getModuleExtensionMetadata().get().getReproducible(); + } + /** Builder type for {@link LockFileModuleExtension}. */ @AutoValue.Builder public abstract static class Builder { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java index 491427a6d3263a..cdddef1c3a1479 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java @@ -206,11 +206,24 @@ public boolean rootModuleHasNonDevDependency() { @ParamType(type = String.class), @ParamType(type = NoneType.class) }), + @Param( + name = "reproducible", + doc = + "States that this module extension ensures complete reproducibility, thereby it " + + "should not be stored in the lockfile.", + positional = false, + named = true, + defaultValue = "False", + allowedTypes = { + @ParamType(type = Boolean.class), + }), }) public ModuleExtensionMetadata extensionMetadata( - Object rootModuleDirectDepsUnchecked, Object rootModuleDirectDevDepsUnchecked) + Object rootModuleDirectDepsUnchecked, + Object rootModuleDirectDevDepsUnchecked, + boolean reproducible) throws EvalException { return ModuleExtensionMetadata.create( - rootModuleDirectDepsUnchecked, rootModuleDirectDevDepsUnchecked); + rootModuleDirectDepsUnchecked, rootModuleDirectDevDepsUnchecked, reproducible); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java index 79444ae4251d5e..539d7db8b26eaf 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java @@ -63,10 +63,13 @@ public abstract class ModuleExtensionMetadata implements StarlarkValue { abstract UseAllRepos getUseAllRepos(); + abstract boolean getReproducible(); + private static ModuleExtensionMetadata create( @Nullable Set explicitRootModuleDirectDeps, @Nullable Set explicitRootModuleDirectDevDeps, - UseAllRepos useAllRepos) { + UseAllRepos useAllRepos, + boolean reproducible) { return new AutoValue_ModuleExtensionMetadata( explicitRootModuleDirectDeps != null ? ImmutableSet.copyOf(explicitRootModuleDirectDeps) @@ -74,27 +77,30 @@ private static ModuleExtensionMetadata create( explicitRootModuleDirectDevDeps != null ? ImmutableSet.copyOf(explicitRootModuleDirectDevDeps) : null, - useAllRepos); + useAllRepos, + reproducible); } static ModuleExtensionMetadata create( - Object rootModuleDirectDepsUnchecked, Object rootModuleDirectDevDepsUnchecked) + Object rootModuleDirectDepsUnchecked, + Object rootModuleDirectDevDepsUnchecked, + boolean reproducible) throws EvalException { if (rootModuleDirectDepsUnchecked == Starlark.NONE && rootModuleDirectDevDepsUnchecked == Starlark.NONE) { - return create(null, null, UseAllRepos.NO); + return create(null, null, UseAllRepos.NO, reproducible); } // When root_module_direct_deps = "all", accept both root_module_direct_dev_deps = None and // root_module_direct_dev_deps = [], but not root_module_direct_dev_deps = ["some_repo"]. if (rootModuleDirectDepsUnchecked.equals("all") && rootModuleDirectDevDepsUnchecked.equals(StarlarkList.immutableOf())) { - return create(null, null, UseAllRepos.REGULAR); + return create(null, null, UseAllRepos.REGULAR, reproducible); } if (rootModuleDirectDevDepsUnchecked.equals("all") && rootModuleDirectDepsUnchecked.equals(StarlarkList.immutableOf())) { - return create(null, null, UseAllRepos.DEV); + return create(null, null, UseAllRepos.DEV, reproducible); } if (rootModuleDirectDepsUnchecked.equals("all") @@ -152,7 +158,11 @@ static ModuleExtensionMetadata create( } } - return create(explicitRootModuleDirectDeps, explicitRootModuleDirectDevDeps, UseAllRepos.NO); + return create( + explicitRootModuleDirectDeps, + explicitRootModuleDirectDevDeps, + UseAllRepos.NO, + reproducible); } public void evaluate( 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 9dbae9b61ad61e..0d3ba48ff35540 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 @@ -152,20 +152,27 @@ public SkyValue compute(SkyKey skyKey, Environment env) } // Check the lockfile first for that module extension + LockFileModuleExtension lockedExtension = null; LockfileMode lockfileMode = BazelLockFileFunction.LOCKFILE_MODE.get(env); if (!lockfileMode.equals(LockfileMode.OFF)) { BazelLockFileValue lockfile = (BazelLockFileValue) env.getValue(BazelLockFileValue.KEY); if (lockfile == null) { return null; } - try { - SingleExtensionEvalValue singleExtensionEvalValue = - tryGettingValueFromLockFile(env, extensionId, extension, usagesValue, lockfile); - if (singleExtensionEvalValue != null) { - return singleExtensionEvalValue; + var lockedExtensionMap = lockfile.getModuleExtensions().get(extensionId); + lockedExtension = + lockedExtensionMap == null ? null : lockedExtensionMap.get(extension.getEvalFactors()); + if (lockedExtension != null) { + try { + SingleExtensionEvalValue singleExtensionEvalValue = + tryGettingValueFromLockFile( + env, extensionId, extension, usagesValue, lockfile, lockedExtension); + if (singleExtensionEvalValue != null) { + return singleExtensionEvalValue; + } + } catch (NeedsSkyframeRestartException e) { + return null; } - } catch (NeedsSkyframeRestartException e) { - return null; } } @@ -182,6 +189,24 @@ public SkyValue compute(SkyKey skyKey, Environment env) Optional moduleExtensionMetadata = moduleExtensionResult.getModuleExtensionMetadata(); + if (lockfileMode.equals(LockfileMode.ERROR)) { + boolean extensionShouldHaveBeenLocked = + moduleExtensionMetadata.map(metadata -> !metadata.getReproducible()).orElse(true); + // If this extension was not found in the lockfile, and after evaluation we found that it is + // not reproducible, then error indicating that it was expected to be in the lockfile. + if (lockedExtension == null && extensionShouldHaveBeenLocked) { + throw new SingleExtensionEvalFunctionException( + ExternalDepsException.withMessage( + Code.BAD_MODULE, + "The module extension '%s'%s does not exist in the lockfile", + extensionId, + extension.getEvalFactors().isEmpty() + ? "" + : " for platform " + extension.getEvalFactors()), + Transience.PERSISTENT); + } + } + // At this point the extension has been evaluated successfully, but SingleExtensionEvalFunction // may still fail if imported repositories were not generated. However, since imports do not // influence the evaluation of the extension and the validation also runs when the extension @@ -220,30 +245,13 @@ private SingleExtensionEvalValue tryGettingValueFromLockFile( ModuleExtensionId extensionId, RunnableExtension extension, SingleExtensionUsagesValue usagesValue, - BazelLockFileValue lockfile) + BazelLockFileValue lockfile, + LockFileModuleExtension lockedExtension) throws SingleExtensionEvalFunctionException, InterruptedException, NeedsSkyframeRestartException { LockfileMode lockfileMode = BazelLockFileFunction.LOCKFILE_MODE.get(env); - var lockedExtensionMap = lockfile.getModuleExtensions().get(extensionId); - LockFileModuleExtension lockedExtension = - lockedExtensionMap == null ? null : lockedExtensionMap.get(extension.getEvalFactors()); - if (lockedExtension == null) { - if (lockfileMode.equals(LockfileMode.ERROR)) { - throw new SingleExtensionEvalFunctionException( - ExternalDepsException.withMessage( - Code.BAD_MODULE, - "The module extension '%s'%s does not exist in the lockfile", - extensionId, - extension.getEvalFactors().isEmpty() - ? "" - : " for platform " + extension.getEvalFactors()), - Transience.PERSISTENT); - } - return null; - } - ImmutableMap lockedExtensionUsages; try { // TODO(salmasamy) might be nicer to precompute this table when we construct diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java index 210f7c28564155..278710464c09df 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java @@ -173,12 +173,14 @@ public ParallelismConverter() throws OptionsParsingException { public int maxDirectoriesToEagerlyVisitInGlobbing; @Option( - name = "fetch", - defaultValue = "true", - documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, - effectTags = {OptionEffectTag.UNKNOWN}, - help = "Allows the command to fetch external dependencies" - ) + name = "fetch", + defaultValue = "true", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "Allows the command to fetch external dependencies. If set to false, the command will" + + " utilize any cached version of the dependency, and if none exists, the command" + + " will result in failure.") public boolean fetch; @Option( diff --git a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py index c7df0c9f97f038..f64c617217b84f 100644 --- a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py +++ b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py @@ -35,8 +35,6 @@ def setUp(self): 'aaa', '1.1' ).createCcModule('bbb', '1.0', {'aaa': '1.0'}).createCcModule( 'bbb', '1.1', {'aaa': '1.1'} - ).createCcModule( - 'ccc', '1.1', {'aaa': '1.1', 'bbb': '1.1'} ) self.ScratchFile( '.bazelrc', @@ -117,13 +115,7 @@ def testChangeModuleInRegistryWithLockfile(self): ], ) self.ScratchFile('BUILD', ['filegroup(name = "hello")']) - self.RunBazel( - [ - 'build', - '--nobuild', - '//:all', - ], - ) + self.RunBazel(['build', '--nobuild', '//:all']) # Change registry -> update 'sss' module file (corrupt it) module_dir = self.main_registry.root.joinpath('modules', 'sss', '1.3') @@ -146,9 +138,7 @@ def testChangeFlagWithLockfile(self): ], ) self.ScratchFile('BUILD', ['filegroup(name = "hello")']) - self.RunBazel( - ['build', '--nobuild', '//:all'], - ) + self.RunBazel(['build', '--nobuild', '//:all']) # Change registry -> update 'sss' module file (corrupt it) module_dir = self.main_registry.root.joinpath('modules', 'sss', '1.3') @@ -1900,6 +1890,131 @@ def testExtensionRepoMappingChange_mainRepoEvalCycleWithWorkspace(self): _, _, stderr = self.RunBazel(['build', '--enable_workspace', ':lol']) self.assertNotIn('ran the extension!', '\n'.join(stderr)) + def testReproducibleExtensionsIgnoredInLockfile(self): + self.ScratchFile( + 'MODULE.bazel', + [ + 'ext1 = use_extension("extension.bzl", "ext1")', + 'ext2 = use_extension("extension.bzl", "ext2")', + 'use_repo(ext1, "repo1")', + 'use_repo(ext2, "repo2")', + ], + ) + self.ScratchFile('BUILD.bazel') + self.ScratchFile( + 'extension.bzl', + [ + 'def _repo_rule_impl(ctx):', + ' ctx.file("BUILD", "filegroup(name=\'lala\')")', + 'repo_rule = repository_rule(implementation=_repo_rule_impl)', + '', + 'def _should_lock_impl(ctx): repo_rule(name="repo1")', + 'def _no_lock_impl(ctx):', + ' repo_rule(name="repo2")', + ' return ctx.extension_metadata(', + ' root_module_direct_deps=[],', + ' root_module_direct_dev_deps=[],', + ' reproducible=True', + ' )', + 'ext1 = module_extension(implementation=_should_lock_impl)', + 'ext2 = module_extension(implementation=_no_lock_impl)', + ], + ) + + self.RunBazel(['build', '@repo1//:all', '@repo2//:all']) + with open(self.Path('MODULE.bazel.lock'), 'r') as f: + lockfile = json.loads(f.read().strip()) + self.assertIn('//:extension.bzl%ext1', lockfile['moduleExtensions']) + self.assertNotIn('//:extension.bzl%ext2', lockfile['moduleExtensions']) + + # Update extensions implementations to the opposite + self.ScratchFile( + 'extension.bzl', + [ + 'def _repo_rule_impl(ctx):', + ' ctx.file("BUILD", "filegroup(name=\'lala\')")', + 'repo_rule = repository_rule(implementation=_repo_rule_impl)', + '', + 'def _should_lock_impl(ctx): repo_rule(name="repo2")', + 'def _no_lock_impl(ctx):', + ' repo_rule(name="repo1")', + ' return ctx.extension_metadata(', + ' root_module_direct_deps=[],', + ' root_module_direct_dev_deps=[],', + ' reproducible=True', + ' )', + 'ext1 = module_extension(implementation=_no_lock_impl)', + 'ext2 = module_extension(implementation=_should_lock_impl)', + ], + ) + + # Assert updates in the lockfile + self.RunBazel(['build', '@repo1//:all', '@repo2//:all']) + with open(self.Path('MODULE.bazel.lock'), 'r') as f: + lockfile = json.loads(f.read().strip()) + self.assertNotIn('//:extension.bzl%ext1', lockfile['moduleExtensions']) + self.assertIn('//:extension.bzl%ext2', lockfile['moduleExtensions']) + + def testReproducibleExtensionInLockfileErrorMode(self): + self.ScratchFile( + 'MODULE.bazel', + [ + 'ext = use_extension("extension.bzl", "ext")', + 'use_repo(ext, "repo")', + ], + ) + self.ScratchFile('BUILD.bazel') + self.ScratchFile( + 'extension.bzl', + [ + 'def _repo_rule_impl(ctx):', + ' ctx.file("BUILD", "filegroup(name=\'lala\')")', + 'repo_rule = repository_rule(implementation=_repo_rule_impl)', + '', + 'def _ext_impl(ctx):', + ' repo_rule(name="repo")', + ' return ctx.extension_metadata(', + ' root_module_direct_deps=[],', + ' root_module_direct_dev_deps=[],', + ' reproducible=True', + ' )', + 'ext = module_extension(implementation=_ext_impl)', + ], + ) + + self.RunBazel(['build', '@repo//:all']) + with open(self.Path('MODULE.bazel.lock'), 'r') as f: + lockfile = json.loads(f.read().strip()) + self.assertNotIn('//:extension.bzl%ext', lockfile['moduleExtensions']) + + # Assert ext does NOT fail in error mode + self.RunBazel(['build', '@repo//:all', '--lockfile_mode=error']) + + # Update extension to not be reproducible + self.ScratchFile( + 'extension.bzl', + [ + 'def _repo_rule_impl(ctx):', + ' ctx.file("BUILD", "filegroup(name=\'lala\')")', + 'repo_rule = repository_rule(implementation=_repo_rule_impl)', + '', + 'def _ext_impl(ctx): repo_rule(name="repo")', + 'ext = module_extension(implementation=_ext_impl)', + ], + ) + + # Assert ext does FAIL in error mode + _, _, stderr = self.RunBazel( + ['build', '@repo//:all', '--lockfile_mode=error'], allow_failure=True + ) + self.assertIn( + 'ERROR: The module extension ' + "'ModuleExtensionId{bzlFileLabel=//:extension.bzl, " + "extensionName=ext, isolationKey=Optional.empty}' does " + 'not exist in the lockfile', + stderr, + ) + if __name__ == '__main__': absltest.main()