Rollback of https://github.com/bazelbuild/bazel/commit/21436e062a12b64c8bee665b0cf79dfe48cff114.

That change broke module maps that depended on the transitive headers from ObjC protos.

RELNOTES: None
PiperOrigin-RevId: 165010275

8 files changed, 21 insertions, 62 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java
index fcea10bc03..ea880d8f6d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java
@@ -117,7 +117,6 @@ public class AppleStaticLibrary implements RuleConfiguredTargetFactory {
                   protosToAvoid,
                   ImmutableList.<ProtoSourcesProvider>of(),
                   objcProtoProviders,
-                  ImmutableList.of(),
                   ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders))
               .registerGenerationActions()
               .registerCompilationActions();
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 bc4277b4e4..e627a7a71c 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
@@ -93,7 +93,6 @@ abstract class BinaryLinkingTargetFactory implements RuleConfiguredTargetFactory
                 ruleContext.getConfiguration(),
                 ImmutableList.<ProtoSourcesProvider>of(),
                 objcProtoProviders,
-                ImmutableList.of(),
                 ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders))
             .registerGenerationActions()
             .registerCompilationActions();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java
index a8e5af0962..9cd27253cd 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IosTest.java
@@ -98,7 +98,6 @@ public final class IosTest implements RuleConfiguredTargetFactory {
                 ruleContext.getConfiguration(),
                 ImmutableList.<ProtoSourcesProvider>of(),
                 objcProtoProviders,
-                ImmutableList.of(),
                 ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders))
             .registerGenerationActions()
             .registerCompilationActions();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchBinarySupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchBinarySupport.java
index da5e230a2e..6cf378355a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchBinarySupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchBinarySupport.java
@@ -243,7 +243,6 @@ public class MultiArchBinarySupport {
                     protosToAvoid,
                     ImmutableList.<ProtoSourcesProvider>of(),
                     objcProtoProviders,
-                    ImmutableList.of(),
                     ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders))
                 .registerGenerationActions()
                 .registerCompilationActions();
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 ac22e9f64c..cd4553c5e1 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
@@ -39,6 +39,7 @@ 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.LINKMAP_FILE;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.LINKOPT;
+import static com.google.devtools.build.lib.rules.objc.ObjcProvider.MODULE_MAP;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.SDK_DYLIB;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.SDK_FRAMEWORK;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.SOURCE;
@@ -46,6 +47,8 @@ import static com.google.devtools.build.lib.rules.objc.ObjcProvider.STATIC_FRAME
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.STATIC_FRAMEWORK_FILE;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.STORYBOARD;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.STRINGS;
+import static com.google.devtools.build.lib.rules.objc.ObjcProvider.TOP_LEVEL_MODULE_MAP;
+import static com.google.devtools.build.lib.rules.objc.ObjcProvider.UMBRELLA_HEADER;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.WEAK_SDK_FRAMEWORK;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.XCASSETS_DIR;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.XCDATAMODEL;
@@ -70,6 +73,7 @@ import com.google.devtools.build.lib.rules.apple.AppleToolchain;
 import com.google.devtools.build.lib.rules.cpp.CcLinkParams;
 import com.google.devtools.build.lib.rules.cpp.CcLinkParamsProvider;
 import com.google.devtools.build.lib.rules.cpp.CppCompilationContext;
+import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
 import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -555,7 +559,13 @@ public final class ObjcCommon {
       }
 
       if (hasModuleMap) {
-        objcProvider.addModuleMap(intermediateArtifacts.moduleMap());
+        CppModuleMap moduleMap = intermediateArtifacts.moduleMap();
+        Optional<Artifact> umbrellaHeader = moduleMap.getUmbrellaHeader();
+        if (umbrellaHeader.isPresent()) {
+          objcProvider.add(UMBRELLA_HEADER, umbrellaHeader.get());
+        }
+        objcProvider.add(MODULE_MAP, moduleMap.getArtifact());
+        objcProvider.add(TOP_LEVEL_MODULE_MAP, moduleMap);
       }
 
       objcProvider
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibrary.java
index 8839892deb..92f96ec95d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibrary.java
@@ -55,17 +55,12 @@ public class ObjcProtoLibrary implements RuleConfiguredTargetFactory {
     Iterable<ObjcProtoProvider> objcProtoProviders =
         ruleContext.getPrerequisites("deps", Mode.TARGET, ObjcProtoProvider.class);
 
-    Iterable<ObjcProvider> additionalDependencies =
-        ruleContext.getPrerequisites(
-            ObjcRuleClasses.PROTO_LIB_ATTR, Mode.TARGET, ObjcProvider.SKYLARK_CONSTRUCTOR);
-
     ProtobufSupport protoSupport =
         new ProtobufSupport(
                 ruleContext,
                 ruleContext.getConfiguration(),
                 protoProviders,
                 objcProtoProviders,
-                additionalDependencies,
                 getPortableProtoFilters(ruleContext, objcProtoProviders, protoProviders))
             .registerGenerationActions()
             .addFilesToBuild(filesToBuild);
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 4039722fbd..51a3c435fa 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
@@ -18,7 +18,6 @@ import static com.google.devtools.build.lib.collect.nestedset.Order.LINK_ORDER;
 import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
@@ -751,19 +750,6 @@ public final class ObjcProvider extends Info {
     }
 
     /**
-     * Convenience method to add a modulemap to this ObjcProvider.
-     */
-    public Builder addModuleMap(CppModuleMap moduleMap) {
-      Optional<Artifact> umbrellaHeader = moduleMap.getUmbrellaHeader();
-      if (umbrellaHeader.isPresent()) {
-        add(UMBRELLA_HEADER, umbrellaHeader.get());
-      }
-      add(MODULE_MAP, moduleMap.getArtifact());
-      add(TOP_LEVEL_MODULE_MAP, moduleMap);
-      return this;
-    }
-
-    /**
      * Add all elements from providers, and propagate them to any (transitive) dependers on this
      * ObjcProvider.
      */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java
index 08e595f8a0..93f0c9a1b9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java
@@ -73,7 +73,6 @@ final class ProtobufSupport {
   private final IntermediateArtifacts intermediateArtifacts;
   private final Set<PathFragment> dylibHandledProtoPaths;
   private final Iterable<ObjcProtoProvider> objcProtoProviders;
-  private final Iterable<ObjcProvider> dependencyObjcProviders;
   private final NestedSet<Artifact> portableProtoFilters;
 
   // Each entry of this map represents a generation action and a compilation action. The input set
@@ -103,15 +102,12 @@ final class ProtobufSupport {
    *     targets in a split configuration
    * @param protoProviders the list of ProtoSourcesProviders that this proto support should process
    * @param objcProtoProviders the list of ObjcProtoProviders that this proto support should process
-   * @param dependencyObjcProviders the list of extra objc dependencies to add to this support's
-   *     ObjcProvider
    */
   public ProtobufSupport(
       RuleContext ruleContext,
       BuildConfiguration buildConfiguration,
       Iterable<ProtoSourcesProvider> protoProviders,
       Iterable<ObjcProtoProvider> objcProtoProviders,
-      Iterable<ObjcProvider> dependencyObjcProviders,
       NestedSet<Artifact> portableProtoFilters) {
     this(
         ruleContext,
@@ -119,7 +115,6 @@ final class ProtobufSupport {
         NestedSetBuilder.<Artifact>stableOrder().build(),
         protoProviders,
         objcProtoProviders,
-        dependencyObjcProviders,
         portableProtoFilters);
   }
 
@@ -137,8 +132,6 @@ final class ProtobufSupport {
    *     symbols
    * @param protoProviders the list of ProtoSourcesProviders that this proto support should process
    * @param objcProtoProviders the list of ObjcProtoProviders that this proto support should process
-   * @param dependencyObjcProviders the list of extra objc dependencies to add to this support's
-   *     ObjcProvider
    */
   public ProtobufSupport(
       RuleContext ruleContext,
@@ -146,14 +139,12 @@ final class ProtobufSupport {
       NestedSet<Artifact> dylibHandledProtos,
       Iterable<ProtoSourcesProvider> protoProviders,
       Iterable<ObjcProtoProvider> objcProtoProviders,
-      Iterable<ObjcProvider> dependencyObjcProviders,
       NestedSet<Artifact> portableProtoFilters) {
     this.ruleContext = ruleContext;
     this.buildConfiguration = buildConfiguration;
     this.attributes = new ProtoAttributes(ruleContext);
     this.dylibHandledProtoPaths = runfilesPaths(dylibHandledProtos.toSet());
     this.objcProtoProviders = objcProtoProviders;
-    this.dependencyObjcProviders = dependencyObjcProviders;
     this.portableProtoFilters = portableProtoFilters;
     this.intermediateArtifacts =
         ObjcRuleClasses.intermediateArtifacts(ruleContext, buildConfiguration);
@@ -268,7 +259,12 @@ final class ProtobufSupport {
       return Optional.absent();
     }
 
-    ObjcProvider.Builder objcProviderBuilder = new ObjcProvider.Builder();
+    Iterable<PathFragment> includes = ImmutableList.of(getWorkspaceRelativeOutputDir());
+    ObjcCommon.Builder commonBuilder = new ObjcCommon.Builder(ruleContext);
+
+    if (!isLinkingTarget()) {
+      commonBuilder.setIntermediateArtifacts(intermediateArtifacts).setHasModuleMap();
+    }
 
     int actionId = 0;
     for (ImmutableSet<Artifact> inputProtos : inputsToOutputsMap.keySet()) {
@@ -279,41 +275,17 @@ final class ProtobufSupport {
           getCompilationArtifacts(intermediateArtifacts, inputProtos, outputProtos);
 
       ObjcCommon common = getCommon(intermediateArtifacts, compilationArtifacts);
-      if (isLinkingTarget()) {
-        // If ProtobufSupport is being called from a linking target (i.e. apple_binary), we
-        // propagate the generated headers (contained inside common.getObjcProvider()) into the
-        // ObjcProvider, in case the generated headers are used from dependent ios_test targets,
-        // which uses XcTestAppProvider to access those headers.
-        // TODO(b/64152968): Remove conditional when ios_test is gone and only propagate to
-        //     direct dependents.
-        objcProviderBuilder.addTransitiveAndPropagate(common.getObjcProvider());
-      } else {
-        // If ProtobufSupport is being called from a non-linking target (i.e. objc_proto_library),
-        // we propagate the headers only to direct dependents of this target. This avoids over
-        // accumulation of header files when a top-level target depends on many objc_proto_library
-        // targets.
-        objcProviderBuilder.addAsDirectDeps(common.getObjcProvider());
-      }
+      commonBuilder.addDepObjcProviders(ImmutableSet.of(common.getObjcProvider()));
       actionId++;
     }
 
-    // Same as with the headers, we only propagate the includes to direct dependents for non-linking
-    // targets (i.e. objc_proto_library), and we fully propagate the includes for linking targets
-    // in case ios_test dependents need to use the headers.
-    // TODO(b/64152968): Remove conditional when ios_test is gone and only propagate to direct
-    //     dependents.
-    Iterable<PathFragment> includes = ImmutableList.of(getWorkspaceRelativeOutputDir());
     if (isLinkingTarget()) {
-      objcProviderBuilder.addAll(ObjcProvider.INCLUDE, includes);
+      commonBuilder.addIncludes(includes);
     } else {
-      objcProviderBuilder.addAllForDirectDependents(ObjcProvider.INCLUDE, includes);
-      // For non linking targets, also create and propagate this target's module map.
-      objcProviderBuilder.addModuleMap(intermediateArtifacts.moduleMap());
+      commonBuilder.addDirectDependencyIncludes(includes);
     }
 
-    objcProviderBuilder.addTransitiveAndPropagate(dependencyObjcProviders);
-
-    return Optional.of(objcProviderBuilder.build());
+    return Optional.of(commonBuilder.build().getObjcProvider());
   }
 
   private NestedSet<Artifact> getProtobufHeaders() {