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: 501371490
Change-Id: I326c91e64a6be88826b1a3877c83e1d1d3f11c38

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

@@ -44,17 +44,23 @@
 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;
@@ -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 @@ 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) {
@@ -355,6 +373,34 @@ 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.
    *
@@ -1031,6 +1077,27 @@ 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,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;
@@ -221,6 +225,32 @@ 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,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 @@ _CC_TOOLCHAIN_RULE = {
 
 _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(

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 _create_context_and_provider(
     )
 
 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 = []

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 @@ 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),
-        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 @@ def _objc_library_impl(ctx):
         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),
     ]
 

src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java

@@ -2509,30 +2509,6 @@ 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)