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
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 fea33ae..73519f9 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 @@
}
@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 a935857..e0a2d20 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 @@
@VisibleForTesting
public final class BaselineCoverageAction extends AbstractFileWriteAction
implements NotifyOnActionCacheHit {
-
private final Iterable<Artifact> instrumentedFiles;
private BaselineCoverageAction(
@@ -112,15 +111,13 @@
* 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 b782869..b6096b3 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 @@
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 @@
if (provider != null) {
instrumentedFilesBuilder.addTransitive(provider.getInstrumentedFiles());
metadataFilesBuilder.addTransitive(provider.getInstrumentationMetadataFiles());
- baselineCoverageArtifactsBuilder.addTransitive(provider.getBaselineCoverageArtifacts());
+ baselineCoverageInstrumentedFilesBuilder.addTransitive(
+ provider.getBaselineCoverageInstrumentedFiles());
}
}
@@ -84,6 +86,8 @@
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 @@
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 @@
}
// 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 2d06204..24017da 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 @@
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 @@
*/
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 b25b632..a56e8d8 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 @@
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 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 @@
}
@Override
+ public NestedSet<Artifact> getBaselineCoverageInstrumentedFiles() {
+ return baselineCoverageFiles;
+ }
+
+ @Override
public NestedSet<Artifact> getBaselineCoverageArtifacts() {
return baselineCoverageArtifacts;
}
