Allow absolute symlinks in TreeArtifacts. Let the downstream action (if there is one) worry about it.

--
PiperOrigin-RevId: 148249223
MOS_MIGRATED_REVID=148249223

2 files changed, 32 insertions, 10 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
index 0f86ad1934..5a132c362a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
@@ -191,14 +191,12 @@ class TreeArtifactValue implements SkyValue {
             pathToExplode.getChild(subpath.getBaseName()), valuesBuilder);
       } else if (subpath.isSymbolicLink()) {
         PathFragment linkTarget = subpath.readSymbolicLinkUnchecked();
+        valuesBuilder.add(canonicalSubpathFragment);
         if (linkTarget.isAbsolute()) {
-          String errorMessage = String.format(
-              "A TreeArtifact may not contain absolute symlinks, found %s pointing to %s.",
-              subpath,
-              linkTarget);
-          throw new IOException(errorMessage);
+          // We tolerate absolute symlinks here. They will probably be dangling if any downstream
+          // consumer tries to read them, but let that be downstream's problem.
+          continue;
         }
-
         // We visit each path segment of the link target to catch any path traversal outside of the
         // TreeArtifact root directory. For example, for TreeArtifact a/b/c, it is possible to have
         // a symlink, a/b/c/sym_link that points to ../outside_dir/../c/link_target. Although this
@@ -216,7 +214,6 @@ class TreeArtifactValue implements SkyValue {
             throw new IOException(errorMessage);
           }
         }
-        valuesBuilder.add(canonicalSubpathFragment);
       } else if (subpath.isFile()) {
         valuesBuilder.add(canonicalSubpathFragment);
       } else {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
index 307e4a68d9..099b307116 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
@@ -564,7 +564,7 @@ public class TreeArtifactBuildTest extends TimestampBuilderTestCase {
   }
 
   @Test
-  public void testAbsoluteSymlinkRejected() throws Exception {
+  public void testAbsoluteSymlinkBadTargetRejected() throws Exception {
     // Failure expected
     StoredEventHandler storingEventHandler = new StoredEventHandler();
     reporter.removeHandler(failFastHandler);
@@ -596,13 +596,38 @@ public class TreeArtifactBuildTest extends TimestampBuilderTestCase {
       List<Event> errors = ImmutableList.copyOf(
           Iterables.filter(storingEventHandler.getEvents(), IS_ERROR_EVENT));
       assertThat(errors).hasSize(2);
-      assertThat(errors.get(0).getMessage()).contains(
-          "A TreeArtifact may not contain absolute symlinks");
+      assertThat(errors.get(0).getMessage()).contains("Failed to resolve relative path links/link");
       assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
     }
   }
 
   @Test
+  public void testAbsoluteSymlinkAccepted() throws Exception {
+    scratch.overwriteFile("/random/pointer");
+
+    final Artifact out = createTreeArtifact("output");
+
+    TreeArtifactTestAction action =
+        new TreeArtifactTestAction(out) {
+          @Override
+          public void execute(ActionExecutionContext actionExecutionContext) {
+            try {
+              writeFile(out.getPath().getChild("one"), "one");
+              writeFile(out.getPath().getChild("two"), "two");
+              FileSystemUtils.ensureSymbolicLink(
+                  out.getPath().getChild("links").getChild("link"), "/random/pointer");
+            } catch (Exception e) {
+              throw new RuntimeException(e);
+            }
+          }
+        };
+
+    registerAction(action);
+
+    buildArtifact(action.getSoleOutput());
+  }
+
+  @Test
   public void testRelativeSymlinkTraversingOutsideOfTreeArtifactRejected() throws Exception {
     // Failure expected
     StoredEventHandler storingEventHandler = new StoredEventHandler();