diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index 9273f68f28fbff..578ac936a154a4 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -349,7 +349,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:provider_collection", "//src/main/java/com/google/devtools/build/lib/analysis:server_directories", "//src/main/java/com/google/devtools/build/lib/analysis:template_expansion_exception", - "//src/main/java/com/google/devtools/build/lib/analysis:test/coverage_action_finished_event", "//src/main/java/com/google/devtools/build/lib/analysis:test/coverage_artifacts_known_event", "//src/main/java/com/google/devtools/build/lib/analysis:test/coverage_report_action_factory", "//src/main/java/com/google/devtools/build/lib/analysis:top_level_artifact_context", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index 4a1cd08c5359bb..26dbb2deaff8df 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -437,7 +437,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", - "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", "//src/main/java/com/google/devtools/common/options", "//src/main/java/net/starlark/java/annot", @@ -2741,16 +2740,6 @@ java_library( ], ) -java_library( - name = "test/coverage_action_finished_event", - srcs = ["test/CoverageActionFinishedEvent.java"], - deps = [ - "//src/main/java/com/google/devtools/build/lib/buildeventstream", - "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", - "//third_party:guava", - ], -) - java_library( name = "test/coverage_artifacts_known_event", srcs = ["test/CoverageArtifactsKnownEvent.java"], diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index ed902642cbf604..e4416ee42aba97 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -853,32 +853,39 @@ private ImmutableSet memoizedGetCoverageArtifactsHelper( EventBus eventBus, TargetPatternPhaseValue loadingResult) throws InterruptedException { - if (memoizedCoverageArtifacts != null) { - return memoizedCoverageArtifacts; + if (memoizedCoverageArtifacts == null) { + memoizedCoverageArtifacts = + constructCoverageArtifacts( + configuredTargets, allTargetsToTest, eventHandler, eventBus, loadingResult); } - ImmutableSet.Builder resultBuilder = ImmutableSet.builder(); - // Coverage - NestedSet baselineCoverageArtifacts = getBaselineCoverageArtifacts(configuredTargets); - resultBuilder.addAll(baselineCoverageArtifacts.toList()); + return memoizedCoverageArtifacts; + } - if (coverageReportActionFactory != null) { - CoverageReportActionsWrapper actionsWrapper = - coverageReportActionFactory.createCoverageReportActionsWrapper( - eventHandler, - eventBus, - directories, - allTargetsToTest, - baselineCoverageArtifacts, - getArtifactFactory(), - skyframeExecutor.getActionKeyContext(), - CoverageReportValue.COVERAGE_REPORT_KEY, - loadingResult.getWorkspaceName()); - if (actionsWrapper != null) { - skyframeExecutor.injectCoverageReportData(actionsWrapper.getActions()); - actionsWrapper.getCoverageOutputs().forEach(resultBuilder::add); - } + private ImmutableSet constructCoverageArtifacts( + Set configuredTargets, + Set allTargetsToTest, + EventHandler eventHandler, + EventBus eventBus, + TargetPatternPhaseValue loadingResult) + throws InterruptedException { + if (coverageReportActionFactory == null) { + return ImmutableSet.of(); } - memoizedCoverageArtifacts = resultBuilder.build(); - return memoizedCoverageArtifacts; + CoverageReportActionsWrapper actionsWrapper = + coverageReportActionFactory.createCoverageReportActionsWrapper( + eventHandler, + eventBus, + directories, + allTargetsToTest, + getBaselineCoverageArtifacts(configuredTargets), + getArtifactFactory(), + skyframeExecutor.getActionKeyContext(), + CoverageReportValue.COVERAGE_REPORT_KEY, + loadingResult.getWorkspaceName()); + if (actionsWrapper == null) { + return ImmutableSet.of(); + } + skyframeExecutor.injectCoverageReportData(actionsWrapper.getActions()); + return ImmutableSet.copyOf(actionsWrapper.getCoverageOutputs()); } } 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 5b3ee7871a0147..52316f713302d9 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 @@ -174,7 +174,6 @@ private TargetCompleteEvent( instrumentedFilesProvider.getBaselineCoverageArtifacts(); if (!baselineCoverageArtifacts.isEmpty()) { this.baselineCoverageArtifacts = baselineCoverageArtifacts; - postedAfterBuilder.add(BuildEventIdUtil.coverageActionsFinished()); } else { this.baselineCoverageArtifacts = null; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageActionFinishedEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageActionFinishedEvent.java deleted file mode 100644 index 0e271de2ea790c..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageActionFinishedEvent.java +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright 2023 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.analysis.test; - -import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.buildeventstream.BuildEvent; -import com.google.devtools.build.lib.buildeventstream.BuildEventContext; -import com.google.devtools.build.lib.buildeventstream.BuildEventIdUtil; -import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; -import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId; -import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent; -import java.util.Collection; - -/** - * Signal that the coverage actions are finished. Only used as a prerequisite for {@link - * com.google.devtools.build.lib.analysis.TargetCompleteEvent} in Skymeld mode. - */ -public class CoverageActionFinishedEvent implements BuildEvent { - - @Override - public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext context) - throws InterruptedException { - return GenericBuildEvent.protoChaining(this).build(); - } - - @Override - public BuildEventId getEventId() { - return BuildEventIdUtil.coverageActionsFinished(); - } - - @Override - public Collection getChildrenEvents() { - return ImmutableList.of(); - } -} diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventIdUtil.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventIdUtil.java index 8f0c66079256a1..bd8070828adff1 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventIdUtil.java +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventIdUtil.java @@ -162,12 +162,6 @@ public static BuildEventId targetConfigured(Label label) { return BuildEventId.newBuilder().setTargetConfigured(configuredId).build(); } - public static BuildEventId coverageActionsFinished() { - return BuildEventId.newBuilder() - .setCoverageActionsFinished(BuildEventId.CoverageActionsFinishedId.getDefaultInstance()) - .build(); - } - public static BuildEventId aspectConfigured(Label label, String aspect) { BuildEventId.TargetConfiguredId configuredId = BuildEventId.TargetConfiguredId.newBuilder() diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto index 7dfe4b9a6b22d3..816871a00b2812 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto @@ -248,12 +248,11 @@ message BuildEventId { // Identifier of an event providing convenience symlinks information. message ConvenienceSymlinksIdentifiedId {} - // Identifier of an event signalling that the coverage actions are finished. - message CoverageActionsFinishedId {} - // Identifier of an event providing the ExecRequest of a run command. message ExecRequestId {} + reserved 27; + oneof id { UnknownBuildEventId unknown = 1; ProgressId progress = 2; @@ -282,7 +281,6 @@ message BuildEventId { WorkspaceConfigId workspace = 23; BuildMetadataId build_metadata = 24; ConvenienceSymlinksIdentifiedId convenience_symlinks_identified = 25; - CoverageActionsFinishedId coverage_actions_finished = 27; ExecRequestId exec_request = 28; } } 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 81488208efa1c4..cd75af0de53847 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 @@ -34,7 +34,6 @@ import com.google.devtools.build.lib.actions.TestExecException; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.TopLevelArtifactContext; -import com.google.devtools.build.lib.analysis.test.CoverageActionFinishedEvent; import com.google.devtools.build.lib.analysis.test.TestProvider; import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.buildtool.buildevent.ExecutionProgressReceiverAvailableEvent; @@ -127,9 +126,6 @@ public void buildArtifacts( skyframeExecutor .getEventBus() .post(new ExecutionProgressReceiverAvailableEvent(executionProgressReceiver)); - // When not in Skymeld mode, TargetCompleteEvents don't need to be held back. - // See {@link CoverageActionFinishedEvent}. - skyframeExecutor.getEventBus().post(new CoverageActionFinishedEvent()); List detailedExitCodes = new ArrayList<>(); EvaluationResult result; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index df4c63f5deb12e..a44065b474fe1c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -274,8 +274,8 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:target_configured_event", "//src/main/java/com/google/devtools/build/lib/analysis:template_expansion_exception", "//src/main/java/com/google/devtools/build/lib/analysis:test/analysis_failure_propagation_exception", - "//src/main/java/com/google/devtools/build/lib/analysis:test/coverage_action_finished_event", "//src/main/java/com/google/devtools/build/lib/analysis:test/coverage_artifacts_known_event", + "//src/main/java/com/google/devtools/build/lib/analysis:test/instrumented_files_info", "//src/main/java/com/google/devtools/build/lib/analysis:toolchain_collection", "//src/main/java/com/google/devtools/build/lib/analysis:toolchain_context", "//src/main/java/com/google/devtools/build/lib/analysis:top_level_artifact_context", 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 d658161d90c3e4..c0d97ae5e00c01 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 @@ -43,6 +43,7 @@ import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsInOutputGroup; import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsToBuild; import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.SuccessfulArtifactFilter; +import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo; import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.causes.Cause; import com.google.devtools.build.lib.causes.LabelCause; @@ -170,8 +171,20 @@ public SkyValue compute(SkyKey skyKey, Environment env) ValueT value = valueAndArtifactsToBuild.first; ArtifactsToBuild artifactsToBuild = valueAndArtifactsToBuild.second; + // Ensure that coverage artifacts are built before a target is considered completed. ImmutableList allArtifacts = artifactsToBuild.getAllArtifacts().toList(); - SkyframeLookupResult inputDeps = env.getValuesAndExceptions(Artifact.keys(allArtifacts)); + InstrumentedFilesInfo instrumentedFilesInfo = + value.getConfiguredObject().get(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR); + Iterable keysToRequest; + if (value.getConfiguredObject() instanceof ConfiguredTarget && instrumentedFilesInfo != null) { + keysToRequest = + Iterables.concat( + Artifact.keys(allArtifacts), + Artifact.keys(instrumentedFilesInfo.getBaselineCoverageArtifacts().toList())); + } else { + keysToRequest = Artifact.keys(allArtifacts); + } + SkyframeLookupResult inputDeps = env.getValuesAndExceptions(keysToRequest); boolean allArtifactsAreImportant = artifactsToBuild.areAllOutputGroupsImportant(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java index a3bb784183a7de..3298d246fa29fb 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java @@ -73,7 +73,6 @@ import com.google.devtools.build.lib.analysis.config.StarlarkTransitionCache; import com.google.devtools.build.lib.analysis.starlark.StarlarkAttributeTransitionProvider; import com.google.devtools.build.lib.analysis.test.AnalysisFailurePropagationException; -import com.google.devtools.build.lib.analysis.test.CoverageActionFinishedEvent; import com.google.devtools.build.lib.analysis.test.CoverageArtifactsKnownEvent; import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.bugreport.BugReporter; @@ -757,18 +756,15 @@ public SkyframeAnalysisResult analyzeAndExecuteTargets( } // Coverage report generation should only be requested after all tests have executed. - // We could generate baseline coverage artifacts earlier; it is only the timing of the - // combined report that matters. // When --nokeep_going and there's an earlier error, we should skip this and fail fast. if ((!mainEvaluationResult.hasError() && !hasExclusiveTestsError) || keepGoing) { - ImmutableSet coverageArtifacts = - coverageReportActionsWrapperSupplier.getCoverageArtifacts( + ImmutableSet coverageReportArtifacts = + coverageReportActionsWrapperSupplier.getCoverageReportArtifacts( buildResultListener.getAnalyzedTargets(), buildResultListener.getAnalyzedTests()); - eventBus.post(CoverageArtifactsKnownEvent.create(coverageArtifacts)); + eventBus.post(CoverageArtifactsKnownEvent.create(coverageReportArtifacts)); additionalArtifactsResult = skyframeExecutor.evaluateSkyKeys( - eventHandler, Artifact.keys(coverageArtifacts), keepGoing); - eventBus.post(new CoverageActionFinishedEvent()); + eventHandler, Artifact.keys(coverageReportArtifacts), keepGoing); if (additionalArtifactsResult.hasError()) { detailedExitCodes.add( SkyframeErrorProcessor.processErrors( @@ -1483,7 +1479,7 @@ public void reset() { /** Provides the list of coverage artifacts to be built. */ @FunctionalInterface public interface CoverageReportActionsWrapperSupplier { - ImmutableSet getCoverageArtifacts( + ImmutableSet getCoverageReportArtifacts( Set configuredTargets, Set allTargetsToTest) throws InterruptedException; }