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 b4ef46b87b32d9..71c5780c7ae8ce 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,12 +44,10 @@ 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; @@ -57,10 +55,6 @@ 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; @@ -93,7 +87,6 @@ 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; @@ -159,17 +152,6 @@ public static final FilesToRunProvider xcrunwrapper(RuleContext ruleContext) { 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 { ExtraLinkArgs(String... args) { @@ -373,34 +355,6 @@ public CompilationSupport build() throws InterruptedException, RuleErrorExceptio } } - /** - * 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 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. * @@ -1077,27 +1031,6 @@ private void registerBinaryStripAction(Artifact binaryToLink, StrippingType stri .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 artifacts, - AnalysisEnvironment analysisEnvironment, - NestedSetBuilder 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 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 92d73c86a856b0..1683b47032498b 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,15 +24,11 @@ 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; @@ -225,32 +221,6 @@ public Sequence createj2objcProvidersFromDeps(StarlarkRuleContext st ImmutableList.of(j2ObjcEntryClassProvider, j2ObjcMappingFileProvider)); } - @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, diff --git a/src/main/starlark/builtins_bzl/common/objc/attrs.bzl b/src/main/starlark/builtins_bzl/common/objc/attrs.bzl index 63490490bf7e4d..38732914b4ddce 100644 --- a/src/main/starlark/builtins_bzl/common/objc/attrs.bzl +++ b/src/main/starlark/builtins_bzl/common/objc/attrs.bzl @@ -15,6 +15,7 @@ """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 @@ -29,28 +30,11 @@ _CC_TOOLCHAIN_RULE = { _COMPILING_RULE = { "srcs": attr.label_list( - 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", - ], + allow_files = extensions.NON_CPP_SOURCES + + extensions.CPP_SOURCES + + extensions.ASSEMBLY_SOURCES + + extensions.OBJECT_FILE_SOURCES + + extensions.HEADERS, 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 550d89cc883c8d..515b86941c469c 100644 --- a/src/main/starlark/builtins_bzl/common/objc/objc_common.bzl +++ b/src/main/starlark/builtins_bzl/common/objc/objc_common.bzl @@ -12,12 +12,26 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Builds the Objective-C provider""" +"""Common functionality for Objc rules.""" 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, @@ -241,7 +255,7 @@ def _create_context_and_provider( ) def _is_cpp_source(source_file): - return source_file.extension in ["cc", "cpp", "mm", "cxx", "C"] + return "." + source_file.extension in CPP_SOURCES 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 fc62bdfccbe55d..07e54c4fe534a6 100644 --- a/src/main/starlark/builtins_bzl/common/objc/objc_library.bzl +++ b/src/main/starlark/builtins_bzl/common/objc/objc_library.bzl @@ -16,6 +16,7 @@ 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") @@ -63,6 +64,19 @@ def _objc_library_impl(ctx): 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), + # TODO(cmita): Use ctx.coverage_instrumented() instead when rules_swift can access + # cc_toolchain.coverage_files and the coverage_support_files parameter of + # coverage_common.instrumented_files_info(...) + coverage_support_files = cc_toolchain.coverage_files() if ctx.configuration.coverage_enabled else depset([]), + metadata_files = compilation_outputs.gcno_files() + compilation_outputs.pic_gcno_files(), + ) + return [ DefaultInfo( files = depset(files), @@ -75,12 +89,7 @@ def _objc_library_impl(ctx): objc_provider, j2objc_providers[0], j2objc_providers[1], - objc_internal.instrumented_files_info( - ctx = ctx, - cc_toolchain = cc_toolchain, - config = ctx.configuration, - object_files = compilation_outputs.objects, - ), + instrumented_files_info, 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 43e275cbaa64e8..58cf3b3c61caaa 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,6 +2509,30 @@ public void testDisableCoverageDoesNotPropagateSupportFiles() throws Exception { 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 getCcInfoUserLinkFlagsFromTarget(String target) throws LabelSyntaxException { return getConfiguredTarget(target)