Fix a bug with the event-based result collection.
For null incremental builds, CompletionFunctions won't be called at all. By moving the built status events to ExecutionProgressReceiver, we can make sure that the appropriate event is collected every build.
PiperOrigin-RevId: 449725542
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 2c4ecd8..9ef0b8a 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
@@ -13,19 +13,27 @@
// limitations under the License.
package com.google.devtools.build.lib.buildtool;
-
+import com.google.common.base.Preconditions;
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.ActionExecutionStatusReporter;
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.MiddlemanType;
+import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.skyframe.ActionExecutionInactivityWatchdog;
import com.google.devtools.build.lib.skyframe.AspectCompletionValue;
+import com.google.devtools.build.lib.skyframe.AspectKeyCreator;
import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey;
+import com.google.devtools.build.lib.skyframe.BuildDriverKey;
+import com.google.devtools.build.lib.skyframe.BuildDriverValue;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor;
import com.google.devtools.build.lib.skyframe.TargetCompletionValue;
+import com.google.devtools.build.lib.skyframe.TopLevelAspectsValue;
+import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.AspectBuiltEvent;
+import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetBuiltEvent;
import com.google.devtools.build.skyframe.ErrorInfo;
import com.google.devtools.build.skyframe.EvaluationProgressReceiver;
import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -61,6 +69,7 @@
private final Set<ActionLookupData> enqueuedActions = Sets.newConcurrentHashSet();
private final Set<ActionLookupData> completedActions = Sets.newConcurrentHashSet();
private final Set<ActionLookupData> ignoredActions = Sets.newConcurrentHashSet();
+ private final EventBus eventBus;
/** Number of exclusive tests. To be accounted for in progress messages. */
private final int exclusiveTestsCount;
@@ -70,11 +79,15 @@
* permitted while this receiver is active.
*/
ExecutionProgressReceiver(
- Set<ConfiguredTargetKey> builtTargets, Set<AspectKey> builtAspects, int exclusiveTestsCount) {
+ Set<ConfiguredTargetKey> builtTargets,
+ Set<AspectKey> builtAspects,
+ int exclusiveTestsCount,
+ EventBus eventBus) {
// TODO(b/227138583) Remove these.
this.builtTargets = Collections.synchronizedSet(builtTargets);
this.builtAspects = Collections.synchronizedSet(builtAspects);
this.exclusiveTestsCount = exclusiveTestsCount;
+ this.eventBus = eventBus;
}
@Override
@@ -110,18 +123,54 @@
Supplier<EvaluationSuccessState> evaluationSuccessState,
EvaluationState state) {
SkyFunctionName type = skyKey.functionName();
- if (type.equals(SkyFunctions.TARGET_COMPLETION)) {
- if (evaluationSuccessState.get().succeeded()) {
- builtTargets.add(((TargetCompletionValue.TargetCompletionKey) skyKey).actionLookupKey());
- }
- } else if (type.equals(SkyFunctions.ASPECT_COMPLETION)) {
- if (evaluationSuccessState.get().succeeded()) {
- builtAspects.add(((AspectCompletionValue.AspectCompletionKey) skyKey).actionLookupKey());
- }
- } else if (type.equals(SkyFunctions.ACTION_EXECUTION)) {
+ if (type.equals(SkyFunctions.ACTION_EXECUTION)) {
// Remember all completed actions, even those in error, regardless of having been cached or
// really executed.
actionCompleted((ActionLookupData) skyKey.argument());
+ return;
+ }
+
+ if (!evaluationSuccessState.get().succeeded()) {
+ return;
+ }
+
+ if (type.equals(SkyFunctions.TARGET_COMPLETION)) {
+ ConfiguredTargetKey configuredTargetKey =
+ ((TargetCompletionValue.TargetCompletionKey) skyKey).actionLookupKey();
+ eventBus.post(TopLevelTargetBuiltEvent.create(configuredTargetKey));
+ // TODO(b/227138583) Remove this.
+ builtTargets.add(configuredTargetKey);
+ return;
+ }
+
+ if (type.equals(SkyFunctions.ASPECT_COMPLETION)) {
+ AspectKeyCreator.AspectKey aspectKey =
+ ((AspectCompletionValue.AspectCompletionKey) skyKey).actionLookupKey();
+ eventBus.post(AspectBuiltEvent.create(aspectKey));
+ // TODO(b/227138583) Remove this.
+ builtAspects.add(aspectKey);
+ return;
+ }
+
+ if (type.equals(SkyFunctions.BUILD_DRIVER)) {
+ BuildDriverKey buildDriverKey = (BuildDriverKey) skyKey;
+ // BuildDriverKeys are re-evaluated every build.
+ BuildDriverValue buildDriverValue = (BuildDriverValue) Preconditions.checkNotNull(newValue);
+
+ if (buildDriverValue.isSkipped()) {
+ return;
+ }
+
+ if (buildDriverKey.isTopLevelAspectDriver()) {
+ ((TopLevelAspectsValue) buildDriverValue.getWrappedSkyValue())
+ .getTopLevelAspectsValues()
+ .forEach(x -> eventBus.post(AspectBuiltEvent.create(((AspectValue) x).getKey())));
+ return;
+ }
+
+ eventBus.post(
+ TopLevelTargetBuiltEvent.create(
+ (ConfiguredTargetKey) buildDriverKey.getActionLookupKey()));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
index f4d0de9..ff43b02 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -332,7 +332,10 @@
// TODO(leba): count test actions
ExecutionProgressReceiver executionProgressReceiver =
new ExecutionProgressReceiver(
- Preconditions.checkNotNull(builtTargets), Preconditions.checkNotNull(builtAspects), 0);
+ Preconditions.checkNotNull(builtTargets),
+ Preconditions.checkNotNull(builtAspects),
+ /*exclusiveTestsCount=*/ 0,
+ env.getEventBus());
skyframeExecutor
.getEventBus()
.post(new ExecutionProgressReceiverAvailableEvent(executionProgressReceiver));
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 3ca36ca..38ef57d 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
@@ -118,7 +118,8 @@
new ExecutionProgressReceiver(
Preconditions.checkNotNull(builtTargets),
Preconditions.checkNotNull(builtAspects),
- countTestActions(exclusiveTests));
+ countTestActions(exclusiveTests),
+ skyframeExecutor.getEventBus());
skyframeExecutor
.getEventBus()
.post(new ExecutionProgressReceiverAvailableEvent(executionProgressReceiver));
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletor.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletor.java
index 2c4f14c..4f13033 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletor.java
@@ -29,11 +29,9 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
-import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
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.CompletionFunction.Completor;
-import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.AspectBuiltEvent;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import javax.annotation.Nullable;
@@ -130,10 +128,4 @@
artifactsToBuild.getAllArtifactsByOutputGroup(),
configurationEventId);
}
-
- @Override
- public Postable getTargetOrAspectBuiltEventForResultSummary(
- AspectCompletionKey unused, AspectValue value) {
- return AspectBuiltEvent.create(value.getKey());
- }
}
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 7f14152..273c977 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
@@ -179,7 +179,7 @@
}
// We consider the evaluation of this BuildDriverKey successful at this point, even when
// the target is skipped.
- return new BuildDriverValue(topLevelSkyValue);
+ return new BuildDriverValue(topLevelSkyValue, /*skipped=*/ true);
}
} catch (TargetCompatibilityCheckException e) {
throw new BuildDriverFunctionException(e);
@@ -209,7 +209,7 @@
topLevelSkyValue, ((ConfiguredTargetValue) topLevelSkyValue).getConfiguredTarget());
}
- return new BuildDriverValue(topLevelSkyValue);
+ return new BuildDriverValue(topLevelSkyValue, /*skipped=*/ false);
}
/**
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 d2193c8..ef40f22 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
@@ -29,18 +29,21 @@
private final TestType testType;
private final boolean strictActionConflictCheck;
private final boolean explicitlyRequested;
+ private final boolean isTopLevelAspectDriver;
private BuildDriverKey(
ActionLookupKey actionLookupKey,
TopLevelArtifactContext topLevelArtifactContext,
boolean strictActionConflictCheck,
boolean explicitlyRequested,
+ boolean isTopLevelAspectDriver,
TestType testType) {
this.actionLookupKey = actionLookupKey;
this.topLevelArtifactContext = topLevelArtifactContext;
this.strictActionConflictCheck = strictActionConflictCheck;
- this.testType = testType;
this.explicitlyRequested = explicitlyRequested;
+ this.isTopLevelAspectDriver = isTopLevelAspectDriver;
+ this.testType = testType;
}
public static BuildDriverKey ofTopLevelAspect(
@@ -53,6 +56,7 @@
topLevelArtifactContext,
strictActionConflictCheck,
explicitlyRequested,
+ /*isTopLevelAspectDriver=*/ true,
TestType.NOT_TEST);
}
@@ -67,6 +71,7 @@
topLevelArtifactContext,
strictActionConflictCheck,
explicitlyRequested,
+ /*isTopLevelAspectDriver=*/ false,
testType);
}
@@ -94,6 +99,10 @@
return explicitlyRequested;
}
+ public boolean isTopLevelAspectDriver() {
+ return isTopLevelAspectDriver;
+ }
+
@Override
public SkyFunctionName functionName() {
return SkyFunctions.BUILD_DRIVER;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverValue.java
index 466d411..dde368c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildDriverValue.java
@@ -18,12 +18,18 @@
/** The result of evaluating a {@link BuildDriverKey}. */
public class BuildDriverValue implements SkyValue {
private final SkyValue wrappedSkyValue;
+ private final boolean skipped;
- BuildDriverValue(SkyValue wrappedSkyValue) {
+ BuildDriverValue(SkyValue wrappedSkyValue, boolean skipped) {
this.wrappedSkyValue = wrappedSkyValue;
+ this.skipped = skipped;
}
public SkyValue getWrappedSkyValue() {
return wrappedSkyValue;
}
+
+ public boolean isSkipped() {
+ return skipped;
+ }
}
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 079c2ae..86b3f16 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
@@ -41,7 +41,6 @@
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.events.ExtendedEventHandler.Postable;
import com.google.devtools.build.lib.skyframe.ArtifactFunction.MissingArtifactValue;
import com.google.devtools.build.lib.skyframe.ArtifactFunction.SourceArtifactException;
import com.google.devtools.build.lib.skyframe.MetadataConsumerForMetrics.FilesMetricConsumer;
@@ -120,9 +119,6 @@
ArtifactsToBuild artifactsToBuild,
Environment env)
throws InterruptedException;
-
- /** Builds the appropriate event among {@link TopLevelStatusEvents}. */
- Postable getTargetOrAspectBuiltEventForResultSummary(KeyT key, ValueT value);
}
private final PathResolverFactory pathResolverFactory;
@@ -319,7 +315,6 @@
env.getListener().post(postable);
topLevelArtifactsMetric.mergeIn(currentConsumer);
- env.getListener().post(completor.getTargetOrAspectBuiltEventForResultSummary(key, value));
return completor.getResult();
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExclusiveTestBuildDriverValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExclusiveTestBuildDriverValue.java
index acc0188..44e7d6c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ExclusiveTestBuildDriverValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExclusiveTestBuildDriverValue.java
@@ -23,7 +23,8 @@
ExclusiveTestBuildDriverValue(
SkyValue wrappedSkyValue, ConfiguredTarget exclusiveTestConfiguredTarget) {
- super(wrappedSkyValue);
+ // If an exclusive test was run at all, it wasn't skipped.
+ super(wrappedSkyValue, /*skipped=*/ false);
this.exclusiveTestConfiguredTarget = exclusiveTestConfiguredTarget;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletor.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletor.java
index 264ff76..51e95c8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletor.java
@@ -28,10 +28,8 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
-import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
import com.google.devtools.build.lib.skyframe.CompletionFunction.Completor;
import com.google.devtools.build.lib.skyframe.TargetCompletionValue.TargetCompletionKey;
-import com.google.devtools.build.lib.skyframe.TopLevelStatusEvents.TopLevelTargetBuiltEvent;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import javax.annotation.Nullable;
@@ -156,10 +154,4 @@
skyframeActionExecutor.publishTargetSummaries());
}
}
-
- @Override
- public Postable getTargetOrAspectBuiltEventForResultSummary(
- TargetCompletionKey key, ConfiguredTargetValue unused) {
- return TopLevelTargetBuiltEvent.create(key.actionLookupKey());
- }
}
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 1d40ae4..bf4deed 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
@@ -58,7 +58,7 @@
public abstract static class TopLevelTargetBuiltEvent implements ProgressLike {
abstract ConfiguredTargetKey configuredTargetKey();
- static TopLevelTargetBuiltEvent create(ConfiguredTargetKey configuredTargetKey) {
+ public static TopLevelTargetBuiltEvent create(ConfiguredTargetKey configuredTargetKey) {
return new AutoValue_TopLevelStatusEvents_TopLevelTargetBuiltEvent(configuredTargetKey);
}
}
@@ -94,11 +94,12 @@
}
}
+ /** An event that marks the successful building of an aspect. */
@AutoValue
- abstract static class AspectBuiltEvent implements ProgressLike {
+ public abstract static class AspectBuiltEvent implements ProgressLike {
abstract AspectKey aspectKey();
- static AspectBuiltEvent create(AspectKey aspectKey) {
+ public static AspectBuiltEvent create(AspectKey aspectKey) {
return new AutoValue_TopLevelStatusEvents_AspectBuiltEvent(aspectKey);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/BuildResultListenerIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/BuildResultListenerIntegrationTest.java
index 85e797d..d30d56f 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/BuildResultListenerIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/BuildResultListenerIntegrationTest.java
@@ -239,4 +239,26 @@
assertThat(getLabelsOfBuiltTargets()).containsExactly("//foo:good");
assertThat(getLabelsOfSkippedTargets()).containsExactly("//foo:bad");
}
+
+ @Test
+ public void nullIncrementalBuild_correctAnalyzedAndBuiltTargets() throws Exception {
+ writeMyRuleBzl();
+ write(
+ "foo/BUILD",
+ "load('//foo:my_rule.bzl', 'my_rule')",
+ "my_rule(name = 'foo', srcs = ['foo.in'])");
+ write("foo/foo.in");
+
+ BuildResult result = buildTarget("//foo:foo");
+
+ assertThat(result.getSuccess()).isTrue();
+ assertThat(getLabelsOfAnalyzedTargets()).containsExactly("//foo:foo");
+ assertThat(getLabelsOfBuiltTargets()).containsExactly("//foo:foo");
+
+ result = buildTarget("//foo:foo");
+
+ assertThat(result.getSuccess()).isTrue();
+ assertThat(getLabelsOfAnalyzedTargets()).containsExactly("//foo:foo");
+ assertThat(getLabelsOfBuiltTargets()).containsExactly("//foo:foo");
+ }
}
