Repository: bazelbuild/bazel
Issue: Request API in repository_ctx to download multiple files simultaneously
Labels: type: feature request
P2
team-ExternalDeps
team-Core
help wanted

### Description of the feature request:


tl;dr: Request an API that is similar to repository_ctx.download, but download multiple files simultaneously.



### Which category does this issue belong to?


Core, External Dependency

### What underlying problem are you trying to solve with this feature?


Example to reproduce the issue:

my_repo_rule.bzl
```
def _impl(repository_ctx):
    repository_ctx.download(
        url = "https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb",
        output = "chrome.deb",
    )
    repository_ctx.download(
        url = "https://dl.google.com/linux/direct/google-chrome-stable_current_x86_64.rpm",
        output = "chrome.rpm",
    )

    repository_ctx.file("WORKSPACE", "")
    repository_ctx.file("BUILD", """exports_files(glob(["**"]))""")

my_repo_rule = repository_rule(
    implementation = _impl,
)
```

WORKSPACE
```
load(":my_repo_rule.bzl", "my_repo_rule")

my_repo_rule(
    name = "my_ext_repo",
)
```

BUILD
```
# empty file
```

Then run
```
bazel fetch @my_ext_repo//...
```

You can see that the two files are downloaded in serial, not in parallel.

To workaround this issue, one could define two repositories. For example:

my_repo_rule.bzl
```
def _impl(repository_ctx):
    repository_ctx.download(
        url = repository_ctx.attr.url,
        output = "myfile",
    )

    repository_ctx.file("WORKSPACE", "")
    repository_ctx.file("BUILD", """exports_files(glob(["**"]))""")

my_repo_rule = repository_rule(
    implementation = _impl,
    attrs = {"url": attr.string()}
)
```

WORKSPACE
```
load(":my_repo_rule.bzl", "my_repo_rule")

my_repo_rule(
    name = "chrome_deb",
    url = "https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb"
)

my_repo_rule(
    name = "chrome_rpm",
    url = "https://dl.google.com/linux/direct/google-chrome-stable_current_x86_64.rpm"
)
```

However, when there are a lot of files, this approach requires creating a lot of repositories.

The benefit of downloading in parallel is:

- If there are a lot of small files to download, the time savings are big, because most of the time spent on setting up the connection is parallelized. If there are a few big files to download, the time savings are small.
- I could also do something like the following with good parallelization (pseudocode):

```
def _impl(repository_ctx):
    repository_ctx.download(<metadata file>)
    metadata = repository_ctx.read(<metadata file>)
    repository_ctx.download_multiple(metadata.urls)
```

Without the API to download multiple files, the above can only be done in WORKSPACE only.

```
# WORKSPACE
load(":download.bzl", "download_metadata", "download")
download_metadata(name = "metadata")

load("@metadata//:metadata.bzl", "metadata")
[
  download(name = elem.name, url = elem.url)
  for elem in meatadata
]
```

I can't hide all these in a single macro because the load statements interleave with the repository definitions.

### Which operating system are you running Bazel on?


Linux

### What is the output of `bazel info release`?


release 6.3.2

### If `bazel info release` returns `development version` or `(@non-git)`, tell us how you built Bazel.


_No response_

### What's the output of `git remote get-url origin; git rev-parse master; git rev-parse HEAD` ?


_No response_

### Have you found anything relevant by searching the web?

https://bazel.build/rules/lib/builtins/repository_ctx#download

### Any other information, logs, or outputs that you want to share?

_No response_

Issue comments:
This is how Kleaf works around the issue right now (by defining multiple repositories):

https://cs.android.com/android/kernel/superproject/+/common-android-mainline:build/kernel/kleaf/download_repo.bzl;l=356;drc=93375eb99ad216911bca8366cd8b70dd3d00def8

This feature request blocks DDKv2 development ([Internal bug 298416462](https://issuetracker.google.com/issues/298416462))
@meteorcloudy @Wyverald WDYT?

I could imagine implementing something like `ctx.download_multiple()`; it would be a non-trivial but small addition to our API surface, although the official Bazel Way at the moment is, as you say, to define multiple repositories and achieve parallelism that way.

@jacky8hyf How big are your downloads and how many of them are there and what pain do you experience from this? As long as you don't have an enormous number, repository-level parallelization should work, even if it is, as you say, a bit clunky.

What I have pretty strong feelings about is that I'd much rather not get into the business of implementing something like Futures (`tokens = [ctx.download(url) for url in urls]; [token.wait() for token in tokens]`) in Starlark.

From that internal bug, do I understand correctly that this is not a hard blocker, "only" a significant usability hurdle?


I agree this would be a nice feature. But the team is quite overloaded currently, community contribution is welcome!
I am working on rules to download kernel prebuilts from ci.android.com, and here are some numbers that may provide some context:

For a typical GKI build, I need to download **24** files to support external driver developments. The files range from empty (modules_blocklist), a few bytes (e.g. toolchain_version) to hundreds of megabytes (vmlinux, 294M)

For a certain build number that involves driver development kit (DDK):

```
42983565   gki_prebuilts_boot_img_tar_gz/file/boot-img.tar.gz
35205632   gki_prebuilts_Image/file/Image
14210442   gki_prebuilts_Image_gz/file/Image.gz
16654085   gki_prebuilts_Image_lz4/file/Image.lz4
146731     gki_prebuilts_kernel_aarch64_config_outdir_tar_gz/file/kernel_aarch64_config_outdir.tar.gz
191        gki_prebuilts_kernel_aarch64_ddk_headers_archive_tar_gz/metadata.bzl
9607113    gki_prebuilts_kernel_aarch64_ddk_headers_archive_tar_gz/file/kernel_aarch64_ddk_headers_archive.tar.gz
13099      gki_prebuilts_kernel_aarch64_env_sh/file/kernel_aarch64_env.sh
181195     gki_prebuilts_kernel_aarch64_internal_outs_tar_gz/file/kernel_aarch64_internal_outs.tar.gz
8875380    gki_prebuilts_kernel_aarch64_module_env_tar_gz/file/kernel_aarch64_module_env.tar.gz
1673       gki_prebuilts_kernel_aarch64_modules/file/kernel_aarch64_modules
1661306    gki_prebuilts_kernel_uapi_headers_tar_gz/file/kernel-uapi-headers.tar.gz
20328      gki_prebuilts_modules_builtin/file/modules.builtin
133459     gki_prebuilts_modules_builtin_modinfo/file/modules.builtin.modinfo
3922357    gki_prebuilts_modules_prepare_outdir_tar_gz/file/modules_prepare_outdir.tar.gz
7640754    gki_prebuilts_modules_staging_dir_tar_gz/file/modules_staging_dir.tar.gz
0          gki_prebuilts_system_dlkm_modules_blocklist/file/system_dlkm.modules.blocklist
2135       gki_prebuilts_system_dlkm_modules_load/file/system_dlkm.modules.load
7684967    gki_prebuilts_system_dlkm_staging_archive_tar_gz/file/system_dlkm_staging_archive.tar.gz
4710929    gki_prebuilts_System_map/file/System.map
9          gki_prebuilts_toolchain_version/file/toolchain_version
35398730   gki_prebuilts_unstripped_modules_tar_gz/file/unstripped_modules.tar.gz
307406096  gki_prebuilts_vmlinux/file/vmlinux
779305     gki_prebuilts_vmlinux_symvers/file/vmlinux.symvers
```

> From that internal bug, do I understand correctly that this is not a hard blocker, "only" a significant usability hurdle?

This is correct.

I think I'm hitting into a use case for this as well, though in my case splitting the downloads across multiple repos doesn't help, as passing a list of labels to a subsequent repo rule seems to still force downloads to happen serially. Details copied from the [Slack thread][slack].

To add some detail, I'm attempting to write a module extension that downloads several artifacts, extracts them, and then manipulates the output (in a way that's not amenable to e.g. a `genrule`).

Initially I went for the obvious implementation (`rctx.download_and_extract()` in a loop). Which worked, but had the disadvantage that prompted this FR, i.e. the downloads being serial.

I tried to work around this by changing it to more of a hub-and-spoke model: the `module_extension` generates a number of `http_archive`s, and then another repo rule takes a label list with the targets from the `http_archive`s.

However, this also seems to result in downloads happening serially -- I'm guessing this is because the label attr prefetch mechanism mentioned in the [docs][prefetch].

I've got a minimal-ish repro [here][repro] if it helps clarify the use-case at all. Please let me know if there's anything I've missed that would make the workaround as described ineffective.

[slack]: https://bazelbuild.slack.com/archives/CA31HN1T3/p1700269436249229
[repro]: https://gist.github.com/colatkinson/d3c0fbe0860d759b7ae88747a43eb67c
[prefetch]: https://bazel.build/extending/repo#restarting_the_implementation_function

Draft pull request: https://github.com/bazelbuild/bazel/pull/20309
@bazel-io fork 7.1.0
Just wanted to say thanks for developing this feature, it makes it much easier to implement `bzlmod` extensions that can do a few lightweight queries to the internet before calling repository rules without forcing users to wait a long time.
We're glad we could help :)
May I ask if this is enabled on 7.1.0 onwards? We are on 7.0.2 right now so we are looking for the version number to update to.
Yes, this is in 7.1.0.
A fix for this issue has been included in [Bazel 7.1.0 RC1](https://releases.bazel.build/7.1.0/rc1/index.html). Please test out the release candidate and report any issues as soon as possible. Thanks!

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD
index 69cee4b2f4a084..f3ed273ba387a1 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD
@@ -23,6 +23,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/clock",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//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/remote/util",
         "//src/main/java/com/google/devtools/build/lib/util",
         "//src/main/java/com/google/devtools/build/lib/util:os",
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 02656d8221f3e8..43853fdfdd6701 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
@@ -20,9 +20,11 @@
 import com.google.auth.Credentials;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Strings;
+import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import com.google.devtools.build.lib.authandtls.StaticCredentials;
 import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache;
 import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache.KeyType;
@@ -30,6 +32,8 @@
 import com.google.devtools.build.lib.bazel.repository.downloader.UrlRewriter.RewrittenURL;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.profiler.Profiler;
+import com.google.devtools.build.lib.profiler.SilentCloseable;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -42,6 +46,10 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import javax.annotation.Nullable;
 
 /**
@@ -51,6 +59,16 @@
  * to disk.
  */
 public class DownloadManager {
+  private static final ExecutorService DOWNLOAD_EXECUTOR =
+      Executors.newFixedThreadPool(
+          // There is also GrpcRemoteDownloader so if we set the thread pool to the same size as
+          // the allowed number of HTTP downloads, it might unnecessarily block. No, this is not a
+          // very
+          // principled approach; ideally, we'd grow the thread pool as needed with some generous
+          // upper
+          // limit.
+          2 * HttpDownloader.MAX_PARALLEL_DOWNLOADS,
+          new ThreadFactoryBuilder().setNameFormat("download-manager-%d").build());
 
   private final RepositoryCache repositoryCache;
   private List<Path> distdir = ImmutableList.of();
@@ -96,6 +114,69 @@ public void setCredentialFactory(CredentialFactory credentialFactory) {
     this.credentialFactory = credentialFactory;
   }
 
+  public Future<Path> startDownload(
+      List<URL> originalUrls,
+      Map<URI, Map<String, List<String>>> authHeaders,
+      Optional<Checksum> checksum,
+      String canonicalId,
+      Optional<String> type,
+      Path output,
+      ExtendedEventHandler eventHandler,
+      Map<String, String> clientEnv,
+      String context) {
+    return DOWNLOAD_EXECUTOR.submit(
+        () -> {
+          try (SilentCloseable c = Profiler.instance().profile("fetching: " + context)) {
+            return downloadInExecutor(
+                originalUrls,
+                authHeaders,
+                checksum,
+                canonicalId,
+                type,
+                output,
+                eventHandler,
+                clientEnv,
+                context);
+          }
+        });
+  }
+
+  public Path finalizeDownload(Future<Path> download) throws IOException, InterruptedException {
+    try {
+      return download.get();
+    } catch (ExecutionException e) {
+      Throwables.throwIfInstanceOf(e.getCause(), IOException.class);
+      Throwables.throwIfInstanceOf(e.getCause(), InterruptedException.class);
+      Throwables.throwIfUnchecked(e.getCause());
+      throw new IllegalStateException(e);
+    }
+  }
+
+  public Path download(
+      List<URL> originalUrls,
+      Map<URI, Map<String, List<String>>> authHeaders,
+      Optional<Checksum> checksum,
+      String canonicalId,
+      Optional<String> type,
+      Path output,
+      ExtendedEventHandler eventHandler,
+      Map<String, String> clientEnv,
+      String context)
+      throws IOException, InterruptedException {
+    Future<Path> future =
+        startDownload(
+            originalUrls,
+            authHeaders,
+            checksum,
+            canonicalId,
+            type,
+            output,
+            eventHandler,
+            clientEnv,
+            context);
+    return finalizeDownload(future);
+  }
+
   /**
    * Downloads file to disk and returns path.
    *
@@ -114,7 +195,7 @@ public void setCredentialFactory(CredentialFactory credentialFactory) {
    * @throws IOException if download was attempted and ended up failing
    * @throws InterruptedException if this thread is being cast into oblivion
    */
-  public Path download(
+  private Path downloadInExecutor(
       List<URL> originalUrls,
       Map<URI, Map<String, List<String>>> authHeaders,
       Optional<Checksum> checksum,
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
index 1cc14c1dd183c8..3e9e0f150a6ac8 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
@@ -47,7 +47,8 @@
  * file to disk.
  */
 public class HttpDownloader implements Downloader {
-  private static final int MAX_PARALLEL_DOWNLOADS = 8;
+  static final int MAX_PARALLEL_DOWNLOADS = 8;
+
   private static final Semaphore SEMAPHORE = new Semaphore(MAX_PARALLEL_DOWNLOADS, true);
   private static final Clock CLOCK = new JavaClock();
   private static final Sleeper SLEEPER = new JavaSleeper();
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java
index f34fb44603f63d..1374306b6be39c 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java
@@ -24,6 +24,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.collect.Maps;
+import com.google.common.util.concurrent.Futures;
 import com.google.devtools.build.lib.actions.FileValue;
 import com.google.devtools.build.lib.bazel.debug.WorkspaceRuleEvent;
 import com.google.devtools.build.lib.bazel.repository.DecompressorDescriptor;
@@ -34,6 +35,8 @@
 import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
 import com.google.devtools.build.lib.bazel.repository.downloader.HttpUtils;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.events.ExtendedEventHandler.FetchProgress;
 import com.google.devtools.build.lib.packages.StarlarkInfo;
 import com.google.devtools.build.lib.packages.StructImpl;
@@ -77,12 +80,16 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import javax.annotation.Nullable;
 import net.starlark.java.annot.Param;
 import net.starlark.java.annot.ParamType;
 import net.starlark.java.annot.StarlarkMethod;
 import net.starlark.java.eval.Dict;
 import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Printer;
 import net.starlark.java.eval.Sequence;
 import net.starlark.java.eval.Starlark;
 import net.starlark.java.eval.StarlarkInt;
@@ -93,6 +100,31 @@
 
 /** A common base class for Starlark "ctx" objects related to external dependencies. */
 public abstract class StarlarkBaseExternalContext implements StarlarkValue {
+
+  /**
+   * An asynchronous task run as part of fetching the repository.
+   *
+   * <p>The main property of such tasks is that they should under no circumstances keep running
+   * after fetching the repository is finished, whether successfully or not. To this end, the {@link
+   * #cancel()} method must stop all such work.
+   */
+  private interface AsyncTask {
+    /** Returns a user-friendly description of the task. */
+    String getDescription();
+
+    /** Returns where the task was started from. */
+    Location getLocation();
+
+    /**
+     * Cancels the task, if not done yet. Returns false if the task was still in progress.
+     *
+     * <p>No means of error reporting is provided. Any errors should be reported by other means. The
+     * only possible error reported as a consequence of calling this method is one that tells the
+     * user that they didn't wait for an async task they should have waited for.
+     */
+    boolean cancel();
+  }
+
   /** Max. number of command line args added as a profiler description. */
   private static final int MAX_PROFILE_ARGS_LEN = 80;
 
@@ -106,6 +138,7 @@ public abstract class StarlarkBaseExternalContext implements StarlarkValue {
   protected final StarlarkSemantics starlarkSemantics;
   private final HashMap<Label, String> accumulatedFileDigests = new HashMap<>();
   private final RepositoryRemoteExecutor remoteExecutor;
+  private final List<AsyncTask> asyncTasks;
 
   protected StarlarkBaseExternalContext(
       Path workingDirectory,
@@ -125,6 +158,31 @@ protected StarlarkBaseExternalContext(
     this.processWrapper = processWrapper;
     this.starlarkSemantics = starlarkSemantics;
     this.remoteExecutor = remoteExecutor;
+    this.asyncTasks = new ArrayList<>();
+  }
+
+  public boolean ensureNoPendingAsyncTasks(EventHandler eventHandler, boolean forSuccessfulFetch) {
+    boolean hadPendingItems = false;
+    for (AsyncTask task : asyncTasks) {
+      if (!task.cancel()) {
+        hadPendingItems = true;
+        if (forSuccessfulFetch) {
+          eventHandler.handle(
+              Event.error(
+                  task.getLocation(),
+                  "Work pending after repository rule finished execution: "
+                      + task.getDescription()));
+        }
+      }
+    }
+
+    return hadPendingItems;
+  }
+
+  // There is no unregister(). We don't have that many futures in each repository and it just
+  // introduces the failure mode of erroneously unregistering async work that's not done.
+  protected void registerAsyncTask(AsyncTask task) {
+    asyncTasks.add(task);
   }
 
   /** A string that can be used to identify this context object. Used for logging purposes. */
@@ -363,6 +421,101 @@ private StructImpl calculateDownloadResult(Optional<Checksum> checksum, Path dow
     return StarlarkInfo.create(StructProvider.STRUCT, out.buildOrThrow(), Location.BUILTIN);
   }
 
+  private class PendingDownload implements StarlarkValue, AsyncTask {
+    private final boolean executable;
+    private final boolean allowFail;
+    private final StarlarkPath outputPath;
+    private final Optional<Checksum> checksum;
+    private final RepositoryFunctionException checksumValidation;
+    private final Future<Path> future;
+    private final Location location;
+
+    private PendingDownload(
+        boolean executable,
+        boolean allowFail,
+        StarlarkPath outputPath,
+        Optional<Checksum> checksum,
+        RepositoryFunctionException checksumValidation,
+        Future<Path> future,
+        Location location) {
+      this.executable = executable;
+      this.allowFail = allowFail;
+      this.outputPath = outputPath;
+      this.checksum = checksum;
+      this.checksumValidation = checksumValidation;
+      this.future = future;
+      this.location = location;
+    }
+
+    @Override
+    public String getDescription() {
+      return String.format("downloading to '%s'", outputPath);
+    }
+
+    @Override
+    public Location getLocation() {
+      return location;
+    }
+
+    @Override
+    public boolean cancel() {
+      if (!future.cancel(true)) {
+        return true;
+      }
+
+      try {
+        future.get();
+        return false;
+      } catch (InterruptedException | ExecutionException | CancellationException e) {
+        // Ignore. The only thing we care about is that there is no async work in progress after
+        // this point. Any error reporting should have been done before.
+        return false;
+      }
+    }
+
+    @StarlarkMethod(
+        name = "wait",
+        doc =
+            "Blocks until the completion of the download and returns or throws as blocking "
+                + " download() call would")
+    public StructImpl await() throws InterruptedException, RepositoryFunctionException {
+      return completeDownload(this);
+    }
+
+    @Override
+    public void repr(Printer printer) {
+      printer.append(String.format("<pending download to '%s'>", outputPath));
+    }
+  }
+
+  private StructImpl completeDownload(PendingDownload pendingDownload)
+      throws RepositoryFunctionException, InterruptedException {
+    Path downloadedPath;
+    try {
+      downloadedPath = downloadManager.finalizeDownload(pendingDownload.future);
+      if (pendingDownload.executable) {
+        pendingDownload.outputPath.getPath().setExecutable(true);
+      }
+    } catch (IOException e) {
+      if (pendingDownload.allowFail) {
+        return StarlarkInfo.create(
+            StructProvider.STRUCT, ImmutableMap.of("success", false), Location.BUILTIN);
+      } else {
+        throw new RepositoryFunctionException(e, Transience.TRANSIENT);
+      }
+    } catch (InvalidPathException e) {
+      throw new RepositoryFunctionException(
+          Starlark.errorf(
+              "Could not create output path %s: %s", pendingDownload.outputPath, e.getMessage()),
+          Transience.PERSISTENT);
+    }
+    if (pendingDownload.checksumValidation != null) {
+      throw pendingDownload.checksumValidation;
+    }
+
+    return calculateDownloadResult(pendingDownload.checksum, downloadedPath);
+  }
+
   @StarlarkMethod(
       name = "download",
       doc =
@@ -435,8 +588,18 @@ private StructImpl calculateDownloadResult(Optional<Checksum> checksum, Path dow
                     + " risk to omit the checksum as remote files can change. At best omitting this"
                     + " field will make your build non-hermetic. It is optional to make development"
                     + " easier but should be set before shipping."),
+        @Param(
+            name = "block",
+            defaultValue = "True",
+            named = true,
+            positional = false,
+            doc =
+                "If set to false, the call returns immediately and instead of the regular return"
+                    + " value, it returns a token with one single method, wait(), which blocks"
+                    + " until the download is finished and returns the usual return value or"
+                    + " throws as usual.")
       })
-  public StructImpl download(
+  public Object download(
       Object url,
       Object output,
       String sha256,
@@ -445,17 +608,20 @@ public StructImpl download(
       String canonicalId,
       Dict<?, ?> authUnchecked, // <String, Dict> expected
       String integrity,
+      Boolean block,
       StarlarkThread thread)
       throws RepositoryFunctionException, EvalException, InterruptedException {
+    PendingDownload download = null;
     ImmutableMap<URI, Map<String, List<String>>> authHeaders =
         getAuthHeaders(getAuthContents(authUnchecked, "auth"));
 
     ImmutableList<URL> urls =
         getUrls(
             url,
-            /*ensureNonEmpty=*/ !allowFail,
-            /*checksumGiven=*/ !Strings.isNullOrEmpty(sha256) || !Strings.isNullOrEmpty(integrity));
-    Optional<Checksum> checksum;
+            /* ensureNonEmpty= */ !allowFail,
+            /* checksumGiven= */ !Strings.isNullOrEmpty(sha256)
+                || !Strings.isNullOrEmpty(integrity));
+    Optional<Checksum> checksum = null;
     RepositoryFunctionException checksumValidation = null;
     try {
       checksum = validateChecksum(sha256, integrity, urls);
@@ -475,13 +641,24 @@ public StructImpl download(
             getIdentifyingStringForLogging(),
             thread.getCallerLocation());
     env.getListener().post(w);
-    Path downloadedPath;
-    try (SilentCloseable c =
-        Profiler.instance().profile("fetching: " + getIdentifyingStringForLogging())) {
+
+    try {
       checkInOutputDirectory("write", outputPath);
       makeDirectories(outputPath.getPath());
-      downloadedPath =
-          downloadManager.download(
+    } catch (IOException e) {
+      download =
+          new PendingDownload(
+              executable,
+              allowFail,
+              outputPath,
+              checksum,
+              checksumValidation,
+              Futures.immediateFailedFuture(e),
+              thread.getCallerLocation());
+    }
+    if (download == null) {
+      Future<Path> downloadFuture =
+          downloadManager.startDownload(
               urls,
               authHeaders,
               checksum,
@@ -491,26 +668,22 @@ public StructImpl download(
               env.getListener(),
               envVariables,
               getIdentifyingStringForLogging());
-      if (executable) {
-        outputPath.getPath().setExecutable(true);
-      }
-    } catch (IOException e) {
-      if (allowFail) {
-        return StarlarkInfo.create(
-            StructProvider.STRUCT, ImmutableMap.of("success", false), Location.BUILTIN);
-      } else {
-        throw new RepositoryFunctionException(e, Transience.TRANSIENT);
-      }
-    } catch (InvalidPathException e) {
-      throw new RepositoryFunctionException(
-          Starlark.errorf("Could not create output path %s: %s", outputPath, e.getMessage()),
-          Transience.PERSISTENT);
+      download =
+          new PendingDownload(
+              executable,
+              allowFail,
+              outputPath,
+              checksum,
+              checksumValidation,
+              downloadFuture,
+              thread.getCallerLocation());
+      registerAsyncTask(download);
     }
-    if (checksumValidation != null) {
-      throw checksumValidation;
+    if (!block) {
+      return download;
+    } else {
+      return completeDownload(download);
     }
-
-    return calculateDownloadResult(checksum, downloadedPath);
   }
 
   @StarlarkMethod(
@@ -669,17 +842,15 @@ public StructImpl downloadAndExtract(
 
     Path downloadedPath;
     Path downloadDirectory;
-    try (SilentCloseable c =
-        Profiler.instance().profile("fetching: " + getIdentifyingStringForLogging())) {
-
+    try {
       // Download to temp directory inside the outputDirectory and delete it after extraction
       java.nio.file.Path tempDirectory =
           Files.createTempDirectory(Paths.get(outputPath.toString()), "temp");
       downloadDirectory =
           workingDirectory.getFileSystem().getPath(tempDirectory.toFile().getAbsolutePath());
 
-      downloadedPath =
-          downloadManager.download(
+      Future<Path> pendingDownload =
+          downloadManager.startDownload(
               urls,
               authHeaders,
               checksum,
@@ -689,6 +860,7 @@ public StructImpl downloadAndExtract(
               env.getListener(),
               envVariables,
               getIdentifyingStringForLogging());
+      downloadedPath = downloadManager.finalizeDownload(pendingDownload);
     } catch (IOException e) {
       env.getListener().post(w);
       if (allowFail) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java
index b3aabda85f2157..db616741fbdbbc 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java
@@ -282,6 +282,7 @@ private RepositoryDirectoryValue.Builder fetchInternal(
       // it possible to return null and not block but it doesn't seem to be easy with Starlark
       // structure as it is.
       Object result;
+      boolean fetchSuccessful = false;
       try (SilentCloseable c =
           Profiler.instance()
               .profile(ProfilerTask.STARLARK_REPOSITORY_FN, () -> rule.getLabel().toString())) {
@@ -291,7 +292,19 @@ private RepositoryDirectoryValue.Builder fetchInternal(
                 function,
                 /*args=*/ ImmutableList.of(starlarkRepositoryContext),
                 /*kwargs=*/ ImmutableMap.of());
+        fetchSuccessful = true;
+      } finally {
+        if (starlarkRepositoryContext.ensureNoPendingAsyncTasks(
+            env.getListener(), fetchSuccessful)) {
+          if (fetchSuccessful) {
+            throw new RepositoryFunctionException(
+                new EvalException(
+                    "Pending asynchronous work after repository rule finished running"),
+                Transience.PERSISTENT);
+          }
+        }
       }
+
       RepositoryResolvedEvent resolved =
           new RepositoryResolvedEvent(
               rule, starlarkRepositoryContext.getAttr(), outputDirectory, result);


Test patch:
diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh
index 83223138173ed7..e378a7074ccc4b 100755
--- a/src/test/shell/bazel/external_integration_test.sh
+++ b/src/test/shell/bazel/external_integration_test.sh
@@ -49,6 +49,9 @@ EOF
 
 tear_down() {
   shutdown_server
+  if [ -d "${TEST_TMPDIR}/server_dir" ]; then
+    rm -fr "${TEST_TMPDIR}/server_dir"
+  fi
 }
 
 function zip_up() {
@@ -410,6 +413,188 @@ EOF
   expect_log "404 Not Found"
 }
 
+function test_deferred_download_unwaited() {
+  cat >> $(create_workspace_with_default_repos WORKSPACE) <<'EOF'
+load("hang.bzl", "hang")
+
+hang(name="hang")
+EOF
+
+  cat > hang.bzl <<'EOF'
+def _hang_impl(rctx):
+  hangs = rctx.download(
+    # This URL will definitely not work, but that's OK -- we don't need a
+    # successful request for this test
+    url = "https://127.0.0.1:0/does_not_exist",
+    output = "does_not_exist",
+    block = False)
+
+hang = repository_rule(implementation = _hang_impl)
+EOF
+
+  touch BUILD
+  bazel query @hang//:all >& $TEST_log && fail "Bazel unexpectedly succeeded"
+  expect_log "Pending asynchronous work"
+}
+
+function test_deferred_download_two_parallel_downloads() {
+  local server_dir="${TEST_TMPDIR}/server_dir"
+  local gate_socket="${server_dir}/gate_socket"
+  local served_apple="APPLE"
+  local served_banana="BANANA"
+  local apple_sha256=$(echo "${served_apple}" | sha256sum | cut -f1 -d' ')
+  local banana_sha256=$(echo "${served_banana}" | sha256sum | cut -f1 -d' ')
+
+  mkdir -p "${server_dir}"
+
+  mkfifo "${server_dir}/apple" || fail "cannot mkfifo"
+  mkfifo "${server_dir}/banana" || fail "cannot mkfifo"
+  mkfifo $gate_socket || fail "cannot mkfifo"
+
+  startup_server "${server_dir}"
+
+  cat >> $(create_workspace_with_default_repos WORKSPACE) <<'EOF'
+load("defer.bzl", "defer")
+
+defer(name="defer")
+EOF
+
+  cat > defer.bzl <<EOF
+def _defer_impl(rctx):
+  requests = [
+    ["apple", "${apple_sha256}"],
+    ["banana", "${banana_sha256}"]
+  ]
+  pending = [
+    rctx.download(
+        url = "http://127.0.0.1:${fileserver_port}/" + name,
+        sha256 = sha256,
+        output = name,
+        block = False)
+    for name, sha256 in requests]
+
+  # Tell the rest of the test to unblock the HTTP server
+  rctx.execute(["/bin/sh", "-c", "echo ok > ${server_dir}/gate_socket"])
+
+  # Wait until the requess are done
+  [p.wait() for p in pending]
+
+  rctx.file("WORKSPACE", "")
+  rctx.file("BUILD", "filegroup(name='f', srcs=glob(['**']))")
+
+defer = repository_rule(implementation = _defer_impl)
+EOF
+
+  touch BUILD
+
+  # Start Bazel
+  bazel query @defer//:all >& $TEST_log &
+  local bazel_pid=$!
+
+  # Wait until the .download() calls return
+  cat "${server_dir}/gate_socket"
+
+  # Tell the test server the strings it should serve. In parallel because the
+  # test server apparently cannot serve two HTTP requests in parallel, so if we
+  # wait for request A to be completely served while unblocking request B, it is
+  # possible that the test server wants to serve request B first, which is a
+  # deadlock.
+  echo "${served_apple}" > "${server_dir}/apple" &
+  local apple_pid=$!
+  echo "${served_banana}" > "${server_dir}/banana" &
+  local banana_pid=$!
+  wait $apple_pid
+  wait $banana_pid
+
+  # Wait until Bazel returns
+  wait "${bazel_pid}" || fail "Bazel failed"
+  expect_log "@defer//:f"
+}
+
+function test_deferred_download_error() {
+  cat >> $(create_workspace_with_default_repos WORKSPACE) <<'EOF'
+load("defer.bzl", "defer")
+
+defer(name="defer")
+EOF
+
+  cat > defer.bzl <<EOF
+def _defer_impl(rctx):
+  deferred = rctx.download(
+    url = "https://127.0.0.1:0/doesnotexist",
+    output = "deferred",
+    block = False)
+
+  deferred.wait()
+  print("survived wait")
+  rctx.file("WORKSPACE", "")
+  rctx.file("BUILD", "filegroup(name='f', srcs=glob(['**']))")
+
+defer = repository_rule(implementation = _defer_impl)
+EOF
+
+  touch BUILD
+
+  # Start Bazel
+  bazel query @defer//:all >& $TEST_log && fail "Bazel unexpectedly succeeded"
+  expect_log "Error downloading.*doesnotexist"
+  expect_not_log "survived wait"
+}
+
+function test_deferred_download_smoke() {
+  local server_dir="${TEST_TMPDIR}/server_dir"
+  local served_socket="${server_dir}/served_socket"
+  local gate_socket="${server_dir}/gate_socket"
+  local served_string="DEFERRED"
+  local served_sha256=$(echo "${served_string}" | sha256sum | cut -f1 -d' ')
+
+  mkdir -p "${server_dir}"
+
+  mkfifo $served_socket || fail "cannot mkfifo"
+  mkfifo $gate_socket || fail "cannot mkfifo"
+
+  startup_server "${server_dir}"
+
+  cat >> $(create_workspace_with_default_repos WORKSPACE) <<'EOF'
+load("defer.bzl", "defer")
+
+defer(name="defer")
+EOF
+
+  cat > defer.bzl <<EOF
+def _defer_impl(rctx):
+  deferred = rctx.download(
+    url = "http://127.0.0.1:${fileserver_port}/served_socket",
+    sha256 = "${served_sha256}",
+    output = "deferred",
+    block = False)
+
+  # Tell the rest of the test to unblock the HTTP server
+  rctx.execute(["/bin/sh", "-c", "echo ok > ${server_dir}/gate_socket"])
+  deferred.wait()
+  rctx.file("WORKSPACE", "")
+  rctx.file("BUILD", "filegroup(name='f', srcs=glob(['**']))")
+
+defer = repository_rule(implementation = _defer_impl)
+EOF
+
+  touch BUILD
+
+  # Start Bazel
+  bazel query @defer//:all-targets >& $TEST_log &
+  local bazel_pid=$!
+
+  # Wait until the .download() call returns
+  cat "${server_dir}/gate_socket"
+
+  # Tell the test server the string it should serve
+  echo "${served_string}" > "${server_dir}/served_socket"
+
+  # Wait until Bazel returns
+  wait "${bazel_pid}" || fail "Bazel failed"
+  expect_log "@defer//:deferred"
+}
+
 # Tests downloading a file and using it as a dependency.
 function test_http_download() {
   local test_file=$TEST_TMPDIR/toto