Correctly match regex with tree artifact (bazelbuild#16958)

It doesn't work when setting `--experimental_remote_download_regex` to match files inside tree artifact after bazelbuild@e01e7f5. This change fixes that. Fixes bazelbuild#16922. Closes bazelbuild#16949. PiperOrigin-RevId: 493838296 Change-Id: I6eceffbffce30949173d10120a9120c6c608a983
fweikert · Dec 8, 2022 · cccb0d6 · cccb0d6
1 parent 7ccc661
commit cccb0d6
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 4 deletions.
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
@@ -548,11 +548,15 @@ public void finalizeAction(Action action, MetadataHandler metadataHandler) {
         inputsToDownload.add(output);
       }
 
-      for (Pattern pattern : patternsToDownload) {
-        if (pattern.matcher(output.getExecPathString()).matches()) {
-          outputsToDownload.add(output);
-          break;
+      if (output.isTreeArtifact()) {
+        var children = metadataHandler.getTreeArtifactChildren((SpecialArtifact) output);
+        for (var file : children) {
+          if (outputMatchesPattern(file)) {
+            outputsToDownload.add(file);
+          }
         }
+      } else if (outputMatchesPattern(output)) {
+        outputsToDownload.add(output);
       }
     }
 
@@ -565,6 +569,15 @@ public void finalizeAction(Action action, MetadataHandler metadataHandler) {
     }
   }
 
+  private boolean outputMatchesPattern(Artifact output) {
+    for (var pattern : patternsToDownload) {
+      if (pattern.matcher(output.getExecPathString()).matches()) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   public void flushOutputTree() throws InterruptedException {
     downloadCache.awaitInProgressTasks();
   }

diff --git a/...st/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java b/...st/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java
@@ -101,6 +101,87 @@ public void downloadOutputsWithRegex() throws Exception {
     assertOutputsDoNotExist("//:foobar");
   }
 
+  @Test
+  public void downloadOutputsWithRegex_treeOutput_regexMatchesTreeFile() throws Exception {
+    // Disable on Windows since it fails for unknown reasons.
+    // TODO(chiwang): Enable it on windows.
+    if (OS.getCurrent() == OS.WINDOWS) {
+      return;
+    }
+
+    writeOutputDirRule();
+    write(
+        "BUILD",
+        "load(':output_dir.bzl', 'output_dir')",
+        "output_dir(",
+        "  name = 'foo',",
+        "  manifest = ':manifest',",
+        ")");
+    write("manifest", "file-1", "file-2", "file-3");
+    addOptions("--experimental_remote_download_regex=.*foo/file-2$");
+
+    buildTarget("//:foo");
+    waitDownloads();
+
+    assertValidOutputFile("foo/file-2", "file-2\n");
+    assertOutputDoesNotExist("foo/file-1");
+    assertOutputDoesNotExist("foo/file-3");
+  }
+
+  @Test
+  public void downloadOutputsWithRegex_treeOutput_regexMatchesTreeRoot() throws Exception {
+    writeOutputDirRule();
+    write(
+        "BUILD",
+        "load(':output_dir.bzl', 'output_dir')",
+        "output_dir(",
+        "  name = 'foo',",
+        "  manifest = ':manifest',",
+        ")");
+    write("manifest", "file-1", "file-2", "file-3");
+    addOptions("--experimental_remote_download_regex=.*foo$");
+
+    buildTarget("//:foo");
+    waitDownloads();
+
+    assertThat(getOutputPath("foo").exists()).isTrue();
+    assertOutputDoesNotExist("foo/file-1");
+    assertOutputDoesNotExist("foo/file-2");
+    assertOutputDoesNotExist("foo/file-3");
+  }
+
+  @Test
+  public void downloadOutputsWithRegex_regexMatchParentPath_filesNotDownloaded() throws Exception {
+    write(
+        "BUILD",
+        "genrule(",
+        "  name = 'file-1',",
+        "  srcs = [],",
+        "  outs = ['foo/file-1'],",
+        "  cmd = 'echo file-1 > $@',",
+        ")",
+        "genrule(",
+        "  name = 'file-2',",
+        "  srcs = [],",
+        "  outs = ['foo/file-2'],",
+        "  cmd = 'echo file-2 > $@',",
+        ")",
+        "genrule(",
+        "  name = 'file-3',",
+        "  srcs = [],",
+        "  outs = ['foo/file-3'],",
+        "  cmd = 'echo file-3 > $@',",
+        ")");
+    addOptions("--experimental_remote_download_regex=.*foo$");
+
+    buildTarget("//:file-1", "//:file-2", "//:file-3");
+    waitDownloads();
+
+    assertOutputDoesNotExist("foo/file-1");
+    assertOutputDoesNotExist("foo/file-2");
+    assertOutputDoesNotExist("foo/file-3");
+  }
+
   @Test
   public void intermediateOutputsAreInputForLocalActions_prefetchIntermediateOutputs()
       throws Exception {