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 \