aboutsummaryrefslogtreecommitdiffhomepage
path: root/src
diff options
context:
space:
mode:
authorGravatar Lukacs Berki <lberki@google.com>2016-06-29 09:14:10 +0000
committerGravatar Damien Martin-Guillerez <dmarting@google.com>2016-06-29 10:57:53 +0000
commit8f460f399123709d5b07c68184147ab024e1c593 (patch)
tree39bee12ac3024e653c1bd30ffe39cb5d25439750 /src
parent02f2348c72f07fc4df4fbc83a86725c0e5eb9146 (diff)
Refactor how coverage support files get to test actions.
Previously we used labels in each configuration fragment that then got added to every test action. Instead, we now have a filegroup under //tools/test for coverage files that truly need to be on the inputs of every test action and collect language-specific support files in InstrumentedFilesProvider. This makes configuration creation simpler and makes it possible to turn --crosstool_top into something else other than a filegroup (previously, it was that filegroup that got added to every test action) -- MOS_MIGRATED_REVID=126170241
Diffstat (limited to 'src')
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java61
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java6
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java82
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java72
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java1
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java8
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java9
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java8
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java11
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java4
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java22
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java2
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java16
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProvider.java8
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProviderImpl.java11
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java22
-rw-r--r--src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java5
17 files changed, 130 insertions, 218 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
index 031325e121..e43c609149 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java
@@ -51,13 +51,6 @@ import java.util.List;
* Rule class definitions used by (almost) every rule.
*/
public class BaseRuleClasses {
- /**
- * Label of the pseudo-filegroup that contains all the targets that are needed
- * for running tests in coverage mode.
- */
- private static final Label COVERAGE_SUPPORT_LABEL =
- Label.parseAbsoluteUnchecked("//tools/defaults:coverage");
-
private static final Attribute.ComputedDefault testonlyDefault =
new Attribute.ComputedDefault() {
@Override
@@ -91,43 +84,10 @@ public class BaseRuleClasses {
}
};
- private static final LateBoundLabelList<BuildConfiguration> COVERAGE_SUPPORT =
- new LateBoundLabelList<BuildConfiguration>(ImmutableList.of(COVERAGE_SUPPORT_LABEL)) {
- @Override
- public List<Label> resolve(Rule rule, AttributeMap attributes,
- BuildConfiguration configuration) {
- return configuration.isCodeCoverageEnabled()
- ? ImmutableList.copyOf(configuration.getCoverageLabels())
- : ImmutableList.<Label>of();
- }
- };
-
- private static final LateBoundLabelList<BuildConfiguration> GCOV =
- new LateBoundLabelList<BuildConfiguration>(ImmutableList.of(COVERAGE_SUPPORT_LABEL)) {
- @Override
- public List<Label> resolve(Rule rule, AttributeMap attributes,
- BuildConfiguration configuration) {
- return configuration.isCodeCoverageEnabled()
- ? ImmutableList.copyOf(configuration.getGcovLabels())
- : ImmutableList.<Label>of();
- }
- };
-
- private static final LateBoundLabelList<BuildConfiguration> COVERAGE_REPORT_GENERATOR =
- new LateBoundLabelList<BuildConfiguration>(ImmutableList.of(COVERAGE_SUPPORT_LABEL)) {
- @Override
- public List<Label> resolve(Rule rule, AttributeMap attributes,
- BuildConfiguration configuration) {
- return configuration.isCodeCoverageEnabled()
- ? ImmutableList.copyOf(configuration.getCoverageReportGeneratorLabels())
- : ImmutableList.<Label>of();
- }
- };
-
/**
* Implementation for the :run_under attribute.
*/
- private static final LateBoundLabel<BuildConfiguration> RUN_UNDER =
+ public static final LateBoundLabel<BuildConfiguration> RUN_UNDER =
new LateBoundLabel<BuildConfiguration>() {
@Override
public Label resolve(Rule rule, AttributeMap attributes,
@@ -168,17 +128,18 @@ public class BaseRuleClasses {
.nonconfigurable("policy decision: should be consistent across configurations"))
.add(attr("args", STRING_LIST)
.nonconfigurable("policy decision: should be consistent across configurations"))
+ // Input files for every test action
.add(attr("$test_runtime", LABEL_LIST).cfg(HOST).value(ImmutableList.of(
env.getToolsLabel("//tools/test:runtime"))))
-
- // TODO(bazel-team): TestActions may need to be run with coverage, so all tests
- // implicitly depend on crosstool, which provides gcov. We could add gcov to
- // InstrumentedFilesProvider.getInstrumentationMetadataFiles() (or a new method) for
- // all the test rules that have C++ in their transitive closure. Then this could go.
- .add(attr(":gcov", LABEL_LIST).cfg(HOST).value(GCOV))
- .add(attr(":coverage_support", LABEL_LIST).cfg(HOST).value(COVERAGE_SUPPORT))
- .add(attr(":coverage_report_generator", LABEL_LIST).cfg(HOST)
- .value(COVERAGE_REPORT_GENERATOR))
+ // Input files for test actions collecting code coverage
+ .add(attr("$coverage_support", LABEL)
+ .cfg(HOST)
+ .value(env.getLabel("//tools/defaults:coverage_support")))
+ // Used in the one-per-build coverage report generation action.
+ .add(attr("$coverage_report_generator", LABEL)
+ .cfg(HOST)
+ .value(env.getLabel("//tools/defaults:coverage_report_generator"))
+ .singleArtifact())
// The target itself and run_under both run on the same machine. We use the DATA config
// here because the run_under acts like a data dependency (e.g. no LIPO optimization).
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java
index b24162610d..a7a56a0e6d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java
@@ -76,6 +76,12 @@ public class OutputFileConfiguredTarget extends FileConfiguredTarget
.getBaselineCoverageArtifacts();
}
+ @Override
+ public NestedSet<Artifact> getCoverageSupportFiles() {
+ return getProvider(InstrumentedFilesProvider.class, InstrumentedFilesProviderImpl.EMPTY)
+ .getCoverageSupportFiles();
+ }
+
/**
* Returns the corresponding provider from the generating rule, if it is non-null, or {@code
* defaultValue} otherwise.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 012e72062c..9005801e44 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -113,7 +113,6 @@ import javax.annotation.Nullable;
doc = "Data required for the analysis of a target that comes from targets that "
+ "depend on it and not targets that it depends on.")
public final class BuildConfiguration {
-
/**
* An interface for language-specific configurations.
*
@@ -181,27 +180,6 @@ public final class BuildConfiguration {
}
/**
- * Returns the labels required to run coverage for the fragment.
- */
- public ImmutableList<Label> getCoverageLabels() {
- return ImmutableList.of();
- }
-
- /**
- * Returns all labels required to run gcov, if provided by this fragment.
- */
- public ImmutableList<Label> getGcovLabels() {
- return ImmutableList.of();
- }
-
- /**
- * Returns the coverage report generator tool labels.
- */
- public ImmutableList<Label> getCoverageReportGeneratorLabels() {
- return ImmutableList.of();
- }
-
- /**
* Returns a fragment of the output directory name for this configuration. The output
* directory for the whole configuration contains all the short names by all fragments.
*/
@@ -644,6 +622,23 @@ public final class BuildConfiguration {
)
public boolean collectMicroCoverage;
+ @Option(name = "coverage_support",
+ converter = LabelConverter.class,
+ defaultValue = "@bazel_tools//tools/test:coverage_support",
+ category = "testing",
+ help = "Location of support files that are required on the inputs of every test action "
+ + "that collects code coverage. Defaults to '//tools/test:coverage_support'.")
+ public Label coverageSupport;
+
+ @Option(name = "coverage_report_generator",
+ converter = LabelConverter.class,
+ defaultValue = "@bazel_tools//tools/test:coverage_report_generator",
+ category = "testing",
+ help = "Location of the binary that is used to generate coverage reports. This must "
+ + "currently be a filegroup that contains a single file, the binary. Defaults to "
+ + "'//tools/test:coverage_report_generator'.")
+ public Label coverageReportGenerator;
+
@Option(name = "cache_test_results",
defaultValue = "auto",
category = "testing",
@@ -920,6 +915,12 @@ public final class BuildConfiguration {
labelMap.put("RunUnder", runUnder.getLabel());
}
}
+ @Override
+ public Map<String, Set<Label>> getDefaultsLabels(BuildConfiguration.Options commonOptions) {
+ return ImmutableMap.<String, Set<Label>>of(
+ "coverage_support", ImmutableSet.of(coverageSupport),
+ "coverage_report_generator", ImmutableSet.of(coverageReportGenerator));
+ }
}
/**
@@ -1028,10 +1029,6 @@ public final class BuildConfiguration {
/** If false, AnalysisEnviroment doesn't register any actions created by the ConfiguredTarget. */
private final boolean actionsEnabled;
- private final ImmutableSet<Label> coverageLabels;
- private final ImmutableSet<Label> coverageReportGeneratorLabels;
- private final ImmutableSet<Label> gcovLabels;
-
// TODO(bazel-team): Move this to a configuration fragment.
private final PathFragment shExecutable;
@@ -1222,18 +1219,6 @@ public final class BuildConfiguration {
? outputRoots
: new OutputRoots(directories, outputDirName);
- ImmutableSet.Builder<Label> coverageLabelsBuilder = ImmutableSet.builder();
- ImmutableSet.Builder<Label> coverageReportGeneratorLabelsBuilder = ImmutableSet.builder();
- ImmutableSet.Builder<Label> gcovLabelsBuilder = ImmutableSet.builder();
- for (Fragment fragment : fragments.values()) {
- coverageLabelsBuilder.addAll(fragment.getCoverageLabels());
- coverageReportGeneratorLabelsBuilder.addAll(fragment.getCoverageReportGeneratorLabels());
- gcovLabelsBuilder.addAll(fragment.getGcovLabels());
- }
- this.coverageLabels = coverageLabelsBuilder.build();
- this.coverageReportGeneratorLabels = coverageReportGeneratorLabelsBuilder.build();
- this.gcovLabels = gcovLabelsBuilder.build();
-
this.localShellEnvironment = setupShellEnvironment();
this.transitiveOptionsMap = computeOptionsMap(buildOptions, fragments.values());
@@ -2056,27 +2041,6 @@ public final class BuildConfiguration {
}
/**
- * Returns the set of labels for coverage.
- */
- public Set<Label> getCoverageLabels() {
- return coverageLabels;
- }
-
- /**
- * Returns the set of labels for gcov.
- */
- public Set<Label> getGcovLabels() {
- return gcovLabels;
- }
-
- /**
- * Returns the set of labels for the coverage report generator.
- */
- public Set<Label> getCoverageReportGeneratorLabels() {
- return coverageReportGeneratorLabels;
- }
-
- /**
* Returns true if bazel should show analyses results, even if it did not
* re-run the analysis.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index 8d71aa7b45..dcd9035b8f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.rules;
+import static com.google.devtools.build.lib.analysis.BaseRuleClasses.RUN_UNDER;
import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.DATA;
import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.HOST;
import static com.google.devtools.build.lib.packages.Attribute.attr;
@@ -37,8 +38,6 @@ import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.OutputGroupProvider;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
-import com.google.devtools.build.lib.analysis.config.RunUnder;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.LabelValidator;
@@ -48,8 +47,6 @@ import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
-import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel;
-import com.google.devtools.build.lib.packages.Attribute.LateBoundLabelList;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.AttributeValueSource;
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SkylarkImplicitOutputsFunctionWithCallback;
@@ -58,7 +55,6 @@ import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
import com.google.devtools.build.lib.packages.PredicateWithMessage;
-import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
@@ -112,52 +108,6 @@ public class SkylarkRuleClassFunctions {
doc = "Specifies a transition to the host configuration.")
private static final Object hostTransition = ConfigurationTransition.HOST;
- private static final LateBoundLabel<BuildConfiguration> RUN_UNDER =
- new LateBoundLabel<BuildConfiguration>() {
- @Override
- public Label resolve(Rule rule, AttributeMap attributes,
- BuildConfiguration configuration) {
- RunUnder runUnder = configuration.getRunUnder();
- return runUnder == null ? null : runUnder.getLabel();
- }
- };
-
- private static final Label COVERAGE_SUPPORT_LABEL =
- Label.parseAbsoluteUnchecked("//tools/defaults:coverage");
-
- private static final LateBoundLabelList<BuildConfiguration> GCOV =
- new LateBoundLabelList<BuildConfiguration>(ImmutableList.of(COVERAGE_SUPPORT_LABEL)) {
- @Override
- public List<Label> resolve(Rule rule, AttributeMap attributes,
- BuildConfiguration configuration) {
- return configuration.isCodeCoverageEnabled()
- ? ImmutableList.copyOf(configuration.getGcovLabels())
- : ImmutableList.<Label>of();
- }
- };
-
- private static final LateBoundLabelList<BuildConfiguration> COVERAGE_REPORT_GENERATOR =
- new LateBoundLabelList<BuildConfiguration>(ImmutableList.of(COVERAGE_SUPPORT_LABEL)) {
- @Override
- public List<Label> resolve(Rule rule, AttributeMap attributes,
- BuildConfiguration configuration) {
- return configuration.isCodeCoverageEnabled()
- ? ImmutableList.copyOf(configuration.getCoverageReportGeneratorLabels())
- : ImmutableList.<Label>of();
- }
- };
-
- private static final LateBoundLabelList<BuildConfiguration> COVERAGE_SUPPORT =
- new LateBoundLabelList<BuildConfiguration>(ImmutableList.of(COVERAGE_SUPPORT_LABEL)) {
- @Override
- public List<Label> resolve(Rule rule, AttributeMap attributes,
- BuildConfiguration configuration) {
- return configuration.isCodeCoverageEnabled()
- ? ImmutableList.copyOf(configuration.getCoverageLabels())
- : ImmutableList.<Label>of();
- }
- };
-
// TODO(bazel-team): Copied from ConfiguredRuleClassProvider for the transition from built-in
// rules to skylark extensions. Using the same instance would require a large refactoring.
// If we don't want to support old built-in rules and Skylark simultaneously
@@ -192,7 +142,7 @@ public class SkylarkRuleClassFunctions {
.build();
/** Parent rule class for test Skylark rules. */
- public static final RuleClass getTestBaseRule(String toolsRespository) {
+ public static final RuleClass getTestBaseRule(String toolsRepository) {
return new RuleClass.Builder("$test_base_rule", RuleClassType.ABSTRACT, true, baseRule)
.add(attr("size", STRING).value("medium").taggable()
.nonconfigurable("used in loading phase rule validation logic"))
@@ -218,15 +168,19 @@ public class SkylarkRuleClassFunctions {
.nonconfigurable("policy decision: this should be consistent across configurations"))
.add(attr("args", STRING_LIST)
.nonconfigurable("policy decision: should be consistent across configurations"))
+ // Input files for every test action
.add(attr("$test_runtime", LABEL_LIST).cfg(HOST).value(ImmutableList.of(
- labelCache.getUnchecked(toolsRespository + "//tools/test:runtime"))))
- .add(attr(":run_under", LABEL).cfg(DATA).value(RUN_UNDER))
- .add(attr(":gcov", LABEL_LIST).cfg(HOST).value(GCOV))
- .add(attr(":coverage_support", LABEL_LIST).cfg(HOST).value(COVERAGE_SUPPORT))
- .add(
- attr(":coverage_report_generator", LABEL_LIST)
+ labelCache.getUnchecked(toolsRepository + "//tools/test:runtime"))))
+ // Input files for test actions collecting code coverage
+ .add(attr("$coverage_support", LABEL)
+ .cfg(HOST)
+ .value(labelCache.getUnchecked("//tools/defaults:coverage_support")))
+ // Used in the one-per-build coverage report generation action.
+ .add(attr("$coverage_report_generator", LABEL)
.cfg(HOST)
- .value(COVERAGE_REPORT_GENERATOR))
+ .value(labelCache.getUnchecked("//tools/defaults:coverage_report_generator"))
+ .singleArtifact())
+ .add(attr(":run_under", LABEL).cfg(DATA).value(RUN_UNDER))
.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index 8ccd6e1dea..a4cecb798e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -497,6 +497,7 @@ public final class CcCommon {
? InstrumentedFilesProviderImpl.EMPTY
: InstrumentedFilesCollector.collect(
ruleContext, CppRuleClasses.INSTRUMENTATION_SPEC, CC_METADATA_COLLECTOR, files,
+ CppHelper.getGcovFilesIfNeeded(ruleContext),
withBaselineCoverage);
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 0d5d16430a..d2ddf4c4e5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -2000,14 +2000,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
}
@Override
- public ImmutableList<Label> getGcovLabels() {
- // TODO(bazel-team): Using a gcov-specific crosstool filegroup here could reduce the number of
- // inputs significantly. We'd also need to add logic in tools/coverage/collect_coverage.sh to
- // drop crosstool dependency if metadataFiles does not contain *.gcno artifacts.
- return ImmutableList.of(crosstoolTop);
- }
-
- @Override
public String getOutputDirectoryName() {
String lipoSuffix;
if (getLipoMode() != LipoMode.OFF && !isAutoFdoLipo()) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index fe99b08ae3..4a20c22d82 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -32,6 +32,7 @@ import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
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.packages.BuildType;
import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import com.google.devtools.build.lib.rules.cpp.CcLinkParams.Linkstamp;
@@ -222,6 +223,14 @@ public class CppHelper {
.getFdoSupport();
}
+ public static NestedSet<Artifact> getGcovFilesIfNeeded(RuleContext ruleContext) {
+ if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
+ return CppHelper.getToolchain(ruleContext).getCrosstool();
+ } else {
+ return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
+ }
+ }
+
/**
* This almost trivial method looks up the :cc_toolchain attribute on the rule context, makes sure
* that it refers to a rule that has a {@link CcToolchainProvider} (gives an error otherwise), and
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 8072197ef9..e5eb9308f8 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
@@ -636,8 +636,12 @@ public class JavaCommon {
private static InstrumentedFilesProvider getInstrumentationFilesProvider(RuleContext ruleContext,
NestedSet<Artifact> filesToBuild, InstrumentationSpec instrumentationSpec) {
return InstrumentedFilesCollector.collect(
- ruleContext, instrumentationSpec, JAVA_METADATA_COLLECTOR,
- filesToBuild, /*withBaselineCoverage*/!TargetUtils.isTestRule(ruleContext.getTarget()));
+ ruleContext,
+ instrumentationSpec,
+ JAVA_METADATA_COLLECTOR,
+ filesToBuild,
+ NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER),
+ /*withBaselineCoverage*/!TargetUtils.isTestRule(ruleContext.getTarget()));
}
public void addGenJarsProvider(RuleConfiguredTargetBuilder builder,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
index 6b71ae01eb..b93a26bc22 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
@@ -72,6 +72,7 @@ import com.google.devtools.build.lib.analysis.actions.SpawnAction;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
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;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction;
@@ -853,6 +854,15 @@ public final class CompilationSupport {
return this;
}
+ private NestedSet<Artifact> getGcovForObjectiveCIfNeeded() {
+ if (ruleContext.getConfiguration().isCodeCoverageEnabled()
+ && ruleContext.attributes().has(IosTest.OBJC_GCOV_ATTR, BuildType.LABEL)) {
+ return PrerequisiteArtifacts.nestedSet(ruleContext, IosTest.OBJC_GCOV_ATTR, Mode.HOST);
+ } else {
+ return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
+ }
+ }
+
/**
* Returns a provider that collects this target's instrumented sources as well as those of its
* dependencies.
@@ -875,6 +885,7 @@ public final class CompilationSupport {
INSTRUMENTATION_SPEC,
new ObjcCoverageMetadataCollector(),
oFiles.build(),
+ getGcovForObjectiveCIfNeeded(),
!TargetUtils.isTestRule(ruleContext.getTarget()));
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java
index 4e1560355f..3d2eb972ec 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java
@@ -32,7 +32,6 @@ import com.google.devtools.build.lib.analysis.RunfilesSupport;
import com.google.devtools.build.lib.analysis.actions.ExecutionRequirements;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
import com.google.devtools.build.lib.rules.objc.CompilationSupport.ExtraLinkArgs;
@@ -53,7 +52,7 @@ public final class IosTest implements RuleConfiguredTargetFactory {
// Attributes for IosTest rules.
// Documentation on usage is in {@link IosTestRule@}.
- static final String GCOV_ATTR = ":gcov";
+ static final String OBJC_GCOV_ATTR = "$objc_gcov";
static final String DEVICE_ARG_ATTR = "ios_device_arg";
static final String IS_XCTEST_ATTR = "xctest";
static final String MEMLEAKS_DEP_ATTR = "$memleaks_dep";
@@ -198,6 +197,7 @@ public final class IosTest implements RuleConfiguredTargetFactory {
ruleContext.getWorkspaceName(),
ruleContext.getConfiguration().legacyExternalRunfiles())
.addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES);
+
NestedSetBuilder<Artifact> filesToBuildBuilder =
NestedSetBuilder.<Artifact>stableOrder().addTransitive(filesToBuildSet);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java
index 4292f4a9b0..4d6313b50b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTestRule.java
@@ -21,7 +21,6 @@ import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
import static com.google.devtools.build.lib.syntax.Type.STRING_LIST;
-import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
@@ -29,7 +28,6 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Attribute.ComputedDefault;
import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel;
-import com.google.devtools.build.lib.packages.Attribute.LateBoundLabelList;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction;
import com.google.devtools.build.lib.packages.Rule;
@@ -40,8 +38,6 @@ import com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.BundlingRule;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.util.FileType;
-import java.util.List;
-
/**
* Rule definition for {@code ios_test} rule in Bazel.
*/
@@ -49,8 +45,6 @@ public class IosTestRule implements RuleDefinition {
@Override
public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironment env) {
- final ImmutableList<Label> gcov =
- ImmutableList.of(env.getToolsLabel("//tools/objc:gcov"));
final Label mcov = env.getToolsLabel("//tools/objc:mcov");
return builder
.requiresConfigurationFragments(
@@ -142,20 +136,10 @@ public class IosTestRule implements RuleDefinition {
.add(
attr(IosTest.MEMLEAKS_PLUGIN_ATTR, LABEL)
.value(env.getToolsLabel("//tools/objc:memleaks_plugin")))
- .override(
- attr(IosTest.GCOV_ATTR, LABEL_LIST)
+ .add(
+ attr(IosTest.OBJC_GCOV_ATTR, LABEL)
.cfg(HOST)
- .value(
- new LateBoundLabelList<BuildConfiguration>(gcov) {
- @Override
- public List<Label> resolve(
- Rule rule, AttributeMap attributes, BuildConfiguration configuration) {
- if (!configuration.isCodeCoverageEnabled()) {
- return ImmutableList.of();
- }
- return gcov;
- }
- }))
+ .value(env.getToolsLabel("//tools/objc:gcov")))
.add(
attr(IosTest.MCOV_TOOL_ATTR, LABEL)
.cfg(HOST)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java
index 7b911c5866..e813e19319 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java
@@ -265,7 +265,7 @@ public class TestSupport {
if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
envBuilder.put("COVERAGE_GCOV_PATH",
- ruleContext.getHostPrerequisiteArtifact(IosTest.GCOV_ATTR).getExecPathString());
+ ruleContext.getHostPrerequisiteArtifact(IosTest.OBJC_GCOV_ATTR).getExecPathString());
envBuilder.put("APPLE_COVERAGE", "1");
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java
index 5cc39ab059..d0357f16d7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java
@@ -62,20 +62,23 @@ public final class InstrumentedFilesCollector {
InstrumentationSpec spec,
LocalMetadataCollector localMetadataCollector,
Iterable<Artifact> rootFiles) {
- return collect(ruleContext, spec, localMetadataCollector, rootFiles, false);
+ return collect(ruleContext, spec, localMetadataCollector, rootFiles,
+ NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER), false);
}
/**
* Collects transitive instrumentation data from dependencies, collects local source files from
* dependencies, collects local metadata files by traversing the action graph of the current
- * configured target, and creates baseline coverage actions for the transitive closure of source
- * files (if <code>withBaselineCoverage</code> is true).
+ * configured target, collect rule-specific instrumentation support file sand creates baseline
+ * coverage actions for the transitive closure of source files (if
+ * <code>withBaselineCoverage</code> is true).
*/
public static InstrumentedFilesProvider collect(
RuleContext ruleContext,
InstrumentationSpec spec,
LocalMetadataCollector localMetadataCollector,
Iterable<Artifact> rootFiles,
+ NestedSet<Artifact> coverageSupportFiles,
boolean withBaselineCoverage) {
Preconditions.checkNotNull(ruleContext);
Preconditions.checkNotNull(spec);
@@ -88,6 +91,9 @@ public final class InstrumentedFilesCollector {
NestedSetBuilder<Artifact> metadataFilesBuilder = NestedSetBuilder.stableOrder();
NestedSetBuilder<Artifact> baselineCoverageInstrumentedFilesBuilder =
NestedSetBuilder.stableOrder();
+ NestedSetBuilder<Artifact> coverageSupportFilesBuilder =
+ NestedSetBuilder.<Artifact>stableOrder()
+ .addTransitive(coverageSupportFiles);
// Transitive instrumentation data.
for (TransitiveInfoCollection dep :
@@ -98,6 +104,7 @@ public final class InstrumentedFilesCollector {
metadataFilesBuilder.addTransitive(provider.getInstrumentationMetadataFiles());
baselineCoverageInstrumentedFilesBuilder.addTransitive(
provider.getBaselineCoverageInstrumentedFiles());
+ coverageSupportFilesBuilder.addTransitive(provider.getCoverageSupportFiles());
}
}
@@ -143,7 +150,8 @@ public final class InstrumentedFilesCollector {
instrumentedFilesBuilder.build(),
metadataFilesBuilder.build(),
baselineCoverageFiles,
- baselineCoverageArtifacts);
+ baselineCoverageArtifacts,
+ coverageSupportFilesBuilder.build());
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProvider.java
index 3677868346..23e5e68733 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProvider.java
@@ -49,4 +49,12 @@ public interface InstrumentedFilesProvider extends TransitiveInfoProvider {
// particular because baseline coverage is language-specific (it requires a parser for the
// specific language), and we don't want to depend on all language parsers from any single rule.
NestedSet<Artifact> getBaselineCoverageArtifacts();
+
+ /**
+ * Extra files that are needed on the inputs of test actions for coverage collection to happen,
+ * for example, {@code gcov}.
+ *
+ * <p>They aren't mentioned in the instrumented files manifest.
+ */
+ NestedSet<Artifact> getCoverageSupportFiles();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProviderImpl.java b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProviderImpl.java
index 279117dec1..3397d2d35d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProviderImpl.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesProviderImpl.java
@@ -27,22 +27,26 @@ public final class InstrumentedFilesProviderImpl implements InstrumentedFilesPro
NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER),
NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER),
NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER),
+ NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER),
NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER));
private final NestedSet<Artifact> instrumentedFiles;
private final NestedSet<Artifact> instrumentationMetadataFiles;
private final NestedSet<Artifact> baselineCoverageFiles;
private final NestedSet<Artifact> baselineCoverageArtifacts;
+ private final NestedSet<Artifact> coverageSupportFiles;
public InstrumentedFilesProviderImpl(
NestedSet<Artifact> instrumentedFiles,
NestedSet<Artifact> instrumentationMetadataFiles,
NestedSet<Artifact> baselineCoverageFiles,
- NestedSet<Artifact> baselineCoverageArtifacts) {
+ NestedSet<Artifact> baselineCoverageArtifacts,
+ NestedSet<Artifact> coverageSupportFiles) {
this.instrumentedFiles = instrumentedFiles;
this.instrumentationMetadataFiles = instrumentationMetadataFiles;
this.baselineCoverageFiles = baselineCoverageFiles;
this.baselineCoverageArtifacts = baselineCoverageArtifacts;
+ this.coverageSupportFiles = coverageSupportFiles;
}
@Override
@@ -64,4 +68,9 @@ public final class InstrumentedFilesProviderImpl implements InstrumentedFilesPro
public NestedSet<Artifact> getBaselineCoverageArtifacts() {
return baselineCoverageArtifacts;
}
+
+ @Override
+ public NestedSet<Artifact> getCoverageSupportFiles() {
+ return coverageSupportFiles;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java
index 44ed5ad4a6..6f3614f8e3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java
@@ -20,13 +20,11 @@ import com.google.common.collect.Lists;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Root;
import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
-import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts;
import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.RunfilesSupport;
-import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -201,18 +199,14 @@ public final class TestActionBuilder {
TestTargetExecutionSettings executionSettings;
if (collectCodeCoverage) {
+ inputsBuilder.addTransitive(instrumentedFiles.getCoverageSupportFiles());
// Add instrumented file manifest artifact to the list of inputs. This file will contain
// exec paths of all source files that should be included into the code coverage output.
NestedSet<Artifact> metadataFiles = instrumentedFiles.getInstrumentationMetadataFiles();
inputsBuilder.addTransitive(metadataFiles);
- for (TransitiveInfoCollection dep :
- ruleContext.getPrerequisites(":coverage_support", Mode.HOST)) {
- inputsBuilder.addTransitive(dep.getProvider(FileProvider.class).getFilesToBuild());
- }
- for (TransitiveInfoCollection dep :
- ruleContext.getPrerequisites(":gcov", Mode.HOST)) {
- inputsBuilder.addTransitive(dep.getProvider(FileProvider.class).getFilesToBuild());
- }
+ inputsBuilder.addTransitive(PrerequisiteArtifacts.nestedSet(
+ ruleContext, "$coverage_support", Mode.HOST));
+
Artifact instrumentedFileManifest =
InstrumentedFileManifestAction.getInstrumentedFileManifest(ruleContext,
instrumentedFiles.getInstrumentedFiles(), metadataFiles);
@@ -273,8 +267,12 @@ public final class TestActionBuilder {
}
}
// TODO(bazel-team): Passing the reportGenerator to every TestParams is a bit strange.
- Artifact reportGenerator = collectCodeCoverage
- ? ruleContext.getPrerequisiteArtifact(":coverage_report_generator", Mode.HOST) : null;
+ Artifact reportGenerator = null;
+ if (collectCodeCoverage) {
+ reportGenerator = ruleContext.getPrerequisiteArtifact(
+ "$coverage_report_generator", Mode.HOST);
+ }
+
return new TestParams(runsPerTest, shards, TestTimeout.getTestTimeout(ruleContext.getRule()),
ruleContext.getRule().getRuleClass(), ImmutableList.copyOf(results),
coverageArtifacts.build(), reportGenerator);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
index 6606fc23b8..2cf963e4b3 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
@@ -132,7 +132,10 @@ public final class BazelAnalysisMock extends AnalysisMock {
"java_import(name = 'jarjar_import',",
" jars = [ 'jarjar.jar' ])");
- config.create("/bazel_tools_workspace/tools/test/BUILD", "filegroup(name = 'runtime')");
+ config.create("/bazel_tools_workspace/tools/test/BUILD",
+ "filegroup(name = 'runtime')",
+ "filegroup(name = 'coverage_support')",
+ "filegroup(name = 'coverage_report_generator', srcs = ['coverage_report_generator.sh'])");
config.create(
"/bazel_tools_workspace/tools/python/BUILD",