BEP: Report configuration for all actions
Whenever we report an action in the build event protocol that has
a configuration associated with it, report the configuration as
well in the protocol (and not only the checksum, if the configuration
is not that of one of the top-level configured targets).
Change-Id: I9b085d5381b3c3509b4f3b99c8a77bc8fba6abfe
PiperOrigin-RevId: 161789745
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 e00555e..c8d9572 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
@@ -19,7 +19,9 @@
import com.google.devtools.build.lib.buildeventstream.BuildEventConverters;
import com.google.devtools.build.lib.buildeventstream.BuildEventId;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
+import com.google.devtools.build.lib.buildeventstream.BuildEventWithConfiguration;
import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent;
+import com.google.devtools.build.lib.buildeventstream.NullConfiguration;
import com.google.devtools.build.lib.buildeventstream.PathConverter;
import com.google.devtools.build.lib.causes.ActionFailed;
import com.google.devtools.build.lib.causes.Cause;
@@ -30,7 +32,7 @@
* This event is fired during the build, when an action is executed. It contains information about
* the action: the Action itself, and the output file names its stdout and stderr are recorded in.
*/
-public class ActionExecutedEvent implements BuildEvent {
+public class ActionExecutedEvent implements BuildEventWithConfiguration {
private final Action action;
private final ActionExecutionException exception;
private final Path stdout;
@@ -84,6 +86,19 @@
}
@Override
+ public Collection<BuildEvent> getConfigurations() {
+ if (action.getOwner() != null) {
+ BuildEvent configuration = action.getOwner().getConfiguration();
+ if (configuration == null) {
+ configuration = new NullConfiguration();
+ }
+ return ImmutableList.of(configuration);
+ } else {
+ return ImmutableList.<BuildEvent>of();
+ }
+ }
+
+ @Override
public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventConverters converters) {
PathConverter pathConverter = converters.pathConverter();
BuildEventStreamProtos.ActionExecuted.Builder actionBuilder =
@@ -108,13 +123,12 @@
if (action.getOwner() != null && action.getOwner().getLabel() != null) {
actionBuilder.setLabel(action.getOwner().getLabel().toString());
}
- // TODO(aehlig): ensure the configuration is shown in the stream, even if it is not
- // one of the configurations of a top-level configured target.
if (action.getOwner() != null) {
- actionBuilder.setConfiguration(
- BuildEventStreamProtos.BuildEventId.ConfigurationId.newBuilder()
- .setId(action.getOwner().getConfigurationChecksum())
- .build());
+ BuildEvent configuration = action.getOwner().getConfiguration();
+ if (configuration == null) {
+ configuration = new NullConfiguration();
+ }
+ actionBuilder.setConfiguration(configuration.getEventId().asStreamProto().getConfiguration());
}
if (exception == null) {
actionBuilder.setPrimaryOutput(
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java
index 72ca914..b8c0dc8 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java
@@ -15,6 +15,7 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.buildeventstream.BuildEvent;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.events.Location;
@@ -34,8 +35,15 @@
public abstract class ActionOwner {
/** An action owner for special cases. Usage is strongly discouraged. */
public static final ActionOwner SYSTEM_ACTION_OWNER =
- ActionOwner.create(null, ImmutableList.<AspectDescriptor>of(),
- null, "system", "empty target kind", "system", null);
+ ActionOwner.create(
+ null,
+ ImmutableList.<AspectDescriptor>of(),
+ null,
+ "system",
+ "empty target kind",
+ "system",
+ null,
+ null);
public static ActionOwner create(
@Nullable Label label,
@@ -44,6 +52,7 @@
@Nullable String mnemonic,
@Nullable String targetKind,
String configurationChecksum,
+ @Nullable BuildEvent configuration,
@Nullable String additionalProgressInfo) {
return new AutoValue_ActionOwner(
location,
@@ -51,6 +60,7 @@
aspectDescriptors,
mnemonic,
Preconditions.checkNotNull(configurationChecksum),
+ configuration,
targetKind,
additionalProgressInfo);
}
@@ -77,6 +87,13 @@
*/
public abstract String getConfigurationChecksum();
+ /**
+ * Return the configuration of the action owner, if any, as it should be reported in the build
+ * event protocol.
+ */
+ @Nullable
+ public abstract BuildEvent getConfiguration();
+
/** Returns the target kind (rule class name) for this ActionOwner, if any; null otherwise. */
@Nullable
public abstract String getTargetKind();
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 61daf33..986a93b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -423,6 +423,7 @@
configuration.getMnemonic(),
rule.getTargetKind(),
configuration.checksum(),
+ configuration,
configuration.isHostConfiguration() ? HOST_CONFIGURATION_PROGRESS_TAG : null);
}
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
index 6ff3e3b..af9d68f 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
@@ -211,6 +211,7 @@
"dummy-configuration-mnemonic",
null,
"dummy-configuration",
+ null,
null);
public static final ArtifactOwner NULL_ARTIFACT_OWNER =
diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java b/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java
index 20ec2b5..cf17e08 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/util/FakeOwner.java
@@ -52,6 +52,7 @@
mnemonic,
/*targetKind=*/ null,
"configurationChecksum",
+ /* configuration=*/ null,
"additionalProgressInfo");
}
@@ -150,4 +151,4 @@
public boolean shouldReportPathPrefixConflict(ActionAnalysisMetadata action) {
throw new UnsupportedOperationException();
}
-}
\ No newline at end of file
+}
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalStateTrackerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalStateTrackerTest.java
index 6212a7f..09b5dda 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalStateTrackerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalStateTrackerTest.java
@@ -431,8 +431,9 @@
"/home/user/bazel/out/abcdef/some/very/very/long/path/for/some/library/directory/foo.jar");
Label label =
Label.parseAbsolute("//some/very/very/long/path/for/some/library/directory:libfoo");
- ActionOwner owner = ActionOwner.create(
- label, ImmutableList.<AspectDescriptor>of(), null, null, null, "fedcba", null);
+ ActionOwner owner =
+ ActionOwner.create(
+ label, ImmutableList.<AspectDescriptor>of(), null, null, null, "fedcba", null, null);
when(action.getOwner()).thenReturn(owner);
clock.advanceMillis(TimeUnit.SECONDS.toMillis(3));
@@ -563,8 +564,15 @@
ConfiguredTarget targetFooTest = Mockito.mock(ConfiguredTarget.class);
when(targetFooTest.getLabel()).thenReturn(labelFooTest);
ActionOwner fooOwner =
- ActionOwner.create(labelFooTest,
- ImmutableList.<AspectDescriptor>of(), null, null, null, "abcdef", null);
+ ActionOwner.create(
+ labelFooTest,
+ ImmutableList.<AspectDescriptor>of(),
+ null,
+ null,
+ null,
+ "abcdef",
+ null,
+ null);
Label labelBarTest = Label.parseAbsolute("//baz:bartest");
ConfiguredTarget targetBarTest = Mockito.mock(ConfiguredTarget.class);
@@ -573,8 +581,15 @@
when(filteringComplete.getTestTargets())
.thenReturn(ImmutableSet.of(targetFooTest, targetBarTest));
ActionOwner barOwner =
- ActionOwner.create(labelBarTest,
- ImmutableList.<AspectDescriptor>of(), null, null, null, "fedcba", null);
+ ActionOwner.create(
+ labelBarTest,
+ ImmutableList.<AspectDescriptor>of(),
+ null,
+ null,
+ null,
+ "fedcba",
+ null,
+ null);
stateTracker.testFilteringComplete(filteringComplete);
diff --git a/src/test/shell/integration/build_event_stream_test.sh b/src/test/shell/integration/build_event_stream_test.sh
index 6ef198c..348cb3c 100755
--- a/src/test/shell/integration/build_event_stream_test.sh
+++ b/src/test/shell/integration/build_event_stream_test.sh
@@ -145,6 +145,20 @@
cmd = "cp \$< \$@",
)
EOF
+mkdir -p failingtool
+cat > failingtool/BUILD <<'EOF'
+genrule(
+ name = "tool",
+ outs = ["tool.sh"],
+ cmd = "false",
+)
+genrule(
+ name = "usestool",
+ outs = ["out.txt"],
+ tools = [":tool"],
+ cmd = "$(location :tool) > $@",
+)
+EOF
}
#### TESTS #############################################################
@@ -449,6 +463,17 @@
|| fail "failed action not before compelted target"
}
+function test_action_conf() {
+ # Verify that the expected configurations for actions are reported.
+ # The example contains a configuration transition (from building for
+ # target to building for host). As the action fails, we expect the
+ # configuration of the action to be reported as well.
+ (bazel build --build_event_text_file=$TEST_log \
+ -k failingtool/... && fail "build failure expected") || true
+ count=`grep '^configuration' "${TEST_log}" | wc -l`
+ [ "${count}" -eq 2 ] || fail "Expected 2 configurations, found $count."
+}
+
function test_loading_failure() {
# Verify that if loading fails, this is properly reported as the
# reason for the target expansion event not resulting in targets
