bazel syntax: stop Starlark execution at the first execution error
The Starlark spec requires that execution stop at the first execution
error. However, Blaze's BUILD file interpreter (now Starlark interpreter)
has long behaved as if there were an exception handler around each
top-level statement that reports the error but allows execution to continue.
This change removes that (mis)feature, so this program:
x = 1 // 0 # error: division by zero
print("hello")
does not execute the print statement. This change affects
all Starlark files, including BUILD and .bzl files.
The motivation for the historical behavior was that it allowed blaze
to return partial information even in the face of errors, similar to
the way the --keep_going flag operates. However, it introduces
unnecessary complexity, and contravenes the spec, hence this change.
Unnecessary, because the vast majority of BUILD files are valid, and
those that are not valid are generally completely neglected, so this
form of error recovery has no benefit in practice.
Also, --keep_going for blaze query or build makes sense because it
allows one to make progress in the face of broken dependencies that
one has no hope of fixing. By contrast, it is always trivial to fix a
BUILD file so that it doesn't contain an error simply by deleting the
failing top-level statement and any subsequent ones.
Note that errors encountered within Blaze built-ins such as 'rule' or
'attr' cause error events to be reported but do not cause Starlark
execution to fail. So, in the program below, the print statement is
still executed:
cc_binary(name='b', srcs=0) # reports error event: "for srcs, got int, want labels"
print("hello")
Several tests needed fixing, either because they contained irrelevant
failures that were ignored, or because the test asserted expected an
error other than the first, or because errant packages no longer
contain rules defined after the first error.
RELNOTES: BUILD/.bzl execution errors cause execution to stop, even at top-level
PiperOrigin-RevId: 271416461
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
index e20ab8c..1fd71cd 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
@@ -226,10 +226,9 @@
Root pkgRoot = getSkyframeExecutor().getPathEntries().get(0);
scratch.file(
"foo/BUILD",
- "subinclude('//a:a')",
- "sh_library(name = 'foo', srcs = glob(['bar/**/baz.sh']))");
- scratch.file("a/BUILD");
- scratch.file("a/a");
+ "sh_library(name = 'foo', srcs = glob(['bar/**/baz.sh']))",
+ "x = 1//0" // causes 'foo' to be marked in error
+ );
Path bazFile = scratch.file("foo/bar/baz/baz.sh");
Path bazDir = bazFile.getParentDirectory();
Path barDir = bazDir.getParentDirectory();