Add --experimental_repository_cache_urls_as_default_canonical_id to h…

…elp detect broken repository URLs (#14989)

This new flag can be used to force redownloading when repository URLs are changed. Otherwise, it's possible broken URLs can be masked by the presence of a repository cache entry with the same hash. Specifying a `canonical_id` works as before, overriding this behavior.

Closes #14128.

Closes #14268.

PiperOrigin-RevId: 420976730
(cherry picked from commit f9882e4)

Co-authored-by: Ken Micklas <kmicklas@uber.com>

src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java

@@ -274,6 +274,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
       if (repoOptions.repositoryDownloaderRetries >= 0) {
         downloadManager.setRetries(repoOptions.repositoryDownloaderRetries);
       }
+      downloadManager.setUrlsAsDefaultCanonicalId(repoOptions.urlsAsDefaultCanonicalId);
 
       repositoryCache.setHardlink(repoOptions.useHardlinks);
       if (repoOptions.experimentalScaleTimeouts > 0.0) {

src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java

@@ -215,6 +215,19 @@ public class RepositoryOptions extends OptionsBase {
               + " to escalate it to a resolution failure.")
   public CheckDirectDepsMode checkDirectDependencies;
 
+  @Option(
+      name = "experimental_repository_cache_urls_as_default_canonical_id",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      metadataTags = {OptionMetadataTag.EXPERIMENTAL},
+      help =
+          "If true, use a string derived from the URLs of repository downloads as the canonical_id "
+              + "if not specified. This causes a change in the URLs to result in a redownload even "
+              + "if the cache contains a download with the same hash. This can be used to verify "
+              + "that URL changes don't result in broken repositories being masked by the cache.")
+  public boolean urlsAsDefaultCanonicalId;
+
   /** An enum for specifying different modes for checking direct dependency accuracy. */
   public enum CheckDirectDepsMode {
     OFF, // Don't check direct dependency accuracy.

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java

@@ -36,6 +36,7 @@
 import java.net.URL;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
 /**
@@ -52,6 +53,7 @@ public class DownloadManager {
   private final Downloader downloader;
   private boolean disableDownload = false;
   private int retries = 0;
+  private boolean urlsAsDefaultCanonicalId;
 
   public DownloadManager(RepositoryCache repositoryCache, Downloader downloader) {
     this.repositoryCache = repositoryCache;
@@ -75,6 +77,10 @@ public void setRetries(int retries) {
     this.retries = retries;
   }
 
+  public void setUrlsAsDefaultCanonicalId(boolean urlsAsDefaultCanonicalId) {
+    this.urlsAsDefaultCanonicalId = urlsAsDefaultCanonicalId;
+  }
+
   /**
    * Downloads file to disk and returns path.
    *
@@ -109,6 +115,10 @@ public Path download(
       throw new InterruptedException();
     }
 
+    if (Strings.isNullOrEmpty(canonicalId) && urlsAsDefaultCanonicalId) {
+      canonicalId = originalUrls.stream().map(URL::toExternalForm).collect(Collectors.joining(" "));
+    }
+
     ImmutableList<URL> rewrittenUrls = ImmutableList.copyOf(originalUrls);
     Map<URI, Map<String, String>> rewrittenAuthHeaders = authHeaders;
 

src/test/shell/bazel/bazel_repository_cache_test.sh

@@ -465,4 +465,68 @@ EOF
   expect_log "Error downloading"
 }
 
+function test_break_url() {
+  setup_repository
+
+  bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
+    || echo "Expected fetch to succeed"
+
+  shutdown_server
+
+  bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
+    || echo "Expected fetch to succeed"
+
+  # Break url in WORKSPACE
+  rm WORKSPACE
+  cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+
+http_archive(
+    name = 'endangered',
+    url = 'http://localhost:$nc_port/bleh.broken',
+    sha256 = '$sha256',
+    type = 'zip',
+)
+EOF
+
+  # By default, cache entry will still match by sha256, even if url is changed.
+  bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
+    || echo "Expected fetch to succeed"
+}
+
+function test_experimental_repository_cache_urls_as_default_canonical_id() {
+  setup_repository
+
+  bazel fetch --repository_cache="$repo_cache_dir" \
+        --experimental_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 \
+        //zoo:breeding-program >& $TEST_log \
+    || echo "Expected fetch to succeed"
+
+  # Break url in WORKSPACE
+  rm WORKSPACE
+  cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+
+http_archive(
+    name = 'endangered',
+    url = 'http://localhost:$nc_port/bleh.broken',
+    sha256 = '$sha256',
+    type = 'zip',
+)
+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 \
+        //zoo:breeding-program >& $TEST_log \
+    && fail "expected failure" || :
+}
+
 run_suite "repository cache tests"