Partially revert change to collect baseline coverage from individual libraries.

We still want to do that, but not like this. Our infrastructure supports
per-target coverage, and so we also need to support per-target baseline
coverage.

I'm working on better documentation (not hard to be better than no docs),
which will cover this. I left a couple of TODOs to explain how we want to do
it in the future.

--
MOS_MIGRATED_REVID=103379710

5 files changed, 52 insertions, 12 deletions

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 fea33aee36..73519f95a7 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
@@ -68,6 +68,12 @@ public class OutputFileConfiguredTarget extends FileConfiguredTarget
   }
 
   @Override
+  public NestedSet<Artifact> getBaselineCoverageInstrumentedFiles() {
+    return getProvider(InstrumentedFilesProvider.class, InstrumentedFilesProviderImpl.EMPTY)
+        .getBaselineCoverageInstrumentedFiles();
+  }
+
+  @Override
   public NestedSet<Artifact> getBaselineCoverageArtifacts() {
     return getProvider(InstrumentedFilesProvider.class, InstrumentedFilesProviderImpl.EMPTY)
         .getBaselineCoverageArtifacts();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java b/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java
index a93585758d..e0a2d20f98 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java
@@ -44,7 +44,6 @@ import java.util.List;
 @VisibleForTesting
 public final class BaselineCoverageAction extends AbstractFileWriteAction
     implements NotifyOnActionCacheHit {
-
   private final Iterable<Artifact> instrumentedFiles;
 
   private BaselineCoverageAction(
@@ -112,15 +111,13 @@ public final class BaselineCoverageAction extends AbstractFileWriteAction
    * Returns collection of baseline coverage artifacts associated with the given target.
    * Will always return 0 or 1 elements.
    */
-  static NestedSet<Artifact> getBaselineCoverageArtifacts(RuleContext ruleContext,
-      Iterable<Artifact> instrumentedFiles) {
+  static NestedSet<Artifact> create(RuleContext ruleContext, Iterable<Artifact> instrumentedFiles) {
     // Baseline coverage artifacts will still go into "testlogs" directory.
     Artifact coverageData = ruleContext.getPackageRelativeArtifact(
         new PathFragment(ruleContext.getTarget().getName()).getChild("baseline_coverage.dat"),
         ruleContext.getConfiguration().getTestLogsDirectory());
     ruleContext.registerAction(new BaselineCoverageAction(
         ruleContext.getActionOwner(), instrumentedFiles, coverageData));
-
     return NestedSetBuilder.create(Order.STABLE_ORDER, coverageData);
   }
 }
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 b7828697f8..b6096b30b2 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
@@ -65,7 +65,8 @@ public final class InstrumentedFilesCollector {
 
     NestedSetBuilder<Artifact> instrumentedFilesBuilder = NestedSetBuilder.stableOrder();
     NestedSetBuilder<Artifact> metadataFilesBuilder = NestedSetBuilder.stableOrder();
-    NestedSetBuilder<Artifact> baselineCoverageArtifactsBuilder = NestedSetBuilder.stableOrder();
+    NestedSetBuilder<Artifact> baselineCoverageInstrumentedFilesBuilder =
+        NestedSetBuilder.stableOrder();
 
     Iterable<TransitiveInfoCollection> prereqs = getAllPrerequisites(ruleContext, spec);
 
@@ -75,7 +76,8 @@ public final class InstrumentedFilesCollector {
       if (provider != null) {
         instrumentedFilesBuilder.addTransitive(provider.getInstrumentedFiles());
         metadataFilesBuilder.addTransitive(provider.getInstrumentationMetadataFiles());
-        baselineCoverageArtifactsBuilder.addTransitive(provider.getBaselineCoverageArtifacts());
+        baselineCoverageInstrumentedFilesBuilder.addTransitive(
+            provider.getBaselineCoverageInstrumentedFiles());
       }
     }
 
@@ -84,6 +86,8 @@ public final class InstrumentedFilesCollector {
     if (shouldIncludeLocalSources(ruleContext)) {
       NestedSetBuilder<Artifact> localSourcesBuilder = NestedSetBuilder.stableOrder();
       for (TransitiveInfoCollection dep : prereqs) {
+        // TODO(ulfjack): Use different sets of attributes to collect transitive instrumentation
+        // data and to collect local sources, and then remove this if-statement.
         if (dep.getProvider(InstrumentedFilesProvider.class) != null) {
           continue;
         }
@@ -97,6 +101,12 @@ public final class InstrumentedFilesCollector {
       localSources = localSourcesBuilder.build();
     }
     instrumentedFilesBuilder.addTransitive(localSources);
+    if (withBaselineCoverage) {
+      // Also add the local sources to the baseline coverage instrumented sources, if the current
+      // rule supports baseline coverage.
+      // TODO(ulfjack): Generate a local baseline coverage action, and then merge at the leaves.
+      baselineCoverageInstrumentedFilesBuilder.addTransitive(localSources);
+    }
 
     // Local metadata files.
     if (localMetadataCollector != null) {
@@ -105,13 +115,15 @@ public final class InstrumentedFilesCollector {
     }
 
     // Baseline coverage actions.
-    if (withBaselineCoverage) {
-      baselineCoverageArtifactsBuilder.addTransitive(
-          BaselineCoverageAction.getBaselineCoverageArtifacts(ruleContext, localSources));
-    }
+    NestedSet<Artifact> baselineCoverageFiles = baselineCoverageInstrumentedFilesBuilder.build();
+
+    // Create one baseline coverage action per target, but for the transitive closure of files.
+    NestedSet<Artifact> baselineCoverageArtifacts =
+        BaselineCoverageAction.create(ruleContext, baselineCoverageFiles);
     return new InstrumentedFilesProviderImpl(instrumentedFilesBuilder.build(),
         metadataFilesBuilder.build(),
-        baselineCoverageArtifactsBuilder.build(),
+        baselineCoverageFiles,
+        baselineCoverageArtifacts,
         ImmutableMap.<String, String>of());
   }
 
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 2d06204bcc..24017da115 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
@@ -26,7 +26,7 @@ import java.util.Map;
 public interface InstrumentedFilesProvider extends TransitiveInfoProvider {
 
   /**
-   * Returns a collection of source files for instrumented binaries.
+   * The transitive closure of instrumented source files.
    */
   NestedSet<Artifact> getInstrumentedFiles();
 
@@ -35,6 +35,21 @@ public interface InstrumentedFilesProvider extends TransitiveInfoProvider {
    */
   NestedSet<Artifact> getInstrumentationMetadataFiles();
 
+  /**
+   * The transitive closure of instrumented source files for which baseline coverage should be
+   * generated. In general, this is a subset of the instrumented source files: it only contains
+   * instrumented source files from rules that support baseline coverage.
+   */
+  NestedSet<Artifact> getBaselineCoverageInstrumentedFiles();
+
+  /**
+   * The output artifact of the baseline coverage action; this is only ever a single artifact, which
+   * contains baseline coverage for the entire transitive closure of source files.
+   */
+  // TODO(ulfjack): Change this to a single Artifact. Also change how it's generated. It's better to
+  // generate actions such that each action only covers the source files of a single rule, in
+  // 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();
 
   /**
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 b25b632935..a56e8d89e3 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
@@ -29,18 +29,22 @@ 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),
       ImmutableMap.<String, String>of());
 
   private final NestedSet<Artifact> instrumentedFiles;
   private final NestedSet<Artifact> instrumentationMetadataFiles;
+  private final NestedSet<Artifact> baselineCoverageFiles;
   private final NestedSet<Artifact> baselineCoverageArtifacts;
   private final ImmutableMap<String, String> extraEnv;
 
   public InstrumentedFilesProviderImpl(NestedSet<Artifact> instrumentedFiles,
       NestedSet<Artifact> instrumentationMetadataFiles,
+      NestedSet<Artifact> baselineCoverageFiles,
       NestedSet<Artifact> baselineCoverageArtifacts, Map<String, String> extraEnv) {
     this.instrumentedFiles = instrumentedFiles;
     this.instrumentationMetadataFiles = instrumentationMetadataFiles;
+    this.baselineCoverageFiles = baselineCoverageFiles;
     this.baselineCoverageArtifacts = baselineCoverageArtifacts;
     this.extraEnv = ImmutableMap.copyOf(extraEnv);
   }
@@ -48,6 +52,7 @@ public final class InstrumentedFilesProviderImpl implements InstrumentedFilesPro
   public InstrumentedFilesProviderImpl(NestedSet<Artifact> instrumentedFiles,
       NestedSet<Artifact> instrumentationMetadataFiles, Map<String, String> extraEnv) {
     this(instrumentedFiles, instrumentationMetadataFiles,
+        NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER),
         NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER), extraEnv);
   }
 
@@ -62,6 +67,11 @@ public final class InstrumentedFilesProviderImpl implements InstrumentedFilesPro
   }
 
   @Override
+  public NestedSet<Artifact> getBaselineCoverageInstrumentedFiles() {
+    return baselineCoverageFiles;
+  }
+
+  @Override
   public NestedSet<Artifact> getBaselineCoverageArtifacts() {
     return baselineCoverageArtifacts;
   }