Skip to content

Commit

Permalink
Make --experimental_remote_download_outputs none experimental
Browse files Browse the repository at this point in the history
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
  • Loading branch information
buchgr authored and copybara-github committed Sep 3, 2019
1 parent d190bb1 commit bb26694
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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",
Expand All @@ -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",
Expand Down
62 changes: 29 additions & 33 deletions src/test/shell/bazel/remote/remote_execution_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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 ]]) \
Expand All @@ -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(
Expand All @@ -866,15 +866,15 @@ 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 ]] \
|| fail "Expected fail.txt of failing target //a:fail to be downloaded"
}

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'
Expand All @@ -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 ]]) \
Expand All @@ -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"
Expand All @@ -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'
Expand All @@ -933,24 +933,24 @@ 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"

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
Expand Down Expand Up @@ -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
Expand All @@ -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'
Expand All @@ -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 ]]) \
Expand All @@ -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"
Expand All @@ -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

Expand Down Expand Up @@ -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} ]] \
Expand All @@ -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'
Expand All @@ -1124,17 +1124,17 @@ 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 ]] \
|| fail "Expected toplevel output bazel-bin/a/foo.txt to be downloaded"
}

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
Expand All @@ -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 ]]) \
Expand All @@ -1178,17 +1176,15 @@ 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 ]]) \
|| fail "Expected test binary bazel-bin/a/test to be downloaded"
}

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'
Expand All @@ -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"

Expand Down Expand Up @@ -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"

Expand All @@ -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"
}
Expand Down

0 comments on commit bb26694

Please sign in to comment.