From 4fca652fdf3c04aeb34ca0572e67ceb2da2e1fda Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 8 Jul 2022 01:00:00 -0700 Subject: [PATCH] Add the missing build status for skymeld. Skymeld, prior to this CL, has been missing the aggregators setup for targets. Therefore the targets' statuses in the BEP were always reported as "aborted". This CL adds the aggregators when a target (including tests) is ready to be executed. PiperOrigin-RevId: 459696157 Change-Id: I109cbdef7051cbcc73cc0b11e5364a7612c140a3 --- .../lib/runtime/TargetSummaryPublisher.java | 52 +++++++++++++++---- .../lib/skyframe/BuildDriverFunction.java | 12 +++-- .../build/lib/skyframe/BuildDriverKey.java | 2 +- .../lib/skyframe/TopLevelStatusEvents.java | 25 +++++++-- 4 files changed, 72 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TargetSummaryPublisher.java b/src/main/java/com/google/devtools/build/lib/runtime/TargetSummaryPublisher.java index 3f72a311ebab05..6ec903572ed6fd 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/TargetSummaryPublisher.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/TargetSummaryPublisher.java @@ -34,7 +34,9 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety; import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; +import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetPendingExecutionEvent; import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.errorprone.annotations.concurrent.GuardedBy; import java.util.Collection; import java.util.concurrent.ConcurrentHashMap; @@ -79,8 +81,6 @@ public void buildStarting(BuildStartingEvent event) { */ @Subscribe public void populateTargets(TestFilteringCompleteEvent event) { - int expectedCompletions = aspectCount.get() + 1; // + 1 for target itself - checkState(expectedCompletions > 0, "Haven't received BuildStartingEvent"); ImmutableSet testTargets = event.getTestTargets() != null ? ImmutableSet.copyOf(event.getTestTargets()) @@ -92,16 +92,48 @@ public void populateTargets(TestFilteringCompleteEvent event) { // we'll still get (and ignore) a TestSummary event, but that event isn't published to BEP. continue; } - // We want target summaries for alias targets, but note they don't receive test summaries. - TargetSummaryAggregator aggregator = - new TargetSummaryAggregator( - target, - expectedCompletions, - !AliasProvider.isAlias(target) && testTargets.contains(target)); - TargetSummaryAggregator oldAggregator = aggregators.put(asKey(target), aggregator); + TargetSummaryAggregator oldAggregator = + replaceAggregatorForTarget(/*isTest=*/ testTargets.contains(target), target); checkState( - oldAggregator == null, "target: %s, values: %s %s", target, oldAggregator, aggregator); + oldAggregator == null, + "target: %s, values: %s %s", + target, + oldAggregator, + aggregators.get(asKey(target))); + } + } + + /** + * Populates the aggregator for a particular top level target, including test targets. + * + *

Since the event is fired from within a SkyFunction, it is possible to receive duplicate + * events. In case of duplication, simply return without creating any new aggregator. + */ + @Subscribe + @AllowConcurrentEvents + public void populateTarget(TopLevelTargetPendingExecutionEvent event) { + replaceAggregatorForTarget(event.isTest(), event.configuredTarget()); + } + + /** + * Creates a TargetSummaryAggregator for the given target and stores it in {@link aggregators} + * + * @return the existing aggregator, if any. + */ + @Nullable + @CanIgnoreReturnValue + private TargetSummaryAggregator replaceAggregatorForTarget( + boolean isTest, ConfiguredTarget target) { + if (aggregators.containsKey(asKey(target))) { + return null; } + int expectedCompletions = aspectCount.get() + 1; // + 1 for target itself + checkState(expectedCompletions > 0, "Haven't received BuildStartingEvent"); + // We want target summaries for alias targets, but note they don't receive test summaries. + TargetSummaryAggregator aggregator = + new TargetSummaryAggregator( + target, expectedCompletions, isTest && !AliasProvider.isAlias(target)); + return aggregators.put(asKey(target), aggregator); } @Subscribe diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java index 4d212e6f84861b..9cc32a4a664311 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverFunction.java @@ -54,15 +54,15 @@ import com.google.devtools.build.lib.skyframe.AspectCompletionValue.AspectCompletionKey; import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey; import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.AspectAnalyzedEvent; +import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.SomeExecutionStartedEvent; import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TestAnalyzedEvent; import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetAnalyzedEvent; -import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetExecutionStartedEvent; +import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetPendingExecutionEvent; import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetSkippedEvent; import com.google.devtools.build.lib.util.RegexFilter; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunction.Environment.SkyKeyComputeState; import com.google.devtools.build.skyframe.SkyFunctionException; -import com.google.devtools.build.skyframe.SkyFunctionException.Transience; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.SkyframeIterableResult; @@ -187,6 +187,10 @@ public SkyValue compute(SkyKey skyKey, Environment env) } } + env.getListener() + .post( + TopLevelTargetPendingExecutionEvent.create( + configuredTarget, buildDriverKey.isTest())); requestConfiguredTargetExecution( configuredTarget, buildDriverKey, @@ -296,7 +300,7 @@ private void requestConfiguredTargetExecution( ImmutableSet.Builder artifactsToBuild = ImmutableSet.builder(); addExtraActionsIfRequested( configuredTarget.getProvider(ExtraActionArtifactsProvider.class), artifactsToBuild); - env.getListener().post(TopLevelTargetExecutionStartedEvent.create()); + env.getListener().post(SomeExecutionStartedEvent.create()); if (NOT_TEST.equals(buildDriverKey.getTestType())) { declareDependenciesAndCheckValues( env, @@ -337,7 +341,7 @@ private void requestAspectExecution( TopLevelArtifactContext topLevelArtifactContext) throws InterruptedException { - env.getListener().post(TopLevelTargetExecutionStartedEvent.create()); + env.getListener().post(SomeExecutionStartedEvent.create()); ImmutableSet.Builder artifactsToBuild = ImmutableSet.builder(); List aspectCompletionKeys = new ArrayList<>(); for (SkyValue aspectValue : topLevelAspectsValue.getTopLevelAspectsValues()) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverKey.java index ef40f22ad9bb57..ff15bb97233ca5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverKey.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverKey.java @@ -84,7 +84,7 @@ public ActionLookupKey getActionLookupKey() { } public boolean isTest() { - return TestType.NOT_TEST.equals(testType); + return !TestType.NOT_TEST.equals(testType); } public TestType getTestType() { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TopLevelStatusEvents.java b/src/main/java/com/google/devtools/build/lib/skyframe/TopLevelStatusEvents.java index c3f01f9dcf8caa..f0ddd1f1cf073f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TopLevelStatusEvents.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TopLevelStatusEvents.java @@ -53,12 +53,29 @@ public static TopLevelTargetSkippedEvent create(ConfiguredTarget configuredTarge } } - /** An event that marks the start of execution of a top-level target, including tests. */ + /** + * An event that marks that a top-level target won't be skipped and is pending execution, + * including test targets. + */ @AutoValue - public abstract static class TopLevelTargetExecutionStartedEvent implements Postable { + public abstract static class TopLevelTargetPendingExecutionEvent implements Postable { + public abstract ConfiguredTarget configuredTarget(); + + public abstract boolean isTest(); + + public static TopLevelTargetPendingExecutionEvent create( + ConfiguredTarget configuredTarget, boolean isTest) { + return new AutoValue_TopLevelStatusEvents_TopLevelTargetPendingExecutionEvent( + configuredTarget, isTest); + } + } + + /** An event that denotes that some execution has started in this build. */ + @AutoValue + public abstract static class SomeExecutionStartedEvent implements Postable { - public static TopLevelTargetExecutionStartedEvent create() { - return new AutoValue_TopLevelStatusEvents_TopLevelTargetExecutionStartedEvent(); + public static SomeExecutionStartedEvent create() { + return new AutoValue_TopLevelStatusEvents_SomeExecutionStartedEvent(); } } /** An event that marks the successful build of a top-level target, including tests. */