From 75bc18a6290f9112077884460d61f34bec325814 Mon Sep 17 00:00:00 2001 From: janakr Date: Fri, 13 Jul 2018 00:52:17 -0700 Subject: For all top-level artifacts, track the labels that own them when that is available. The owning labels are the labels of the top-level configured targets that requested this artifact to be built (there may be many such targets). In cases where the artifact is added not through a configured target (build-info artifacts and coverage artifacts), the label of the artifact's owner is used. PiperOrigin-RevId: 204432951 --- .../build/lib/analysis/AnalysisResult.java | 12 ++- .../devtools/build/lib/analysis/BuildView.java | 110 +++++++++++---------- .../build/lib/analysis/TopLevelArtifactHelper.java | 80 ++++++++------- .../build/lib/buildtool/ExecutionTool.java | 23 ++--- .../build/lib/exec/ActionContextProvider.java | 5 +- 5 files changed, 116 insertions(+), 114 deletions(-) (limited to 'src/main') diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java index 401551333b..ff8dbe435b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java @@ -16,10 +16,12 @@ package com.google.devtools.build.lib.analysis; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.SetMultimap; import com.google.devtools.build.lib.actions.ActionGraph; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.PackageRoots; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.skyframe.AspectValue; import java.util.Collection; import java.util.List; @@ -35,7 +37,7 @@ public final class AnalysisResult { private final ImmutableSet targetsToSkip; @Nullable private final String error; private final ActionGraph actionGraph; - private final ImmutableSet artifactsToBuild; + private final SetMultimap topLevelArtifactsToOwnerLabels; private final ImmutableSet parallelTests; private final ImmutableSet exclusiveTests; @Nullable private final TopLevelArtifactContext topLevelContext; @@ -52,7 +54,7 @@ public final class AnalysisResult { Collection targetsToSkip, @Nullable String error, ActionGraph actionGraph, - Collection artifactsToBuild, + SetMultimap topLevelArtifactsToOwnerLabels, Collection parallelTests, Collection exclusiveTests, TopLevelArtifactContext topLevelContext, @@ -66,7 +68,7 @@ public final class AnalysisResult { this.targetsToSkip = ImmutableSet.copyOf(targetsToSkip); this.error = error; this.actionGraph = actionGraph; - this.artifactsToBuild = ImmutableSet.copyOf(artifactsToBuild); + this.topLevelArtifactsToOwnerLabels = topLevelArtifactsToOwnerLabels; this.parallelTests = ImmutableSet.copyOf(parallelTests); this.exclusiveTests = ImmutableSet.copyOf(exclusiveTests); this.topLevelContext = topLevelContext; @@ -120,8 +122,8 @@ public final class AnalysisResult { return targetsToSkip; } - public ImmutableSet getAdditionalArtifactsToBuild() { - return artifactsToBuild; + public SetMultimap getTopLevelArtifactsToOwnerLabels() { + return topLevelArtifactsToOwnerLabels; } public ImmutableSet getExclusiveTests() { 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 2143efaa19..caca6a7aef 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 @@ -14,18 +14,17 @@ package com.google.devtools.build.lib.analysis; -import static com.google.common.collect.Iterables.concat; - import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Multimap; +import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; @@ -378,7 +377,7 @@ public class BuildView { allTargetsToTest = filterTestsByTargets(configuredTargets, testsToRun); } - Set artifactsToBuild = new HashSet<>(); + SetMultimap topLevelArtifactsToOwnerLabels = HashMultimap.create(); Set parallelTests = new HashSet<>(); Set exclusiveTests = new HashSet<>(); @@ -386,15 +385,15 @@ public class BuildView { Collection buildInfoArtifacts = skyframeExecutor.getWorkspaceStatusArtifacts(eventHandler); Preconditions.checkState(buildInfoArtifacts.size() == 2, buildInfoArtifacts); - artifactsToBuild.addAll(buildInfoArtifacts); + addArtifactsWithNoOwner(buildInfoArtifacts, topLevelArtifactsToOwnerLabels); // Extra actions addExtraActionsIfRequested( - viewOptions, configuredTargets, aspects, artifactsToBuild, eventHandler); + viewOptions, configuredTargets, aspects, topLevelArtifactsToOwnerLabels, eventHandler); // Coverage - NestedSet baselineCoverageArtifacts = getBaselineCoverageArtifacts(configuredTargets); - Iterables.addAll(artifactsToBuild, baselineCoverageArtifacts); + NestedSet baselineCoverageArtifacts = + getBaselineCoverageArtifacts(configuredTargets, topLevelArtifactsToOwnerLabels); if (coverageReportActionFactory != null) { CoverageReportActionsWrapper actionsWrapper; actionsWrapper = @@ -409,7 +408,8 @@ public class BuildView { if (actionsWrapper != null) { ImmutableList actions = actionsWrapper.getActions(); skyframeExecutor.injectCoverageReportData(actions); - artifactsToBuild.addAll(actionsWrapper.getCoverageOutputs()); + addArtifactsWithNoOwner( + actionsWrapper.getCoverageOutputs(), topLevelArtifactsToOwnerLabels); } } @@ -453,7 +453,7 @@ public class BuildView { targetsToSkip, error, actionGraph, - artifactsToBuild, + topLevelArtifactsToOwnerLabels, parallelTests, exclusiveTests, topLevelOptions, @@ -462,6 +462,11 @@ public class BuildView { topLevelTargetsWithConfigs); } + private static void addArtifactsWithNoOwner( + Collection artifacts, SetMultimap topLevelArtifactsToOwnerLabels) { + artifacts.forEach((a) -> topLevelArtifactsToOwnerLabels.put(a, a.getOwnerLabel())); + } + @Nullable public static String createErrorMessage( LoadingResult loadingResult, @Nullable SkyframeAnalysisResult skyframeAnalysisResult) { @@ -476,11 +481,17 @@ public class BuildView { } private static NestedSet getBaselineCoverageArtifacts( - Collection configuredTargets) { + Collection configuredTargets, + SetMultimap topLevelArtifactsToOwnerLabels) { NestedSetBuilder baselineCoverageArtifacts = NestedSetBuilder.stableOrder(); for (ConfiguredTarget target : configuredTargets) { InstrumentedFilesProvider provider = target.getProvider(InstrumentedFilesProvider.class); if (provider != null) { + TopLevelArtifactHelper.addArtifactsWithOwnerLabel( + provider.getBaselineCoverageArtifacts(), + null, + target.getLabel(), + topLevelArtifactsToOwnerLabels); baselineCoverageArtifacts.addTransitive(provider.getBaselineCoverageArtifacts()); } } @@ -491,28 +502,9 @@ public class BuildView { AnalysisOptions viewOptions, Collection configuredTargets, Collection aspects, - Set artifactsToBuild, + SetMultimap artifactsToTopLevelLabelsMap, ExtendedEventHandler eventHandler) { - Iterable extraActionArtifacts = - concat( - addExtraActionsFromTargets(viewOptions, configuredTargets, eventHandler), - addExtraActionsFromAspects(viewOptions, aspects)); - RegexFilter filter = viewOptions.extraActionFilter; - for (Artifact artifact : extraActionArtifacts) { - boolean filterMatches = - filter == null || filter.isIncluded(artifact.getOwnerLabel().toString()); - if (filterMatches) { - artifactsToBuild.add(artifact); - } - } - } - - private NestedSet addExtraActionsFromTargets( - AnalysisOptions viewOptions, - Collection configuredTargets, - ExtendedEventHandler eventHandler) { - NestedSetBuilder builder = NestedSetBuilder.stableOrder(); for (ConfiguredTarget target : configuredTargets) { ExtraActionArtifactsProvider provider = target.getProvider(ExtraActionArtifactsProvider.class); @@ -530,17 +522,46 @@ public class BuildView { for (Attribute attr : actualTarget.getAssociatedRule().getAttributes()) { aspectClasses.addAll(attr.getAspectClasses()); } - - builder.addTransitive(provider.getExtraActionArtifacts()); + TopLevelArtifactHelper.addArtifactsWithOwnerLabel( + provider.getExtraActionArtifacts(), + filter, + target.getLabel(), + artifactsToTopLevelLabelsMap); if (!aspectClasses.isEmpty()) { - builder.addAll(filterTransitiveExtraActions(provider, aspectClasses)); + TopLevelArtifactHelper.addArtifactsWithOwnerLabel( + filterTransitiveExtraActions(provider, aspectClasses), + filter, + target.getLabel(), + artifactsToTopLevelLabelsMap); } } else { - builder.addTransitive(provider.getTransitiveExtraActionArtifacts()); + TopLevelArtifactHelper.addArtifactsWithOwnerLabel( + provider.getTransitiveExtraActionArtifacts(), + filter, + target.getLabel(), + artifactsToTopLevelLabelsMap); + } + } + } + for (AspectValue aspect : aspects) { + ExtraActionArtifactsProvider provider = + aspect.getConfiguredAspect().getProvider(ExtraActionArtifactsProvider.class); + if (provider != null) { + if (viewOptions.extraActionTopLevelOnly) { + TopLevelArtifactHelper.addArtifactsWithOwnerLabel( + provider.getExtraActionArtifacts(), + filter, + aspect.getLabel(), + artifactsToTopLevelLabelsMap); + } else { + TopLevelArtifactHelper.addArtifactsWithOwnerLabel( + provider.getTransitiveExtraActionArtifacts(), + filter, + aspect.getLabel(), + artifactsToTopLevelLabelsMap); } } } - return builder.build(); } /** @@ -563,23 +584,6 @@ public class BuildView { return artifacts.build(); } - private NestedSet addExtraActionsFromAspects( - AnalysisOptions viewOptions, Collection aspects) { - NestedSetBuilder builder = NestedSetBuilder.stableOrder(); - for (AspectValue aspect : aspects) { - ExtraActionArtifactsProvider provider = - aspect.getConfiguredAspect().getProvider(ExtraActionArtifactsProvider.class); - if (provider != null) { - if (viewOptions.extraActionTopLevelOnly) { - builder.addTransitive(provider.getExtraActionArtifacts()); - } else { - builder.addTransitive(provider.getTransitiveExtraActionArtifacts()); - } - } - } - return builder.build(); - } - private static void scheduleTestsIfRequested( Collection targetsToTest, Collection targetsToTestExclusive, diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java index bb714903b6..7730719fe1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java @@ -16,14 +16,17 @@ package com.google.devtools.build.lib.analysis; import static com.google.common.base.Preconditions.checkNotNull; -import com.google.common.collect.ImmutableCollection; -import com.google.common.collect.ImmutableList; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.HashMultimap; +import com.google.common.collect.SetMultimap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.test.TestProvider; +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.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.skyframe.AspectValue; +import com.google.devtools.build.lib.util.RegexFilter; import javax.annotation.Nullable; /** @@ -115,48 +118,51 @@ public final class TopLevelArtifactHelper { // Prevent instantiation. } - /** - * Utility function to form a list of all test output Artifacts of the given targets to test. - */ - public static ImmutableCollection getAllArtifactsToTest( - Iterable targets) { - if (targets == null) { - return ImmutableList.of(); + @VisibleForTesting + public static SetMultimap makeTopLevelArtifactsToOwnerLabels( + AnalysisResult analysisResult, Iterable aspects) { + SetMultimap topLevelArtifactsToOwnerLabels = + HashMultimap.create(analysisResult.getTopLevelArtifactsToOwnerLabels()); + TopLevelArtifactContext artifactContext = analysisResult.getTopLevelContext(); + for (ConfiguredTarget target : analysisResult.getTargetsToBuild()) { + addArtifactsWithOwnerLabel( + getAllArtifactsToBuild(target, artifactContext).getAllArtifacts(), + null, + target.getLabel(), + topLevelArtifactsToOwnerLabels); } - ImmutableList.Builder allTestArtifacts = ImmutableList.builder(); - for (TransitiveInfoCollection target : targets) { - allTestArtifacts.addAll(TestProvider.getTestStatusArtifacts(target)); + for (AspectValue aspect : aspects) { + addArtifactsWithOwnerLabel( + getAllArtifactsToBuild(aspect, artifactContext).getAllArtifacts(), + null, + aspect.getLabel(), + topLevelArtifactsToOwnerLabels); } - return allTestArtifacts.build(); - } - - /** - * Utility function to form a NestedSet of all top-level Artifacts of the given targets. - */ - public static ArtifactsToBuild getAllArtifactsToBuild( - Iterable targets, TopLevelArtifactContext context) { - NestedSetBuilder artifacts = NestedSetBuilder.stableOrder(); - for (TransitiveInfoCollection target : targets) { - ArtifactsToBuild targetArtifacts = getAllArtifactsToBuild(target, context); - artifacts.addTransitive(targetArtifacts.getAllArtifactsByOutputGroup()); + if (analysisResult.getTargetsToTest() != null) { + for (ConfiguredTarget target : analysisResult.getTargetsToTest()) { + addArtifactsWithOwnerLabel( + TestProvider.getTestStatusArtifacts(target), + null, + target.getLabel(), + topLevelArtifactsToOwnerLabels); + } } - return new ArtifactsToBuild(artifacts.build()); + // TODO(dslomov): Artifacts to test from aspects? + return topLevelArtifactsToOwnerLabels; } - /** - * Utility function to form a NestedSet of all top-level Artifacts of the given targets. - */ - public static ArtifactsToBuild getAllArtifactsToBuildFromAspects( - Iterable aspects, TopLevelArtifactContext context) { - NestedSetBuilder artifacts = NestedSetBuilder.stableOrder(); - for (AspectValue aspect : aspects) { - ArtifactsToBuild aspectArtifacts = getAllArtifactsToBuild(aspect, context); - artifacts.addTransitive(aspectArtifacts.getAllArtifactsByOutputGroup()); + public static void addArtifactsWithOwnerLabel( + Iterable artifacts, + @Nullable RegexFilter filter, + Label ownerLabel, + SetMultimap artifactToTopLevelLabels) { + for (Artifact artifact : artifacts) { + if (filter == null || filter.isIncluded(artifact.getOwnerLabel().toString())) { + artifactToTopLevelLabels.put(artifact, ownerLabel); + } } - return new ArtifactsToBuild(artifacts.build()); } - /** * Returns all artifacts to build if this target is requested as a top-level target. The resulting * set includes the temps and either the files to compile, if @@ -184,7 +190,7 @@ public final class TopLevelArtifactHelper { context); } - public static ArtifactsToBuild getAllArtifactsToBuild( + static ArtifactsToBuild getAllArtifactsToBuild( @Nullable OutputGroupInfo outputGroupInfo, @Nullable FileProvider fileProvider, TopLevelArtifactContext context) { 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 2e61ef8cf5..d31e0d1f4b 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 @@ -21,7 +21,7 @@ import com.google.common.base.Stopwatch; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; +import com.google.common.collect.SetMultimap; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionCacheChecker; import com.google.devtools.build.lib.actions.ActionGraph; @@ -49,6 +49,7 @@ import com.google.devtools.build.lib.analysis.actions.SymlinkTreeActionContext; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.buildtool.buildevent.ExecutionPhaseCompleteEvent; import com.google.devtools.build.lib.buildtool.buildevent.ExecutionStartingEvent; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; @@ -226,8 +227,6 @@ public class ExecutionTool { ActionGraph actionGraph = analysisResult.getActionGraph(); - // Get top-level artifacts. - ImmutableSet additionalArtifacts = analysisResult.getAdditionalArtifactsToBuild(); OutputService outputService = env.getOutputService(); ModifiedFileSet modifiedOutputFiles = ModifiedFileSet.EVERYTHING_MODIFIED; @@ -303,18 +302,8 @@ public class ExecutionTool { Set builtAspects = new HashSet<>(); Collection aspects = analysisResult.getAspects(); - Iterable allArtifactsForProviders = - Iterables.concat( - additionalArtifacts, - TopLevelArtifactHelper.getAllArtifactsToBuild( - analysisResult.getTargetsToBuild(), analysisResult.getTopLevelContext()) - .getAllArtifacts(), - TopLevelArtifactHelper.getAllArtifactsToBuildFromAspects( - aspects, analysisResult.getTopLevelContext()) - .getAllArtifacts(), - //TODO(dslomov): Artifacts to test from aspects? - TopLevelArtifactHelper.getAllArtifactsToTest(analysisResult.getTargetsToTest())); - + SetMultimap topLevelArtifactsToOwnerLabels = + TopLevelArtifactHelper.makeTopLevelArtifactsToOwnerLabels(analysisResult, aspects); if (request.isRunningInEmacs()) { // The syntax of this message is tightly constrained by lisp/progmodes/compile.el in emacs request @@ -327,7 +316,7 @@ public class ExecutionTool { for (ActionContextProvider actionContextProvider : actionContextProviders) { try (SilentCloseable c = Profiler.instance().profile(actionContextProvider + ".executionPhaseStarting")) { - actionContextProvider.executionPhaseStarting(actionGraph, allArtifactsForProviders); + actionContextProvider.executionPhaseStarting(actionGraph, topLevelArtifactsToOwnerLabels); } } executor.executionPhaseStarting(); @@ -350,7 +339,7 @@ public class ExecutionTool { builder.buildArtifacts( env.getReporter(), - additionalArtifacts, + analysisResult.getTopLevelArtifactsToOwnerLabels().keySet(), analysisResult.getParallelTests(), analysisResult.getExclusiveTests(), analysisResult.getTargetsToBuild(), diff --git a/src/main/java/com/google/devtools/build/lib/exec/ActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/exec/ActionContextProvider.java index 0afdb1366b..909f36a146 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/ActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/exec/ActionContextProvider.java @@ -13,11 +13,13 @@ // limitations under the License. package com.google.devtools.build.lib.exec; +import com.google.common.collect.SetMultimap; import com.google.devtools.build.lib.actions.ActionContext; import com.google.devtools.build.lib.actions.ActionGraph; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ExecutorInitException; import com.google.devtools.build.lib.actions.MetadataProvider; +import com.google.devtools.build.lib.cmdline.Label; /** * An object that provides execution strategies to {@link BlazeExecutor}. @@ -50,8 +52,7 @@ public abstract class ActionContextProvider { /** Called when the execution phase is started. */ public void executionPhaseStarting( - ActionGraph actionGraph, - Iterable topLevelArtifacts) + ActionGraph actionGraph, SetMultimap topLevelArtifactsToOwnerLabels) throws ExecutorInitException, InterruptedException {} /** -- cgit v1.2.3