Skip to content

Commit f585783

Browse files
coeuvrecopybara-github
authored andcommitted
Remote: Don't check TreeArtifact output
With a151116, we ignore action cache entry when checking the remote cache if a mandatory output of the Spawn is missing. However, this breaks Spawns that declare a directory output but don't generate anything under it. Since remote servers typically don't create directory ahead of time, and if the action itself doesn't create files under the directory, that directory will be eliminated from the action cache entry. This PR fixes that by not checking TreeArtifact output. Closes bazelbuild#15077. PiperOrigin-RevId: 436163970
1 parent b95a61a commit f585783

File tree

2 files changed

+64
-0
lines changed

2 files changed

+64
-0
lines changed

src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java

+7
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,13 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
10141014
// Check that all mandatory outputs are created.
10151015
for (ActionInput output : action.spawn.getOutputFiles()) {
10161016
if (action.spawn.isMandatoryOutput(output)) {
1017+
// Don't check output that is tree artifact since spawn could generate nothing under that
1018+
// directory. Remote server typically doesn't create directory ahead of time resulting in
1019+
// empty tree artifact missing from action cache entry.
1020+
if (output instanceof Artifact && ((Artifact) output).isTreeArtifact()) {
1021+
continue;
1022+
}
1023+
10171024
Path localPath = execRoot.getRelative(output.getExecPath());
10181025
if (!metadata.files.containsKey(localPath)
10191026
&& !metadata.directories.containsKey(localPath)

src/test/shell/bazel/remote/remote_execution_test.sh

+57
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,63 @@ EOF
10011001
|| fail "Failed to run //a:starlark_output_dir_test with remote execution"
10021002
}
10031003

1004+
function generate_empty_treeartifact_build() {
1005+
mkdir -p a
1006+
cat > a/BUILD <<'EOF'
1007+
load(":output_dir.bzl", "gen_output_dir")
1008+
gen_output_dir(
1009+
name = "output-dir",
1010+
outdir = "dir",
1011+
)
1012+
EOF
1013+
cat > a/output_dir.bzl <<'EOF'
1014+
def _gen_output_dir_impl(ctx):
1015+
output_dir = ctx.actions.declare_directory(ctx.attr.outdir)
1016+
ctx.actions.run_shell(
1017+
outputs = [output_dir],
1018+
inputs = [],
1019+
command = "",
1020+
arguments = [output_dir.path],
1021+
)
1022+
return [
1023+
DefaultInfo(files = depset(direct = [output_dir])),
1024+
]
1025+
1026+
gen_output_dir = rule(
1027+
implementation = _gen_output_dir_impl,
1028+
attrs = {
1029+
"outdir": attr.string(mandatory = True),
1030+
},
1031+
)
1032+
EOF
1033+
}
1034+
1035+
function test_empty_treeartifact_works_with_remote_execution() {
1036+
# Test that empty tree artifact works with remote execution
1037+
generate_empty_treeartifact_build
1038+
1039+
bazel build \
1040+
--remote_executor=grpc://localhost:${worker_port} \
1041+
//a:output-dir >& $TEST_log || fail "Failed to build"
1042+
}
1043+
1044+
function test_empty_treeartifact_works_with_remote_cache() {
1045+
# Test that empty tree artifact works with remote cache
1046+
generate_empty_treeartifact_build
1047+
1048+
bazel build \
1049+
--remote_cache=grpc://localhost:${worker_port} \
1050+
//a:output-dir >& $TEST_log || fail "Failed to build"
1051+
1052+
bazel clean
1053+
1054+
bazel build \
1055+
--remote_cache=grpc://localhost:${worker_port} \
1056+
//a:output-dir >& $TEST_log || fail "Failed to build"
1057+
1058+
expect_log "remote cache hit"
1059+
}
1060+
10041061
function test_downloads_minimal() {
10051062
# Test that genrule outputs are not downloaded when using
10061063
# --remote_download_minimal

0 commit comments

Comments
 (0)