diff options
11 files changed, 69 insertions, 98 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaselineCoverageArtifactsProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/BaselineCoverageArtifactsProvider.java deleted file mode 100644 index ab74581dfe..0000000000 --- a/src/main/java/com/google/devtools/build/lib/analysis/BaselineCoverageArtifactsProvider.java +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2014 Google Inc. 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; - -import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; - -/** - * A {@link TransitiveInfoProvider} that has baseline coverage artifacts. - */ -@Immutable -public final class BaselineCoverageArtifactsProvider implements TransitiveInfoProvider { - private final ImmutableList<Artifact> baselineCoverageArtifacts; - - public BaselineCoverageArtifactsProvider(ImmutableList<Artifact> baselineCoverageArtifacts) { - this.baselineCoverageArtifacts = baselineCoverageArtifacts; - } - - /** - * Returns a set of baseline coverage artifacts for a given set of configured targets. - * - * <p>These artifacts represent "empty" code coverage data for non-test libraries and binaries and - * used to establish correct baseline when calculating code coverage ratios since they would cover - * completely non-tested code as well. - */ - public ImmutableList<Artifact> getBaselineCoverageArtifacts() { - return baselineCoverageArtifacts; - } -} 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 59adaf7892..91a92560ce 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 @@ -40,6 +40,7 @@ import com.google.devtools.build.lib.analysis.config.BinTools; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; @@ -767,17 +768,18 @@ public class BuildView { artifactsToBuild, parallelTests, exclusiveTests, topLevelOptions); } - private static ImmutableSet<Artifact> getBaselineCoverageArtifacts( + private static NestedSet<Artifact> getBaselineCoverageArtifacts( Collection<ConfiguredTarget> configuredTargets) { - Set<Artifact> baselineCoverageArtifacts = Sets.newHashSet(); + NestedSetBuilder<Artifact> baselineCoverageArtifacts = NestedSetBuilder.stableOrder(); for (ConfiguredTarget target : configuredTargets) { - BaselineCoverageArtifactsProvider provider = - target.getProvider(BaselineCoverageArtifactsProvider.class); + TopLevelArtifactProvider provider = target.getProvider(TopLevelArtifactProvider.class); if (provider != null) { - baselineCoverageArtifacts.addAll(provider.getBaselineCoverageArtifacts()); + baselineCoverageArtifacts.addTransitive(provider.getOutputGroup( + TopLevelArtifactProvider.BASELINE_COVERAGE + )); } } - return ImmutableSet.copyOf(baselineCoverageArtifacts); + return baselineCoverageArtifacts.build(); } private void addExtraActionsIfRequested(BuildView.Options viewOptions, diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java index f0742ea96f..daa24ec380 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java @@ -52,7 +52,6 @@ import com.google.devtools.build.lib.syntax.Label; import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; -import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -419,15 +418,6 @@ public final class RuleConfiguredTargetBuilder { } /** - * Set the baseline coverage Artifacts. - */ - public RuleConfiguredTargetBuilder setBaselineCoverageArtifacts( - Collection<Artifact> artifacts) { - return add(BaselineCoverageArtifactsProvider.class, - new BaselineCoverageArtifactsProvider(ImmutableList.copyOf(artifacts))); - } - - /** * Set the mandatory stamp files. */ public RuleConfiguredTargetBuilder setMandatoryStampFiles(ImmutableList<Artifact> files) { 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 70510f702c..b6cbdf91e0 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 @@ -118,8 +118,12 @@ public final class TopLevelArtifactHelper { for (String outputGroup : context.outputGroups()) { NestedSet<Artifact> results = topLevelArtifactProvider.getOutputGroup(outputGroup); if (results != null) { - importantBuilder.addTransitive(results); - } + if (outputGroup.startsWith(TopLevelArtifactProvider.HIDDEN_OUTPUT_GROUP_PREFIX)) { + allBuilder.addTransitive(results); + } else { + importantBuilder.addTransitive(results); + } + } } } @@ -157,29 +161,8 @@ public final class TopLevelArtifactHelper { } } - allBuilder.addAll(getCoverageArtifacts(target, context)); - NestedSet<Artifact> importantArtifacts = importantBuilder.build(); allBuilder.addTransitive(importantArtifacts); return new ArtifactsToBuild(importantArtifacts, allBuilder.build()); } - - private static Iterable<Artifact> getCoverageArtifacts(TransitiveInfoCollection target, - TopLevelArtifactContext topLevelOptions) { - if (!topLevelOptions.compileOnly() && !topLevelOptions.compilationPrerequisitesOnly() - && topLevelOptions.shouldRunTests()) { - // Add baseline code coverage artifacts if we are collecting code coverage. We do that only - // when running tests. - // It might be slightly faster to first check if any configuration has coverage enabled. - if (target.getConfiguration() != null - && target.getConfiguration().isCodeCoverageEnabled()) { - BaselineCoverageArtifactsProvider provider = - target.getProvider(BaselineCoverageArtifactsProvider.class); - if (provider != null) { - return provider.getBaselineCoverageArtifacts(); - } - } - } - return ImmutableList.of(); - } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactProvider.java index bab5d4a1de..c91b90f3dd 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactProvider.java @@ -17,6 +17,8 @@ package com.google.devtools.build.lib.analysis; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; /** @@ -27,17 +29,30 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; * <p>The artifacts are grouped into "output groups". Which output groups are built is controlled * by the {@code --output_groups} undocumented command line option, which in turn is added to the * command line at the discretion of the build command being run. + * + * <p>Output groups starting with an underscore are "not important". This means that artifacts built + * because such an output group is mentioned in a {@code --output_groups} command line option are + * not mentioned on the output. */ @Immutable public final class TopLevelArtifactProvider implements TransitiveInfoProvider { + public static final String HIDDEN_OUTPUT_GROUP_PREFIX = "_"; + public static final String BASELINE_COVERAGE = HIDDEN_OUTPUT_GROUP_PREFIX + "_baseline_coverage"; + private final ImmutableMap<String, NestedSet<Artifact>> outputGroups; TopLevelArtifactProvider(ImmutableMap<String, NestedSet<Artifact>> outputGroups) { this.outputGroups = outputGroups; } - /** Return the artifacts in a particular output group. */ + /** Return the artifacts in a particular output group. + * + * @return the artifacts in the output group with the given name. The return value is never null. + * If the specified output group is not present, the empty set is returned. + */ public NestedSet<Artifact> getOutputGroup(String outputGroupName) { - return outputGroups.get(outputGroupName); + return outputGroups.containsKey(outputGroupName) + ? outputGroups.get(outputGroupName) + : NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER); } } diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java index 30d9a83c33..49336877fd 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java @@ -24,6 +24,8 @@ import com.google.common.collect.ImmutableSortedSet; import com.google.devtools.build.lib.Constants; import com.google.devtools.build.lib.analysis.BuildView; import com.google.devtools.build.lib.analysis.TopLevelArtifactContext; +import com.google.devtools.build.lib.analysis.TopLevelArtifactProvider; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.pkgcache.LoadingPhaseRunner; @@ -43,6 +45,8 @@ import com.google.devtools.common.options.OptionsProvider; import java.util.ArrayList; import java.util.List; +import java.util.Set; +import java.util.TreeSet; import java.util.UUID; import java.util.concurrent.ExecutionException; @@ -512,7 +516,20 @@ public class BuildRequest implements OptionsClassProvider { getBuildOptions().compileOnly, getBuildOptions().compilationPrerequisitesOnly, getBuildOptions().buildDefaultArtifacts, getOptions(ExecutionOptions.class).testStrategy.equals("exclusive"), - ImmutableSet.<String>copyOf(getBuildOptions().outputGroups), shouldRunTests()); + determineOutputGroups(), shouldRunTests()); + } + + private ImmutableSet<String> determineOutputGroups() { + Set<String> current = new TreeSet<>(); + current.addAll(getBuildOptions().outputGroups); + if (getOptions(Options.class).collectCodeCoverage + && !getBuildOptions().compileOnly + && !getBuildOptions().compilationPrerequisitesOnly + && runTests) { + current.add(TopLevelArtifactProvider.BASELINE_COVERAGE); + } + + return ImmutableSet.copyOf(current); } public String getSymlinkPrefix() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java index b0882c677d..7c3f2ca036 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.RunfilesSupport; +import com.google.devtools.build.lib.analysis.TopLevelArtifactProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.Util; import com.google.devtools.build.lib.analysis.actions.FileWriteAction; @@ -303,11 +304,11 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { CppDebugPackageProvider.class, new CppDebugPackageProvider(strippedFile, executable, explicitDwpFile)) .setRunfilesSupport(runfilesSupport, executable) - .setBaselineCoverageArtifacts(createBaselineCoverageArtifacts( - ruleContext, common, ccCompilationOutputs, fake)) .addProvider(LipoContextProvider.class, new LipoContextProvider( cppCompilationContext, ImmutableMap.copyOf(scannableMap))) .addProvider(CppLinkAction.Context.class, linkContext) + .addOutputGroup(TopLevelArtifactProvider.BASELINE_COVERAGE, + createBaselineCoverageArtifacts(ruleContext, common, ccCompilationOutputs, fake)) .build(); } @@ -623,7 +624,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { return builder.build(); } - private static ImmutableList<Artifact> createBaselineCoverageArtifacts( + private static NestedSet<Artifact> createBaselineCoverageArtifacts( RuleContext context, CcCommon common, CcCompilationOutputs compilationOutputs, boolean fake) { if (!TargetUtils.isTestRule(context.getRule()) && !fake) { @@ -632,7 +633,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { return BaselineCoverageAction.getBaselineCoverageArtifacts(context, common.getInstrumentedFilesProvider(objectFiles).getInstrumentedFiles()); } else { - return ImmutableList.of(); + return NestedSetBuilder.emptySet(Order.STABLE_ORDER); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java index 428eedb6e2..7dc5d80fc9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesProvider; +import com.google.devtools.build.lib.analysis.TopLevelArtifactProvider; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; @@ -262,12 +263,14 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { .add(RunfilesProvider.class, RunfilesProvider.withData(staticRunfiles, sharedRunfiles)) // Remove this? .add(CppRunfilesProvider.class, new CppRunfilesProvider(staticRunfiles, sharedRunfiles)) - .setBaselineCoverageArtifacts(BaselineCoverageAction.getBaselineCoverageArtifacts( - ruleContext, instrumentedFilesProvider.getInstrumentedFiles())) .add(ImplementedCcPublicLibrariesProvider.class, new ImplementedCcPublicLibrariesProvider(getImplementedCcPublicLibraries(ruleContext))) .add(AlwaysBuiltArtifactsProvider.class, - new AlwaysBuiltArtifactsProvider(artifactsToForce)); + new AlwaysBuiltArtifactsProvider(artifactsToForce)) + .addOutputGroup(TopLevelArtifactProvider.BASELINE_COVERAGE, + BaselineCoverageAction.getBaselineCoverageArtifacts( + ruleContext, instrumentedFilesProvider.getInstrumentedFiles())); + } private static NestedSet<Artifact> collectArtifactsToForce(RuleContext ruleContext, diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java index c55a74e0bb..627040ddad 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.analysis.FilesToCompileProvider; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.TopLevelArtifactProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.analysis.Util; @@ -491,10 +492,9 @@ public class JavaCommon { .add(JavaExportsProvider.class, new JavaExportsProvider(collectTransitiveExports())); if (!TargetUtils.isTestRule(ruleContext.getTarget())) { - ImmutableList<Artifact> baselineCoverageArtifacts = + builder.addOutputGroup(TopLevelArtifactProvider.BASELINE_COVERAGE, BaselineCoverageAction.getBaselineCoverageArtifacts(ruleContext, - instrumentedFilesCollector.getInstrumentedFiles()); - builder.setBaselineCoverageArtifacts(baselineCoverageArtifacts); + instrumentedFilesCollector.getInstrumentedFiles())); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java b/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java index 6a19f92def..b341b32856 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java @@ -24,6 +24,9 @@ import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Util; import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.syntax.Label; import com.google.devtools.build.lib.util.Fingerprint; @@ -117,7 +120,7 @@ public class BaselineCoverageAction extends AbstractFileWriteAction * Returns collection of baseline coverage artifacts associated with the given target. * Will always return 0 or 1 elements. */ - public static ImmutableList<Artifact> getBaselineCoverageArtifacts(RuleContext ruleContext, + public static NestedSet<Artifact> getBaselineCoverageArtifacts(RuleContext ruleContext, Iterable<Artifact> instrumentedFiles) { // Baseline coverage artifacts will still go into "testlogs" directory. Artifact coverageData = ruleContext.getAnalysisEnvironment().getDerivedArtifact( @@ -126,7 +129,7 @@ public class BaselineCoverageAction extends AbstractFileWriteAction ruleContext.registerAction(new BaselineCoverageAction( ruleContext.getActionOwner(), instrumentedFiles, coverageData)); - return ImmutableList.of(coverageData); + return NestedSetBuilder.create(Order.STABLE_ORDER, coverageData); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/CoverageReportActionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/test/CoverageReportActionFactory.java index 9354d4340b..3a1a6137d8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/CoverageReportActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/CoverageReportActionFactory.java @@ -23,7 +23,6 @@ import com.google.devtools.build.lib.actions.ArtifactOwner; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import java.util.Collection; -import java.util.Set; import javax.annotation.Nullable; @@ -70,6 +69,6 @@ public interface CoverageReportActionFactory { @Nullable public CoverageReportActionsWrapper createCoverageReportActionsWrapper( Collection<ConfiguredTarget> targetsToTest, - Set<Artifact> baselineCoverageArtifacts, + Iterable<Artifact> baselineCoverageArtifacts, ArtifactFactory artifactFactory, ArtifactOwner artifactOwner); } |