Skip to content

Commit 36fa2a5

Browse files
fmeumiancha1992
authored andcommitted
Fix crash on lost inputs that are symlinks
The downloads in `AbstractActionInputPrefetcher#prefetchInputs` reported lost inputs that are symlinks to other artifacts with the exec path of the target, which results in a crash when the target is not also an input to the action. This is fixed by always reporting the exec path of the symlink. Fixes bazelbuild#25841 Closes bazelbuild#25847. PiperOrigin-RevId: 747819887 Change-Id: Iff9125c95a2598851b8a31ed4ca18fd7218a97aa
1 parent f873f9d commit 36fa2a5

File tree

3 files changed

+142
-8
lines changed

3 files changed

+142
-8
lines changed

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

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import com.google.devtools.build.lib.events.Reporter;
4949
import com.google.devtools.build.lib.profiler.Profiler;
5050
import com.google.devtools.build.lib.profiler.ProfilerTask;
51+
import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
5152
import com.google.devtools.build.lib.remote.util.AsyncTaskCache;
5253
import com.google.devtools.build.lib.util.TempPathGenerator;
5354
import com.google.devtools.build.lib.vfs.FileSymlinkLoopException;
@@ -358,14 +359,24 @@ private ListenableFuture<Void> prefetchFile(
358359

359360
Completable result =
360361
downloadFileNoCheckRx(
361-
action,
362-
execRoot.getRelative(execPath),
363-
treeRootExecPath != null ? execRoot.getRelative(treeRootExecPath) : null,
364-
dirsWithOutputPermissions,
365-
input,
366-
metadata,
367-
priority,
368-
reason);
362+
action,
363+
execRoot.getRelative(execPath),
364+
treeRootExecPath != null ? execRoot.getRelative(treeRootExecPath) : null,
365+
dirsWithOutputPermissions,
366+
input,
367+
metadata,
368+
priority,
369+
reason)
370+
.onErrorResumeNext(
371+
t -> {
372+
if (t instanceof CacheNotFoundException cacheNotFoundException) {
373+
// Only the symlink itself is guaranteed to be an input to the action, so
374+
// report its path for rewinding.
375+
cacheNotFoundException.setExecPath(input.getExecPath());
376+
return Completable.error(cacheNotFoundException);
377+
}
378+
return Completable.error(t);
379+
});
369380

370381
if (symlink != null) {
371382
result = result.andThen(plantSymlink(symlink));

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

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,127 @@ public void remoteCacheEvictBlobs_whenPrefetchingInput_exitWithCode39() throws E
357357
assertThat(error.getDetailedExitCode().getExitCode().getNumericExitCode()).isEqualTo(39);
358358
}
359359

360+
@Test
361+
public void remoteCacheEvictBlobs_whenPrefetchingSymlinkedInput_exitWithCode39()
362+
throws Exception {
363+
// Arrange: Prepare workspace and populate remote cache
364+
writeSymlinkRule();
365+
write(
366+
"a/BUILD",
367+
"""
368+
load("//:symlink.bzl", "symlink")
369+
370+
genrule(
371+
name = "foo",
372+
srcs = ["foo.in"],
373+
outs = ["foo.out"],
374+
cmd = "cat $(SRCS) > $@",
375+
)
376+
377+
symlink(
378+
name = "symlinked_foo",
379+
target_artifact = ":foo.out",
380+
)
381+
382+
genrule(
383+
name = "bar",
384+
srcs = [
385+
":symlinked_foo",
386+
"bar.in",
387+
],
388+
outs = ["bar.out"],
389+
cmd = "cat $(SRCS) > $@",
390+
tags = ["no-remote-exec"],
391+
)
392+
""");
393+
write("a/foo.in", "foo");
394+
write("a/bar.in", "bar");
395+
396+
// Populate remote cache
397+
buildTarget("//a:bar");
398+
var bytes = readContent(getOutputPath("a/foo.out"));
399+
var hashCode = getDigestHashFunction().getHashFunction().hashBytes(bytes);
400+
getOnlyElement(getArtifacts("//a:symlinked_foo")).getPath().delete();
401+
getOutputPath("a/foo.out").delete();
402+
getOutputPath("a/bar.out").delete();
403+
getOutputBase().getRelative("action_cache").deleteTreesBelow();
404+
restartServer();
405+
406+
// Clean build, foo.out isn't downloaded
407+
buildTarget("//a:bar");
408+
assertOutputDoesNotExist("a/foo.out");
409+
assertOutputsDoNotExist("//a:symlinked_foo");
410+
411+
// Act: Evict blobs from remote cache and do an incremental build
412+
evictAllBlobs();
413+
write("a/bar.in", "updated bar");
414+
var error = assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar"));
415+
416+
// Assert: Exit code is 39
417+
assertThat(error).hasMessageThat().contains("Lost inputs no longer available remotely");
418+
assertThat(error).hasMessageThat().contains("a/symlinked_foo");
419+
assertThat(error).hasMessageThat().contains(String.format("%s/%s", hashCode, bytes.length));
420+
assertThat(error.getDetailedExitCode().getExitCode().getNumericExitCode()).isEqualTo(39);
421+
}
422+
423+
@Test
424+
public void remoteCacheEvictBlobs_whenPrefetchingSymlinkedInput_succeedsWithActionRewinding()
425+
throws Exception {
426+
writeSymlinkRule();
427+
write(
428+
"a/BUILD",
429+
"""
430+
load("//:symlink.bzl", "symlink")
431+
432+
genrule(
433+
name = "foo",
434+
srcs = ["foo.in"],
435+
outs = ["foo.out"],
436+
cmd = "cat $(SRCS) > $@",
437+
)
438+
439+
symlink(
440+
name = "symlinked_foo",
441+
target_artifact = ":foo.out",
442+
)
443+
444+
genrule(
445+
name = "bar",
446+
srcs = [
447+
":symlinked_foo",
448+
"bar.in",
449+
],
450+
outs = ["bar.out"],
451+
cmd = "cat $(SRCS) > $@",
452+
tags = ["no-remote-exec"],
453+
)
454+
""");
455+
write("a/foo.in", "foo");
456+
write("a/bar.in", "bar");
457+
458+
// Populate remote cache
459+
buildTarget("//a:bar");
460+
getOnlyElement(getArtifacts("//a:symlinked_foo")).getPath().delete();
461+
getOutputPath("a/foo.out").delete();
462+
getOutputPath("a/bar.out").delete();
463+
getOutputBase().getRelative("action_cache").deleteTreesBelow();
464+
restartServer();
465+
466+
// Clean build, foo.out isn't downloaded
467+
buildTarget("//a:bar");
468+
assertOutputDoesNotExist("a/foo.out");
469+
assertOutputsDoNotExist("//a:symlinked_foo");
470+
471+
// Act: Evict blobs from remote cache and do an incremental build
472+
evictAllBlobs();
473+
write("a/bar.in", "updated bar");
474+
enableActionRewinding();
475+
buildTarget("//a:bar");
476+
477+
// Assert: target was successfully built
478+
assertValidOutputFile("a/bar.out", "foo" + lineSeparator() + "updated bar" + lineSeparator());
479+
}
480+
360481
@Test
361482
public void remoteCacheEvictBlobs_whenUploadingInput_exitWithCode39() throws Exception {
362483
// Arrange: Prepare workspace and populate remote cache

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import com.google.devtools.build.lib.util.CommandBuilder;
3838
import com.google.devtools.build.lib.util.OS;
3939
import com.google.devtools.build.lib.util.io.RecordingOutErr;
40+
import com.google.devtools.build.lib.vfs.FileSystemUtils;
4041
import com.google.devtools.build.lib.vfs.Path;
4142
import com.google.devtools.build.lib.vfs.PathFragment;
4243
import java.io.IOException;
@@ -2029,6 +2030,7 @@ protected void assertSymlink(String binRelativeLinkPath, PathFragment targetPath
20292030
}
20302031

20312032
protected void writeSymlinkRule() throws IOException {
2033+
FileSystemUtils.touchFile(getWorkspace().getRelative("BUILD"));
20322034
write(
20332035
"symlink.bzl",
20342036
"""

0 commit comments

Comments
 (0)