Fix event id for action_completed BEP events

The id should be the exec path not the absolute path. Otherwise the stdout and stderr don't line up correctly. PiperOrigin-RevId: 215369211
talya · Oct 2, 2018 · 5f312dd · 5f312dd
1 parent f59fad7
commit 5f312dd
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 5 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
@@ -28,6 +28,7 @@
 import com.google.devtools.build.lib.buildeventstream.PathConverter;
 import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -39,6 +40,7 @@
 public class ActionExecutedEvent implements BuildEventWithConfiguration, ProgressLike {
   private static final Logger logger = Logger.getLogger(ActionExecutedEvent.class.getName());
 
+  private final PathFragment actionId;
   private final Action action;
   private final ActionExecutionException exception;
   private final Path primaryOutput;
@@ -47,12 +49,14 @@ public class ActionExecutedEvent implements BuildEventWithConfiguration, Progres
   private final ErrorTiming timing;
 
   public ActionExecutedEvent(
+      PathFragment actionId,
       Action action,
       ActionExecutionException exception,
       Path primaryOutput,
       Path stdout,
       Path stderr,
       ErrorTiming timing) {
+    this.actionId = actionId;
     this.action = action;
     this.exception = exception;
     this.primaryOutput = primaryOutput;
@@ -93,10 +97,10 @@ public String getStderr() {
   @Override
   public BuildEventId getEventId() {
     if (action.getOwner() == null) {
-      return BuildEventId.actionCompleted(primaryOutput);
+      return BuildEventId.actionCompleted(actionId);
     } else {
       return BuildEventId.actionCompleted(
-          primaryOutput,
+          actionId,
           action.getOwner().getLabel(),
           action.getOwner().getConfigurationChecksum());
     }

diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BUILD b/src/main/java/com/google/devtools/build/lib/buildeventstream/BUILD
@@ -19,6 +19,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/build/lib/vfs",
+        "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/com/google/devtools/common/options",
         "//third_party:guava",
         "//third_party:jsr305",

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
@@ -19,7 +19,7 @@
 import com.google.devtools.build.lib.causes.Cause;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
-import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.protobuf.TextFormat;
 import java.io.Serializable;
 import java.util.List;
@@ -233,12 +233,12 @@ public static BuildEventId fromCause(Cause cause) {
     return new BuildEventId(cause.getIdProto());
   }
 
-  public static BuildEventId actionCompleted(Path path) {
+  public static BuildEventId actionCompleted(PathFragment path) {
     return actionCompleted(path, null, null);
   }
 
   public static BuildEventId actionCompleted(
-      Path path, @Nullable Label label, @Nullable String configurationChecksum) {
+      PathFragment path, @Nullable Label label, @Nullable String configurationChecksum) {
     ActionCompletedId.Builder actionId =
         ActionCompletedId.newBuilder().setPrimaryOutput(path.toString());
     if (label != null) {

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -1260,6 +1260,7 @@ private void reportActionExecution(
     }
     eventHandler.post(
         new ActionExecutedEvent(
+            action.getPrimaryOutput().getExecPath(),
             action,
             exception,
             actionExecutionContext.getInputPath(action.getPrimaryOutput()),

diff --git a/...ava/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java b/...ava/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java
@@ -60,6 +60,7 @@ public class BazelBuildEventServiceModuleTest {
 
   private static final ActionExecutedEvent SUCCESSFUL_ACTION_EXECUTED_EVENT =
       new ActionExecutedEvent(
+          ActionsTestUtil.DUMMY_ARTIFACT.getExecPath(),
           new ActionsTestUtil.NullAction(),
           /* exception= */ null,
           ActionsTestUtil.DUMMY_ARTIFACT.getPath(),

diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java
@@ -86,6 +86,7 @@ public class BuildEventStreamerTest extends FoundationTestCase {
 
   private static final ActionExecutedEvent SUCCESSFUL_ACTION_EXECUTED_EVENT =
       new ActionExecutedEvent(
+          ActionsTestUtil.DUMMY_ARTIFACT.getExecPath(),
           new ActionsTestUtil.NullAction(),
           /* exception= */ null,
           ActionsTestUtil.DUMMY_ARTIFACT.getPath(),
@@ -912,6 +913,7 @@ public void testSuccessfulActionsAreNotPublishedByDefault() {
 
     ActionExecutedEvent failedActionExecutedEvent =
         new ActionExecutedEvent(
+            ActionsTestUtil.DUMMY_ARTIFACT.getExecPath(),
             new ActionsTestUtil.NullAction(),
             new ActionExecutionException("Exception", /* action= */ null, /* catastrophe= */ false),
             ActionsTestUtil.DUMMY_ARTIFACT.getPath(),
@@ -942,6 +944,7 @@ public void testSuccessfulActionsCanBePublished() {
 
     ActionExecutedEvent failedActionExecutedEvent =
         new ActionExecutedEvent(
+            ActionsTestUtil.DUMMY_ARTIFACT.getExecPath(),
             new ActionsTestUtil.NullAction(),
             new ActionExecutionException("Exception", /* action= */ null, /* catastrophe= */ false),
             ActionsTestUtil.DUMMY_ARTIFACT.getPath(),