From 57c18017de2c6f3ec655d931e3a3ed8060cf84c8 Mon Sep 17 00:00:00 2001
From: Googler <wyv@google.com>
Date: Fri, 26 Jan 2024 10:55:56 -0800
Subject: [PATCH] Enable worker thread for repo fetching by default

We use virtual threads if available (JDK 21+); if not, we fall back to not using worker threads at all. This approach is slightly safer than straight up setting the default to `virtual` as that would break certain obscure platforms without an available JDK 21. The plan is to cherry-pick this into 7.1.0.

Also removed test_reexecute in bazel_workspaces_test since, well, that's the whole point!

Fixes https://github.com/bazelbuild/bazel/issues/10515

RELNOTES: The flag `--experimental_worker_for_repo_fetching` now defaults to `auto`, which uses virtual threads from JDK 21 if it's available. This eliminates restarts during repo fetching.
PiperOrigin-RevId: 601810629
Change-Id: I9d2ec59688586356d90604fdd328cc985e75d1d4
---
 .../lib/bazel/BazelRepositoryModule.java      | 15 ++++--
 .../bazel/repository/RepositoryOptions.java   |  9 ++--
 src/test/shell/bazel/bazel_workspaces_test.sh | 48 -------------------
 3 files changed, 16 insertions(+), 56 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
index ff8cc2df8b8bef..401e1759da7886 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
@@ -344,6 +344,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
           starlarkRepositoryFunction.setWorkerExecutorService(repoFetchingWorkerThreadPool);
           break;
         case VIRTUAL:
+        case AUTO:
           try {
             // Since Google hasn't migrated to JDK 21 yet, we can't directly call
             // Executors.newVirtualThreadPerTaskExecutor here. But a bit of reflection never hurt
@@ -354,11 +355,15 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
                         .getDeclaredMethod("newVirtualThreadPerTaskExecutor")
                         .invoke(null));
           } catch (ReflectiveOperationException e) {
-            throw new AbruptExitException(
-                detailedExitCode(
-                    "couldn't create virtual worker thread executor for repo fetching",
-                    Code.BAD_DOWNLOADER_CONFIG),
-                e);
+            if (repoOptions.workerForRepoFetching == RepositoryOptions.WorkerForRepoFetching.AUTO) {
+              starlarkRepositoryFunction.setWorkerExecutorService(null);
+            } else {
+              throw new AbruptExitException(
+                  detailedExitCode(
+                      "couldn't create virtual worker thread executor for repo fetching",
+                      Code.BAD_DOWNLOADER_CONFIG),
+                  e);
+            }
           }
       }
       downloadManager.setDisableDownload(repoOptions.disableDownload);
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 fe13a082486579..a223f80f479ac4 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
@@ -216,7 +216,8 @@ public class RepositoryOptions extends OptionsBase {
   public enum WorkerForRepoFetching {
     OFF,
     PLATFORM,
-    VIRTUAL;
+    VIRTUAL,
+    AUTO;
 
     static class Converter extends EnumConverter<WorkerForRepoFetching> {
       public Converter() {
@@ -227,14 +228,16 @@ public Converter() {
 
   @Option(
       name = "experimental_worker_for_repo_fetching",
-      defaultValue = "off",
+      defaultValue = "auto",
       converter = WorkerForRepoFetching.Converter.class,
       documentationCategory = OptionDocumentationCategory.REMOTE,
       effectTags = {OptionEffectTag.UNKNOWN},
       help =
           "The threading mode to use for repo fetching. If set to 'off', no worker thread is used,"
               + " and the repo fetching is subject to restarts. Otherwise, uses a platform thread"
-              + " (i.e. OS thread) if set to 'platform' or a virtual thread if set to 'virtual'.")
+              + " (i.e. OS thread) if set to 'platform' or a virtual thread if set to 'virtual'. If"
+              + " set to 'auto', virtual threads are used if available (i.e. running on JDK 21+),"
+              + " otherwise no worker thread is used.")
   public WorkerForRepoFetching workerForRepoFetching;
 
   @Option(
diff --git a/src/test/shell/bazel/bazel_workspaces_test.sh b/src/test/shell/bazel/bazel_workspaces_test.sh
index e65a60377e8aa6..b600f6a3eb8337 100755
--- a/src/test/shell/bazel/bazel_workspaces_test.sh
+++ b/src/test/shell/bazel/bazel_workspaces_test.sh
@@ -96,54 +96,6 @@ function test_execute() {
   ensure_contains_exactly "location: .*repos.bzl:2:25" 0
 }
 
-# The workspace is set up so that the function is interrupted and re-executed.
-# The log should contain both instances.
-function test_reexecute() {
-  create_new_workspace
-  cat > BUILD <<'EOF'
-genrule(
-   name="test",
-   srcs=["@repo//:t.txt"],
-   outs=["out.txt"],
-   cmd="echo Result > $(location out.txt)"
-)
-EOF
-  cat >> repos.bzl <<EOF
-def _executeMe(repository_ctx):
-  repository_ctx.execute(["echo", "testing!"])
-  build_contents = "package(default_visibility = ['//visibility:public'])\n\n"
-  build_contents += "exports_files([\"t.txt\"])\n"
-  repository_ctx.file("BUILD", build_contents, False)
-  repository_ctx.symlink(Label("@another//:dummy.txt"), "t.txt")
-
-ex_repo = repository_rule(
-  implementation = _executeMe,
-  local = True,
-)
-
-def _another(repository_ctx):
-  build_contents = "exports_files([\"dummy.txt\"])\n"
-  repository_ctx.file("BUILD", build_contents, False)
-  repository_ctx.file("dummy.txt", "dummy\n", False)
-
-a_repo = repository_rule(
-  implementation = _another,
-  local = True,
-)
-EOF
-  cat >> WORKSPACE <<EOF
-load("//:repos.bzl", "ex_repo")
-load("//:repos.bzl", "a_repo")
-ex_repo(name = "repo")
-a_repo(name = "another")
-EOF
-
-  build_and_process_log
-
-  ensure_contains_atleast "location: .*repos.bzl:2:" 2
-}
-
-
 # Ensure details of the specific functions are present
 function test_execute2() {
   set_workspace_command 'repository_ctx.execute(["echo", "test_contents"], 21, {"Arg1": "Val1"}, True)'