commit c924b4d42a4e74975bf9f96a4ea978682f147915
author janakr <janakr@google.com> Fri Sep 25 17:00:43 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Fri Sep 25 17:01:56 2020 -0700
tree 0102178628b82938e31853ab3a8ef15f861e1915
parent 859c9acc895d7e1add7e52b9b2a4c43718b4c352

Enrich FailAction's failure message a bit: always include the owning label, and in the last remaining "generic" case, be more specific in the error message.

Also do some minor clean-ups around FailAction: AutoCodec not needed, and include error message in cache key.

PiperOrigin-RevId: 333835998

src/main/java/com/google/devtools/build/lib/actions/FailAction.java

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 ab7628d..81c2084 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
@@ -21,7 +21,6 @@
 import com.google.devtools.build.lib.server.FailureDetails;
 import com.google.devtools.build.lib.server.FailureDetails.FailAction.Code;
 import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.build.lib.util.Fingerprint;
 import javax.annotation.Nullable;
@@ -30,7 +29,6 @@
  * FailAction is an Action that always fails to execute. (Used as scaffolding for rules we haven't
  * yet implemented. Also useful for testing.)
  */
-@AutoCodec
 @Immutable
 public final class FailAction extends AbstractAction {
 
@@ -61,7 +59,11 @@
         false,
         DetailedExitCode.of(
             FailureDetail.newBuilder()
-                .setMessage("FailAction intentional failure: " + errorMessage)
+                .setMessage(
+                    "FailAction intentional failure: "
+                        + errorMessage
+                        + " caused by "
+                        + getOwner().getLabel())
                 .setFailAction(
                     FailureDetails.FailAction.newBuilder().setCode(Code.INTENTIONAL_FAILURE))
                 .build()));
@@ -73,6 +75,8 @@
       @Nullable ArtifactExpander artifactExpander,
       Fingerprint fp) {
     fp.addString(GUID);
+    // Should never be cached, but just be safe.
+    fp.addString(getErrorMessage());
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java

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 3d66fda..f61fd6a 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
@@ -330,7 +330,7 @@
     }
 
     try {
-      boolean creatingFailActions = false;
+      Class<?> missingFragmentClass = null;
       for (Class<?> fragmentClass :
           configurationFragmentPolicy.getRequiredConfigurationFragments()) {
         if (!configuration.hasFragment(fragmentClass.asSubclass(Fragment.class))) {
@@ -344,12 +344,12 @@
               return null;
             }
             // Otherwise missingFragmentPolicy == MissingFragmentPolicy.CREATE_FAIL_ACTIONS:
-            creatingFailActions = true;
+            missingFragmentClass = fragmentClass;
           }
         }
       }
-      if (creatingFailActions) {
-        return createFailConfiguredTarget(ruleContext);
+      if (missingFragmentClass != null) {
+        return createFailConfiguredTargetForMissingFragmentClass(ruleContext, missingFragmentClass);
       }
       if (rule.getRuleClassObject().isStarlark()) {
         // TODO(bazel-team): maybe merge with RuleConfiguredTargetBuilder?
@@ -614,16 +614,24 @@
 
   /**
    * A pseudo-implementation for configured targets that creates fail actions for all declared
-   * outputs, both implicit and explicit.
+   * outputs, both implicit and explicit, due to a missing fragment class.
    */
-  private static ConfiguredTarget createFailConfiguredTarget(RuleContext ruleContext)
-      throws RuleErrorException, ActionConflictException {
+  private static ConfiguredTarget createFailConfiguredTargetForMissingFragmentClass(
+      RuleContext ruleContext, Class<?> missingFragmentClass) {
     RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext);
     if (!ruleContext.getOutputArtifacts().isEmpty()) {
-      ruleContext.registerAction(new FailAction(ruleContext.getActionOwner(),
-          ruleContext.getOutputArtifacts(), "Can't build this"));
+      ruleContext.registerAction(
+          new FailAction(
+              ruleContext.getActionOwner(),
+              ruleContext.getOutputArtifacts(),
+              "Missing fragment class: " + missingFragmentClass.getName()));
     }
     builder.add(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY));
-    return builder.build();
+    try {
+      return builder.build();
+    } catch (ActionConflictException e) {
+      throw new IllegalStateException(
+          "Can't have an action conflict with one action: " + ruleContext.getLabel(), e);
+    }
   }
 }