From 8f460f399123709d5b07c68184147ab024e1c593 Mon Sep 17 00:00:00 2001
From: Lukacs Berki <lberki@google.com>
Date: Wed, 29 Jun 2016 09:14:10 +0000
Subject: 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
---
 .../build/lib/analysis/BaseRuleClasses.java        | 61 +++-------------
 .../lib/analysis/OutputFileConfiguredTarget.java   |  6 ++
 .../lib/analysis/config/BuildConfiguration.java    | 82 ++++++----------------
 .../build/lib/rules/SkylarkRuleClassFunctions.java | 72 ++++---------------
 .../devtools/build/lib/rules/cpp/CcCommon.java     |  1 +
 .../build/lib/rules/cpp/CppConfiguration.java      |  8 ---
 .../devtools/build/lib/rules/cpp/CppHelper.java    |  9 +++
 .../devtools/build/lib/rules/java/JavaCommon.java  |  8 ++-
 .../build/lib/rules/objc/CompilationSupport.java   | 11 +++
 .../devtools/build/lib/rules/objc/IosTest.java     |  4 +-
 .../devtools/build/lib/rules/objc/IosTestRule.java | 22 +-----
 .../devtools/build/lib/rules/objc/TestSupport.java |  2 +-
 .../lib/rules/test/InstrumentedFilesCollector.java | 16 +++--
 .../lib/rules/test/InstrumentedFilesProvider.java  |  8 +++
 .../rules/test/InstrumentedFilesProviderImpl.java  | 11 ++-
 .../build/lib/rules/test/TestActionBuilder.java    | 22 +++---
 16 files changed, 126 insertions(+), 217 deletions(-)

(limited to 'src/main/java')

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.
    *
@@ -180,27 +179,6 @@ public final class BuildConfiguration {
       return ImmutableMap.of();
     }
 
-    /**
-     * 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());
@@ -2055,27 +2040,6 @@ public final class BuildConfiguration {
     return options.instrumentTestTargets;
   }
 
-  /**
-   * 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
@@ -1999,14 +1999,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     return env.build();
   }
 
-  @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;
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);
-- 
cgit v1.2.3