Automated rollback of commit cf95a2b.

*** Reason for rollback ***

Rollforward with fix.

*** Original change description ***

Automated rollback of commit 2f39c04.

*** Reason for rollback ***

breaks coverage builds: b/265134564

*** Original change description ***

Use coverage_common.instrumented_files_info in objc_library.bzl.

PiperOrigin-RevId: 501577275
Change-Id: I0e8afde4855594818ceda093d151c60728dcf603

src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java

@@ -44,23 +44,17 @@
 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;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 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<String> {
     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<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.
    *
@@ -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<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"));

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<NativeInfo> 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,

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(

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 = []

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),
     ]
 

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<String> getCcInfoUserLinkFlagsFromTarget(String target)
       throws LabelSyntaxException {
     return getConfiguredTarget(target)