From 5f312dd1678878fb7563eae0cd184f2270346352 Mon Sep 17 00:00:00 2001 From: ulfjack Date: Tue, 2 Oct 2018 04:58:53 -0700 Subject: [PATCH] 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 --- .../devtools/build/lib/actions/ActionExecutedEvent.java | 8 ++++++-- .../com/google/devtools/build/lib/buildeventstream/BUILD | 1 + .../devtools/build/lib/buildeventstream/BuildEventId.java | 6 +++--- .../build/lib/skyframe/SkyframeActionExecutor.java | 1 + .../BazelBuildEventServiceModuleTest.java | 1 + .../build/lib/runtime/BuildEventStreamerTest.java | 3 +++ 6 files changed, 15 insertions(+), 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 index 9c038131dcd53d..e528c5d6ab5b73 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 @@ -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 index 4bb7f329f8f3d0..a47c6790d186a3 100644 --- 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 index 97ff7a6844f19c..f5848ce284a9a9 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 @@ -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 index 7ed0f92ad4f303..c3d2febeb6791d 100644 --- 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/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java index 78fcaff13392a3..7ce832c88abd25 100644 --- a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java +++ b/src/test/java/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 index 53d4a74d9cf236..02b130df7ce88c 100644 --- 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(),