Skip to content

Commit

Permalink
Correctly match regex with tree artifact
Browse files Browse the repository at this point in the history
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
  • Loading branch information
coeuvre authored and copybara-github committed Dec 8, 2022
1 parent 87437fd commit 86dee6d
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 86dee6d

Please sign in to comment.