Add additional FailAction error codes to make the different errors more unique. Expand the reasoning behind the FailActions by making the error codes more specific to the issue that caused the failure. This includes changes to ConfiguredTargetFactory.java, AndroidBinary.java, CcLibrary.java, CppHelper.java, PyCommon.java RELNOTES: None. PiperOrigin-RevId: 334200809
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FailAction.java b/src/main/java/com/google/devtools/build/lib/actions/FailAction.java index 81c2084..829a74e 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FailAction.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FailAction.java
@@ -34,11 +34,16 @@ private static final String GUID = "626cb78a-810f-4af3-979c-ee194955f04c"; - private final String errorMessage; + private final FailureDetail failureDetail; - public FailAction(ActionOwner owner, Iterable<Artifact> outputs, String errorMessage) { + public FailAction( + ActionOwner owner, Iterable<Artifact> outputs, String errorMessage, Code failActionCode) { super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), outputs); - this.errorMessage = errorMessage; + this.failureDetail = + FailureDetail.newBuilder() + .setMessage(errorMessage + " caused by " + getOwner().getLabel()) + .setFailAction(FailureDetails.FailAction.newBuilder().setCode(failActionCode).build()) + .build(); } @Override @@ -47,26 +52,14 @@ } public String getErrorMessage() { - return errorMessage; + return failureDetail.getMessage(); } @Override public ActionResult execute(ActionExecutionContext actionExecutionContext) throws ActionExecutionException { throw new ActionExecutionException( - errorMessage, - this, - false, - DetailedExitCode.of( - FailureDetail.newBuilder() - .setMessage( - "FailAction intentional failure: " - + errorMessage - + " caused by " - + getOwner().getLabel()) - .setFailAction( - FailureDetails.FailAction.newBuilder().setCode(Code.INTENTIONAL_FAILURE)) - .build())); + failureDetail.getMessage(), this, false, DetailedExitCode.of(failureDetail)); } @Override
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index f61fd6a..52c24aa 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -67,6 +67,7 @@ import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.profiler.memory.CurrentRuleTracker; import com.google.devtools.build.lib.rules.cpp.DeniedImplicitOutputMarkerProvider; +import com.google.devtools.build.lib.server.FailureDetails.FailAction.Code; import com.google.devtools.build.lib.skyframe.AspectValueKey; import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; @@ -624,7 +625,8 @@ new FailAction( ruleContext.getActionOwner(), ruleContext.getOutputArtifacts(), - "Missing fragment class: " + missingFragmentClass.getName())); + "Missing fragment class: " + missingFragmentClass.getName(), + Code.FRAGMENT_CLASS_MISSING)); } builder.add(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY)); try {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java index d08471a..bad9b7f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
@@ -81,6 +81,7 @@ import com.google.devtools.build.lib.rules.java.JavaToolchainProvider; import com.google.devtools.build.lib.rules.java.OneVersionCheckActionBuilder; import com.google.devtools.build.lib.rules.java.ProguardSpecProvider; +import com.google.devtools.build.lib.server.FailureDetails.FailAction.Code; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.Serializable; import java.util.ArrayList; @@ -833,7 +834,8 @@ new FailAction( ruleContext.getActionOwner(), failures.build().toSet(), - String.format("Can't run Proguard without proguard_specs"))); + "Can't run Proguard without proguard_specs", + Code.PROGUARD_SPECS_MISSING)); return new ProguardOutput(deployJarArtifact, null, null, null, null, null, null); }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/BUILD b/src/main/java/com/google/devtools/build/lib/rules/android/BUILD index 559a5de..42e45fe 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/android/BUILD
@@ -89,6 +89,7 @@ "//src/main/java/com/google/devtools/common/options", "//src/main/java/net/starlark/java/eval", "//src/main/protobuf:android_deploy_info_java_proto", + "//src/main/protobuf:failure_details_java_proto", "//third_party:android_common_25_0_0_lite", "//third_party:auto_value", "//third_party:guava",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java index f0ac8dd..0076362 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -52,6 +52,7 @@ import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.CompilationInfo; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; +import com.google.devtools.build.lib.server.FailureDetails.FailAction.Code; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; @@ -268,8 +269,12 @@ ruleContext.getConfiguration(), LinkTargetType.INTERFACE_DYNAMIC_LIBRARY)); } - ruleContext.registerAction(new FailAction(ruleContext.getActionOwner(), - dynamicLibraries.build(), "Toolchain does not support dynamic linking")); + ruleContext.registerAction( + new FailAction( + ruleContext.getActionOwner(), + dynamicLibraries.build(), + "Toolchain does not support dynamic linking", + Code.DYNAMIC_LINKING_NOT_SUPPORTED)); } else if (!createDynamicLibrary && ruleContext.attributes().isConfigurable("srcs")) { // If "srcs" is configurable, the .so output is always declared because the logic that @@ -293,9 +298,13 @@ ruleContext.getConfiguration(), LinkTargetType.INTERFACE_DYNAMIC_LIBRARY)); } - ruleContext.registerAction(new FailAction(ruleContext.getActionOwner(), - dynamicLibraries.build(), "configurable \"srcs\" triggers an implicit .so output " - + "even though there are no sources to compile in this configuration")); + ruleContext.registerAction( + new FailAction( + ruleContext.getActionOwner(), + dynamicLibraries.build(), + "configurable \"srcs\" triggers an implicit .so output even though there are no" + + " sources to compile in this configuration", + Code.SOURCE_FILES_MISSING)); } CompilationInfo compilationInfo = compilationHelper.compile(ruleContext::ruleError);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index 0f29222..d76a5aa 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -66,6 +66,7 @@ import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; +import com.google.devtools.build.lib.server.FailureDetails.FailAction.Code; import com.google.devtools.build.lib.shell.ShellUtils; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.Fingerprint; @@ -490,7 +491,8 @@ ImmutableList.of(linuxDefault), String.format( "the given toolchain supports creation of %s instead of %s", - result.getExecPathString(), linuxDefault.getExecPathString()))); + result.getExecPathString(), linuxDefault.getExecPathString()), + Code.INCORRECT_TOOLCHAIN)); } return result;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/BUILD b/src/main/java/com/google/devtools/build/lib/rules/python/BUILD index 63b821d..186f585 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/python/BUILD
@@ -54,6 +54,7 @@ "//src/main/java/net/starlark/java/eval", "//src/main/java/net/starlark/java/syntax", "//src/main/protobuf:extra_actions_base_java_proto", + "//src/main/protobuf:failure_details_java_proto", "//third_party:error_prone_annotations", "//third_party:guava", "//third_party:jsr305",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java index c9da2de..18fb6ae 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java
@@ -49,6 +49,7 @@ import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.rules.cpp.CcInfo; import com.google.devtools.build.lib.rules.cpp.CppFileTypes; +import com.google.devtools.build.lib.server.FailureDetails.FailAction.Code; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.OS; @@ -596,7 +597,11 @@ return false; } else { ruleContext.registerAction( - new FailAction(ruleContext.getActionOwner(), ImmutableList.of(executable), error)); + new FailAction( + ruleContext.getActionOwner(), + ImmutableList.of(executable), + error, + Code.INCORRECT_PYTHON_VERSION)); return true; } }
diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index 639b574..82537cd 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto
@@ -1023,6 +1023,13 @@ enum Code { FAIL_ACTION_UNKNOWN = 0 [(metadata) = { exit_code: 37 }]; INTENTIONAL_FAILURE = 1 [(metadata) = { exit_code: 1 }]; + INCORRECT_PYTHON_VERSION = 2 [(metadata) = { exit_code: 1 }]; + PROGUARD_SPECS_MISSING = 3 [(metadata) = { exit_code: 1 }]; + DYNAMIC_LINKING_NOT_SUPPORTED = 4 [(metadata) = { exit_code: 1 }]; + SOURCE_FILES_MISSING = 5 [(metadata) = { exit_code: 1 }]; + INCORRECT_TOOLCHAIN = 6 [(metadata) = { exit_code: 1 }]; + FRAGMENT_CLASS_MISSING = 7 [(metadata) = { exit_code: 1 }]; + reserved 8, 9; // For internal use } Code code = 1;
diff --git a/src/test/java/com/google/devtools/build/lib/actions/BUILD b/src/test/java/com/google/devtools/build/lib/actions/BUILD index 0c7deb6..4a56e9f 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/test/java/com/google/devtools/build/lib/actions/BUILD
@@ -60,6 +60,7 @@ "//src/main/java/com/google/devtools/common/options", "//src/main/java/net/starlark/java/eval", "//src/main/protobuf:action_cache_java_proto", + "//src/main/protobuf:failure_details_java_proto", "//src/test/java/com/google/devtools/build/lib/actions/util", "//src/test/java/com/google/devtools/build/lib/events:testutil", "//src/test/java/com/google/devtools/build/lib/testutil",
diff --git a/src/test/java/com/google/devtools/build/lib/actions/FailActionTest.java b/src/test/java/com/google/devtools/build/lib/actions/FailActionTest.java index 601a412..afb6d49 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/FailActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/FailActionTest.java
@@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; +import com.google.devtools.build.lib.server.FailureDetails.FailAction.Code; import com.google.devtools.build.lib.testutil.Scratch; import java.util.Collection; import org.junit.Before; @@ -46,7 +47,7 @@ ActionsTestUtil.createArtifact( ArtifactRoot.asDerivedRoot(scratch.dir("/"), "out"), scratch.file("/out/foo")); outputs = ImmutableList.of(anOutput); - failAction = new FailAction(NULL_ACTION_OWNER, outputs, errorMessage); + failAction = new FailAction(NULL_ACTION_OWNER, outputs, errorMessage, Code.FAIL_ACTION_UNKNOWN); actionGraph.registerAction(failAction); assertThat(actionGraph.getGeneratingAction(anOutput)).isSameInstanceAs(failAction); } @@ -55,7 +56,7 @@ public void testExecutingItYieldsExceptionWithErrorMessage() { ActionExecutionException e = assertThrows(ActionExecutionException.class, () -> failAction.execute(null)); - assertThat(e).hasMessageThat().isEqualTo(errorMessage); + assertThat(e).hasMessageThat().contains(errorMessage); } @Test
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/BUILD b/src/test/java/com/google/devtools/build/lib/testutil/BUILD index 0395805..528acb0 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/BUILD +++ b/src/test/java/com/google/devtools/build/lib/testutil/BUILD
@@ -65,6 +65,7 @@ "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", "//src/main/java/net/starlark/java/syntax", + "//src/main/protobuf:failure_details_java_proto", "//third_party:guava", "//third_party:jsr305", "//third_party:junit4",
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/UnknownRuleConfiguredTarget.java b/src/test/java/com/google/devtools/build/lib/testutil/UnknownRuleConfiguredTarget.java index 33b0fae..ea0bed6 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/UnknownRuleConfiguredTarget.java +++ b/src/test/java/com/google/devtools/build/lib/testutil/UnknownRuleConfiguredTarget.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.server.FailureDetails.FailAction.Code; /** * A null implementation of ConfiguredTarget for rules we don't know how to build. @@ -55,7 +56,8 @@ new FailAction( context.getActionOwner(), filesToBuild.toList(), - "cannot build " + rule.getRuleClass() + " rules such as " + rule.getLabel())); + "cannot build " + rule.getRuleClass() + " rules such as " + rule.getLabel(), + Code.FAIL_ACTION_UNKNOWN)); return new RuleConfiguredTargetBuilder(context) .setFilesToBuild(filesToBuild) .add(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY))