BEP: move configuration-independent information to TargetConfigured event
Some information about a target is configuration independent and therefore can
already be provided at a target level (i.e., in the TargetConfigured event). Do
so, to have that information available earlier and, once the deprecation period
is over, avoid redundant information in the stream.
Change-Id: I8021ce3dd2a8168d409ea513190c4e3a349dbc2f
PiperOrigin-RevId: 164967059
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 87cb74a..559cad1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -507,8 +507,8 @@
for (TargetAndConfiguration pair : topLevelTargetsWithConfigs) {
byLabel.put(pair.getLabel(), pair.getConfiguration());
}
- for (Label label : byLabel.keySet()) {
- eventBus.post(new TargetConfiguredEvent(label, byLabel.get(label)));
+ for (Target target : targets) {
+ eventBus.post(new TargetConfiguredEvent(target, byLabel.get(target.getLabel())));
}
List<ConfiguredTargetKey> topLevelCtKeys = Lists.transform(topLevelTargetsWithConfigs,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java
index 48ac8fb..4f2ee3b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java
@@ -146,21 +146,6 @@
return childrenBuilder.build();
}
- private BuildEventStreamProtos.TestSize bepTestSize(TestSize size) {
- switch (size) {
- case SMALL:
- return BuildEventStreamProtos.TestSize.SMALL;
- case MEDIUM:
- return BuildEventStreamProtos.TestSize.MEDIUM;
- case LARGE:
- return BuildEventStreamProtos.TestSize.LARGE;
- case ENORMOUS:
- return BuildEventStreamProtos.TestSize.ENORMOUS;
- default:
- return BuildEventStreamProtos.TestSize.UNKNOWN;
- }
- }
-
@Override
public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventConverters converters) {
BuildEventStreamProtos.TargetComplete.Builder builder =
@@ -173,7 +158,8 @@
if (isTest) {
builder.setTestSize(
- bepTestSize(TestSize.getTestSize(target.getTarget().getAssociatedRule())));
+ TargetConfiguredEvent.bepTestSize(
+ TestSize.getTestSize(target.getTarget().getAssociatedRule())));
}
// TODO(aehlig): remove direct reporting of artifacts as soon as clients no longer
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TargetConfiguredEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/TargetConfiguredEvent.java
index 56ddc4d..8395337 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TargetConfiguredEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TargetConfiguredEvent.java
@@ -22,17 +22,19 @@
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.cmdline.Label;
+import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.packages.TargetUtils;
+import com.google.devtools.build.lib.packages.TestSize;
import java.util.Collection;
/** Event reporting about the configurations associated with a given target */
public class TargetConfiguredEvent implements BuildEventWithConfiguration {
- private final Label label;
+ private final Target target;
private final Collection<BuildConfiguration> configurations;
- TargetConfiguredEvent(Label label, Collection<BuildConfiguration> configurations) {
- this.label = label;
+ TargetConfiguredEvent(Target target, Collection<BuildConfiguration> configurations) {
this.configurations = configurations;
+ this.target = target;
}
@Override
@@ -50,7 +52,7 @@
@Override
public BuildEventId getEventId() {
- return BuildEventId.targetConfigured(label);
+ return BuildEventId.targetConfigured(target.getLabel());
}
@Override
@@ -58,19 +60,37 @@
ImmutableList.Builder childrenBuilder = ImmutableList.builder();
for (BuildConfiguration config : configurations) {
if (config != null) {
- childrenBuilder.add(BuildEventId.targetCompleted(label, config.getEventId()));
+ childrenBuilder.add(BuildEventId.targetCompleted(target.getLabel(), config.getEventId()));
} else {
childrenBuilder.add(
- BuildEventId.targetCompleted(label, BuildEventId.nullConfigurationId()));
+ BuildEventId.targetCompleted(target.getLabel(), BuildEventId.nullConfigurationId()));
}
}
return childrenBuilder.build();
}
+ static BuildEventStreamProtos.TestSize bepTestSize(TestSize size) {
+ switch (size) {
+ case SMALL:
+ return BuildEventStreamProtos.TestSize.SMALL;
+ case MEDIUM:
+ return BuildEventStreamProtos.TestSize.MEDIUM;
+ case LARGE:
+ return BuildEventStreamProtos.TestSize.LARGE;
+ case ENORMOUS:
+ return BuildEventStreamProtos.TestSize.ENORMOUS;
+ default:
+ return BuildEventStreamProtos.TestSize.UNKNOWN;
+ }
+ }
+
@Override
public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventConverters converters) {
- return GenericBuildEvent.protoChaining(this)
- .setConfigured(BuildEventStreamProtos.TargetConfigured.getDefaultInstance())
- .build();
+ BuildEventStreamProtos.TargetConfigured.Builder builder =
+ BuildEventStreamProtos.TargetConfigured.newBuilder().setTargetKind(target.getTargetKind());
+ if (TargetUtils.isTestRule(target)) {
+ builder.setTestSize(bepTestSize(TestSize.getTestSize(target.getAssociatedRule())));
+ }
+ return GenericBuildEvent.protoChaining(this).setConfigured(builder.build()).build();
}
}
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 ea50943..c803498 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
@@ -303,11 +303,27 @@
message PatternExpanded {
}
+// Enumeration type characterizing the size of a test, as specified by the
+// test rule.
+enum TestSize {
+ UNKNOWN = 0;
+ SMALL = 1;
+ MEDIUM = 2;
+ LARGE = 3;
+ ENORMOUS = 4;
+}
+
// Payload of the event indicating that the configurations for a target have
// been identified. As with pattern expansion the main information is in the
// chaining part: the id will contain the target that was configured and the
// children id will ctontain the configured targets it was configured to.
message TargetConfigured {
+ // The kind of target (e.g., e.g. "cc_library rule", "source file",
+ // "generated file") where the completion is reported.
+ string target_kind = 1;
+
+ // The size of the test, if the target is a test target. Unset otherwise.
+ TestSize test_size = 2;
}
message File {
@@ -373,16 +389,6 @@
repeated BuildEventId.NamedSetOfFilesId file_sets = 3;
}
-// Enumeration type characterizing the size of a test, as specified by the
-// test rule.
-enum TestSize {
- UNKNOWN = 0;
- SMALL = 1;
- MEDIUM = 2;
- LARGE = 3;
- ENORMOUS = 4;
-}
-
// Payload of the event indicating the completion of a target. The target is
// specified in the id. If the target failed the root causes are provided as
// children events.
@@ -391,10 +397,12 @@
// The kind of target (e.g., e.g. "cc_library rule", "source file",
// "generated file") where the completion is reported.
- string target_kind = 5;
+ // Deprecated: use the target_kind field in TargetConfigured instead.
+ string target_kind = 5 [deprecated = true];
// The size of the test, if the target is a test target. Unset otherwise.
- TestSize test_size = 6;
+ // Deprecated: use the test_size field in TargetConfigured instead.
+ TestSize test_size = 6 [deprecated = true];
// The output files are arranged by their output group. If an output file
// is part of multiple output groups, it appears once in each output
diff --git a/src/test/shell/integration/build_event_stream_test.sh b/src/test/shell/integration/build_event_stream_test.sh
index a87349c..1bbdbba 100755
--- a/src/test/shell/integration/build_event_stream_test.sh
+++ b/src/test/shell/integration/build_event_stream_test.sh
@@ -198,6 +198,27 @@
expect_log 'key: "TARGET_CPU"'
}
+function test_target_information_early() {
+ # Verify that certain information is present in the log as part of
+ # the TargetConfigured event (verifying that it comes at least before
+ # the first TargetCompleted event, which is fine, if we only ask for
+ # a single target).
+ bazel test --build_event_text_file=$TEST_log pkg:true \
+ || fail "bazel test failed"
+ expect_log '^completed'
+ ed $TEST_log <<'EOF'
+1
+/^completed/+1,$d
+a
+...[cut here]
+.
+w
+q
+EOF
+ expect_log 'target_kind:.*sh'
+ expect_log 'test_size: SMALL'
+}
+
function test_workspace_status() {
bazel test --build_event_text_file=$TEST_log \
--workspace_status_command=sample_workspace_status pkg:true \