Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.1.0] Cherry-picks for elimination of repo rule restarts #21082

Merged
merged 2 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -340,10 +340,27 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
starlarkRepositoryFunction.setWorkerExecutorService(repoFetchingWorkerThreadPool);
break;
case VIRTUAL:
throw new AbruptExitException(
detailedExitCode(
"using a virtual worker thread for repo fetching is not yet supported",
Code.BAD_DOWNLOADER_CONFIG));
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
// anyone... right? (OSS Bazel already ships with a bundled JDK 21)
starlarkRepositoryFunction.setWorkerExecutorService(
(ExecutorService)
Executors.class
.getDeclaredMethod("newVirtualThreadPerTaskExecutor")
.invoke(null));
} catch (ReflectiveOperationException 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);
if (repoOptions.repositoryDownloaderRetries >= 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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(
Expand Down
48 changes: 0 additions & 48 deletions src/test/shell/bazel/bazel_workspaces_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)'
Expand Down
6 changes: 6 additions & 0 deletions src/test/shell/bazel/starlark_repository_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2384,6 +2384,12 @@ EOF
bazel build @foo//:bar --experimental_worker_for_repo_fetching=platform >& $TEST_log \
|| fail "Expected build to succeed"
expect_log_n "hello world!" 1

# virtual worker thread, never restarts
bazel shutdown
bazel build @foo//:bar --experimental_worker_for_repo_fetching=virtual >& $TEST_log \
|| fail "Expected build to succeed"
expect_log_n "hello world!" 1
}

function test_duplicate_value_in_environ() {
Expand Down
Loading