Skip to content

Commit 86dee6d

Browse files
coeuvrecopybara-github
authored andcommitted
Correctly match regex with tree artifact
It doesn't work when setting `--experimental_remote_download_regex` to match files inside tree artifact after e01e7f5. This change fixes that. Fixes #16922. Closes #16949. PiperOrigin-RevId: 493838296 Change-Id: I6eceffbffce30949173d10120a9120c6c608a983
1 parent 87437fd commit 86dee6d

File tree

2 files changed

+98
-4
lines changed

2 files changed

+98
-4
lines changed

src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java

+17-4
Original file line numberDiff line numberDiff line change
@@ -548,11 +548,15 @@ public void finalizeAction(Action action, MetadataHandler metadataHandler) {
548548
inputsToDownload.add(output);
549549
}
550550

551-
for (Pattern pattern : patternsToDownload) {
552-
if (pattern.matcher(output.getExecPathString()).matches()) {
553-
outputsToDownload.add(output);
554-
break;
551+
if (output.isTreeArtifact()) {
552+
var children = metadataHandler.getTreeArtifactChildren((SpecialArtifact) output);
553+
for (var file : children) {
554+
if (outputMatchesPattern(file)) {
555+
outputsToDownload.add(file);
556+
}
555557
}
558+
} else if (outputMatchesPattern(output)) {
559+
outputsToDownload.add(output);
556560
}
557561
}
558562

@@ -565,6 +569,15 @@ public void finalizeAction(Action action, MetadataHandler metadataHandler) {
565569
}
566570
}
567571

572+
private boolean outputMatchesPattern(Artifact output) {
573+
for (var pattern : patternsToDownload) {
574+
if (pattern.matcher(output.getExecPathString()).matches()) {
575+
return true;
576+
}
577+
}
578+
return false;
579+
}
580+
568581
public void flushOutputTree() throws InterruptedException {
569582
downloadCache.awaitInProgressTasks();
570583
}

src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java

+81
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,87 @@ public void downloadOutputsWithRegex() throws Exception {
101101
assertOutputsDoNotExist("//:foobar");
102102
}
103103

104+
@Test
105+
public void downloadOutputsWithRegex_treeOutput_regexMatchesTreeFile() throws Exception {
106+
// Disable on Windows since it fails for unknown reasons.
107+
// TODO(chiwang): Enable it on windows.
108+
if (OS.getCurrent() == OS.WINDOWS) {
109+
return;
110+
}
111+
112+
writeOutputDirRule();
113+
write(
114+
"BUILD",
115+
"load(':output_dir.bzl', 'output_dir')",
116+
"output_dir(",
117+
" name = 'foo',",
118+
" manifest = ':manifest',",
119+
")");
120+
write("manifest", "file-1", "file-2", "file-3");
121+
addOptions("--experimental_remote_download_regex=.*foo/file-2$");
122+
123+
buildTarget("//:foo");
124+
waitDownloads();
125+
126+
assertValidOutputFile("foo/file-2", "file-2\n");
127+
assertOutputDoesNotExist("foo/file-1");
128+
assertOutputDoesNotExist("foo/file-3");
129+
}
130+
131+
@Test
132+
public void downloadOutputsWithRegex_treeOutput_regexMatchesTreeRoot() throws Exception {
133+
writeOutputDirRule();
134+
write(
135+
"BUILD",
136+
"load(':output_dir.bzl', 'output_dir')",
137+
"output_dir(",
138+
" name = 'foo',",
139+
" manifest = ':manifest',",
140+
")");
141+
write("manifest", "file-1", "file-2", "file-3");
142+
addOptions("--experimental_remote_download_regex=.*foo$");
143+
144+
buildTarget("//:foo");
145+
waitDownloads();
146+
147+
assertThat(getOutputPath("foo").exists()).isTrue();
148+
assertOutputDoesNotExist("foo/file-1");
149+
assertOutputDoesNotExist("foo/file-2");
150+
assertOutputDoesNotExist("foo/file-3");
151+
}
152+
153+
@Test
154+
public void downloadOutputsWithRegex_regexMatchParentPath_filesNotDownloaded() throws Exception {
155+
write(
156+
"BUILD",
157+
"genrule(",
158+
" name = 'file-1',",
159+
" srcs = [],",
160+
" outs = ['foo/file-1'],",
161+
" cmd = 'echo file-1 > $@',",
162+
")",
163+
"genrule(",
164+
" name = 'file-2',",
165+
" srcs = [],",
166+
" outs = ['foo/file-2'],",
167+
" cmd = 'echo file-2 > $@',",
168+
")",
169+
"genrule(",
170+
" name = 'file-3',",
171+
" srcs = [],",
172+
" outs = ['foo/file-3'],",
173+
" cmd = 'echo file-3 > $@',",
174+
")");
175+
addOptions("--experimental_remote_download_regex=.*foo$");
176+
177+
buildTarget("//:file-1", "//:file-2", "//:file-3");
178+
waitDownloads();
179+
180+
assertOutputDoesNotExist("foo/file-1");
181+
assertOutputDoesNotExist("foo/file-2");
182+
assertOutputDoesNotExist("foo/file-3");
183+
}
184+
104185
@Test
105186
public void intermediateOutputsAreInputForLocalActions_prefetchIntermediateOutputs()
106187
throws Exception {

0 commit comments

Comments
 (0)