Automated rollback of commit 2f39c04ea4b93788bfef5c475b7dd40a8a289aef.
*** Reason for rollback ***
breaks coverage builds: b/265134564
*** Original change description ***
Use coverage_common.instrumented_files_info in objc_library.bzl.
PiperOrigin-RevId: 501371490
Change-Id: I326c91e64a6be88826b1a3877c83e1d1d3f11c38
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 71c5780..b4ef46b 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
@@ -44,10 +44,12 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
+import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.actions.ParamFileInfo;
import com.google.devtools.build.lib.actions.ParameterFile;
+import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts;
import com.google.devtools.build.lib.analysis.RuleContext;
@@ -55,6 +57,10 @@
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
+import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector;
+import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector.InstrumentationSpec;
+import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector.LocalMetadataCollector;
+import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
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;
@@ -87,6 +93,7 @@
import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode;
import com.google.devtools.build.lib.rules.objc.ObjcVariablesExtension.VariableCategory;
+import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
@@ -152,6 +159,17 @@
return ruleContext.getExecutablePrerequisite("$xcrunwrapper");
}
+ /**
+ * 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
+ * InstrumentedFilesInfo}).
+ */
+ private static final InstrumentationSpec INSTRUMENTATION_SPEC =
+ new InstrumentationSpec(
+ FileTypeSet.of(ObjcRuleClasses.NON_CPP_SOURCES, ObjcRuleClasses.CPP_SOURCES, HEADERS))
+ .withSourceAttributes("srcs", "non_arc_srcs", "hdrs")
+ .withDependencyAttributes("deps", "data", "binary", "xctest_app");
+
/** Iterable wrapper providing strong type safety for arguments to binary linking. */
static final class ExtraLinkArgs extends IterableWrapper<String> {
ExtraLinkArgs(String... args) {
@@ -356,6 +374,34 @@
}
/**
+ * Returns a provider that collects this target's instrumented sources as well as those of its
+ * dependencies.
+ *
+ * @param ruleContext the rule context of the target
+ * @param toolchain the toolchain used by the target
+ * @param buildConfiguration the build configuration of the target
+ * @param objectFiles the object files generated by the target
+ * @return an instrumented files provider
+ */
+ protected static InstrumentedFilesInfo getInstrumentedFilesProvider(
+ RuleContext ruleContext,
+ CcToolchainProvider toolchain,
+ BuildConfigurationValue buildConfiguration,
+ ImmutableList<Artifact> objectFiles)
+ throws RuleErrorException {
+ CppConfiguration cppConfiguration = buildConfiguration.getFragment(CppConfiguration.class);
+ return InstrumentedFilesCollector.collect(
+ ruleContext,
+ INSTRUMENTATION_SPEC,
+ OBJC_METADATA_COLLECTOR,
+ objectFiles,
+ CppHelper.getGcovFilesIfNeeded(ruleContext, toolchain),
+ CppHelper.getCoverageEnvironmentIfNeeded(ruleContext, cppConfiguration, toolchain),
+ /* withBaselineCoverage= */ true,
+ /* reportedToActualSources= */ NestedSetBuilder.create(Order.STABLE_ORDER));
+ }
+
+ /**
* Validates compilation-related attributes on this rule.
*
* @return this compilation support
@@ -1031,6 +1077,27 @@
.build(ruleContext));
}
+ /**
+ * Collector that, given a list of output artifacts, finds and registers coverage notes metadata
+ * for any compilation action.
+ */
+ private static final LocalMetadataCollector OBJC_METADATA_COLLECTOR =
+ new LocalMetadataCollector() {
+ @Override
+ public void collectMetadataArtifacts(
+ Iterable<Artifact> artifacts,
+ AnalysisEnvironment analysisEnvironment,
+ NestedSetBuilder<Artifact> metadataFilesBuilder) {
+ for (Artifact artifact : artifacts) {
+ ActionAnalysisMetadata action = analysisEnvironment.getLocalGeneratingAction(artifact);
+ if (action.getMnemonic().equals("ObjcCompile")
+ || action.getMnemonic().equals("ObjcCompileHeader")) {
+ addOutputs(metadataFilesBuilder, action, ObjcRuleClasses.COVERAGE_NOTES);
+ }
+ }
+ }
+ };
+
public static Optional<Artifact> getCustomModuleMap(RuleContext ruleContext) {
if (ruleContext.attributes().has("module_map", BuildType.LABEL)) {
return Optional.fromNullable(ruleContext.getPrerequisiteArtifact("module_map"));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkInternal.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkInternal.java
index 1683b47..92d73c8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkInternal.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkInternal.java
@@ -24,11 +24,15 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.LocationExpander;
import com.google.devtools.build.lib.analysis.TemplateVariableInfo;
+import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext;
+import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo;
import com.google.devtools.build.lib.packages.NativeInfo;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.rules.cpp.CcCompilationContext;
import com.google.devtools.build.lib.rules.cpp.CcLinkingContext;
+import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
import com.google.devtools.build.lib.shell.ShellUtils;
import com.google.devtools.build.lib.shell.ShellUtils.TokenizationException;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -222,6 +226,32 @@
}
@StarlarkMethod(
+ name = "instrumented_files_info",
+ documented = false,
+ parameters = {
+ @Param(name = "ctx", positional = false, named = true),
+ @Param(name = "cc_toolchain", positional = false, named = true),
+ @Param(name = "config", positional = false, named = true),
+ @Param(name = "object_files", positional = false, defaultValue = "[]", named = true),
+ })
+ public InstrumentedFilesInfo createInstrumentedFilesInfo(
+ StarlarkRuleContext starlarkRuleContext,
+ CcToolchainProvider ccToolchain,
+ BuildConfigurationValue config,
+ Sequence<?> objectFiles)
+ throws EvalException {
+ try {
+ return CompilationSupport.getInstrumentedFilesProvider(
+ starlarkRuleContext.getRuleContext(),
+ ccToolchain,
+ config,
+ Sequence.cast(objectFiles, Artifact.class, "object_files").getImmutableList());
+ } catch (RuleErrorException e) {
+ throw new EvalException(e);
+ }
+ }
+
+ @StarlarkMethod(
name = "create_compilation_context",
documented = false,
parameters = {
diff --git a/src/main/starlark/builtins_bzl/common/objc/attrs.bzl b/src/main/starlark/builtins_bzl/common/objc/attrs.bzl
index 3873291..6349049 100644
--- a/src/main/starlark/builtins_bzl/common/objc/attrs.bzl
+++ b/src/main/starlark/builtins_bzl/common/objc/attrs.bzl
@@ -15,7 +15,6 @@
"""Attributes common to Objc rules"""
load("@_builtins//:common/objc/semantics.bzl", "semantics")
-load("@_builtins//:common/objc/objc_common.bzl", "extensions")
CcInfo = _builtins.toplevel.CcInfo
AppleDynamicFrameworkInfo = _builtins.toplevel.apple_common.AppleDynamicFramework
@@ -30,11 +29,28 @@
_COMPILING_RULE = {
"srcs": attr.label_list(
- allow_files = extensions.NON_CPP_SOURCES +
- extensions.CPP_SOURCES +
- extensions.ASSEMBLY_SOURCES +
- extensions.OBJECT_FILE_SOURCES +
- extensions.HEADERS,
+ allow_files = [
+ # NON_CPP_SOURCES
+ ".m",
+ ".c",
+ # CPP_SOURCES
+ ".cc",
+ ".cpp",
+ ".mm",
+ ".cxx",
+ ".C",
+ # ASSEMBLY_SOURCES
+ ".s",
+ ".S",
+ ".asm",
+ # OBJECT_FILE_SOURCES
+ ".o",
+ # HEADERS
+ ".h",
+ ".inc",
+ ".hpp",
+ ".hh",
+ ],
flags = ["DIRECT_COMPILE_TIME_INPUT"],
),
"non_arc_srcs": attr.label_list(
diff --git a/src/main/starlark/builtins_bzl/common/objc/objc_common.bzl b/src/main/starlark/builtins_bzl/common/objc/objc_common.bzl
index 515b869..550d89c 100644
--- a/src/main/starlark/builtins_bzl/common/objc/objc_common.bzl
+++ b/src/main/starlark/builtins_bzl/common/objc/objc_common.bzl
@@ -12,26 +12,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-"""Common functionality for Objc rules."""
+"""Builds the Objective-C provider"""
objc_internal = _builtins.internal.objc_internal
CcInfo = _builtins.toplevel.CcInfo
apple_common = _builtins.toplevel.apple_common
-CPP_SOURCES = [".cc", ".cpp", ".mm", ".cxx", ".C"]
-NON_CPP_SOURCES = [".m", ".c"]
-ASSEMBLY_SOURCES = [".s", ".S", ".asm"]
-OBJECT_FILE_SOURCES = [".o"]
-HEADERS = [".h", ".inc", ".hpp", ".hh"]
-
-extensions = struct(
- CPP_SOURCES = CPP_SOURCES,
- NON_CPP_SOURCES = NON_CPP_SOURCES,
- ASSEMBLY_SOURCES = ASSEMBLY_SOURCES,
- OBJECT_FILE_SOURCES = OBJECT_FILE_SOURCES,
- HEADERS = HEADERS,
-)
-
def _create_context_and_provider(
ctx,
compilation_attributes,
@@ -255,7 +241,7 @@
)
def _is_cpp_source(source_file):
- return "." + source_file.extension in CPP_SOURCES
+ return source_file.extension in ["cc", "cpp", "mm", "cxx", "C"]
def _add_linkopts(objc_provider_kwargs, linkopts):
non_sdk_linkopts = []
diff --git a/src/main/starlark/builtins_bzl/common/objc/objc_library.bzl b/src/main/starlark/builtins_bzl/common/objc/objc_library.bzl
index 14384ad..fc62bdfc 100644
--- a/src/main/starlark/builtins_bzl/common/objc/objc_library.bzl
+++ b/src/main/starlark/builtins_bzl/common/objc/objc_library.bzl
@@ -16,7 +16,6 @@
load("@_builtins//:common/objc/compilation_support.bzl", "compilation_support")
load("@_builtins//:common/objc/attrs.bzl", "common_attrs")
-load("@_builtins//:common/objc/objc_common.bzl", "extensions")
load("@_builtins//:common/objc/transitions.bzl", "apple_crosstool_transition")
load("@_builtins//:common/cc/cc_helper.bzl", "cc_helper")
@@ -64,16 +63,6 @@
objc_provider = common_variables.objc_provider
- instrumented_files_info = coverage_common.instrumented_files_info(
- ctx = ctx,
- source_attributes = ["srcs", "non_arc_srcs", "hdrs"],
- dependency_attributes = ["deps", "data", "binary", "xctest_app"],
- extensions = extensions.NON_CPP_SOURCES + extensions.CPP_SOURCES + extensions.HEADERS,
- coverage_environment = cc_helper.get_coverage_environment(ctx, ctx.fragments.cpp, cc_toolchain),
- coverage_support_files = cc_toolchain.coverage_files() if ctx.coverage_instrumented() else depset([]),
- metadata_files = compilation_outputs.gcno_files() + compilation_outputs.pic_gcno_files(),
- )
-
return [
DefaultInfo(
files = depset(files),
@@ -86,7 +75,12 @@
objc_provider,
j2objc_providers[0],
j2objc_providers[1],
- instrumented_files_info,
+ objc_internal.instrumented_files_info(
+ ctx = ctx,
+ cc_toolchain = cc_toolchain,
+ config = ctx.configuration,
+ object_files = compilation_outputs.objects,
+ ),
OutputGroupInfo(**output_groups),
]
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
index 58cf3b3..43e275c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
@@ -2509,30 +2509,6 @@
assertThat(instrumentedFilesInfo.getCoverageSupportFiles().toList()).isEmpty();
}
- @Test
- public void testCoverageMetadataFiles() throws Exception {
- scratch.file(
- "a/BUILD",
- "cc_toolchain_alias(name = 'toolchain')",
- "objc_library(",
- " name = 'foo',",
- " srcs = ['foo.m'],",
- ")",
- "objc_library(",
- " name = 'bar',",
- " srcs = ['bar.m'],",
- " deps = [':foo'],",
- ")");
- useConfiguration("--collect_code_coverage", "--instrumentation_filter=//a[:/]");
-
- InstrumentedFilesInfo instrumentedFilesInfo =
- getConfiguredTarget("//a:bar").get(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR);
-
- assertThat(
- Artifact.toRootRelativePaths(instrumentedFilesInfo.getInstrumentationMetadataFiles()))
- .containsExactly("a/_objs/foo/arc/foo.gcno", "a/_objs/bar/arc/bar.gcno");
- }
-
private ImmutableList<String> getCcInfoUserLinkFlagsFromTarget(String target)
throws LabelSyntaxException {
return getConfiguredTarget(target)
