From 7040b47407da6c07180858640f485b0a9623efab Mon Sep 17 00:00:00 2001
From: Googler <tjgq@google.com>
Date: Mon, 2 Jan 2023 02:47:48 -0800
Subject: [PATCH] [6.2.0] Skip empty directories in prefetcher.

While non-empty tree artifacts in the inputs to an action are expanded into the files they contain and omitted from the input mapping, empty tree artifacts are still present, as they signal the need to create the directory.

I'm explicitly skipping the empty tree artifacts in prefetchFiles() as otherwise they get skipped as a result of FileArtifactValue#isRemote() returning false for the FileArtifactValue associated with an empty tree artifact (even if it was produced remotely!), which is extremely subtle.

Closes #17183.

[NOTE: this is a combined cherry pick of 763f966 and 4c57def, as the former left Bazel in a broken state.]

PiperOrigin-RevId: 501207095
Change-Id: Ib52727d6fdc6b7a291a61fba33914e57531fb1f4
---
 .../remote/AbstractActionInputPrefetcher.java |  8 ++++++
 ...ildWithoutTheBytesIntegrationTestBase.java | 25 +++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
index 3c325684ee2ad5..027188364cc6e1 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
@@ -179,6 +179,8 @@ protected Completable onErrorResumeNext(Throwable error) {
    * Fetches remotely stored action outputs, that are inputs to this spawn, and stores them under
    * their path in the output base.
    *
+   * <p>The {@code inputs} may not contain any unexpanded directories.
+   *
    * <p>This method is safe to be called concurrently from spawn runners before running any local
    * spawn.
    *
@@ -197,10 +199,16 @@ protected ListenableFuture<Void> prefetchFiles(
     Map<SpecialArtifact, List<TreeFileArtifact>> trees = new HashMap<>();
     List<ActionInput> files = new ArrayList<>();
     for (ActionInput input : inputs) {
+      // Source artifacts don't need to be fetched.
       if (input instanceof Artifact && ((Artifact) input).isSourceArtifact()) {
         continue;
       }
 
+      // Skip empty tree artifacts (non-empty tree artifacts should have already been expanded).
+      if (input.isDirectory()) {
+        continue;
+      }
+
       if (input instanceof TreeFileArtifact) {
         TreeFileArtifact treeFile = (TreeFileArtifact) input;
         SpecialArtifact treeArtifact = treeFile.getParent();
diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java
index 215df976fd1316..89a191d13f6b3e 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java
@@ -520,6 +520,31 @@ public void treeOutputsFromLocalFileSystem_works() throws Exception {
     assertValidOutputFile("out/foobar.txt", "file-1\nbar\n");
   }
 
+  @Test
+  public void emptyTreeConsumedByLocalAction() throws Exception {
+    // Disable remote execution so that the empty tree artifact is prefetched.
+    addOptions("--modify_execution_info=Genrule=+no-remote-exec");
+    setDownloadToplevel();
+    writeOutputDirRule();
+    write(
+        "BUILD",
+        "load(':output_dir.bzl', 'output_dir')",
+        "output_dir(",
+        "  name = 'foo',",
+        "  manifest = ':manifest',",
+        ")",
+        "genrule(",
+        "  name = 'foobar',",
+        "  srcs = [':foo'],",
+        "  outs = ['foobar.txt'],",
+        "  cmd = 'touch $@',",
+        ")");
+    write("manifest"); // no files
+
+    buildTarget("//:foobar");
+    waitDownloads();
+  }
+
   @Test
   public void incrementalBuild_deleteOutputsInUnwritableParentDirectory() throws Exception {
     write(