Skip to content

Commit a7da0d8

Browse files
fmeumcopybara-github
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 c0c98a2 commit a7da0d8

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

+19-8
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import com.google.devtools.build.lib.events.Reporter;
4848
import com.google.devtools.build.lib.profiler.Profiler;
4949
import com.google.devtools.build.lib.profiler.ProfilerTask;
50+
import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
5051
import com.google.devtools.build.lib.remote.util.AsyncTaskCache;
5152
import com.google.devtools.build.lib.util.TempPathGenerator;
5253
import com.google.devtools.build.lib.vfs.FileSymlinkLoopException;
@@ -394,14 +395,24 @@ private ListenableFuture<Void> prefetchFile(
394395

395396
Completable result =
396397
downloadFileNoCheckRx(
397-
action,
398-
execRoot.getRelative(execPath),
399-
treeRootExecPath != null ? execRoot.getRelative(treeRootExecPath) : null,
400-
dirsWithOutputPermissions,
401-
input,
402-
metadata,
403-
priority,
404-
reason);
398+
action,
399+
execRoot.getRelative(execPath),
400+
treeRootExecPath != null ? execRoot.getRelative(treeRootExecPath) : null,
401+
dirsWithOutputPermissions,
402+
input,
403+
metadata,
404+
priority,
405+
reason)
406+
.onErrorResumeNext(
407+
t -> {
408+
if (t instanceof CacheNotFoundException cacheNotFoundException) {
409+
// Only the symlink itself is guaranteed to be an input to the action, so
410+
// report its path for rewinding.
411+
cacheNotFoundException.setExecPath(input.getExecPath());
412+
return Completable.error(cacheNotFoundException);
413+
}
414+
return Completable.error(t);
415+
});
405416

406417
if (symlink != null) {
407418
result = result.andThen(plantSymlink(symlink));

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

+121
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,127 @@ public void remoteCacheEvictBlobs_whenPrefetchingInput_succeedsWithActionRewindi
418418
assertValidOutputFile("a/bar.out", "foo" + lineSeparator() + "updated bar" + lineSeparator());
419419
}
420420

421+
@Test
422+
public void remoteCacheEvictBlobs_whenPrefetchingSymlinkedInput_exitWithCode39()
423+
throws Exception {
424+
// Arrange: Prepare workspace and populate remote cache
425+
writeSymlinkRule();
426+
write(
427+
"a/BUILD",
428+
"""
429+
load("//:symlink.bzl", "symlink")
430+
431+
genrule(
432+
name = "foo",
433+
srcs = ["foo.in"],
434+
outs = ["foo.out"],
435+
cmd = "cat $(SRCS) > $@",
436+
)
437+
438+
symlink(
439+
name = "symlinked_foo",
440+
target_artifact = ":foo.out",
441+
)
442+
443+
genrule(
444+
name = "bar",
445+
srcs = [
446+
":symlinked_foo",
447+
"bar.in",
448+
],
449+
outs = ["bar.out"],
450+
cmd = "cat $(SRCS) > $@",
451+
tags = ["no-remote-exec"],
452+
)
453+
""");
454+
write("a/foo.in", "foo");
455+
write("a/bar.in", "bar");
456+
457+
// Populate remote cache
458+
buildTarget("//a:bar");
459+
var bytes = readContent(getOutputPath("a/foo.out"));
460+
var hashCode = getDigestHashFunction().getHashFunction().hashBytes(bytes);
461+
getOnlyElement(getArtifacts("//a:symlinked_foo")).getPath().delete();
462+
getOutputPath("a/foo.out").delete();
463+
getOutputPath("a/bar.out").delete();
464+
getOutputBase().getRelative("action_cache").deleteTreesBelow();
465+
restartServer();
466+
467+
// Clean build, foo.out isn't downloaded
468+
buildTarget("//a:bar");
469+
assertOutputDoesNotExist("a/foo.out");
470+
assertOutputsDoNotExist("//a:symlinked_foo");
471+
472+
// Act: Evict blobs from remote cache and do an incremental build
473+
evictAllBlobs();
474+
write("a/bar.in", "updated bar");
475+
var error = assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar"));
476+
477+
// Assert: Exit code is 39
478+
assertThat(error).hasMessageThat().contains("Lost inputs no longer available remotely");
479+
assertThat(error).hasMessageThat().contains("a/symlinked_foo");
480+
assertThat(error).hasMessageThat().contains(String.format("%s/%s", hashCode, bytes.length));
481+
assertThat(error.getDetailedExitCode().getExitCode().getNumericExitCode()).isEqualTo(39);
482+
}
483+
484+
@Test
485+
public void remoteCacheEvictBlobs_whenPrefetchingSymlinkedInput_succeedsWithActionRewinding()
486+
throws Exception {
487+
writeSymlinkRule();
488+
write(
489+
"a/BUILD",
490+
"""
491+
load("//:symlink.bzl", "symlink")
492+
493+
genrule(
494+
name = "foo",
495+
srcs = ["foo.in"],
496+
outs = ["foo.out"],
497+
cmd = "cat $(SRCS) > $@",
498+
)
499+
500+
symlink(
501+
name = "symlinked_foo",
502+
target_artifact = ":foo.out",
503+
)
504+
505+
genrule(
506+
name = "bar",
507+
srcs = [
508+
":symlinked_foo",
509+
"bar.in",
510+
],
511+
outs = ["bar.out"],
512+
cmd = "cat $(SRCS) > $@",
513+
tags = ["no-remote-exec"],
514+
)
515+
""");
516+
write("a/foo.in", "foo");
517+
write("a/bar.in", "bar");
518+
519+
// Populate remote cache
520+
buildTarget("//a:bar");
521+
getOnlyElement(getArtifacts("//a:symlinked_foo")).getPath().delete();
522+
getOutputPath("a/foo.out").delete();
523+
getOutputPath("a/bar.out").delete();
524+
getOutputBase().getRelative("action_cache").deleteTreesBelow();
525+
restartServer();
526+
527+
// Clean build, foo.out isn't downloaded
528+
buildTarget("//a:bar");
529+
assertOutputDoesNotExist("a/foo.out");
530+
assertOutputsDoNotExist("//a:symlinked_foo");
531+
532+
// Act: Evict blobs from remote cache and do an incremental build
533+
evictAllBlobs();
534+
write("a/bar.in", "updated bar");
535+
enableActionRewinding();
536+
buildTarget("//a:bar");
537+
538+
// Assert: target was successfully built
539+
assertValidOutputFile("a/bar.out", "foo" + lineSeparator() + "updated bar" + lineSeparator());
540+
}
541+
421542
@Test
422543
public void remoteCacheEvictBlobs_whenUploadingInput_exitWithCode39() throws Exception {
423544
// Arrange: Prepare workspace and populate remote cache

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

+2
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;
@@ -2033,6 +2034,7 @@ protected void assertSymlink(String binRelativeLinkPath, PathFragment targetPath
20332034
}
20342035

20352036
protected void writeSymlinkRule() throws IOException {
2037+
FileSystemUtils.touchFile(getWorkspace().getRelative("BUILD"));
20362038
write(
20372039
"symlink.bzl",
20382040
"""

0 commit comments

Comments
 (0)