From bb26694806c9a9c2b3f39100ddd4b00e0d8285cd Mon Sep 17 00:00:00 2001 From: Jakob Buchgraber Date: Tue, 3 Sep 2019 00:32:42 -0700 Subject: [PATCH] Make --experimental_remote_download_outputs none experimental Enough people are using and reported good results so that I believe it's time to mark this feature none experimental and here to stay. Minimal mode can be enabled via --remote_download_minimal. Toplevel mode can be enabled via --remote_download_toplevel. Closes #9307. PiperOrigin-RevId: 266869579 --- .../lib/remote/options/RemoteOptions.java | 9 ++- .../bazel/remote/remote_execution_test.sh | 62 +++++++++---------- 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index b03b2c136d2374..7f3167c55e72fc 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java @@ -235,7 +235,8 @@ public final class RemoteOptions extends OptionsBase { public boolean allowSymlinkUpload; @Option( - name = "experimental_remote_download_outputs", + name = "remote_download_outputs", + oldName = "experimental_remote_download_outputs", defaultValue = "all", category = "remote", documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS, @@ -257,7 +258,8 @@ public RemoteOutputsStrategyConverter() { } @Option( - name = "experimental_remote_download_minimal", + name = "remote_download_minimal", + oldName = "experimental_remote_download_minimal", defaultValue = "null", expansion = { "--experimental_inmemory_jdeps_files", @@ -275,7 +277,8 @@ public RemoteOutputsStrategyConverter() { public Void remoteOutputsMinimal; @Option( - name = "experimental_remote_download_toplevel", + name = "remote_download_toplevel", + oldName = "experimental_remote_download_toplevel", defaultValue = "null", expansion = { "--experimental_inmemory_jdeps_files", diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index 3fcedff8ab956c..4bb979b10ad451 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -822,7 +822,7 @@ EOF function test_downloads_minimal() { # Test that genrule outputs are not downloaded when using - # --experimental_remote_download_outputs=minimal + # --remote_download_minimal mkdir -p a cat > a/BUILD <<'EOF' genrule( @@ -843,7 +843,7 @@ EOF bazel build \ --genrule_strategy=remote \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_minimal \ + --remote_download_minimal \ //a:foobar || fail "Failed to build //a:foobar" (! [[ -f bazel-bin/a/foo.txt ]] && ! [[ -f bazel-bin/a/foobar.txt ]]) \ @@ -852,7 +852,7 @@ EOF function test_downloads_minimal_failure() { # Test that outputs of failing actions are downloaded when using - # --experimental_remote_download_outputs=minimal + # --remote_download_minimal mkdir -p a cat > a/BUILD <<'EOF' genrule( @@ -866,7 +866,7 @@ EOF bazel build \ --spawn_strategy=remote \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_minimal \ + --remote_download_minimal \ //a:fail && fail "Expected test failure" || true [[ -f bazel-bin/a/fail.txt ]] \ @@ -874,7 +874,7 @@ EOF } function test_downloads_minimal_prefetch() { - # Test that when using --experimental_remote_download_outputs=minimal a remote-only output that's + # Test that when using --remote_download_minimal a remote-only output that's # an input to a local action is downloaded lazily before executing the local action. mkdir -p a cat > a/BUILD <<'EOF' @@ -897,7 +897,7 @@ EOF bazel build \ --genrule_strategy=remote \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_minimal \ + --remote_download_minimal \ //a:remote || fail "Failed to build //a:remote" (! [[ -f bazel-bin/a/remote.txt ]]) \ @@ -906,7 +906,7 @@ EOF bazel build \ --genrule_strategy=remote,local \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_minimal \ + --remote_download_minimal \ //a:local || fail "Failed to build //a:local" localtxt="bazel-bin/a/local.txt" @@ -918,7 +918,7 @@ EOF } function test_download_outputs_invalidation() { - # Test that when changing values of --experimental_remote_download_outputs all actions are + # Test that when changing values of --remote_download_minimal all actions are # invalidated. mkdir -p a cat > a/BUILD <<'EOF' @@ -933,7 +933,7 @@ EOF bazel build \ --genrule_strategy=remote \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_minimal \ + --remote_download_minimal \ //a:remote >& $TEST_log || fail "Failed to build //a:remote" expect_log "1 process: 1 remote" @@ -941,16 +941,16 @@ EOF bazel build \ --genrule_strategy=remote \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_outputs=all \ + --remote_download_outputs=all \ //a:remote >& $TEST_log || fail "Failed to build //a:remote" - # Changing --experimental_remote_download_outputs to "all" should invalidate SkyFrames in-memory + # Changing --remote_download_outputs to "all" should invalidate SkyFrames in-memory # caching and make it re-run the action. expect_log "1 process: 1 remote" } function test_downloads_minimal_native_prefetch() { - # Test that when using --experimental_remote_download_outputs=minimal a remotely stored output + # Test that when using --remote_download_outputs=minimal a remotely stored output # that's an input to a native action (ctx.actions.expand_template) is staged lazily for action # execution. mkdir -p a @@ -996,7 +996,7 @@ EOF bazel build \ --genrule_strategy=remote \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_minimal \ + --remote_download_minimal \ //a:substitute-buchgr >& $TEST_log || fail "Failed to build //a:substitute-buchgr" # The genrule //a:generate-template should run remotely and //a:substitute-buchgr @@ -1012,7 +1012,7 @@ EOF } function test_downloads_toplevel() { - # Test that when using --experimental_remote_download_outputs=toplevel only the output of the + # Test that when using --remote_download_outputs=toplevel only the output of the # toplevel target is being downloaded. mkdir -p a cat > a/BUILD <<'EOF' @@ -1034,7 +1034,7 @@ EOF bazel build \ --genrule_strategy=remote \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_toplevel \ + --remote_download_toplevel \ //a:foobar || fail "Failed to build //a:foobar" (! [[ -f bazel-bin/a/foo.txt ]]) \ @@ -1050,7 +1050,7 @@ EOF bazel build \ --genrule_strategy=remote \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_toplevel \ + --remote_download_toplevel \ //a:foobar >& $TEST_log || fail "Failed to build //a:foobar" expect_log "1 process: 1 remote cache hit" @@ -1060,7 +1060,7 @@ EOF } function test_downloads_toplevel_runfiles() { - # Test that --experimental_remote_download_outputs=toplevel downloads the top level binary + # Test that --remote_download_toplevel fetches only the top level binaries # and generated runfiles. mkdir -p a @@ -1092,7 +1092,7 @@ EOF bazel build \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_toplevel \ + --remote_download_toplevel \ //a:foo || fail "Failed to build //a:foobar" [[ -f bazel-bin/a/foo${EXE_EXT} ]] \ @@ -1103,7 +1103,7 @@ EOF } function test_downloads_toplevel_src_runfiles() { - # Test that using --experimental_remote_download_outputs=toplevel with a non-generated (source) + # Test that using --remote_download_toplevel with a non-generated (source) # runfile dependency works. mkdir -p a cat > a/create_foo.sh <<'EOF' @@ -1124,7 +1124,7 @@ EOF bazel build \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_toplevel \ + --remote_download_toplevel \ //a:foo || fail "Failed to build //a:foobar" [[ -f bazel-bin/a/foo.txt ]] \ @@ -1132,9 +1132,9 @@ EOF } function test_download_toplevel_test_rule() { - # Test that when using --experimental_remote_download_outputs=toplevel with bazel test only - # the test.log and test.xml file are downloaded but not the test binary. However when building - # a test then the test binary should be downloaded. + # Test that when using --remote_download_toplevel with bazel test only + # the test.log and test.xml file are downloaded but not the test binary. + # However when building a test then the test binary should be downloaded. if [[ "$PLATFORM" == "darwin" ]]; then # TODO(b/37355380): This test is disabled due to RemoteWorker not supporting @@ -1159,9 +1159,7 @@ EOF # When invoking bazel test only test.log and test.xml should be downloaded. bazel test \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_inmemory_jdeps_files \ - --experimental_inmemory_dotd_files \ - --experimental_remote_download_outputs=toplevel \ + --remote_download_toplevel \ //a:test >& $TEST_log || fail "Failed to test //a:test with remote execution" (! [[ -f bazel-bin/a/test ]]) \ @@ -1178,9 +1176,7 @@ EOF # When invoking bazel build the test binary should be downloaded. bazel build \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_inmemory_jdeps_files \ - --experimental_inmemory_dotd_files \ - --experimental_remote_download_outputs=toplevel \ + --remote_download_toplevel \ //a:test >& $TEST_log || fail "Failed to build //a:test with remote execution" ([[ -f bazel-bin/a/test ]]) \ @@ -1188,7 +1184,7 @@ EOF } function test_downloads_minimal_bep() { - # Test that when using --experimental_remote_download_outputs=minimal all URI's in the BEP + # Test that when using --remote_download_minimal all URI's in the BEP # are rewritten as bytestream://.. mkdir -p a cat > a/success.sh <<'EOF' @@ -1212,7 +1208,7 @@ EOF bazel test \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_minimal \ + --remote_download_minimal \ --build_event_text_file=$TEST_log \ //a:foo //a:success_test || fail "Failed to test //a:foo //a:success_test" @@ -1268,7 +1264,7 @@ chmod +x status.sh bazel build \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_minimal \ + --remote_download_minimal \ --workspace_status_command=status.sh \ //a:foo || "Failed to build //a:foo" @@ -1279,7 +1275,7 @@ EOF bazel build \ --remote_executor=grpc://localhost:${worker_port} \ - --experimental_remote_download_minimal \ + --remote_download_minimal \ --workspace_status_command=status.sh \ //a:foo || "Failed to build //a:foo" }