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
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 22afdaa..7e1e1cb 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 @@
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 @@
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 @@
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 @@
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