From fc3460e92597091997fa4154173e2d4e973ede31 Mon Sep 17 00:00:00 2001 From: Son Luong Ngoc Date: Thu, 21 Sep 2023 10:52:31 -0700 Subject: [PATCH] Stablize and flip repository_cache_urls_as_default_canonical_id Introduced in #14268, this flag is very useful for bigger enterprise context where folks often version bumping dependencies without remembering to update the SHA256 of the downloaded file, leading to Bazel picking up older download entries from the repository cache. As we get more questions about this flag in Slack, marking it as stable and flip the default seems to be the right move. Closes #19549. PiperOrigin-RevId: 567356017 Change-Id: I6402e33f569789545e3ce17ebb42c51a8d56126f --- scripts/bootstrap/bootstrap.sh | 3 ++ .../bazel/repository/RepositoryOptions.java | 5 ++- .../lib/blackbox/bazel/DefaultToolsSetup.java | 3 ++ src/test/py/bazel/test_base.py | 3 ++ .../bazel/bazel_repository_cache_test.sh | 43 +++++++++++++++++-- src/test/shell/testenv.sh.tmpl | 3 ++ 6 files changed, 54 insertions(+), 6 deletions(-) diff --git a/scripts/bootstrap/bootstrap.sh b/scripts/bootstrap/bootstrap.sh index 87b9df5579cdbb..2e3b2c7375326e 100755 --- a/scripts/bootstrap/bootstrap.sh +++ b/scripts/bootstrap/bootstrap.sh @@ -31,11 +31,14 @@ fi : ${JAVA_VERSION:="11"} +# TODO: remove `norepository_cache_urls_as_default_canonical_id` once all dependencies are mirrored. +# See https://github.com/bazelbuild/bazel/pull/19549 for more context. _BAZEL_ARGS="--spawn_strategy=standalone \ --nojava_header_compilation \ --strategy=Javac=worker --worker_quit_after_build --ignore_unsupported_sandboxing \ --compilation_mode=opt \ --repository_cache=derived/repository_cache \ + --norepository_cache_urls_as_default_canonical_id \ --extra_toolchains=//scripts/bootstrap:all \ --extra_toolchains=@bazel_tools//tools/python:autodetecting_toolchain \ --enable_bzlmod \ diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java index 95c5cdf5609782..5040d2634252ce 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java @@ -287,8 +287,9 @@ public Converter() { public CheckDirectDepsMode checkDirectDependencies; @Option( - name = "experimental_repository_cache_urls_as_default_canonical_id", - defaultValue = "false", + name = "repository_cache_urls_as_default_canonical_id", + oldName = "experimental_repository_cache_urls_as_default_canonical_id", + defaultValue = "true", documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS, effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, metadataTags = {OptionMetadataTag.EXPERIMENTAL}, diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/bazel/DefaultToolsSetup.java b/src/test/java/com/google/devtools/build/lib/blackbox/bazel/DefaultToolsSetup.java index a226e8771c014a..8a4bef13988adc 100644 --- a/src/test/java/com/google/devtools/build/lib/blackbox/bazel/DefaultToolsSetup.java +++ b/src/test/java/com/google/devtools/build/lib/blackbox/bazel/DefaultToolsSetup.java @@ -78,6 +78,9 @@ public void setup(BlackBoxTestContext context) throws IOException { String sharedRepoCache = System.getenv("REPOSITORY_CACHE"); if (sharedRepoCache != null) { lines.add("common --repository_cache=" + sharedRepoCache); + // TODO(sluongng): Remove this flag once all dependencies are mirrored. + // See https://github.com/bazelbuild/bazel/pull/19549 for more context. + lines.add("common --norepository_cache_urls_as_default_canonical_id"); if (OS.getCurrent() == OS.DARWIN) { // For reducing SSD usage on our physical Mac machines. lines.add("common --experimental_repository_cache_hardlinks"); diff --git a/src/test/py/bazel/test_base.py b/src/test/py/bazel/test_base.py index 334828f250d0a4..24c60f900eeb4c 100644 --- a/src/test/py/bazel/test_base.py +++ b/src/test/py/bazel/test_base.py @@ -127,6 +127,9 @@ def setUp(self): shared_repo_cache = os.environ.get('REPOSITORY_CACHE') if shared_repo_cache: f.write('common --repository_cache={}\n'.format(shared_repo_cache)) + # TODO(sluongng): Remove this flag once all dependencies are mirrored. + # See https://github.com/bazelbuild/bazel/pull/19549 for more context. + f.write('common --norepository_cache_urls_as_default_canonical_id\n') if TestBase.IsDarwin(): # For reducing SSD usage on our physical Mac machines. f.write('common --experimental_repository_cache_hardlinks\n') diff --git a/src/test/shell/bazel/bazel_repository_cache_test.sh b/src/test/shell/bazel/bazel_repository_cache_test.sh index 28235a0557c272..d52d4bfbfbc7bb 100755 --- a/src/test/shell/bazel/bazel_repository_cache_test.sh +++ b/src/test/shell/bazel/bazel_repository_cache_test.sh @@ -494,18 +494,18 @@ EOF || echo "Expected fetch to succeed" } -function test_experimental_repository_cache_urls_as_default_canonical_id() { +function test_repository_cache_urls_as_default_canonical_id() { setup_repository bazel fetch --repository_cache="$repo_cache_dir" \ - --experimental_repository_cache_urls_as_default_canonical_id \ + --repository_cache_urls_as_default_canonical_id \ //zoo:breeding-program >& $TEST_log \ || echo "Expected fetch to succeed" shutdown_server bazel fetch --repository_cache="$repo_cache_dir" \ - --experimental_repository_cache_urls_as_default_canonical_id \ + --repository_cache_urls_as_default_canonical_id \ //zoo:breeding-program >& $TEST_log \ || echo "Expected fetch to succeed" @@ -524,9 +524,44 @@ EOF # As repository cache key should depend on urls, we expect fetching to fail now. bazel fetch --repository_cache="$repo_cache_dir" \ - --experimental_repository_cache_urls_as_default_canonical_id \ + --repository_cache_urls_as_default_canonical_id \ //zoo:breeding-program >& $TEST_log \ && fail "expected failure" || : } +function test_repository_legacy_default_canonical_id() { + setup_repository + + bazel fetch --repository_cache="$repo_cache_dir" \ + --norepository_cache_urls_as_default_canonical_id \ + //zoo:breeding-program >& $TEST_log \ + || echo "Expected fetch to succeed" + + shutdown_server + + bazel fetch --repository_cache="$repo_cache_dir" \ + --norepository_cache_urls_as_default_canonical_id \ + //zoo:breeding-program >& $TEST_log \ + || echo "Expected fetch to succeed" + + # Break url in WORKSPACE + rm WORKSPACE + cat >> $(create_workspace_with_default_repos WORKSPACE) <& $TEST_log \ + || echo "Expected fetch to succeed" +} + run_suite "repository cache tests" diff --git a/src/test/shell/testenv.sh.tmpl b/src/test/shell/testenv.sh.tmpl index a77b67ee9ddcfb..7518f8f2e559d9 100755 --- a/src/test/shell/testenv.sh.tmpl +++ b/src/test/shell/testenv.sh.tmpl @@ -324,6 +324,9 @@ EOF if [[ -n ${REPOSITORY_CACHE:-} ]]; then echo "testenv.sh: Using repository cache at $REPOSITORY_CACHE." echo "common --repository_cache=$REPOSITORY_CACHE" >> $TEST_TMPDIR/bazelrc + # TODO(sluongng): Remove this flag once all dependencies are mirrored. + # See https://github.com/bazelbuild/bazel/pull/19549 for more context. + echo "common --norepository_cache_urls_as_default_canonical_id" >> $TEST_TMPDIR/bazelrc if is_darwin; then # For reducing SSD usage on our physical Mac machines. echo "testenv.sh: Enabling --experimental_repository_cache_hardlinks"