HttpDownloader: also cache files downloaded without provided hash

Even if downloading a file without a predicted hash, add it to the cache.
This step is necessary to avoid redownloading when we switch to resolved
WORKSPACES. Starting from a plain URL, the file is downloaded and the hash
is added to the resolved version of the rule. Then we still want the resolved
form of the rule (which still has different arguments) to download the
same file again.

Even before the design of resolved WORKSPACES is implemented, this caching
is useful for the manual workflow: take the hash that is printed for every
file downloaded without predicted hash and manually add it to the parameters
of the rule. Then we prefer not to redownload the file again.

Change-Id: I90eefa9efed7c47514cc481dc09d9e938efa5a39
PiperOrigin-RevId: 193907109

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

@@ -24,6 +24,7 @@
 import com.google.devtools.build.lib.buildeventstream.FetchEvent;
 import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.clock.JavaClock;
+import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
@@ -164,8 +165,8 @@ public Path download(
 
     Path destination = getDownloadDestination(urls.get(0), type, output);
 
-    // Used to decide whether to cache the download at the end of this method.
-    boolean isCaching = false;
+    // Is set to true if the value should be cached by the sha256 value provided
+    boolean isCachingByProvidedSha256 = false;
 
     if (!sha256.isEmpty()) {
       try {
@@ -180,7 +181,7 @@ public Path download(
       }
 
       if (repositoryCache.isEnabled()) {
-        isCaching = true;
+        isCachingByProvidedSha256 = true;
 
         Path cachedDestination = repositoryCache.get(sha256, destination, KeyType.SHA256);
         if (cachedDestination != null) {
@@ -193,7 +194,7 @@ public Path download(
         Path candidate = dir.getRelative(destination.getBaseName());
         if (RepositoryCache.getChecksum(KeyType.SHA256, candidate).equals(sha256)) {
           // Found the archive in one of the distdirs, no need to download.
-          if (isCaching) {
+          if (isCachingByProvidedSha256) {
             repositoryCache.put(sha256, candidate, KeyType.SHA256);
           }
           FileSystemUtils.createDirectoryAndParents(destination.getParentDirectory());
@@ -231,8 +232,11 @@ public Path download(
       eventHandler.post(new FetchEvent(urls.get(0).toString(), success));
     }
 
-    if (isCaching) {
+    if (isCachingByProvidedSha256) {
       repositoryCache.put(sha256, destination, KeyType.SHA256);
+    } else if (repositoryCache.isEnabled()) {
+      String newSha256 = repositoryCache.put(destination, KeyType.SHA256);
+      eventHandler.handle(Event.info("SHA256 (" + urls.get(0) + ") = " + newSha256));
     }
 
     return destination;

src/test/shell/bazel/BUILD

@@ -451,7 +451,7 @@ sh_test(
     size = "large",
     srcs = ["bazel_repository_cache_test.sh"],
     data = [":test-deps"],
-    shard_count = 4,
+    shard_count = 6,
 )
 
 sh_test(

src/test/shell/bazel/bazel_repository_cache_test.sh

@@ -283,6 +283,50 @@ function test_load_cached_value() {
   expect_log "All external dependencies fetched successfully"
 }
 
+function test_write_cache_without_hash() {
+  setup_repository
+
+  # Have a WORKSPACE file without the specified sha256
+  cat > WORKSPACE <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+
+http_archive(
+    name = 'endangered',
+    url = 'http://localhost:$nc_port/bleh',
+    type = 'zip',
+)
+EOF
+
+  # Fetch; as we did not specify a hash, we expect bazel to tell us the hash
+  # in an info message.
+  bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
+    || fail "expected fetch to succeed"
+
+  expect_log "${sha256}"
+
+  # Shutdown the server; so fetching again won't work
+  shutdown_server
+  bazel clean --expunge
+
+  # As we don't have a predicted cache, we expect fetching to fail now.
+  bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
+    && fail "expected failure" || :
+
+  # However, if we add the hash, the value is taken from cache
+  cat > WORKSPACE <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+
+http_archive(
+    name = 'endangered',
+    url = 'http://localhost:$nc_port/bleh',
+    type = 'zip',
+    sha256 = '${sha256}',
+)
+EOF
+  bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
+    || fail "expected fetch to succeed"
+}
+
 function test_failed_fetch_without_cache() {
   setup_repository