Move TargetCompleteEvent generation to the CompletionFunction
This change breaks without https://github.com/bazelbuild/bazel/commit/a0ea569f7df19b8284846a52854e73747f7ec005; if that change is rolled back, this
change has to be rolled back as well.
It was previously in ExecutionProgressReceiver, and directly hooked into
Skyframe. Now that we have a way to post event bus events directly from
Sky functions, we should just do that.
Also, this allows us to access artifact metdata, since the completion function
has all the necessary artifacts as dependencies, which in turn can be used to
improve the build event protocol implementation, where we want to post URLs to
the CAS, which requires knowing the checksum.
PiperOrigin-RevId: 179903333
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 4a6e8aa..c1e7805 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
@@ -57,7 +57,7 @@
BuildEventWithConfiguration {
private final ConfiguredTarget target;
private final NestedSet<Cause> rootCauses;
- private final Collection<BuildEventId> postedAfter;
+ private final ImmutableList<BuildEventId> postedAfter;
private final Iterable<ArtifactsInOutputGroup> outputs;
private final NestedSet<Artifact> baselineCoverageArtifacts;
private final boolean isTest;
@@ -71,7 +71,7 @@
this.rootCauses =
(rootCauses == null) ? NestedSetBuilder.<Cause>emptySet(Order.STABLE_ORDER) : rootCauses;
- ImmutableList.Builder postedAfterBuilder = ImmutableList.builder();
+ ImmutableList.Builder<BuildEventId> postedAfterBuilder = ImmutableList.builder();
for (Cause cause : getRootCauses()) {
postedAfterBuilder.add(BuildEventId.fromCause(cause));
}
@@ -94,13 +94,13 @@
}
/** Construct a successful target completion event. */
- public static TargetCompleteEvent createSuccessfulTarget(
+ public static TargetCompleteEvent successfulBuild(
ConfiguredTarget ct, NestedSet<ArtifactsInOutputGroup> outputs) {
return new TargetCompleteEvent(ct, null, outputs, false);
}
/** Construct a successful target completion event for a target that will be tested. */
- public static TargetCompleteEvent createSuccessfulTestTarget(ConfiguredTarget ct) {
+ public static TargetCompleteEvent successfulBuildSchedulingTest(ConfiguredTarget ct) {
return new TargetCompleteEvent(ct, null, ImmutableList.<ArtifactsInOutputGroup>of(), true);
}
@@ -147,7 +147,7 @@
@Override
public Collection<BuildEventId> getChildrenEvents() {
- ImmutableList.Builder childrenBuilder = ImmutableList.builder();
+ ImmutableList.Builder<BuildEventId> childrenBuilder = ImmutableList.builder();
for (Cause cause : getRootCauses()) {
childrenBuilder.add(BuildEventId.fromCause(cause));
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
index 4283ff5..ef43195 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
@@ -15,21 +15,13 @@
import com.google.common.base.Supplier;
import com.google.common.collect.Sets;
-import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter;
import com.google.devtools.build.lib.actions.ActionLookupData;
-import com.google.devtools.build.lib.analysis.AspectCompleteEvent;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.TargetCompleteEvent;
-import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
-import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper;
-import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsToBuild;
import com.google.devtools.build.lib.clock.BlazeClock;
import com.google.devtools.build.lib.skyframe.ActionExecutionInactivityWatchdog;
-import com.google.devtools.build.lib.skyframe.AspectCompletionValue;
-import com.google.devtools.build.lib.skyframe.AspectValue;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor;
import com.google.devtools.build.lib.skyframe.TargetCompletionValue;
@@ -62,9 +54,6 @@
private final Object activityIndicator = new Object();
/** Number of exclusive tests. To be accounted for in progress messages. */
private final int exclusiveTestsCount;
- private final Set<ConfiguredTarget> testedTargets;
- private final EventBus eventBus;
- private final TopLevelArtifactContext topLevelArtifactContext;
static {
PROGRESS_MESSAGE_NUMBER_FORMATTER = NumberFormat.getIntegerInstance(Locale.ENGLISH);
@@ -77,15 +66,9 @@
*/
ExecutionProgressReceiver(
Set<ConfiguredTarget> builtTargets,
- int exclusiveTestsCount,
- Set<ConfiguredTarget> testedTargets,
- TopLevelArtifactContext topLevelArtifactContext,
- EventBus eventBus) {
+ int exclusiveTestsCount) {
this.builtTargets = Collections.synchronizedSet(builtTargets);
this.exclusiveTestsCount = exclusiveTestsCount;
- this.testedTargets = testedTargets;
- this.topLevelArtifactContext = topLevelArtifactContext;
- this.eventBus = eventBus;
}
@Override
@@ -123,20 +106,6 @@
ConfiguredTarget target = value.getConfiguredTarget();
builtTargets.add(target);
-
- if (testedTargets.contains(target)) {
- postTestTargetComplete(target);
- } else {
- postBuildTargetComplete(target);
- }
- } else if (type.equals(SkyFunctions.ASPECT_COMPLETION)) {
- AspectCompletionValue value = (AspectCompletionValue) skyValueSupplier.get();
- if (value != null) {
- AspectValue aspectValue = value.getAspectValue();
- ArtifactsToBuild artifacts =
- TopLevelArtifactHelper.getAllArtifactsToBuild(aspectValue, topLevelArtifactContext);
- eventBus.post(AspectCompleteEvent.createSuccessful(aspectValue, artifacts));
- }
} else if (type.equals(SkyFunctions.ACTION_EXECUTION)) {
// Remember all completed actions, even those in error, regardless of having been cached or
// really executed.
@@ -235,16 +204,4 @@
}
};
}
-
- private void postTestTargetComplete(ConfiguredTarget target) {
- eventBus.post(TargetCompleteEvent.createSuccessfulTestTarget(target));
- }
-
- private void postBuildTargetComplete(ConfiguredTarget target) {
- ArtifactsToBuild artifactsToBuild =
- TopLevelArtifactHelper.getAllArtifactsToBuild(target, topLevelArtifactContext);
- eventBus.post(
- TargetCompleteEvent.createSuccessfulTarget(
- target, artifactsToBuild.getAllArtifactsByOutputGroup()));
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
index 99e8841..e72eda9 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
@@ -114,13 +114,7 @@
ExecutionProgressReceiver executionProgressReceiver =
new ExecutionProgressReceiver(
Preconditions.checkNotNull(builtTargets),
- countTestActions(exclusiveTests),
- ImmutableSet.<ConfiguredTarget>builder()
- .addAll(parallelTests)
- .addAll(exclusiveTests)
- .build(),
- topLevelArtifactContext,
- skyframeExecutor.getEventBus());
+ countTestActions(exclusiveTests));
skyframeExecutor
.getEventBus()
.post(new ExecutionProgressReceiverAvailableEvent(executionProgressReceiver));
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
index 103fd73..f4fb386 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.MissingInputFileException;
@@ -29,6 +28,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.skyframe.AspectCompletionValue.AspectCompletionKey;
import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
import com.google.devtools.build.lib.skyframe.TargetCompletionValue.TargetCompletionKey;
@@ -38,7 +38,6 @@
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.ValueOrException2;
import java.util.Map;
-import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nullable;
/**
@@ -86,7 +85,11 @@
TResult createResult(TValue value);
/** Creates a failed completion value. */
- SkyValue createFailed(TValue value, NestedSet<Cause> rootCauses);
+ ExtendedEventHandler.Postable createFailed(TValue value, NestedSet<Cause> rootCauses);
+
+ /** Creates a succeeded completion value. */
+ ExtendedEventHandler.Postable createSucceeded(
+ SkyKey skyKey, TValue value, TopLevelArtifactContext topLevelArtifactContext);
/**
* Extracts a tag given the {@link SkyKey}.
@@ -143,7 +146,8 @@
}
@Override
- public SkyValue createFailed(ConfiguredTargetValue value, NestedSet<Cause> rootCauses) {
+ public ExtendedEventHandler.Postable createFailed(
+ ConfiguredTargetValue value, NestedSet<Cause> rootCauses) {
return TargetCompleteEvent.createFailed(value.getConfiguredTarget(), rootCauses);
}
@@ -152,6 +156,22 @@
return Label.print(
((TargetCompletionKey) skyKey.argument()).configuredTargetKey().getLabel());
}
+
+ @Override
+ public ExtendedEventHandler.Postable createSucceeded(
+ SkyKey skyKey,
+ ConfiguredTargetValue value,
+ TopLevelArtifactContext topLevelArtifactContext) {
+ ConfiguredTarget target = value.getConfiguredTarget();
+ if (((TargetCompletionKey) skyKey.argument()).willTest()) {
+ return TargetCompleteEvent.successfulBuildSchedulingTest(target);
+ } else {
+ ArtifactsToBuild artifactsToBuild =
+ TopLevelArtifactHelper.getAllArtifactsToBuild(target, topLevelArtifactContext);
+ return TargetCompleteEvent.successfulBuild(
+ target, artifactsToBuild.getAllArtifactsByOutputGroup());
+ }
+ }
}
private static class AspectCompletor implements Completor<AspectValue, AspectCompletionValue> {
@@ -203,7 +223,8 @@
}
@Override
- public SkyValue createFailed(AspectValue value, NestedSet<Cause> rootCauses) {
+ public ExtendedEventHandler.Postable createFailed(
+ AspectValue value, NestedSet<Cause> rootCauses) {
return AspectCompleteEvent.createFailed(value, rootCauses);
}
@@ -211,22 +232,27 @@
public String extractTag(SkyKey skyKey) {
return Label.print(((AspectCompletionKey) skyKey.argument()).aspectKey().getLabel());
}
+
+ @Override
+ public ExtendedEventHandler.Postable createSucceeded(
+ SkyKey skyKey, AspectValue value, TopLevelArtifactContext topLevelArtifactContext) {
+ ArtifactsToBuild artifacts =
+ TopLevelArtifactHelper.getAllArtifactsToBuild(value, topLevelArtifactContext);
+ return AspectCompleteEvent.createSuccessful(value, artifacts);
+ }
}
- public static SkyFunction targetCompletionFunction(AtomicReference<EventBus> eventBusRef) {
- return new CompletionFunction<>(eventBusRef, new TargetCompletor());
+ public static SkyFunction targetCompletionFunction() {
+ return new CompletionFunction<>(new TargetCompletor());
}
- public static SkyFunction aspectCompletionFunction(AtomicReference<EventBus> eventBusRef) {
- return new CompletionFunction<>(eventBusRef, new AspectCompletor());
+ public static SkyFunction aspectCompletionFunction() {
+ return new CompletionFunction<>(new AspectCompletor());
}
- private final AtomicReference<EventBus> eventBusRef;
private final Completor<TValue, TResult> completor;
- private CompletionFunction(
- AtomicReference<EventBus> eventBusRef, Completor<TValue, TResult> completor) {
- this.eventBusRef = eventBusRef;
+ private CompletionFunction(Completor<TValue, TResult> completor) {
this.completor = completor;
}
@@ -280,7 +306,7 @@
NestedSet<Cause> rootCauses = rootCausesBuilder.build();
if (!rootCauses.isEmpty()) {
- eventBusRef.get().post(completor.createFailed(value, rootCauses));
+ env.getListener().post(completor.createFailed(value, rootCauses));
if (firstActionExecutionException != null) {
throw new CompletionFunctionException(firstActionExecutionException);
} else {
@@ -288,7 +314,14 @@
}
}
- return env.valuesMissing() ? null : completor.createResult(value);
+ // Only check for missing values *after* reporting errors: if there are missing files in a build
+ // with --nokeep_going, there may be missing dependencies during error bubbling, we still need
+ // to report the error.
+ if (env.valuesMissing()) {
+ return null;
+ }
+ env.getListener().post(completor.createSucceeded(skyKey, value, topLevelContext));
+ return completor.createResult(value);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 6bca567..736b8fd 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -439,8 +439,8 @@
SkyFunctions.WORKSPACE_FILE,
new WorkspaceFileFunction(ruleClassProvider, pkgFactory, directories));
map.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction());
- map.put(SkyFunctions.TARGET_COMPLETION, CompletionFunction.targetCompletionFunction(eventBus));
- map.put(SkyFunctions.ASPECT_COMPLETION, CompletionFunction.aspectCompletionFunction(eventBus));
+ map.put(SkyFunctions.TARGET_COMPLETION, CompletionFunction.targetCompletionFunction());
+ map.put(SkyFunctions.ASPECT_COMPLETION, CompletionFunction.aspectCompletionFunction());
map.put(SkyFunctions.TEST_COMPLETION, new TestCompletionFunction());
map.put(SkyFunctions.ARTIFACT, new ArtifactFunction());
map.put(
@@ -1202,7 +1202,7 @@
Set<Artifact> artifactsToBuild,
Collection<ConfiguredTarget> targetsToBuild,
Collection<AspectValue> aspects,
- Collection<ConfiguredTarget> targetsToTest,
+ Set<ConfiguredTarget> targetsToTest,
boolean exclusiveTesting,
boolean keepGoing,
boolean explain,
@@ -1224,7 +1224,7 @@
progressReceiver.executionProgressReceiver = executionProgressReceiver;
Iterable<SkyKey> artifactKeys = ArtifactSkyKey.mandatoryKeys(artifactsToBuild);
Iterable<SkyKey> targetKeys =
- TargetCompletionValue.keys(targetsToBuild, topLevelArtifactContext);
+ TargetCompletionValue.keys(targetsToBuild, topLevelArtifactContext, targetsToTest);
Iterable<SkyKey> aspectKeys = AspectCompletionValue.keys(aspects, topLevelArtifactContext);
Iterable<SkyKey> testKeys =
TestCompletionValue.keys(targetsToTest, topLevelArtifactContext, exclusiveTesting);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
index c08f5a6..b3b62e1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
@@ -22,6 +22,7 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.Collection;
+import java.util.Set;
/**
* The value of a TargetCompletion. Currently this just stores a ConfiguredTarget.
@@ -38,14 +39,18 @@
}
public static SkyKey key(
- ConfiguredTargetKey configuredTargetKey, TopLevelArtifactContext topLevelArtifactContext) {
+ ConfiguredTargetKey configuredTargetKey,
+ TopLevelArtifactContext topLevelArtifactContext,
+ boolean willTest) {
return LegacySkyKey.create(
SkyFunctions.TARGET_COMPLETION,
- TargetCompletionKey.create(configuredTargetKey, topLevelArtifactContext));
+ TargetCompletionKey.create(configuredTargetKey, topLevelArtifactContext, willTest));
}
- public static Iterable<SkyKey> keys(Collection<ConfiguredTarget> targets,
- final TopLevelArtifactContext ctx) {
+ public static Iterable<SkyKey> keys(
+ Collection<ConfiguredTarget> targets,
+ final TopLevelArtifactContext ctx,
+ final Set<ConfiguredTarget> targetsToTest) {
return Iterables.transform(
targets,
new Function<ConfiguredTarget, SkyKey>() {
@@ -53,7 +58,8 @@
public SkyKey apply(ConfiguredTarget ct) {
return LegacySkyKey.create(
SkyFunctions.TARGET_COMPLETION,
- TargetCompletionKey.create(ConfiguredTargetKey.of(ct), ctx));
+ TargetCompletionKey.create(
+ ConfiguredTargetKey.of(ct), ctx, targetsToTest.contains(ct)));
}
});
}
@@ -61,13 +67,16 @@
@AutoValue
abstract static class TargetCompletionKey {
public static TargetCompletionKey create(
- ConfiguredTargetKey configuredTargetKey, TopLevelArtifactContext topLevelArtifactContext) {
+ ConfiguredTargetKey configuredTargetKey,
+ TopLevelArtifactContext topLevelArtifactContext,
+ boolean willTest) {
return new AutoValue_TargetCompletionValue_TargetCompletionKey(
- configuredTargetKey, topLevelArtifactContext);
+ configuredTargetKey, topLevelArtifactContext, willTest);
}
abstract ConfiguredTargetKey configuredTargetKey();
public abstract TopLevelArtifactContext topLevelArtifactContext();
+ public abstract boolean willTest();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
index d476098..3efc7e8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TestCompletionFunction.java
@@ -34,7 +34,7 @@
(TestCompletionValue.TestCompletionKey) skyKey.argument();
ConfiguredTargetKey lac = key.configuredTargetKey();
TopLevelArtifactContext ctx = key.topLevelArtifactContext();
- if (env.getValue(TargetCompletionValue.key(lac, ctx)) == null) {
+ if (env.getValue(TargetCompletionValue.key(lac, ctx, /*willTest=*/true)) == null) {
return null;
}
