Switch objc rules to standard coverage propagation using InstrumentedFilesCollector.
--
MOS_MIGRATED_REVID=103642172
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 73519f9..bd5bc75 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
@@ -15,15 +15,12 @@
package com.google.devtools.build.lib.analysis;
import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.packages.OutputFile;
import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.rules.test.InstrumentedFilesProviderImpl;
-import java.util.Map;
-
/**
* A ConfiguredTarget for an OutputFile.
*/
@@ -79,11 +76,6 @@
.getBaselineCoverageArtifacts();
}
- @Override
- public Map<String, String> getExtraEnv() {
- return ImmutableMap.of();
- }
-
/**
* 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/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index bec3a9b..887a03c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -43,6 +43,7 @@
import com.google.devtools.build.lib.rules.test.ExecutionInfoProvider;
import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.rules.test.TestActionBuilder;
+import com.google.devtools.build.lib.rules.test.TestEnvironmentProvider;
import com.google.devtools.build.lib.rules.test.TestProvider;
import com.google.devtools.build.lib.rules.test.TestProvider.TestParams;
import com.google.devtools.build.lib.syntax.ClassObject;
@@ -165,19 +166,21 @@
"Having more than 50 shards is indicative of poor test organization. "
+ "Please reduce the number of shards.");
}
- TestActionBuilder testActionBuilder = new TestActionBuilder(ruleContext);
- InstrumentedFilesProvider instrumentedFilesProvider =
- findProvider(InstrumentedFilesProvider.class);
- if (instrumentedFilesProvider != null) {
- testActionBuilder
- .setInstrumentedFiles(instrumentedFilesProvider)
- .setExtraEnv(instrumentedFilesProvider.getExtraEnv());
+ TestActionBuilder testActionBuilder =
+ new TestActionBuilder(ruleContext)
+ .setInstrumentedFiles(findProvider(InstrumentedFilesProvider.class));
+
+ TestEnvironmentProvider environmentProvider = findProvider(TestEnvironmentProvider.class);
+ if (environmentProvider != null) {
+ testActionBuilder.setExtraEnv(environmentProvider.getEnvironment());
}
- final TestParams testParams = testActionBuilder
- .setFilesToRunProvider(filesToRunProvider)
- .setExecutionRequirements(findProvider(ExecutionInfoProvider.class))
- .setShardCount(explicitShardCount)
- .build();
+
+ final TestParams testParams =
+ testActionBuilder
+ .setFilesToRunProvider(filesToRunProvider)
+ .setExecutionRequirements(findProvider(ExecutionInfoProvider.class))
+ .setShardCount(explicitShardCount)
+ .build();
final ImmutableList<String> testTags =
ImmutableList.copyOf(ruleContext.getRule().getRuleTags());
return new TestProvider(testParams, testTags);
@@ -233,9 +236,9 @@
List<Label> actionListenerLabels = configuration.getActionListeners();
if (!actionListenerLabels.isEmpty()
- && ruleContext.getRule().getAttributeDefinition(":action_listener") != null) {
- ExtraActionsVisitor visitor = new ExtraActionsVisitor(ruleContext,
- computeMnemonicsToExtraActionMap());
+ && ruleContext.attributes().getAttributeDefinition(":action_listener") != null) {
+ ExtraActionsVisitor visitor =
+ new ExtraActionsVisitor(ruleContext, computeMnemonicsToExtraActionMap());
// The action list is modified within the body of the loop by the addExtraAction() call,
// thus the copy
@@ -446,7 +449,7 @@
return result;
}
- result = NestedSetBuilder.<Artifact>stableOrder();
+ result = NestedSetBuilder.stableOrder();
outputGroupBuilders.put(name, result);
return result;
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/objc/BazelIosTest.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/objc/BazelIosTest.java
index 150041b..c92e385 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/objc/BazelIosTest.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/objc/BazelIosTest.java
@@ -44,10 +44,11 @@
NestedSetBuilder<Artifact> filesToBuildBuilder = NestedSetBuilder.<Artifact>stableOrder()
.addTransitive(filesToBuild);
- TestSupport testSupport = new TestSupport(ruleContext)
- .registerTestRunnerActions()
- .addRunfiles(runfilesBuilder, common.getObjcProvider())
- .addFilesToBuild(filesToBuildBuilder);
+ TestSupport testSupport =
+ new TestSupport(ruleContext)
+ .registerTestRunnerActions()
+ .addRunfiles(runfilesBuilder)
+ .addFilesToBuild(filesToBuildBuilder);
Artifact executable = testSupport.generatedTestScript();
@@ -59,9 +60,10 @@
.setFilesToBuild(filesToBuildBuilder.build())
.add(XcodeProvider.class, xcodeProvider)
.add(RunfilesProvider.class, RunfilesProvider.simple(runfiles))
- .add(ExecutionInfoProvider.class,
+ .add(
+ ExecutionInfoProvider.class,
new ExecutionInfoProvider(ImmutableMap.of(ExecutionRequirements.REQUIRES_DARWIN, "")))
- .addProviders(testSupport.getExtraProviders(common.getObjcProvider()))
+ .addProviders(testSupport.getExtraProviders())
.setRunfilesSupport(runfilesSupport, executable)
.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java b/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
index 9e757e2..da2607a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
@@ -35,6 +35,7 @@
import com.google.devtools.build.lib.rules.objc.ObjcCommon.CompilationAttributes;
import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes;
import com.google.devtools.build.lib.rules.objc.ReleaseBundlingSupport.LinkedBinary;
+import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
/**
* Implementation for rules that link binaries.
@@ -95,13 +96,14 @@
return null;
}
- new CompilationSupport(ruleContext)
- .registerJ2ObjcCompileAndArchiveActions(objcProvider)
- .registerCompileAndArchiveActions(common)
- .addXcodeSettings(xcodeProviderBuilder, common)
- .registerLinkActions(objcProvider, getExtraLinkArgs(ruleContext),
- ImmutableList.<Artifact>of())
- .validateAttributes();
+ CompilationSupport compilationSupport =
+ new CompilationSupport(ruleContext)
+ .registerJ2ObjcCompileAndArchiveActions(objcProvider)
+ .registerCompileAndArchiveActions(common)
+ .addXcodeSettings(xcodeProviderBuilder, common)
+ .registerLinkActions(
+ objcProvider, getExtraLinkArgs(ruleContext), ImmutableList.<Artifact>of())
+ .validateAttributes();
if (ruleContext.hasErrors()) {
return null;
@@ -157,7 +159,10 @@
RuleConfiguredTargetBuilder targetBuilder =
ObjcRuleClasses.ruleConfiguredTarget(ruleContext, filesToBuild.build())
.addProvider(XcodeProvider.class, xcodeProvider)
- .addProvider(ObjcProvider.class, objcProvider);
+ .addProvider(ObjcProvider.class, objcProvider)
+ .addProvider(
+ InstrumentedFilesProvider.class,
+ compilationSupport.getInstrumentedFilesProvider(common));
if (xcTestAppProvider.isPresent()) {
// TODO(bazel-team): Stop exporting an XcTestAppProvider once objc_binary no longer creates an
// application bundle.
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 1ea050a..e20ae81 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
@@ -41,6 +41,7 @@
import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.SRCS_TYPE;
import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.STRIP;
import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.SWIFT;
+import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.intermediateArtifacts;
import static java.nio.charset.StandardCharsets.ISO_8859_1;
import com.google.common.annotations.VisibleForTesting;
@@ -56,6 +57,7 @@
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ParameterFile;
+import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
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;
@@ -66,6 +68,7 @@
import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction;
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
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.BuildType;
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction;
import com.google.devtools.build.lib.packages.TargetUtils;
@@ -75,7 +78,12 @@
import com.google.devtools.build.lib.rules.java.J2ObjcConfiguration;
import com.google.devtools.build.lib.rules.objc.ObjcCommon.CompilationAttributes;
import com.google.devtools.build.lib.rules.objc.XcodeProvider.Builder;
+import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector;
+import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector.InstrumentationSpec;
+import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector.LocalMetadataCollector;
+import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.shell.ShellUtils;
+import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -106,6 +114,21 @@
static final ImmutableList<String> CLANG_COVERAGE_FLAGS =
ImmutableList.of("-fprofile-arcs", "-ftest-coverage");
+ /**
+ * Files which can be instrumented along with the attributes in which they may occur and the
+ * attributes along which they are propagated from dependencies (via
+ * {@link InstrumentedFilesProvider}).
+ */
+ private static final InstrumentationSpec INSTRUMENTATION_SPEC =
+ new InstrumentationSpec(
+ FileTypeSet.of(
+ ObjcRuleClasses.NON_CPP_SOURCES,
+ ObjcRuleClasses.CPP_SOURCES,
+ ObjcRuleClasses.SWIFT_SOURCES,
+ HEADERS))
+ .withSourceAttributes("srcs", "non_arc_srcs", "hdrs")
+ .withDependencyAttributes("deps", "data", "binary", "xctest_app");
+
private static final String FRAMEWORK_SUFFIX = ".framework";
private static final Predicate<String> INCLUDE_DIR_OPTION_IN_COPTS =
@@ -191,7 +214,7 @@
if (ObjcRuleClasses.objcConfiguration(ruleContext).moduleMapsEnabled()) {
moduleMap = Optional.of(intermediateArtifacts.moduleMap());
} else {
- moduleMap = Optional.<CppModuleMap>absent();
+ moduleMap = Optional.absent();
}
registerCompileAndArchiveActions(
common.getCompilationArtifacts().get(),
@@ -543,6 +566,32 @@
}
/**
+ * Returns a provider that collects this target's instrumented sources as well as those of its
+ * dependencies.
+ *
+ * @param common common information about this rule and its dependencies
+ * @return an instrumented files provider
+ */
+ public InstrumentedFilesProvider getInstrumentedFilesProvider(ObjcCommon common) {
+ ImmutableList.Builder<Artifact> oFiles = ImmutableList.builder();
+
+ if (common.getCompilationArtifacts().isPresent()) {
+ CompilationArtifacts artifacts = common.getCompilationArtifacts().get();
+ IntermediateArtifacts intermediateArtifacts = intermediateArtifacts(ruleContext);
+ for (Artifact artifact : Iterables.concat(artifacts.getSrcs(), artifacts.getNonArcSrcs())) {
+ oFiles.add(intermediateArtifacts.objFile(artifact));
+ }
+ }
+
+ return InstrumentedFilesCollector.collect(
+ ruleContext,
+ INSTRUMENTATION_SPEC,
+ new ObjcCoverageMetadataCollector(),
+ oFiles.build(),
+ !TargetUtils.isTestRule(ruleContext.getTarget()));
+ }
+
+ /**
* Registers any actions necessary to link this rule and its dependencies.
*
* <p>Dsym bundle and breakpad files are generated if
@@ -579,11 +628,11 @@
* Registers an action that will generate a clang module map for this target, using the hdrs
* attribute of this rule.
*/
- public CompilationSupport registerGenerateModuleMapAction(
+ CompilationSupport registerGenerateModuleMapAction(
Optional<CompilationArtifacts> compilationArtifacts) {
if (ObjcRuleClasses.objcConfiguration(ruleContext).moduleMapsEnabled()) {
Iterable<Artifact> publicHeaders = attributes.hdrs();
- Iterable<Artifact> privateHeaders = ImmutableList.<Artifact>of();
+ Iterable<Artifact> privateHeaders = ImmutableList.of();
if (compilationArtifacts.isPresent()) {
CompilationArtifacts artifacts = compilationArtifacts.get();
publicHeaders = Iterables.concat(publicHeaders, artifacts.getAdditionalHdrs());
@@ -864,7 +913,7 @@
if (ObjcRuleClasses.objcConfiguration(ruleContext).moduleMapsEnabled()) {
moduleMap = Optional.of(ObjcRuleClasses.intermediateArtifacts(ruleContext).moduleMap());
} else {
- moduleMap = Optional.<CppModuleMap>absent();
+ moduleMap = Optional.absent();
}
registerCompileAndArchiveActions(
compilationArtifact,
@@ -1035,20 +1084,20 @@
.replaceName(linkedBinary.getFilename())
.relativeTo(dsymOutputDir);
- StringBuilder unzipDsymCommand = new StringBuilder();
- unzipDsymCommand
- .append(
- String.format(
- "unzip -p %s %s > %s",
- dsymBundle.getExecPathString(),
- dsymPlistZipEntry,
- dsymPlist.getExecPathString()))
- .append(
- String.format(
- " && unzip -p %s %s > %s",
- dsymBundle.getExecPathString(),
- debugSymbolFileZipEntry,
- debugSymbolFile.getExecPathString()));
+ StringBuilder unzipDsymCommand =
+ new StringBuilder()
+ .append(
+ String.format(
+ "unzip -p %s %s > %s",
+ dsymBundle.getExecPathString(),
+ dsymPlistZipEntry,
+ dsymPlist.getExecPathString()))
+ .append(
+ String.format(
+ " && unzip -p %s %s > %s",
+ dsymBundle.getExecPathString(),
+ debugSymbolFileZipEntry,
+ debugSymbolFile.getExecPathString()));
ruleContext.registerAction(
new SpawnAction.Builder()
@@ -1092,4 +1141,24 @@
private String getModuleName() {
return ruleContext.getLabel().getName();
}
+
+ /**
+ * Collector that, given a list of output artifacts, finds and registers coverage notes metadata
+ * for any compilation action.
+ */
+ private static class ObjcCoverageMetadataCollector extends LocalMetadataCollector {
+
+ @Override
+ public void collectMetadataArtifacts(
+ Iterable<Artifact> artifacts,
+ AnalysisEnvironment analysisEnvironment,
+ NestedSetBuilder<Artifact> metadataFilesBuilder) {
+ for (Artifact artifact : artifacts) {
+ Action action = analysisEnvironment.getLocalGeneratingAction(artifact);
+ if (action.getMnemonic().equals("ObjcCompile")) {
+ addOutputs(metadataFilesBuilder, action, ObjcRuleClasses.COVERAGE_NOTES);
+ }
+ }
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ExperimentalIosTest.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ExperimentalIosTest.java
index dd034b7..b86e54f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ExperimentalIosTest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ExperimentalIosTest.java
@@ -25,6 +25,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.rules.test.ExecutionInfoProvider;
+import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
/**
* Implementation for {@code experimental_ios_test} rule in Bazel.
@@ -43,10 +44,11 @@
NestedSetBuilder<Artifact> filesToBuildBuilder = NestedSetBuilder.<Artifact>stableOrder()
.addTransitive(filesToBuild);
- TestSupport testSupport = new TestSupport(ruleContext)
- .registerTestRunnerActions()
- .addRunfiles(runfilesBuilder, common.getObjcProvider())
- .addFilesToBuild(filesToBuildBuilder);
+ TestSupport testSupport =
+ new TestSupport(ruleContext)
+ .registerTestRunnerActions()
+ .addRunfiles(runfilesBuilder)
+ .addFilesToBuild(filesToBuildBuilder);
Artifact executable = testSupport.generatedTestScript();
@@ -58,9 +60,13 @@
.setFilesToBuild(filesToBuildBuilder.build())
.add(XcodeProvider.class, xcodeProvider)
.add(RunfilesProvider.class, RunfilesProvider.simple(runfiles))
- .add(ExecutionInfoProvider.class,
+ .add(
+ ExecutionInfoProvider.class,
new ExecutionInfoProvider(ImmutableMap.of(ExecutionRequirements.REQUIRES_DARWIN, "")))
- .addProviders(testSupport.getExtraProviders(common.getObjcProvider()))
+ .addProvider(
+ InstrumentedFilesProvider.class,
+ new CompilationSupport(ruleContext).getInstrumentedFilesProvider(common))
+ .addProviders(testSupport.getExtraProviders())
.setRunfilesSupport(runfilesSupport, executable)
.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
index c2c19d5..8018218 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
@@ -27,14 +27,12 @@
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.FRAMEWORK_FILE;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.Flag.USES_CPP;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.Flag.USES_SWIFT;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.GCNO;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.GENERAL_RESOURCE_DIR;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.GENERAL_RESOURCE_FILE;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.HEADER;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.IMPORTED_LIBRARY;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.INCLUDE;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.INCLUDE_SYSTEM;
-import static com.google.devtools.build.lib.rules.objc.ObjcProvider.INSTRUMENTED_SOURCE;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.LIBRARY;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.LINKED_BINARY;
import static com.google.devtools.build.lib.rules.objc.ObjcProvider.MODULE_MAP;
@@ -68,7 +66,6 @@
import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.util.FileType;
-import com.google.devtools.build.lib.util.RegexFilter;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
@@ -227,7 +224,7 @@
/**
* Returns whether this target uses language features that require clang modules, such as
- * @import.
+ * {@literal @}import.
*/
public boolean enableModules() {
return ruleContext.attributes().has("enable_modules", Type.BOOLEAN)
@@ -518,17 +515,6 @@
.addAll(HEADER, artifacts.getAdditionalHdrs())
.addAll(LIBRARY, artifacts.getArchive().asSet())
.addAll(SOURCE, allSources);
- BuildConfiguration configuration = context.getConfiguration();
- RegexFilter filter = configuration.getInstrumentationFilter();
- if (configuration.isCodeCoverageEnabled()
- && filter.isIncluded(context.getLabel().toString())) {
- for (Artifact source : allSources) {
- if (ObjcRuleClasses.isInstrumentable(source)) {
- objcProvider.add(INSTRUMENTED_SOURCE, source);
- objcProvider.add(GCNO, intermediateArtifacts.gcnoFile(source));
- }
- }
- }
boolean usesCpp = false;
boolean usesSwift = false;
@@ -607,22 +593,12 @@
* compilation information has no sources.
*/
public Optional<Artifact> getCompiledArchive() {
- for (CompilationArtifacts justCompilationArtifacts : compilationArtifacts.asSet()) {
- return justCompilationArtifacts.getArchive();
+ if (compilationArtifacts.isPresent()) {
+ return compilationArtifacts.get().getArchive();
}
return Optional.absent();
}
- /**
- * Reports any known errors to the {@link RuleContext}. This should be called exactly once for
- * a target.
- */
- public void reportErrors() {
-
- // TODO(bazel-team): Report errors for rules that are not actually useful (i.e. objc_library
- // without sources or resources, empty objc_bundles)
- }
-
static ImmutableList<PathFragment> userHeaderSearchPaths(BuildConfiguration configuration) {
return ImmutableList.of(
new PathFragment("."),
@@ -734,4 +710,5 @@
}
return errors;
}
+
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java
index 048dce7..f76cbd8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.rules.cpp.CppCompilationContext;
import com.google.devtools.build.lib.rules.objc.ObjcCommon.CompilationAttributes;
import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes;
+import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
import com.google.devtools.build.lib.syntax.Type;
/**
@@ -93,10 +94,11 @@
NestedSetBuilder<Artifact> filesToBuild = NestedSetBuilder.<Artifact>stableOrder()
.addAll(common.getCompiledArchive().asSet());
- new CompilationSupport(ruleContext)
- .registerCompileAndArchiveActions(common)
- .addXcodeSettings(xcodeProviderBuilder, common)
- .validateAttributes();
+ CompilationSupport compilationSupport =
+ new CompilationSupport(ruleContext)
+ .registerCompileAndArchiveActions(common)
+ .addXcodeSettings(xcodeProviderBuilder, common)
+ .validateAttributes();
new ResourceSupport(ruleContext)
.validateAttributes()
@@ -117,6 +119,9 @@
.addProvider(J2ObjcSrcsProvider.class, J2ObjcSrcsProvider.buildFrom(ruleContext))
.addProvider(
J2ObjcMappingFileProvider.class, ObjcRuleClasses.j2ObjcMappingFileProvider(ruleContext))
+ .addProvider(
+ InstrumentedFilesProvider.class,
+ compilationSupport.getInstrumentedFilesProvider(common))
.build();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
index bbee7c6..5e3e520 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
@@ -88,18 +88,6 @@
public static final Key<Artifact> SOURCE = new Key<>(STABLE_ORDER);
/**
- * Contains all coverage instrumented source files.
- */
- public static final Key<Artifact> INSTRUMENTED_SOURCE = new Key<>(STABLE_ORDER);
-
- /**
- * Contains all .gcno files one for every source file if in coverage mode.
- * It contains information to reconstruct the basic block graphs and assign source line numbers
- * to blocks.
- */
- public static final Key<Artifact> GCNO = new Key<>(STABLE_ORDER);
-
- /**
* Include search paths specified with {@code -I} on the command line. Also known as header search
* paths (and distinct from <em>user</em> header search paths).
*/
@@ -384,16 +372,6 @@
return this;
}
- /**
- * Add element, but don't propagate dependers on this ObjcProvider. These elements will be
- * exposed to {@link #get(Key)} calls, but not to any ObjcProviders which add this provider to
- * themselves.
- */
- public <E> Builder addWithoutPropagating(Key<E> key, E toAdd) {
- uncheckedAddAll(key, ImmutableList.of(toAdd), false);
- return this;
- }
-
public ObjcProvider build() {
ImmutableMap.Builder<Key<?>, NestedSet<?>> propagated = new ImmutableMap.Builder<>();
for (Map.Entry<Key<?>, NestedSetBuilder<?>> typeEntry : items.entrySet()) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
index 07f3ad4..05bdd6c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
@@ -331,7 +331,7 @@
*/
static final FileType CPP_SOURCES = FileType.of(".cc", ".cpp", ".mm", ".cxx", ".C");
- private static final FileType NON_CPP_SOURCES = FileType.of(".m", ".c");
+ static final FileType NON_CPP_SOURCES = FileType.of(".m", ".c");
static final FileType ASSEMBLY_SOURCES = FileType.of(".s", ".S", ".asm");
@@ -366,6 +366,12 @@
static final FileTypeSet HDRS_TYPE = FileTypeSet.ANY_FILE;
/**
+ * Coverage note files which contain information to reconstruct the basic block graphs and assign
+ * source line numbers to blocks.
+ */
+ static final FileType COVERAGE_NOTES = FileType.of(".gcno");
+
+ /**
* Common attributes for {@code objc_*} rules that allow the definition of resources such as
* storyboards. These resources are used during compilation of the declaring rule as well as when
* bundling a dependent bundle (application, extension, etc.).
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java
index b6b5340..a3ea622 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java
@@ -402,10 +402,8 @@
// want to link anything since that stuff is shared automatically by way of the
// -bundle_loader linker flag.
ObjcProvider partialObjcProvider = new ObjcProvider.Builder()
- .addTransitiveAndPropagate(ObjcProvider.GCNO, objcProvider)
.addTransitiveAndPropagate(ObjcProvider.HEADER, objcProvider)
.addTransitiveAndPropagate(ObjcProvider.INCLUDE, objcProvider)
- .addTransitiveAndPropagate(ObjcProvider.INSTRUMENTED_SOURCE, objcProvider)
.addTransitiveAndPropagate(ObjcProvider.SDK_DYLIB, objcProvider)
.addTransitiveAndPropagate(ObjcProvider.SDK_FRAMEWORK, objcProvider)
.addTransitiveAndPropagate(ObjcProvider.SOURCE, objcProvider)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingTargetFactory.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingTargetFactory.java
index 54aeb67..57c488e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingTargetFactory.java
@@ -23,6 +23,8 @@
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.rules.objc.ReleaseBundlingSupport.LinkedBinary;
import com.google.devtools.build.lib.rules.objc.ReleaseBundlingSupport.SplitArchTransition.ConfigurationDistinguisher;
+import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector;
+import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
import javax.annotation.Nullable;
@@ -89,7 +91,10 @@
RuleConfiguredTargetBuilder targetBuilder =
ObjcRuleClasses.ruleConfiguredTarget(ruleContext, filesToBuild.build())
.addProvider(XcTestAppProvider.class, releaseBundlingSupport.xcTestAppProvider())
- .addProvider(XcodeProvider.class, xcodeProviderBuilder.build());
+ .addProvider(XcodeProvider.class, xcodeProviderBuilder.build())
+ .addProvider(
+ InstrumentedFilesProvider.class,
+ InstrumentedFilesCollector.forward(ruleContext, "binary"));
ObjcProvider exposedObjcProvider = exposedObjcProvider(ruleContext);
if (exposedObjcProvider != null) {
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 c882f05..b920e4f 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
@@ -32,9 +32,7 @@
import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.Substitution;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.rules.objc.ObjcProvider.Key;
-import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
-import com.google.devtools.build.lib.rules.test.InstrumentedFilesProviderImpl;
+import com.google.devtools.build.lib.rules.test.TestEnvironmentProvider;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.util.FileType;
@@ -197,12 +195,8 @@
/**
* Adds all files needed to run this test to the passed Runfiles builder.
- *
- * @param objcProvider common information about this rule's attributes and its dependencies
- * @throws InterruptedException
*/
- public TestSupport addRunfiles(Builder runfilesBuilder, ObjcProvider objcProvider)
- throws InterruptedException {
+ public TestSupport addRunfiles(Builder runfilesBuilder) throws InterruptedException {
runfilesBuilder
.addArtifact(testIpa())
.addArtifacts(xctestIpa().asSet())
@@ -218,67 +212,28 @@
runfilesBuilder.addTransitiveArtifacts(labDeviceRunfiles());
}
- if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
- runfilesBuilder
- .addTransitiveArtifacts(objcProvider.get(ObjcProvider.SOURCE))
- .addTransitiveArtifacts(gcnoFiles(objcProvider));
- }
return this;
}
/**
* Returns any additional providers that need to be exported to the rule context to the passed
* builder.
- *
- * @param objcProvider common information about this rule's attributes and its dependencies
*/
- public Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider>
- getExtraProviders(ObjcProvider objcProvider) {
- return ImmutableMap.<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider>of(
- InstrumentedFilesProvider.class,
- new InstrumentedFilesProviderImpl(
- instrumentedFiles(objcProvider), gcnoFiles(objcProvider), gcovEnv()));
+ public Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> getExtraProviders() {
+ if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
+ return ImmutableMap.<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider>of(
+ TestEnvironmentProvider.class, new TestEnvironmentProvider(gcovEnv()));
+ }
+ return ImmutableMap.of();
}
/**
* Returns a map of extra environment variable names to their values used to point to gcov binary,
* which should be added to the test action environment, if coverage is enabled.
*/
- private Map<String, String> gcovEnv() {
- if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {
- return ImmutableMap.of("COVERAGE_GCOV_PATH",
- ruleContext.getHostPrerequisiteArtifact(":gcov").getExecPathString());
- }
- return ImmutableMap.of();
- }
-
- /**
- * Returns all GCC coverage notes files available for computing coverage.
- */
- private NestedSet<Artifact> gcnoFiles(ObjcProvider objcProvider) {
- return filesWithXcTestApp(ObjcProvider.GCNO, objcProvider);
- }
-
- /**
- * Returns all source files from the test (and if present xctest app) which have been
- * instrumented for code coverage.
- */
- private NestedSet<Artifact> instrumentedFiles(ObjcProvider objcProvider) {
- return filesWithXcTestApp(ObjcProvider.INSTRUMENTED_SOURCE, objcProvider);
- }
-
- private NestedSet<Artifact> filesWithXcTestApp(Key<Artifact> key, ObjcProvider objcProvider) {
- NestedSet<Artifact> underlying = objcProvider.get(key);
- XcTestAppProvider provider = ruleContext.getPrerequisite(
- IosTest.XCTEST_APP, Mode.TARGET, XcTestAppProvider.class);
- if (provider == null) {
- return underlying;
- }
-
- return NestedSetBuilder.<Artifact>stableOrder()
- .addTransitive(underlying)
- .addTransitive(provider.getObjcProvider().get(key))
- .build();
+ private ImmutableMap<String, String> gcovEnv() {
+ return ImmutableMap.of(
+ "COVERAGE_GCOV_PATH", ruleContext.getHostPrerequisiteArtifact(":gcov").getExecPathString());
}
/**
@@ -307,7 +262,6 @@
/**
* Adds files which must be built in order to run this test to builder.
- * @throws InterruptedException
*/
public TestSupport addFilesToBuild(NestedSetBuilder<Artifact> builder)
throws InterruptedException {
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 2266b54..84b13a9 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
@@ -15,7 +15,6 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
@@ -35,15 +34,32 @@
import java.util.Collection;
import java.util.List;
-import javax.annotation.Nullable;
-
/**
* A helper class for collecting instrumented files and metadata for a target.
*/
public final class InstrumentedFilesCollector {
- public static InstrumentedFilesProvider collect(RuleContext ruleContext, InstrumentationSpec spec,
- @Nullable LocalMetadataCollector localMetadataCollector,
- @Nullable Iterable<Artifact> rootFiles) {
+
+ /**
+ * Forwards any instrumented files from the given target's dependencies (as defined in
+ * {@code dependencyAttributes}) for further export. No files from this target are considered
+ * instrumented.
+ *
+ * @return instrumented file provider of all dependencies in {@code dependencyAttributes}
+ */
+ public static InstrumentedFilesProvider forward(
+ RuleContext ruleContext, String... dependencyAttributes) {
+ return collect(
+ ruleContext,
+ new InstrumentationSpec(FileTypeSet.NO_FILE).withDependencyAttributes(dependencyAttributes),
+ null,
+ null);
+ }
+
+ public static InstrumentedFilesProvider collect(
+ RuleContext ruleContext,
+ InstrumentationSpec spec,
+ LocalMetadataCollector localMetadataCollector,
+ Iterable<Artifact> rootFiles) {
return collect(ruleContext, spec, localMetadataCollector, rootFiles, false);
}
@@ -53,9 +69,12 @@
* configured target, and 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, @Nullable LocalMetadataCollector localMetadataCollector,
- @Nullable Iterable<Artifact> rootFiles, boolean withBaselineCoverage) {
+ public static InstrumentedFilesProvider collect(
+ RuleContext ruleContext,
+ InstrumentationSpec spec,
+ LocalMetadataCollector localMetadataCollector,
+ Iterable<Artifact> rootFiles,
+ boolean withBaselineCoverage) {
Preconditions.checkNotNull(ruleContext);
Preconditions.checkNotNull(spec);
@@ -118,11 +137,11 @@
// 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(),
+ return new InstrumentedFilesProviderImpl(
+ instrumentedFilesBuilder.build(),
metadataFilesBuilder.build(),
baselineCoverageFiles,
- baselineCoverageArtifacts,
- ImmutableMap.<String, String>of());
+ baselineCoverageArtifacts);
}
/**
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 24017da..0a2be73 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
@@ -18,8 +18,6 @@
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import java.util.Map;
-
/**
* A provider of instrumented file sources and instrumentation metadata.
*/
@@ -51,9 +49,4 @@
// 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();
-
- /**
- * Returns environment variables which should be set for coverage to function.
- */
- Map<String, String> getExtraEnv();
}
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 a56e8d8..439c4e3 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
@@ -13,47 +13,36 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.test;
-import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.Artifact;
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 java.util.Map;
-
/**
* An implementation class for the InstrumentedFilesProvider interface.
*/
public final class InstrumentedFilesProviderImpl implements InstrumentedFilesProvider {
- public static final InstrumentedFilesProvider EMPTY = new InstrumentedFilesProviderImpl(
- 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());
+ public static final InstrumentedFilesProvider EMPTY =
+ new InstrumentedFilesProviderImpl(
+ 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 ImmutableMap<String, String> extraEnv;
- public InstrumentedFilesProviderImpl(NestedSet<Artifact> instrumentedFiles,
+ public InstrumentedFilesProviderImpl(
+ NestedSet<Artifact> instrumentedFiles,
NestedSet<Artifact> instrumentationMetadataFiles,
NestedSet<Artifact> baselineCoverageFiles,
- NestedSet<Artifact> baselineCoverageArtifacts, Map<String, String> extraEnv) {
+ NestedSet<Artifact> baselineCoverageArtifacts) {
this.instrumentedFiles = instrumentedFiles;
this.instrumentationMetadataFiles = instrumentationMetadataFiles;
this.baselineCoverageFiles = baselineCoverageFiles;
this.baselineCoverageArtifacts = baselineCoverageArtifacts;
- this.extraEnv = ImmutableMap.copyOf(extraEnv);
- }
-
- 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);
}
@Override
@@ -75,9 +64,4 @@
public NestedSet<Artifact> getBaselineCoverageArtifacts() {
return baselineCoverageArtifacts;
}
-
- @Override
- public Map<String, String> getExtraEnv() {
- return extraEnv;
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestEnvironmentProvider.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestEnvironmentProvider.java
new file mode 100644
index 0000000..fa98fc6
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestEnvironmentProvider.java
@@ -0,0 +1,43 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.rules.test;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+
+import java.util.Map;
+
+/**
+ * Provider containing any additional environment variables for use in the test action.
+ */
+@Immutable
+public final class TestEnvironmentProvider implements TransitiveInfoProvider {
+ private final ImmutableMap<String, String> environment;
+
+ /**
+ * Constructs a new provider with the given variable name to variable value mapping.
+ */
+ public TestEnvironmentProvider(ImmutableMap<String, String> environment) {
+ this.environment = environment;
+ }
+
+ /**
+ * Returns environment variables which should be set on the test action.
+ */
+ public Map<String, String> getEnvironment() {
+ return environment;
+ }
+}
