Get ConfiguredTargetAndData out of TargetCompleteEvent.

Having it in there makes serializing events harder.

PiperOrigin-RevId: 197648233

2 files changed, 98 insertions, 47 deletions

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 672c406634..7473de21b6 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
@@ -49,10 +49,13 @@ import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.TestTimeout;
 import com.google.devtools.build.lib.rules.AliasConfiguredTarget;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
+import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.util.Collection;
 import java.util.function.Function;
+import javax.annotation.Nullable;
 
 /** This event is fired as soon as a target is either built or fails. */
 public final class TargetCompleteEvent
@@ -60,36 +63,91 @@ public final class TargetCompleteEvent
         BuildEventWithOrderConstraint,
         EventReportingArtifacts,
         BuildEventWithConfiguration {
-  private final ConfiguredTargetAndData targetAndData;
+
+  /** Lightweight data needed about the configured target in this event. */
+  public static class ExecutableTargetData {
+    @Nullable private final Path runfilesDirectory;
+    @Nullable private final Artifact executable;
+
+    private ExecutableTargetData(ConfiguredTargetAndData targetAndData) {
+      FilesToRunProvider provider =
+          targetAndData.getConfiguredTarget().getProvider(FilesToRunProvider.class);
+      if (provider != null) {
+        this.executable = provider.getExecutable();
+        if (null != provider.getRunfilesSupport()) {
+          this.runfilesDirectory = provider.getRunfilesSupport().getRunfilesDirectory();
+        } else {
+          this.runfilesDirectory = null;
+        }
+      } else {
+        this.executable = null;
+        this.runfilesDirectory = null;
+      }
+    }
+
+    @Nullable
+    public Path getRunfilesDirectory() {
+      return runfilesDirectory;
+    }
+
+    @Nullable
+    public Artifact getExecutable() {
+      return executable;
+    }
+  }
+
+  private final Label label;
+  private final ConfiguredTargetKey configuredTargetKey;
   private final NestedSet<Cause> rootCauses;
   private final ImmutableList<BuildEventId> postedAfter;
   private final NestedSet<ArtifactsInOutputGroup> outputs;
   private final NestedSet<Artifact> baselineCoverageArtifacts;
+  private final Label aliasLabel;
   private final boolean isTest;
+  @Nullable private final Long testTimeoutSeconds;
+  @Nullable private final TestProvider.TestParams testParams;
+  private final BuildEvent configurationEvent;
+  private final BuildEventId configEventId;
+  private final Iterable<String> tags;
+  private final ExecutableTargetData executableTargetData;
 
   private TargetCompleteEvent(
       ConfiguredTargetAndData targetAndData,
       NestedSet<Cause> rootCauses,
       NestedSet<ArtifactsInOutputGroup> outputs,
       boolean isTest) {
-    this.targetAndData = targetAndData;
     this.rootCauses =
         (rootCauses == null) ? NestedSetBuilder.<Cause>emptySet(Order.STABLE_ORDER) : rootCauses;
-
+    this.executableTargetData = new ExecutableTargetData(targetAndData);
     ImmutableList.Builder<BuildEventId> postedAfterBuilder = ImmutableList.builder();
-    Label label = getTarget().getLabel();
+    this.label = targetAndData.getConfiguredTarget().getLabel();
     if (targetAndData.getConfiguredTarget() instanceof AliasConfiguredTarget) {
-      label = ((AliasConfiguredTarget) targetAndData.getConfiguredTarget()).getOriginalLabel();
+      this.aliasLabel =
+          ((AliasConfiguredTarget) targetAndData.getConfiguredTarget()).getOriginalLabel();
+    } else {
+      this.aliasLabel = label;
     }
-    postedAfterBuilder.add(BuildEventId.targetConfigured(label));
+    this.configuredTargetKey =
+        ConfiguredTargetKey.of(
+            targetAndData.getConfiguredTarget(), targetAndData.getConfiguration());
+    postedAfterBuilder.add(BuildEventId.targetConfigured(aliasLabel));
     for (Cause cause : getRootCauses()) {
       postedAfterBuilder.add(BuildEventId.fromCause(cause));
     }
     this.postedAfter = postedAfterBuilder.build();
     this.outputs = outputs;
     this.isTest = isTest;
+    this.testTimeoutSeconds = isTest ? getTestTimeoutSeconds(targetAndData) : null;
+    BuildConfiguration configuration = targetAndData.getConfiguration();
+    this.configEventId =
+        configuration != null ? configuration.getEventId() : BuildEventId.nullConfigurationId();
+    this.configurationEvent = configuration != null ? configuration.toBuildEvent() : null;
+    this.testParams =
+        isTest
+            ? targetAndData.getConfiguredTarget().getProvider(TestProvider.class).getTestParams()
+            : null;
     InstrumentedFilesProvider instrumentedFilesProvider =
-        this.targetAndData.getConfiguredTarget().getProvider(InstrumentedFilesProvider.class);
+        targetAndData.getConfiguredTarget().getProvider(InstrumentedFilesProvider.class);
     if (instrumentedFilesProvider == null) {
       this.baselineCoverageArtifacts = null;
     } else {
@@ -101,6 +159,17 @@ public final class TargetCompleteEvent
         this.baselineCoverageArtifacts = null;
       }
     }
+    // For tags, we are only interested in targets that are rules.
+    if (!(targetAndData.getConfiguredTarget() instanceof RuleConfiguredTarget)) {
+      this.tags = ImmutableList.of();
+    } else {
+      AttributeMap attributes =
+          ConfiguredAttributeMapper.of(
+              (Rule) targetAndData.getTarget(),
+              ((RuleConfiguredTarget) targetAndData.getConfiguredTarget()).getConfigConditions());
+      // Every rule (implicitly) has a "tags" attribute.
+      this.tags = attributes.get("tags", Type.STRING_LIST);
+    }
   }
 
   /** Construct a successful target completion event. */
@@ -125,9 +194,17 @@ public final class TargetCompleteEvent
         ct, rootCauses, NestedSetBuilder.emptySet(Order.STABLE_ORDER), false);
   }
 
-  /** Returns the target associated with the event. */
-  public ConfiguredTarget getTarget() {
-    return targetAndData.getConfiguredTarget();
+  /** Returns the label of the target associated with the event. */
+  public Label getLabel() {
+    return label;
+  }
+
+  public ConfiguredTargetKey getConfiguredTargetKey() {
+    return configuredTargetKey;
+  }
+
+  public ExecutableTargetData getExecutableTargetData() {
+    return executableTargetData;
   }
 
   /** Determines whether the target has failed or succeeded. */
@@ -155,14 +232,7 @@ public final class TargetCompleteEvent
 
   @Override
   public BuildEventId getEventId() {
-    Label label = getTarget().getLabel();
-    if (targetAndData.getConfiguredTarget() instanceof AliasConfiguredTarget) {
-      label = ((AliasConfiguredTarget) targetAndData.getConfiguredTarget()).getOriginalLabel();
-    }
-    BuildConfiguration config = targetAndData.getConfiguration();
-    BuildEventId configId =
-        config == null ? BuildEventId.nullConfigurationId() : config.getEventId();
-    return BuildEventId.targetCompleted(label, configId);
+    return BuildEventId.targetCompleted(aliasLabel, configEventId);
   }
 
   @Override
@@ -175,18 +245,13 @@ public final class TargetCompleteEvent
       // For tests, announce all the test actions that will minimally happen (except for
       // interruption). If after the result of a test action another attempt is necessary,
       // it will be announced with the action that made the new attempt necessary.
-      Label label = targetAndData.getConfiguredTarget().getLabel();
-      TestProvider.TestParams params =
-          targetAndData.getConfiguredTarget().getProvider(TestProvider.class).getTestParams();
-      for (int run = 0; run < Math.max(params.getRuns(), 1); run++) {
-        for (int shard = 0; shard < Math.max(params.getShards(), 1); shard++) {
-          childrenBuilder.add(
-              BuildEventId.testResult(
-                  label, run, shard, targetAndData.getConfiguration().getEventId()));
+      Label label = getLabel();
+      for (int run = 0; run < Math.max(testParams.getRuns(), 1); run++) {
+        for (int shard = 0; shard < Math.max(testParams.getShards(), 1); shard++) {
+          childrenBuilder.add(BuildEventId.testResult(label, run, shard, configEventId));
         }
       }
-      childrenBuilder.add(
-          BuildEventId.testSummary(label, targetAndData.getConfiguration().getEventId()));
+      childrenBuilder.add(BuildEventId.testSummary(label, configEventId));
     }
     return childrenBuilder.build();
   }
@@ -222,7 +287,7 @@ public final class TargetCompleteEvent
     builder.addAllOutputGroup(getOutputFilesByGroup(converters.artifactGroupNamer()));
 
     if (isTest) {
-      builder.setTestTimeoutSeconds(getTestTimeoutSeconds(targetAndData));
+      builder.setTestTimeoutSeconds(testTimeoutSeconds);
     }
 
     // TODO(aehlig): remove direct reporting of artifacts as soon as clients no longer
@@ -259,25 +324,11 @@ public final class TargetCompleteEvent
 
   @Override
   public Collection<BuildEvent> getConfigurations() {
-    BuildConfiguration configuration = targetAndData.getConfiguration();
-    if (configuration != null) {
-      return ImmutableList.of(targetAndData.getConfiguration().toBuildEvent());
-    } else {
-      return ImmutableList.<BuildEvent>of();
-    }
+    return configurationEvent != null ? ImmutableList.of(configurationEvent) : ImmutableList.of();
   }
 
   private Iterable<String> getTags() {
-    // We are only interested in targets that are rules.
-    if (!(targetAndData.getConfiguredTarget() instanceof RuleConfiguredTarget)) {
-      return ImmutableList.<String>of();
-    }
-    AttributeMap attributes =
-        ConfiguredAttributeMapper.of(
-            (Rule) targetAndData.getTarget(),
-            ((RuleConfiguredTarget) targetAndData.getConfiguredTarget()).getConfigConditions());
-    // Every rule (implicitly) has a "tags" attribute.
-    return attributes.get("tags", Type.STRING_LIST);
+    return tags;
   }
 
   private Iterable<OutputGroup> getOutputFilesByGroup(ArtifactGroupNamer namer) {
@@ -308,7 +359,7 @@ public final class TargetCompleteEvent
    * rule picks its timeout but ends up with the same effective value as all other rules in that
    * category and configuration.
    */
-  private Long getTestTimeoutSeconds(ConfiguredTargetAndData targetAndData) {
+  private static Long getTestTimeoutSeconds(ConfiguredTargetAndData targetAndData) {
     BuildConfiguration configuration = targetAndData.getConfiguration();
     Rule associatedRule = targetAndData.getTarget().getAssociatedRule();
     TestTimeout categoricalTimeout = TestTimeout.getTestTimeout(associatedRule);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
index 9e851aa509..e2a70d2afb 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
@@ -225,7 +225,7 @@ public class AggregatingTestListener {
   @AllowConcurrentEvents
   public void targetComplete(TargetCompleteEvent event) {
     if (event.failed()) {
-      targetFailure(asKey(event.getTarget()));
+      targetFailure(event.getConfiguredTargetKey());
     }
   }