diff options
author | 2016-06-29 09:14:10 +0000 | |
---|---|---|
committer | 2016-06-29 10:57:53 +0000 | |
commit | 8f460f399123709d5b07c68184147ab024e1c593 (patch) | |
tree | 39bee12ac3024e653c1bd30ffe39cb5d25439750 /src | |
parent | 02f2348c72f07fc4df4fbc83a86725c0e5eb9146 (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')
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", |