From 922b27907c20c23347b700b496f7be3160438c2a Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 15 Feb 2024 01:26:07 -0800 Subject: [PATCH] [7.1.0] Clarify the behavior of --incompatible_remote_symlinks in the presence of a dangling symlink. This CL makes no behavioral changes; it only amends documentation to match reality and adds test coverage for it. PiperOrigin-RevId: 607249997 Change-Id: I448801a4d696347184e2d307ad0df58eb79193cf --- .../lib/remote/options/RemoteOptions.java | 9 +- .../build/lib/remote/UploadManifestTest.java | 96 +++++++++++++++++++ 2 files changed, 99 insertions(+), 6 deletions(-) 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 37e0b643c2dc7c..57e954ce0969cc 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 @@ -407,9 +407,8 @@ public RemoteBuildEventUploadModeConverter() { effectTags = {OptionEffectTag.EXECUTION}, metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, help = - "If set to true, Bazel will represent symlinks in action outputs in the remote" - + " caching/execution protocol as such. Otherwise, symlinks will be followed and" - + " represented as files or directories. See #6631 for details.") + "If set to true, Bazel will upload symlinks as such to a remote or disk cache. Otherwise," + + " non-dangling symlinks will be uploaded as the file or directory they point to.") public boolean incompatibleRemoteSymlinks; @Option( @@ -419,9 +418,7 @@ public RemoteBuildEventUploadModeConverter() { 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.") + help = "If set to true, symlinks uploaded to a remote or disk cache are allowed to dangle.") public boolean incompatibleRemoteDanglingSymlinks; @Option( 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 c0b93f2a4d002c..8443870e598b27 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 @@ -440,6 +440,53 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkAsSymlink() th assertThat(result.build()).isEqualTo(expectedResult.build()); } + @Test + public void actionResult_followSymlinks_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= */ true, + /* allowDanglingSymlinks= */ true, + /* allowAbsoluteSymlinks= */ true); + um.addFiles(ImmutableList.of(link)); + assertThat(um.getDigestToFile()).isEmpty(); + + ActionResult.Builder expectedResult = ActionResult.newBuilder(); + expectedResult.addOutputFileSymlinksBuilder().setPath("link").setTarget("/execroot/target"); + expectedResult.addOutputSymlinksBuilder().setPath("link").setTarget("/execroot/target"); + assertThat(result.build()).isEqualTo(expectedResult.build()); + } + + @Test + public void actionResult_followSymlinks_relativeDanglingSymlinkAsSymlink() throws Exception { + ActionResult.Builder result = ActionResult.newBuilder(); + Path link = execRoot.getRelative("link"); + link.createSymbolicLink(PathFragment.create("target")); + + UploadManifest um = + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /* followSymlinks= */ true, + /* allowDanglingSymlinks= */ true, + /* allowAbsoluteSymlinks= */ false); + um.addFiles(ImmutableList.of(link)); + assertThat(um.getDigestToFile()).isEmpty(); + + ActionResult.Builder expectedResult = ActionResult.newBuilder(); + expectedResult.addOutputFileSymlinksBuilder().setPath("link").setTarget("target"); + expectedResult.addOutputSymlinksBuilder().setPath("link").setTarget("target"); + assertThat(result.build()).isEqualTo(expectedResult.build()); + } + @Test public void actionResult_noFollowSymlinks_noAllowDanglingSymlinks_absoluteDanglingSymlinkError() throws Exception { @@ -903,6 +950,55 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkInDirectoryAsS assertThat(result.build()).isEqualTo(expectedResult.build()); } + @Test + public void + actionResult_followSymlinks_allowDanglingSymlinks_absoluteDanglingSymlinkInDirectoryError() + throws Exception { + ActionResult.Builder result = ActionResult.newBuilder(); + Path dir = execRoot.getRelative("dir"); + dir.createDirectory(); + Path link = dir.getRelative("link"); + Path target = dir.getRelative("target"); + link.createSymbolicLink(target); + + UploadManifest um = + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /* followSymlinks= */ true, + /* allowDanglingSymlinks= */ true, + /* allowAbsoluteSymlinks= */ 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/dir/target"); + } + + @Test + public void + actionResult_followSymlinks_allowDanglingSymlinks_relativeDanglingSymlinkInDirectoryError() + throws Exception { + ActionResult.Builder result = ActionResult.newBuilder(); + Path dir = execRoot.getRelative("dir"); + dir.createDirectory(); + Path link = dir.getRelative("link"); + link.createSymbolicLink(PathFragment.create("target")); + + UploadManifest um = + new UploadManifest( + digestUtil, + remotePathResolver, + result, + /* followSymlinks= */ true, + /* allowDanglingSymlinks= */ true, + /* allowAbsoluteSymlinks= */ false); + 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("target"); + } + @Test public void actionResult_noFollowSymlinks_noAllowDanglingSymlinks_absoluteDanglingSymlinkInDirectory()