Change action failure description to be action.describe(), rather than its primary output.
Remove longstanding kludges to work around the absence of this information. In some cases, we now print the action's description instead of its owning label. I think that's ok.
Main changes: for C++ compilation and file write actions, we print the usual description instead of their bespoke one which had the label. For errors around output file failures, we print action.describe() instead of the rule label. For errors in include scanning, we print the action, then the subtask ("include scanning"), then the failure.
I've left a little kludge, for old times' sake: because ActionExecutionException's message is used both in SkyframeActionExecutor and in SkyframeBuilder when constructing a BuildFailedException, but there are lots of places that construct an ActionExecutionException, it is left to the ActionExecutionException consumer to prepend action.describe() + " failed: ". We could have ActionExecutionException itself do that in its constructor when it is constructed, but there are subtleties involving its message being null that I didn't want to untangle.
RELNOTES: Error messages emitted when an action fails are reworked to be more informative about the failing action. Some tooling may have to be updated as a result.
Closes #11151
PiperOrigin-RevId: 337305888
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/CustomRealFilesystemBuildIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/CustomRealFilesystemBuildIntegrationTest.java
index f2ece1b..1fbbb83 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/CustomRealFilesystemBuildIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/CustomRealFilesystemBuildIntegrationTest.java
@@ -84,7 +84,8 @@
customFileSystem.alwaysError(fooShFile);
assertThrows(BuildFailedException.class, () -> buildTarget("//foo:top"));
- events.assertContainsError("//foo:top: missing input file '//foo:foo.sh': nope");
+ events.assertContainsError(
+ "Executing genrule //foo:top failed: missing input file '//foo:foo.sh': nope");
}
/**
@@ -104,7 +105,8 @@
assertThat(rootCauses).hasSize(1);
assertThat(rootCauses.get(0).getLabel()).isEqualTo(Label.parseAbsoluteUnchecked("//foo:foo"));
assertThat(e.getDetailedExitCode().getExitCode()).isEqualTo(ExitCode.BUILD_FAILURE);
- events.assertContainsError("foo/BUILD:1:11: //foo:foo: missing input file 'foo/foo.h': nope");
+ events.assertContainsError(
+ "foo/BUILD:1:11: Compiling foo/foo.cc failed: missing input file 'foo/foo.h': nope");
}
/** Tests that IOExceptions encountered when not all discovered deps are done are handled. */
@@ -142,7 +144,8 @@
assertThat(rootCauses).hasSize(1);
assertThat(rootCauses.get(0).getLabel()).isEqualTo(Label.parseAbsoluteUnchecked("//foo:foo"));
assertThat(e.getDetailedExitCode().getExitCode()).isEqualTo(ExitCode.BUILD_FAILURE);
- events.assertContainsError("foo/BUILD:1:11: //foo:foo: missing input file 'foo/error.h': nope");
+ events.assertContainsError(
+ "foo/BUILD:1:11: Compiling foo/foo.cc failed: missing input file 'foo/error.h': nope");
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/DanglingSymlinkTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/DanglingSymlinkTest.java
index 0e2e98b..0483939 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/DanglingSymlinkTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/DanglingSymlinkTest.java
@@ -53,7 +53,8 @@
assertThat(e).hasMessageThat().isNull();
events.assertContainsError("output 'test/test.out' is a dangling symbolic link");
- events.assertContainsError("Couldn't build file test/test.out: not all outputs were created");
+ events.assertContainsError(
+ "Executing genrule //test:test_ln failed: not all outputs were created");
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/MissingInputActionTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/MissingInputActionTest.java
index e9477f2..5a45980 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/MissingInputActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/MissingInputActionTest.java
@@ -79,7 +79,9 @@
addOptions("--workspace_status_command=" + sleepPath.getPathString());
for (int i = 0; i < 2; i++) {
assertMissingInputOnBuild("//dummy", 1);
- events.assertContainsError("dummy/BUILD:1:8: //dummy:dummy: missing input file '//dummy:in'");
+ events.assertContainsError(
+ "dummy/BUILD:1:8: Executing genrule //dummy:dummy failed: missing input file"
+ + " '//dummy:in'");
events.assertContainsEventWithFrequency("missing input file", 1);
events.assertDoesNotContainEvent("Failed to determine build info");
events.clear();
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/NoOutputActionTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/NoOutputActionTest.java
index ca66e1b..ec69335 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/NoOutputActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/NoOutputActionTest.java
@@ -46,7 +46,9 @@
assertThrows(BuildFailedException.class, () -> buildTarget("//nooutput"));
assertThat(e)
.hasMessageThat()
- .contains("nooutput/BUILD:1:8 not all outputs were created or valid");
+ .contains(
+ "nooutput/BUILD:1:8 Executing genrule //nooutput:nooutput failed: not all outputs were"
+ + " created or valid");
events.assertContainsError("declared output 'nooutput/out1' was not created by genrule");
events.assertContainsError("declared output 'nooutput/out2' was not created by genrule");
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
index ad8fed7..e0264f8 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
@@ -467,6 +467,11 @@
}
@Override
+ public String describe() {
+ return prettyPrint();
+ }
+
+ @Override
public NestedSet<Artifact> getTools() {
return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
index 5c41342..d004581 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
@@ -1196,6 +1196,11 @@
@Override
public String prettyPrint() {
+ return describe();
+ }
+
+ @Override
+ public String describe() {
return "DummyTemplate";
}
diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
index bd11f56..88bb1b5 100644
--- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
@@ -36,6 +36,7 @@
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
+import com.google.devtools.build.lib.actions.util.ActionsTestUtil.NullAction;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ServerDirectories;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -316,7 +317,7 @@
public void testVerboseFailures() {
ExecException e = assertThrows(ExecException.class, () -> run(createSpawn(getFalseCommand())));
ActionExecutionException actionExecutionException =
- e.toActionExecutionException("messagePrefix", null);
+ e.toActionExecutionException(new NullAction());
assertWithMessage("got: " + actionExecutionException.getMessage())
.that(actionExecutionException.getMessage().contains("failed: error executing command"))
.isTrue();
