Remove BaselineCoverageArtifactsProvider in favor of an output group.
The only slightly different thing here is that now, instead of using target.getConfiguration().isCodeCoverageEnabled() we use BuildRequest.Options.collectCodeCoverage, but the only place where this is not the same I can think of is InputFileCT, which does not have baseline coverage files anyway.
--
MOS_MIGRATED_REVID=86682774
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 ab74581..0000000
--- 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 59adaf7..91a9256 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.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 @@
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 f0742ea..daa24ec 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.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 @@
}
/**
- * 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 70510f7..b6cbdf9 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 @@
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 @@
}
}
- 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 bab5d4a..c91b90f 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 @@
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 @@
* <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 30d9a83..4933687 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.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 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 @@
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 b0882c6..7c3f2ca 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.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 @@
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 @@
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 @@
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 428eedb..7dc5d80 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.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 @@
.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 c55a74e..627040d 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.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 @@
.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 6a19f92..b341b32 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.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 @@
* 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 @@
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 9354d43..3a1a613 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.analysis.ConfiguredTarget;
import java.util.Collection;
-import java.util.Set;
import javax.annotation.Nullable;
@@ -70,6 +69,6 @@
@Nullable
public CoverageReportActionsWrapper createCoverageReportActionsWrapper(
Collection<ConfiguredTarget> targetsToTest,
- Set<Artifact> baselineCoverageArtifacts,
+ Iterable<Artifact> baselineCoverageArtifacts,
ArtifactFactory artifactFactory, ArtifactOwner artifactOwner);
}
