Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly match regex with tree artifact #16949

Closed
wants to merge 4 commits into from
Closed

Correctly match regex with tree artifact #16949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
35538cd
Correctly match regex with tree artifact
coeuvre Dec 7, 2022
933c9db
Add test
coeuvre Dec 7, 2022
dfa0169
Disable one test on Windows
coeuvre Dec 7, 2022
b467ba8
Don't match tree root
coeuvre Dec 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
View file
Open in desktop
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
81 changes: 81 additions & 0 deletions ...st/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java
View file
Open in desktop
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