From d8a9344699da2f6258eafa22566c7e541ec09792 Mon Sep 17 00:00:00 2001 From: ulfjack Date: Fri, 22 Dec 2017 00:59:05 -0800 Subject: 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 --- .../build/lib/analysis/TargetCompleteEvent.java | 10 ++-- .../lib/buildtool/ExecutionProgressReceiver.java | 45 +--------------- .../build/lib/buildtool/SkyframeBuilder.java | 8 +-- .../build/lib/skyframe/CompletionFunction.java | 63 ++++++++++++++++------ .../build/lib/skyframe/SkyframeExecutor.java | 8 +-- .../build/lib/skyframe/TargetCompletionValue.java | 23 +++++--- .../build/lib/skyframe/TestCompletionFunction.java | 2 +- 7 files changed, 76 insertions(+), 83 deletions(-) (limited to 'src') 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 4a6e8aaa71..c1e7805bf7 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 @@ public final class TargetCompleteEvent BuildEventWithConfiguration { private final ConfiguredTarget target; private final NestedSet rootCauses; - private final Collection postedAfter; + private final ImmutableList postedAfter; private final Iterable outputs; private final NestedSet baselineCoverageArtifacts; private final boolean isTest; @@ -71,7 +71,7 @@ public final class TargetCompleteEvent this.rootCauses = (rootCauses == null) ? NestedSetBuilder.emptySet(Order.STABLE_ORDER) : rootCauses; - ImmutableList.Builder postedAfterBuilder = ImmutableList.builder(); + ImmutableList.Builder postedAfterBuilder = ImmutableList.builder(); for (Cause cause : getRootCauses()) { postedAfterBuilder.add(BuildEventId.fromCause(cause)); } @@ -94,13 +94,13 @@ public final class TargetCompleteEvent } /** Construct a successful target completion event. */ - public static TargetCompleteEvent createSuccessfulTarget( + public static TargetCompleteEvent successfulBuild( ConfiguredTarget ct, NestedSet 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.of(), true); } @@ -147,7 +147,7 @@ public final class TargetCompleteEvent @Override public Collection getChildrenEvents() { - ImmutableList.Builder childrenBuilder = ImmutableList.builder(); + ImmutableList.Builder 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 4283ff5418..ef4319573a 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 @@ package com.google.devtools.build.lib.buildtool; 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 @@ public final class ExecutionProgressReceiver private final Object activityIndicator = new Object(); /** Number of exclusive tests. To be accounted for in progress messages. */ private final int exclusiveTestsCount; - private final Set testedTargets; - private final EventBus eventBus; - private final TopLevelArtifactContext topLevelArtifactContext; static { PROGRESS_MESSAGE_NUMBER_FORMATTER = NumberFormat.getIntegerInstance(Locale.ENGLISH); @@ -77,15 +66,9 @@ public final class ExecutionProgressReceiver */ ExecutionProgressReceiver( Set builtTargets, - int exclusiveTestsCount, - Set 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 @@ public final class ExecutionProgressReceiver 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 @@ public final class ExecutionProgressReceiver } }; } - - 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 99e8841f1d..e72eda9ed0 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 @@ public class SkyframeBuilder implements Builder { ExecutionProgressReceiver executionProgressReceiver = new ExecutionProgressReceiver( Preconditions.checkNotNull(builtTargets), - countTestActions(exclusiveTests), - ImmutableSet.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 103fd73167..f4fb386996 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.cmdline.Label; 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.SkyKey; 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 @@ public final class CompletionFunction rootCauses); + ExtendedEventHandler.Postable createFailed(TValue value, NestedSet 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 @@ public final class CompletionFunction rootCauses) { + public ExtendedEventHandler.Postable createFailed( + ConfiguredTargetValue value, NestedSet rootCauses) { return TargetCompleteEvent.createFailed(value.getConfiguredTarget(), rootCauses); } @@ -152,6 +156,22 @@ public final class CompletionFunction { @@ -203,7 +223,8 @@ public final class CompletionFunction rootCauses) { + public ExtendedEventHandler.Postable createFailed( + AspectValue value, NestedSet rootCauses) { return AspectCompleteEvent.createFailed(value, rootCauses); } @@ -211,22 +232,27 @@ public final class CompletionFunction eventBusRef) { - return new CompletionFunction<>(eventBusRef, new TargetCompletor()); + public static SkyFunction targetCompletionFunction() { + return new CompletionFunction<>(new TargetCompletor()); } - public static SkyFunction aspectCompletionFunction(AtomicReference eventBusRef) { - return new CompletionFunction<>(eventBusRef, new AspectCompletor()); + public static SkyFunction aspectCompletionFunction() { + return new CompletionFunction<>(new AspectCompletor()); } - private final AtomicReference eventBusRef; private final Completor completor; - private CompletionFunction( - AtomicReference eventBusRef, Completor completor) { - this.eventBusRef = eventBusRef; + private CompletionFunction(Completor completor) { this.completor = completor; } @@ -280,7 +306,7 @@ public final class CompletionFunction 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 @@ public final class CompletionFunction artifactsToBuild, Collection targetsToBuild, Collection aspects, - Collection targetsToTest, + Set targetsToTest, boolean exclusiveTesting, boolean keepGoing, boolean explain, @@ -1224,7 +1224,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { progressReceiver.executionProgressReceiver = executionProgressReceiver; Iterable artifactKeys = ArtifactSkyKey.mandatoryKeys(artifactsToBuild); Iterable targetKeys = - TargetCompletionValue.keys(targetsToBuild, topLevelArtifactContext); + TargetCompletionValue.keys(targetsToBuild, topLevelArtifactContext, targetsToTest); Iterable aspectKeys = AspectCompletionValue.keys(aspects, topLevelArtifactContext); Iterable 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 c08f5a6522..b3b62e1abb 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.LegacySkyKey; 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 class TargetCompletionValue implements SkyValue { } 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 keys(Collection targets, - final TopLevelArtifactContext ctx) { + public static Iterable keys( + Collection targets, + final TopLevelArtifactContext ctx, + final Set targetsToTest) { return Iterables.transform( targets, new Function() { @@ -53,7 +58,8 @@ public class TargetCompletionValue implements SkyValue { 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 @@ public class TargetCompletionValue implements SkyValue { @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 d476098c69..3efc7e84f6 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 @@ public final class TestCompletionFunction implements SkyFunction { (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; } -- cgit v1.2.3