BEP: Unconditionally report ExtraActions

...irrespective of their success status. While a build typically
contains too many successful actions to report them all, extra actions
included in a build are worth reporting.

--
Change-Id: I6b20935895aa7b16836d6271f456176a7113317e
Reviewed-on: https://cr.bazel.build/9519
PiperOrigin-RevId: 151328633
MOS_MIGRATED_REVID=151328633

4 files changed, 37 insertions, 7 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java
index aed5bd710e..6a4037a31f 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java
@@ -68,9 +68,13 @@ public class ActionExecutedEvent implements BuildEvent {
 
   @Override
   public BuildEventId getEventId() {
-    Cause cause =
-        new ActionFailed(action.getPrimaryOutput().getPath(), action.getOwner().getLabel());
-    return BuildEventId.fromCause(cause);
+    if (getException() != null) {
+      Cause cause =
+          new ActionFailed(action.getPrimaryOutput().getPath(), action.getOwner().getLabel());
+      return BuildEventId.fromCause(cause);
+    } else {
+      return BuildEventId.actionCompleted(action.getPrimaryOutput().getPath());
+    }
   }
 
   @Override
@@ -82,7 +86,7 @@ public class ActionExecutedEvent implements BuildEvent {
   public BuildEventStreamProtos.BuildEvent asStreamProto(PathConverter pathConverter) {
     BuildEventStreamProtos.ActionExecuted.Builder actionBuilder =
         BuildEventStreamProtos.ActionExecuted.newBuilder().setSuccess(getException() == null);
-    if (exception.getExitCode() != null) {
+    if (exception != null && exception.getExitCode() != null) {
       actionBuilder.setExitCode(exception.getExitCode().getNumericExitCode());
     }
     if (stdout != null) {
@@ -99,9 +103,15 @@ public class ActionExecutedEvent implements BuildEvent {
           .setUri(pathConverter.apply(stderr))
           .build());
     }
-    if (action.getOwner() != null) {
+    if (action.getOwner() != null && action.getOwner().getLabel() != null) {
       actionBuilder.setLabel(action.getOwner().getLabel().toString());
     }
+    if (exception == null) {
+      actionBuilder.setPrimaryOutput(
+          BuildEventStreamProtos.File.newBuilder()
+              .setUri(pathConverter.apply(action.getPrimaryOutput().getPath()))
+              .build());
+    }
     return GenericBuildEvent.protoChaining(this).setAction(actionBuilder.build()).build();
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventId.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventId.java
index dd9a933854..7fa1bf1c60 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventId.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventId.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.buildeventstream;
 
 import com.google.devtools.build.lib.causes.Cause;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.vfs.Path;
 import com.google.protobuf.TextFormat;
 import java.io.Serializable;
 import java.util.List;
@@ -145,6 +146,16 @@ public final class BuildEventId implements Serializable {
     return new BuildEventId(cause.getIdProto());
   }
 
+  public static BuildEventId actionCompleted(Path path) {
+    return new BuildEventId(
+        BuildEventStreamProtos.BuildEventId.newBuilder()
+            .setActionCompleted(
+                BuildEventStreamProtos.BuildEventId.ActionCompletedId.newBuilder()
+                    .setPrimaryOutput(path.toString())
+                    .build())
+            .build());
+  }
+
   public static BuildEventId testResult(Label target, Integer run, Integer shard, Integer attempt) {
     BuildEventStreamProtos.BuildEventId.TestResultId resultId =
         BuildEventStreamProtos.BuildEventId.TestResultId.newBuilder()
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
index e53229ebd6..9e8651f8da 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
@@ -230,7 +230,7 @@ message File {
 
 // Payload of the event indicating the completion of an action. The main purpose
 // of posting those events is to provide details on the root cause for a target
-// failing; however, consumers of the build-event protocol should not assume
+// failing; however, consumers of the build-event protocol must not assume
 // that only failed actions are posted.
 message ActionExecuted {
   bool success = 1;
@@ -248,6 +248,9 @@ message ActionExecuted {
 
   // Optionally, the label of the owner of the action, for reference.
   string label = 5;
+
+  // Primary output; only provided for successful actions.
+  File primary_output = 6;
 }
 
 // Collection of all output files belonging to that output group.
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
index a79297d396..9901ceeb49 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
@@ -33,6 +33,7 @@ import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent;
 import com.google.devtools.build.lib.buildtool.buildevent.BuildInterruptedEvent;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.rules.extra.ExtraAction;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.HashSet;
@@ -181,9 +182,14 @@ public class BuildEventStreamer implements EventHandler {
     close();
   }
 
+  /**
+   * Return true, if the action is not worth being reported. This is the case, if the action
+   * executed successfully and is not an ExtraAction.
+   */
   private static boolean isActionWithoutError(BuildEvent event) {
     return event instanceof ActionExecutedEvent
-        && ((ActionExecutedEvent) event).getException() == null;
+        && ((ActionExecutedEvent) event).getException() == null
+        && (!(((ActionExecutedEvent) event).getAction() instanceof ExtraAction));
   }
 
   private boolean bufferUntilPrerequisitesReceived(BuildEvent event) {