Remote: Don't check TreeArtifact output

coeuvre · copybara-github · commit f5857830bb68 · 2022-03-21T03:52:14.000-07:00
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
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
@@ -1014,6 +1014,13 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
     // 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
@@ -1001,6 +1001,63 @@ EOF
            || 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
