Skip to content

Commit

Permalink
Add the missing build status for skymeld.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
joeleba authored and copybara-github committed Jul 8, 2022
1 parent d35f923 commit 4fca652
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<ConfiguredTarget> testTargets =
event.getTestTargets() != null
? ImmutableSet.copyOf(event.getTestTargets())
Expand All @@ -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.
*
* <p>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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -187,6 +187,10 @@ public SkyValue compute(SkyKey skyKey, Environment env)
}
}

env.getListener()
.post(
TopLevelTargetPendingExecutionEvent.create(
configuredTarget, buildDriverKey.isTest()));
requestConfiguredTargetExecution(
configuredTarget,
buildDriverKey,
Expand Down Expand Up @@ -296,7 +300,7 @@ private void requestConfiguredTargetExecution(
ImmutableSet.Builder<Artifact> 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,
Expand Down Expand Up @@ -337,7 +341,7 @@ private void requestAspectExecution(
TopLevelArtifactContext topLevelArtifactContext)
throws InterruptedException {

env.getListener().post(TopLevelTargetExecutionStartedEvent.create());
env.getListener().post(SomeExecutionStartedEvent.create());
ImmutableSet.Builder<Artifact> artifactsToBuild = ImmutableSet.builder();
List<SkyKey> aspectCompletionKeys = new ArrayList<>();
for (SkyValue aspectValue : topLevelAspectsValue.getTopLevelAspectsValues()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down

0 comments on commit 4fca652

Please sign in to comment.