From 89e795aaa964aa0c3321337d33657dc672fa7c04 Mon Sep 17 00:00:00 2001 From: Tiago Quelhas Date: Thu, 29 Sep 2022 14:53:53 +0100 Subject: [PATCH] Upload dangling symlinks found in the outputs of local actions. Since we don't know whether a dangling symlink is supposed to point to a file or a directory, we always report it as a symlink to a file. The distinction only matters in V2 of the protocol, with V2.1 reporting them uniformly, so it doesn't seem to be worth the effort to match up the input and output types. Dangling symlinks to absolute paths are unconditionally uploaded. A followup PR will gate their upload on the presence of the respective server capability (SymlinkAbsolutePathStrategy.ALLOW). Progress towards #16289. --- .../lib/remote/RemoteExecutionService.java | 5 +- .../build/lib/remote/UploadManifest.java | 43 ++- .../lib/remote/options/RemoteOptions.java | 12 + .../lib/actions/util/ActionsTestUtil.java | 6 + .../remote/RemoteExecutionServiceTest.java | 34 +++ .../build/lib/remote/UploadManifestTest.java | 287 ++++++++++++++++-- 6 files changed, 341 insertions(+), 46 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java index 2c61a0038feee9..154d139152e2c9 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java @@ -115,6 +115,7 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.Symlinks; import com.google.protobuf.ByteString; import com.google.protobuf.ExtensionRegistry; import com.google.protobuf.Message; @@ -1257,8 +1258,10 @@ private Single buildUploadManifestAsync( ImmutableList.Builder outputFiles = ImmutableList.builder(); // Check that all mandatory outputs are created. for (ActionInput outputFile : action.getSpawn().getOutputFiles()) { + Symlinks followSymlinks = outputFile.isSymlink() ? Symlinks.NOFOLLOW : Symlinks.FOLLOW; Path localPath = execRoot.getRelative(outputFile.getExecPath()); - if (action.getSpawn().isMandatoryOutput(outputFile) && !localPath.exists()) { + if (action.getSpawn().isMandatoryOutput(outputFile) + && !localPath.exists(followSymlinks)) { throw new IOException( "Expected output " + prettyPrint(outputFile) + " was not created locally."); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java b/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java index 209c956c4bb7c3..a7fc6ed674fe58 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java +++ b/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java @@ -76,6 +76,7 @@ public class UploadManifest { private final RemotePathResolver remotePathResolver; private final ActionResult.Builder result; private final boolean followSymlinks; + private final boolean allowDanglingSymlinks; private final Map digestToFile = new HashMap<>(); private final Map digestToBlobs = new HashMap<>(); @Nullable private ActionKey actionKey; @@ -103,7 +104,8 @@ public static UploadManifest create( digestUtil, remotePathResolver, result, - /* followSymlinks= */ !remoteOptions.incompatibleRemoteSymlinks); + /* followSymlinks= */ !remoteOptions.incompatibleRemoteSymlinks, + /* allowDanglingSymlinks= */ remoteOptions.incompatibleRemoteDanglingSymlinks); manifest.addFiles(outputFiles); manifest.setStdoutStderr(outErr); manifest.addAction(actionKey, action, command); @@ -140,11 +142,13 @@ public UploadManifest( DigestUtil digestUtil, RemotePathResolver remotePathResolver, ActionResult.Builder result, - boolean followSymlinks) { + boolean followSymlinks, + boolean allowDanglingSymlinks) { this.digestUtil = digestUtil; this.remotePathResolver = remotePathResolver; this.result = result; this.followSymlinks = followSymlinks; + this.allowDanglingSymlinks = allowDanglingSymlinks; } private void setStdoutStderr(FileOutErr outErr) throws IOException { @@ -186,25 +190,32 @@ void addFiles(Collection files) throws ExecException, IOException { // Need to resolve the symbolic link to know what to add, file or directory. FileStatus statFollow = file.statIfFound(Symlinks.FOLLOW); if (statFollow == null) { - throw new IOException( - String.format("Action output %s is a dangling symbolic link to %s ", file, target)); - } - if (statFollow.isSpecialFile()) { - illegalOutput(file); - } - Preconditions.checkState( - statFollow.isFile() || statFollow.isDirectory(), "Unknown stat type for %s", file); - if (!followSymlinks && !target.isAbsolute()) { - if (statFollow.isFile()) { + if (allowDanglingSymlinks) { + // Report symlink to a file since we don't know any better. + // TODO(tjgq): Check for the SymlinkAbsolutePathStrategy.ALLOW server capability before + // uploading an absolute symlink. addFileSymbolicLink(file, target); } else { - addDirectorySymbolicLink(file, target); + throw new IOException( + String.format("Action output %s is a dangling symbolic link to %s ", file, target)); } + } else if (statFollow.isSpecialFile()) { + illegalOutput(file); } else { - if (statFollow.isFile()) { - addFile(digestUtil.compute(file), file); + Preconditions.checkState( + statFollow.isFile() || statFollow.isDirectory(), "Unknown stat type for %s", file); + if (!followSymlinks && !target.isAbsolute()) { + if (statFollow.isFile()) { + addFileSymbolicLink(file, target); + } else { + addDirectorySymbolicLink(file, target); + } } else { - addDirectory(file); + if (statFollow.isFile()) { + addFile(digestUtil.compute(file), file); + } else { + addDirectory(file); + } } } } else { diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index 101641aaf5c0fa..c9823a65aa3aaf 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java @@ -412,6 +412,18 @@ public RemoteBuildEventUploadModeConverter() { + " represented as files or directories. See #6631 for details.") public boolean incompatibleRemoteSymlinks; + @Option( + name = "incompatible_remote_dangling_symlinks", + defaultValue = "true", + category = "remote", + documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, + effectTags = {OptionEffectTag.EXECUTION}, + metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, + help = + "If set to true and --incompatible_remote_symlinks is also true, symlinks in action" + + " outputs are allowed to dangle.") + public boolean incompatibleRemoteDanglingSymlinks; + @Option( name = "experimental_remote_cache_compression", defaultValue = "false", diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java index 50ad0bda12c74d..46326b223b03b5 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java @@ -267,6 +267,12 @@ public static SpecialArtifact createTreeArtifactWithGeneratingAction( return treeArtifact; } + public static SpecialArtifact createUnresolvedSymlinkArtifactWithExecPath( + ArtifactRoot root, PathFragment execPath) { + return SpecialArtifact.create( + root, execPath, NULL_ARTIFACT_OWNER, SpecialArtifactType.UNRESOLVED_SYMLINK); + } + public static void assertNoArtifactEndingWith(RuleConfiguredTarget target, String path) { Pattern endPattern = Pattern.compile(path + "$"); for (ActionAnalysisMetadata action : target.getActions()) { diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java index 39b6cc35c7986d..a6e2fe4f1537cf 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java @@ -1367,6 +1367,40 @@ public void uploadOutputs_uploadNestedDirectory_works() throws Exception { .isEmpty(); } + @Test + public void uploadOutputs_uploadRelativeDanglingSymlink_works() throws Exception { + // arrange + Path linkPath = execRoot.getRelative("outputs/link"); + linkPath.createSymbolicLink(PathFragment.create("some/path")); + Artifact outputSymlink = + ActionsTestUtil.createUnresolvedSymlinkArtifactWithExecPath( + artifactRoot, linkPath.relativeTo(execRoot)); + RemoteExecutionService service = newRemoteExecutionService(); + Spawn spawn = newSpawn(ImmutableMap.of(), ImmutableSet.of(outputSymlink)); + FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn); + RemoteAction action = service.buildRemoteAction(spawn, context); + SpawnResult spawnResult = + new SpawnResult.Builder() + .setExitCode(0) + .setStatus(SpawnResult.Status.SUCCESS) + .setRunnerName("test") + .build(); + + // act + UploadManifest manifest = service.buildUploadManifest(action, spawnResult); + service.uploadOutputs(action, spawnResult); + + // assert + ActionResult.Builder expectedResult = ActionResult.newBuilder(); + expectedResult.addOutputFileSymlinksBuilder().setPath("outputs/link").setTarget("some/path"); + assertThat(manifest.getActionResult()).isEqualTo(expectedResult.build()); + // assertThat( + // getFromFuture( + // cache.findMissingDigests( + // remoteActionExecutionContext, ImmutableList.of(barDigest)))) + // .isEmpty(); + } + @Test public void uploadOutputs_emptyOutputs_doNotPerformUpload() throws Exception { // Test that uploading an empty output does not try to perform an upload. diff --git a/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java b/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java index 236fed0d695384..21116c4d966706 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java @@ -202,7 +202,12 @@ public void actionResult_followSymlinks_absoluteFileSymlinkAsFile() throws Excep link.createSymbolicLink(target); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ true); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ true, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(target); assertThat(um.getDigestToFile()).containsExactly(digest, link); @@ -223,7 +228,12 @@ public void actionResult_followSymlinks_absoluteDirectorySymlinkAsDirectory() th link.createSymbolicLink(dir); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ true); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ true, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(foo); assertThat(um.getDigestToFile()).containsExactly(digest, execRoot.getRelative("link/foo")); @@ -250,7 +260,12 @@ public void actionResult_noFollowSymlinks_absoluteFileSymlinkAsFile() throws Exc link.createSymbolicLink(target); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(target); assertThat(um.getDigestToFile()).containsExactly(digest, link); @@ -271,7 +286,12 @@ public void actionResult_noFollowSymlinks_absoluteDirectorySymlinkAsDirectory() link.createSymbolicLink(dir); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(foo); assertThat(um.getDigestToFile()).containsExactly(digest, execRoot.getRelative("link/foo")); @@ -298,7 +318,12 @@ public void actionResult_followSymlinks_relativeFileSymlinkAsFile() throws Excep link.createSymbolicLink(target.relativeTo(execRoot)); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ true); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ true, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(target); assertThat(um.getDigestToFile()).containsExactly(digest, link); @@ -319,7 +344,12 @@ public void actionResult_followSymlinks_relativeDirectorySymlinkAsDirectory() th link.createSymbolicLink(dir.relativeTo(execRoot)); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ true); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ true, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); Digest digest = digestUtil.compute(foo); assertThat(um.getDigestToFile()).containsExactly(digest, execRoot.getRelative("link/foo")); @@ -346,7 +376,12 @@ public void actionResult_noFollowSymlinks_relativeFileSymlinkAsSymlink() throws link.createSymbolicLink(target.relativeTo(execRoot)); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); assertThat(um.getDigestToFile()).isEmpty(); @@ -366,7 +401,12 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkAsSymlink() th link.createSymbolicLink(dir.relativeTo(execRoot)); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(link)); assertThat(um.getDigestToFile()).isEmpty(); @@ -376,14 +416,20 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkAsSymlink() th } @Test - public void actionResult_noFollowSymlinks_absoluteDanglingSymlinkError() throws Exception { + public void actionResult_noFollowSymlinks_noAllowDanglingSymlinks_absoluteDanglingSymlinkError() + throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); Path link = execRoot.getRelative("link"); Path target = execRoot.getRelative("target"); link.createSymbolicLink(target); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); IOException e = assertThrows(IOException.class, () -> um.addFiles(ImmutableList.of(link))); assertThat(e).hasMessageThat().contains("dangling"); assertThat(e).hasMessageThat().contains("/execroot/link"); @@ -391,20 +437,72 @@ public void actionResult_noFollowSymlinks_absoluteDanglingSymlinkError() throws } @Test - public void actionResult_noFollowSymlinks_relativeDanglingSymlinkError() throws Exception { + public void actionResult_noFollowSymlinks_allowDanglingSymlinks_absoluteDanglingSymlinkAsSymlink() + throws Exception { + ActionResult.Builder result = ActionResult.newBuilder(); + Path link = execRoot.getRelative("link"); + Path target = execRoot.getRelative("target"); + link.createSymbolicLink(target); + + UploadManifest um = + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ true); + um.addFiles(ImmutableList.of(link)); + assertThat(um.getDigestToFile()).isEmpty(); + + ActionResult.Builder expectedResult = ActionResult.newBuilder(); + expectedResult.addOutputFileSymlinksBuilder().setPath("link").setTarget("/execroot/target"); + assertThat(result.build()).isEqualTo(expectedResult.build()); + } + + @Test + public void actionResult_noFollowSymlinks_noAllowDanglingSymlinks_relativeDanglingSymlinkError() + throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); Path link = execRoot.getRelative("link"); Path target = execRoot.getRelative("target"); link.createSymbolicLink(target.relativeTo(link.getParentDirectory())); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); IOException e = assertThrows(IOException.class, () -> um.addFiles(ImmutableList.of(link))); assertThat(e).hasMessageThat().contains("dangling"); assertThat(e).hasMessageThat().contains("/execroot/link"); assertThat(e).hasMessageThat().contains("target"); } + @Test + public void actionResult_noFollowSymlinks_allowDanglingSymlinks_relativeDanglingSymlinkAsSymlink() + throws Exception { + ActionResult.Builder result = ActionResult.newBuilder(); + Path link = execRoot.getRelative("link"); + Path target = execRoot.getRelative("target"); + link.createSymbolicLink(target.relativeTo(link.getParentDirectory())); + + UploadManifest um = + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ true); + um.addFiles(ImmutableList.of(link)); + assertThat(um.getDigestToFile()).isEmpty(); + + ActionResult.Builder expectedResult = ActionResult.newBuilder(); + expectedResult.addOutputFileSymlinksBuilder().setPath("link").setTarget("target"); + assertThat(result.build()).isEqualTo(expectedResult.build()); + } + @Test public void actionResult_followSymlinks_absoluteFileSymlinkInDirectoryAsFile() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); @@ -416,7 +514,12 @@ public void actionResult_followSymlinks_absoluteFileSymlinkInDirectoryAsFile() t link.createSymbolicLink(target); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ true); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ true, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(target); assertThat(um.getDigestToFile()).containsExactly(digest, link); @@ -448,7 +551,12 @@ public void actionResult_followSymlinks_absoluteDirectorySymlinkInDirectoryAsDir link.createSymbolicLink(bardir); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ true); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ true, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(foo); assertThat(um.getDigestToFile()).containsExactly(digest, execRoot.getRelative("dir/link/foo")); @@ -485,7 +593,12 @@ public void actionResult_noFollowSymlinks_absoluteFileSymlinkInDirectoryAsFile() link.createSymbolicLink(target); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(target); assertThat(um.getDigestToFile()).containsExactly(digest, link); @@ -517,7 +630,12 @@ public void actionResult_noFollowSymlinks_absoluteDirectorySymlinkInDirectoryAsD link.createSymbolicLink(bardir); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(foo); assertThat(um.getDigestToFile()).containsExactly(digest, execRoot.getRelative("dir/link/foo")); @@ -553,7 +671,12 @@ public void actionResult_followSymlinks_relativeFileSymlinkInDirectoryAsFile() t link.createSymbolicLink(PathFragment.create("../target")); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ true); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ true, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(target); assertThat(um.getDigestToFile()).containsExactly(digest, link); @@ -585,7 +708,12 @@ public void actionResult_followSymlinks_relativeDirectorySymlinkInDirectoryAsDir link.createSymbolicLink(PathFragment.create("../bardir")); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ true); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ true, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); Digest digest = digestUtil.compute(foo); assertThat(um.getDigestToFile()).containsExactly(digest, execRoot.getRelative("dir/link/foo")); @@ -622,7 +750,12 @@ public void actionResult_noFollowSymlinks_relativeFileSymlinkInDirectoryAsSymlin link.createSymbolicLink(PathFragment.create("../target")); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); assertThat(um.getDigestToFile()).isEmpty(); @@ -653,7 +786,12 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkInDirectoryAsS link.createSymbolicLink(PathFragment.create("../bardir")); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); um.addFiles(ImmutableList.of(dir)); assertThat(um.getDigestToFile()).isEmpty(); @@ -671,7 +809,9 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkInDirectoryAsS } @Test - public void actionResult_noFollowSymlinks_absoluteDanglingSymlinkInDirectory() throws Exception { + public void + actionResult_noFollowSymlinks_noAllowDanglingSymlinks_absoluteDanglingSymlinkInDirectory() + throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); Path dir = execRoot.getRelative("dir"); dir.createDirectory(); @@ -680,7 +820,12 @@ public void actionResult_noFollowSymlinks_absoluteDanglingSymlinkInDirectory() t link.createSymbolicLink(target); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); IOException e = assertThrows(IOException.class, () -> um.addFiles(ImmutableList.of(dir))); assertThat(e).hasMessageThat().contains("dangling"); assertThat(e).hasMessageThat().contains("/execroot/dir/link"); @@ -688,8 +833,67 @@ public void actionResult_noFollowSymlinks_absoluteDanglingSymlinkInDirectory() t } @Test - public void actionResult_noFollowSymlinks_relativeDanglingSymlinkInDirectoryAsSymlink() - throws Exception { + public void + actionResult_noFollowSymlinks_allowDanglingSymlinks_absoluteDanglingSymlinkInDirectory() + throws Exception { + ActionResult.Builder result = ActionResult.newBuilder(); + Path dir = execRoot.getRelative("dir"); + dir.createDirectory(); + Path target = execRoot.getRelative("target"); + Path link = execRoot.getRelative("dir/link"); + link.createSymbolicLink(target); + + UploadManifest um = + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ true); + IOException e = assertThrows(IOException.class, () -> um.addFiles(ImmutableList.of(dir))); + assertThat(e).hasMessageThat().contains("dangling"); + assertThat(e).hasMessageThat().contains("/execroot/dir/link"); + assertThat(e).hasMessageThat().contains("/execroot/target"); + } + + @Test + public void + actionResult_noFollowSymlinks_noAllowDanglingSymlinks_relativeDanglingSymlinkInDirectoryAsSymlink() + throws Exception { + ActionResult.Builder result = ActionResult.newBuilder(); + Path dir = execRoot.getRelative("dir"); + dir.createDirectory(); + Path target = execRoot.getRelative("dir/target"); + Path link = execRoot.getRelative("dir/link"); + link.createSymbolicLink(target.relativeTo(link.getParentDirectory())); + + UploadManifest um = + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); + um.addFiles(ImmutableList.of(dir)); + assertThat(um.getDigestToFile()).isEmpty(); + + Tree tree = + Tree.newBuilder() + .setRoot( + Directory.newBuilder() + .addSymlinks(SymlinkNode.newBuilder().setName("link").setTarget("target"))) + .build(); + Digest treeDigest = digestUtil.compute(tree); + + ActionResult.Builder expectedResult = ActionResult.newBuilder(); + expectedResult.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest); + assertThat(result.build()).isEqualTo(expectedResult.build()); + } + + @Test + public void + actionResult_noFollowSymlinks_allowDanglingSymlinks_relativeDanglingSymlinkInDirectoryAsSymlink() + throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); Path dir = execRoot.getRelative("dir"); dir.createDirectory(); @@ -698,7 +902,12 @@ public void actionResult_noFollowSymlinks_relativeDanglingSymlinkInDirectoryAsSy link.createSymbolicLink(target.relativeTo(link.getParentDirectory())); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ true); um.addFiles(ImmutableList.of(dir)); assertThat(um.getDigestToFile()).isEmpty(); @@ -725,7 +934,12 @@ public void actionResult_noFollowSymlinks_specialFileError() throws Exception { Path special = dir.getRelative("special"); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); UserExecException e = assertThrows(UserExecException.class, () -> um.addFiles(ImmutableList.of(special))); assertThat(e).hasMessageThat().contains("special file"); @@ -739,7 +953,12 @@ public void actionResult_followSymlinks_specialFileSymlinkError() throws Excepti Path link = dir.getRelative("link"); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ true); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ true, + /*allowDanglingSymlinks=*/ false); UserExecException e = assertThrows(UserExecException.class, () -> um.addFiles(ImmutableList.of(link))); assertThat(e).hasMessageThat().contains("special file"); @@ -752,7 +971,12 @@ public void actionResult_noFollowSymlinks_specialFileInDirectoryError() throws E Path dir = createDirectoryWithSpecialFile("dir", "special"); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ false); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ false, + /*allowDanglingSymlinks=*/ false); UserExecException e = assertThrows(UserExecException.class, () -> um.addFiles(ImmutableList.of(dir))); assertThat(e).hasMessageThat().contains("special file"); @@ -765,7 +989,12 @@ public void actionResult_followSymlinks_specialFileSymlinkInDirectoryError() thr Path dir = createDirectoryWithSymlinkToSpecialFile("dir", "link", "special"); UploadManifest um = - new UploadManifest(digestUtil, remotePathResolver, result, /*followSymlinks=*/ true); + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /*followSymlinks=*/ true, + /*allowDanglingSymlinks=*/ false); UserExecException e = assertThrows(UserExecException.class, () -> um.addFiles(ImmutableList.of(dir))); assertThat(e).hasMessageThat().contains("special file");