Provide descriptive error messages on external mutable source files encountered during a build

Currently when evaluating a file or symlink leading to an external mutable object, Blaze throws an exception with unclear messages. The message does not contain the actual path but rather [/]/[] instead. This change updates FileFunction to allow bubbling up the error with the accurate path.

--
MOS_MIGRATED_REVID=118381323

1 files changed, 57 insertions, 5 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
index 22afdaa8ec..7e1e1cb040 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.skyframe;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.lib.skyframe.SkyframeExecutor.DEFAULT_THREAD_COUNT;
+import static com.google.devtools.build.skyframe.EvaluationResultSubjectFactory.assertThatEvaluationResult;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -664,8 +665,14 @@ public class FileFunctionTest {
             ImmutableList.of(key), false, DEFAULT_THREAD_COUNT, NullEventHandler.INSTANCE);
 
     assertTrue(result.hasError());
-    assertThat(result.getError(key).getException())
+    assertThatEvaluationResult(result)
+        .hasErrorEntryForKeyThat(key)
+        .hasExceptionThat()
         .isInstanceOf(FileOutsidePackageRootsException.class);
+    assertThatEvaluationResult(result)
+        .hasErrorEntryForKeyThat(key)
+        .hasExceptionThat()
+        .hasMessage("Encountered reference to external mutable [/]/[outsideroot]");
   }
 
   @Test
@@ -680,8 +687,45 @@ public class FileFunctionTest {
             ImmutableList.of(key), false, DEFAULT_THREAD_COUNT, NullEventHandler.INSTANCE);
 
     assertTrue(result.hasError());
-    assertThat(result.getError(key).getException())
-        .isInstanceOf(FileOutsidePackageRootsException.class);
+    assertThatEvaluationResult(result)
+        .hasErrorEntryForKeyThat(key)
+        .hasExceptionThat()
+        .isInstanceOf(SymlinkOutsidePackageRootsException.class);
+    assertThatEvaluationResult(result)
+        .hasErrorEntryForKeyThat(key)
+        .hasExceptionThat()
+        .hasMessage(
+            "Encountered symlink [/root]/[a] linking to external mutable [/]/[outsideroot]");
+  }
+
+  /**
+   * A slightly more complicated negative test to ensure that the error message contains the real
+   * symlink and external path instead of the path of the top-level skyframe file node. In other
+   * words, the error is bubbled up to the top-level node, but the error message stops getting
+   * updated once it enters the internal path boundary.
+   */
+  @Test
+  public void testAbsoluteSymlinksReferredByInternalFilesToFilesOutsideRootWhenExternalDisallowed()
+      throws Exception {
+    file("/outsideroot/src/foo/bar");
+    symlink("/root/src", "/outsideroot/src");
+
+    SequentialBuildDriver driver = makeDriver(/*errorOnExternalFiles=*/ true);
+    SkyKey key = skyKey("/root/src/foo/bar");
+    EvaluationResult<SkyValue> result =
+        driver.evaluate(
+            ImmutableList.of(key), false, DEFAULT_THREAD_COUNT, NullEventHandler.INSTANCE);
+
+    assertTrue(result.hasError());
+    assertThatEvaluationResult(result)
+        .hasErrorEntryForKeyThat(key)
+        .hasExceptionThat()
+        .isInstanceOf(SymlinkOutsidePackageRootsException.class);
+    assertThatEvaluationResult(result)
+        .hasErrorEntryForKeyThat(key)
+        .hasExceptionThat()
+        .hasMessage(
+            "Encountered symlink [/root]/[src] linking to external mutable [/]/[outsideroot/src]");
   }
 
   @Test
@@ -693,9 +737,17 @@ public class FileFunctionTest {
     EvaluationResult<SkyValue> result =
         driver.evaluate(
             ImmutableList.of(key), false, DEFAULT_THREAD_COUNT, NullEventHandler.INSTANCE);
+
     assertTrue(result.hasError());
-    assertThat(result.getError(key).getException())
-        .isInstanceOf(FileOutsidePackageRootsException.class);
+    assertThatEvaluationResult(result)
+        .hasErrorEntryForKeyThat(key)
+        .hasExceptionThat()
+        .isInstanceOf(SymlinkOutsidePackageRootsException.class);
+    assertThatEvaluationResult(result)
+        .hasErrorEntryForKeyThat(key)
+        .hasExceptionThat()
+        .hasMessage(
+            "Encountered symlink [/root]/[a] linking to external mutable [/]/[outsideroot]");
   }
 
   @Test