Remote: Don't check TreeArtifact output (#15085)
With a151116f7f671efea9a95f3e251561e53e535cca, 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 #15077.
PiperOrigin-RevId: 436163970
(cherry picked from commit f5857830bb68bd05ffc257506575ed37a8128933)
Co-authored-by: Chi Wang <chiwang@google.com>
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
index 6a5feb8..94abbaa 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -1013,6 +1013,13 @@
// Check that all mandatory outputs are created.
for (ActionInput output : action.spawn.getOutputFiles()) {
if (action.spawn.isMandatoryOutput(output)) {
+ // Don't check output that is tree artifact since spawn could generate nothing under that
+ // directory. Remote server typically doesn't create directory ahead of time resulting in
+ // empty tree artifact missing from action cache entry.
+ if (output instanceof Artifact && ((Artifact) output).isTreeArtifact()) {
+ continue;
+ }
+
Path localPath = execRoot.getRelative(output.getExecPath());
if (!metadata.files.containsKey(localPath)
&& !metadata.directories.containsKey(localPath)
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index b70c277..42b1269 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -966,6 +966,63 @@
|| fail "Failed to run //a:starlark_output_dir_test with remote execution"
}
+function generate_empty_treeartifact_build() {
+ mkdir -p a
+ cat > a/BUILD <<'EOF'
+load(":output_dir.bzl", "gen_output_dir")
+gen_output_dir(
+ name = "output-dir",
+ outdir = "dir",
+)
+EOF
+ cat > a/output_dir.bzl <<'EOF'
+def _gen_output_dir_impl(ctx):
+ output_dir = ctx.actions.declare_directory(ctx.attr.outdir)
+ ctx.actions.run_shell(
+ outputs = [output_dir],
+ inputs = [],
+ command = "",
+ arguments = [output_dir.path],
+ )
+ return [
+ DefaultInfo(files = depset(direct = [output_dir])),
+ ]
+
+gen_output_dir = rule(
+ implementation = _gen_output_dir_impl,
+ attrs = {
+ "outdir": attr.string(mandatory = True),
+ },
+)
+EOF
+}
+
+function test_empty_treeartifact_works_with_remote_execution() {
+ # Test that empty tree artifact works with remote execution
+ generate_empty_treeartifact_build
+
+ bazel build \
+ --remote_executor=grpc://localhost:${worker_port} \
+ //a:output-dir >& $TEST_log || fail "Failed to build"
+}
+
+function test_empty_treeartifact_works_with_remote_cache() {
+ # Test that empty tree artifact works with remote cache
+ generate_empty_treeartifact_build
+
+ bazel build \
+ --remote_cache=grpc://localhost:${worker_port} \
+ //a:output-dir >& $TEST_log || fail "Failed to build"
+
+ bazel clean
+
+ bazel build \
+ --remote_cache=grpc://localhost:${worker_port} \
+ //a:output-dir >& $TEST_log || fail "Failed to build"
+
+ expect_log "remote cache hit"
+}
+
function test_downloads_minimal() {
# Test that genrule outputs are not downloaded when using
# --remote_download_minimal
