Avoid linking objc protos that are transitively in the "dylibs" attribute

--
PiperOrigin-RevId: 143601292
MOS_MIGRATED_REVID=143601292

6 files changed, 85 insertions, 20 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index 5497fc8cf3..a8350680bf 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -635,7 +635,7 @@ public class BazelRuleClassProvider {
           builder.addRuleDefinition(new ObjcXcodeprojRule());
           builder.addRuleDefinition(new ObjcRuleClasses.CoptsRule());
           builder.addRuleDefinition(new ObjcRuleClasses.BundlingRule());
-          builder.addRuleDefinition(new ObjcRuleClasses.DylibDependingRule());
+          builder.addRuleDefinition(new ObjcRuleClasses.DylibDependingRule(objcProtoAspect));
           builder.addRuleDefinition(new ObjcRuleClasses.ReleaseBundlingRule());
           builder.addRuleDefinition(new ObjcRuleClasses.SimulatorRule());
           builder.addRuleDefinition(new ObjcRuleClasses.CompilingRule());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java
index abd5eddb34..c20133319a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java
@@ -94,6 +94,8 @@ public class AppleBinary implements RuleConfiguredTargetFactory {
         ruleContext.getPrerequisitesByConfiguration("deps", Mode.SPLIT);
     Iterable<ObjcProvider> dylibProviders =
         ruleContext.getPrerequisites(DYLIBS_ATTR_NAME, Mode.TARGET, ObjcProvider.class);
+    Iterable<ObjcProtoProvider> dylibProtoProviders =
+        ruleContext.getPrerequisites(DYLIBS_ATTR_NAME, Mode.TARGET, ObjcProtoProvider.class);
 
     BundleLoaderProvider bundleLoaderProvider =
         ruleContext.getPrerequisite(
@@ -115,6 +117,7 @@ public class AppleBinary implements RuleConfiguredTargetFactory {
             configToDepsCollectionMap,
             configurationToNonPropagatedObjcMap,
             dylibProviders,
+            dylibProtoProviders,
             bundleLoaderObjcProvider);
 
     multiArchBinarySupport.registerActions(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibrary.java
index a4f9f232bf..50f7d10e6d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibrary.java
@@ -34,6 +34,7 @@ import com.google.devtools.build.lib.rules.apple.Platform;
 import com.google.devtools.build.lib.rules.apple.Platform.PlatformType;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
 import com.google.devtools.build.lib.rules.objc.CompilationSupport.ExtraLinkArgs;
+
 import java.util.Map;
 import java.util.Set;
 
@@ -56,17 +57,21 @@ public class AppleDynamicLibrary implements RuleConfiguredTargetFactory {
         ruleContext.getPrerequisitesByConfiguration("deps", Mode.SPLIT);
     Iterable<ObjcProvider> dylibProviders =
         ruleContext.getPrerequisites(DYLIBS_ATTR_NAME, Mode.TARGET, ObjcProvider.class);
+    Iterable<ObjcProtoProvider> dylibProtoProviders =
+        ruleContext.getPrerequisites(DYLIBS_ATTR_NAME, Mode.TARGET, ObjcProtoProvider.class);
     Set<BuildConfiguration> childConfigurations = getChildConfigurations(ruleContext);
     Artifact outputArtifact =
         ObjcRuleClasses.intermediateArtifacts(ruleContext).combinedArchitectureDylib();
 
     MultiArchBinarySupport multiArchBinarySupport = new MultiArchBinarySupport(ruleContext);
+
     Map<BuildConfiguration, ObjcProvider> objcProviderByDepConfiguration =
         multiArchBinarySupport.objcProviderByDepConfiguration(
             childConfigurations,
             configToDepsCollectionMap,
             configurationToNonPropagatedObjcMap,
             dylibProviders,
+            dylibProtoProviders,
             Optional.<ObjcProvider>absent());
 
     multiArchBinarySupport.registerActions(
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 774e00334f..11f8696a28 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
@@ -24,11 +24,13 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.rules.apple.Platform;
 import com.google.devtools.build.lib.rules.objc.CompilationSupport.ExtraLinkArgs;
 import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes;
+
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -131,8 +133,10 @@ public class MultiArchBinarySupport {
    *     current rule have propagated in that configuration
    * @param configurationToNonPropagatedObjcMap a map from child configuration to providers that
    *     "non_propagated_deps" of the current rule have propagated in that configuration
-   * @param dylibProviders providers that dynamic library dependencies of the current rule have
-   *     propagated
+   * @param dylibObjcProviders {@link ObjcProvider}s that dynamic library dependencies of the
+   *     current rule have propagated
+   * @param dylibProtoProviders {@link ObjcProtoProvider} providers that dynamic library
+   *     dependencies of the current rule have propagated
    * @param bundleLoaderObjcProvider Optional ObjcProvider containing artifacts and paths to be
    *     included in this binary's compilation actions
    * @throws RuleErrorException if there are attribute errors in the current rule context
@@ -141,16 +145,19 @@ public class MultiArchBinarySupport {
       Set<BuildConfiguration> childConfigurations,
       ImmutableListMultimap<BuildConfiguration, TransitiveInfoCollection> configToDepsCollectionMap,
       ImmutableListMultimap<BuildConfiguration, ObjcProvider> configurationToNonPropagatedObjcMap,
-      Iterable<ObjcProvider> dylibProviders,
+      Iterable<ObjcProvider> dylibObjcProviders,
+      Iterable<ObjcProtoProvider> dylibProtoProviders,
       Optional<ObjcProvider> bundleLoaderObjcProvider)
       throws RuleErrorException, InterruptedException {
     ImmutableMap.Builder<BuildConfiguration, ObjcProvider> configurationToObjcProviderBuilder =
         ImmutableMap.builder();
+
     for (BuildConfiguration childConfig : childConfigurations) {
       Optional<ObjcProvider> protosObjcProvider;
       if (ObjcRuleClasses.objcConfiguration(ruleContext).enableAppleBinaryNativeProtos()) {
         ProtobufSupport protoSupport =
-            new ProtobufSupport(ruleContext, childConfig)
+            new ProtobufSupport(ruleContext, childConfig,
+                    protoArtifactsToAvoid(dylibProtoProviders))
                 .registerGenerationActions()
                 .registerCompilationActions();
         protosObjcProvider = protoSupport.getObjcProvider();
@@ -163,7 +170,7 @@ public class MultiArchBinarySupport {
 
       Iterable<ObjcProvider> additionalDepProviders =
           Iterables.concat(
-              dylibProviders,
+              dylibObjcProviders,
               ruleContext.getPrerequisites("bundles", Mode.TARGET, ObjcProvider.class),
               protosObjcProvider.asSet(),
               bundleLoaderObjcProvider.asSet());
@@ -176,7 +183,7 @@ public class MultiArchBinarySupport {
               nullToEmptyList(configToDepsCollectionMap.get(childConfig)),
               nullToEmptyList(configurationToNonPropagatedObjcMap.get(childConfig)),
               additionalDepProviders);
-      ObjcProvider objcProvider = common.getObjcProvider().subtractSubtrees(dylibProviders);
+      ObjcProvider objcProvider = common.getObjcProvider().subtractSubtrees(dylibObjcProviders);
 
       configurationToObjcProviderBuilder.put(childConfig, objcProvider);
     }
@@ -218,4 +225,15 @@ public class MultiArchBinarySupport {
   private <T> List<T> nullToEmptyList(List<T> inputList) {
     return inputList != null ? inputList : ImmutableList.<T>of();
   }
+
+  private static NestedSet<Artifact> protoArtifactsToAvoid(
+      Iterable<ObjcProtoProvider> avoidedProviders) {
+    NestedSetBuilder<Artifact> avoidArtifacts = NestedSetBuilder.stableOrder();
+    for (ObjcProtoProvider avoidProvider : avoidedProviders) {
+      for (NestedSet<Artifact> avoidProviderOutputGroup : avoidProvider.getProtoGroups()) {
+        avoidArtifacts.addTransitive(avoidProviderOutputGroup);
+      }
+    }
+    return avoidArtifacts.build();
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
index 7209c7a9bb..14776b30df 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
@@ -957,6 +957,12 @@ public class ObjcRuleClasses {
    */
   public static class DylibDependingRule implements RuleDefinition {
 
+    private final ObjcProtoAspect objcProtoAspect;
+
+    public DylibDependingRule(ObjcProtoAspect objcProtoAspect) {
+      this.objcProtoAspect = objcProtoAspect;
+    }
+
     /**
      * Attribute name for dylib dependencies.
      */
@@ -981,7 +987,8 @@ public class ObjcRuleClasses {
               .direct_compile_time_input()
               .mandatoryNativeProviders(
                   ImmutableList.<Class<? extends TransitiveInfoProvider>>of(ObjcProvider.class))
-              .allowedFileTypes())
+              .allowedFileTypes()
+              .aspect(objcProtoAspect))
           .build();
     }
 
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 f96b8e81a4..f8b8a7deb2 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
@@ -17,9 +17,11 @@ package com.google.devtools.build.lib.rules.objc;
 import static com.google.devtools.build.lib.rules.objc.XcodeProductType.LIBRARY_STATIC;
 
 import com.google.common.base.Optional;
+import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSetMultimap;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Ordering;
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -37,6 +39,7 @@ import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.
 import com.google.devtools.build.lib.rules.proto.ProtoSourcesProvider;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
+
 import java.util.HashMap;
 import java.util.Set;
 
@@ -57,6 +60,9 @@ import java.util.Set;
  */
 final class ProtobufSupport {
 
+  private static final String HEADER_SUFFIX = ".pbobjc.h";
+  private static final String SOURCE_SUFFIX = ".pbobjc.m";
+
   private static final String BUNDLED_PROTOS_IDENTIFIER = "BundledProtos";
 
   private static final String UNIQUE_DIRECTORY_NAME = "_generated_protos";
@@ -65,6 +71,7 @@ final class ProtobufSupport {
   private final BuildConfiguration buildConfiguration;
   private final ProtoAttributes attributes;
   private final IntermediateArtifacts intermediateArtifacts;
+  private final Set<Artifact> dylibHandledProtos;
 
   // Each entry of this map represents a generation action and a compilation action. The input set
   // are dependencies of the output set. The output set is always a subset of, or the same set as,
@@ -90,7 +97,7 @@ final class ProtobufSupport {
    *
    * @param ruleContext context this proto library is constructed in
    */
-  public ProtobufSupport(RuleContext ruleContext) throws RuleErrorException {
+  public ProtobufSupport(RuleContext ruleContext) {
     this(ruleContext, ruleContext.getConfiguration());
   }
 
@@ -104,12 +111,30 @@ final class ProtobufSupport {
    * @param buildConfiguration the configuration from which to get prerequisites when building proto
    *     targets in a split configuration
    */
-  public ProtobufSupport(RuleContext ruleContext, BuildConfiguration buildConfiguration)
-      throws RuleErrorException {
+  public ProtobufSupport(RuleContext ruleContext, BuildConfiguration buildConfiguration) {
+    this(ruleContext, buildConfiguration, NestedSetBuilder.<Artifact>stableOrder().build());
+  }
+
+  /**
+   * Creates a new proto support for the protobuf library. This support code bundles up all the
+   * transitive protos within the groups in which they were defined. We use that information to
+   * minimize the number of inputs per generation/compilation actions by only providing what is
+   * really needed to the actions.
+   *
+   * @param ruleContext context this proto library is constructed in
+   * @param buildConfiguration the configuration from which to get prerequisites when building proto
+   *     targets in a split configuration
+   * @param dylibHandledProtos a set of protos linked into dynamic libraries that the current
+   *     rule depends on; these protos will not be output by this support, thus avoiding duplicate
+   *     symbols
+   */
+  public ProtobufSupport(RuleContext ruleContext, BuildConfiguration buildConfiguration,
+      NestedSet<Artifact> dylibHandledProtos) {
     this.ruleContext = ruleContext;
     this.buildConfiguration = buildConfiguration;
     this.attributes = new ProtoAttributes(ruleContext);
     this.inputsToOutputsMap = getInputsToOutputsMap();
+    this.dylibHandledProtos = dylibHandledProtos.toSet();
     this.intermediateArtifacts =
         ObjcRuleClasses.intermediateArtifacts(ruleContext, buildConfiguration);
   }
@@ -141,7 +166,7 @@ final class ProtobufSupport {
             .setPchFile(Optional.<Artifact>absent())
             .addAdditionalHdrs(getProtobufHeaders())
             .addAdditionalHdrs(
-                getGeneratedProtoOutputs(inputsToOutputsMap.values(), ".pbobjc.h"));
+                getGeneratedProtoOutputs(inputsToOutputsMap.values(), HEADER_SUFFIX));
 
     new LegacyCompilationSupport(
             ruleContext,
@@ -184,8 +209,9 @@ final class ProtobufSupport {
   public ProtobufSupport addFilesToBuild(NestedSetBuilder<Artifact> filesToBuild) {
     for (ImmutableSet<Artifact> inputProtoFiles : inputsToOutputsMap.keySet()) {
       ImmutableSet<Artifact> outputProtoFiles = inputsToOutputsMap.get(inputProtoFiles);
-      Iterable<Artifact> generatedSources = getGeneratedProtoOutputs(outputProtoFiles, ".pbobjc.m");
-      Iterable<Artifact> generatedHeaders = getGeneratedProtoOutputs(outputProtoFiles, ".pbobjc.h");
+      Iterable<Artifact> generatedSources = getProtoSourceFilesForCompilation(outputProtoFiles);
+      Iterable<Artifact> generatedHeaders = getGeneratedProtoOutputs(outputProtoFiles,
+          HEADER_SUFFIX);
 
       filesToBuild.addAll(generatedSources).addAll(generatedHeaders);
     }
@@ -314,8 +340,7 @@ final class ProtobufSupport {
     return protobufHeaderSearchPaths.build();
   }
 
-  private ImmutableSetMultimap<ImmutableSet<Artifact>, Artifact> getInputsToOutputsMap()
-      throws RuleErrorException {
+  private ImmutableSetMultimap<ImmutableSet<Artifact>, Artifact> getInputsToOutputsMap() {
     Iterable<ObjcProtoProvider> objcProtoProviders = getObjcProtoProviders();
     Iterable<ProtoSourcesProvider> protoProviders = getProtoSourcesProviders();
 
@@ -452,16 +477,23 @@ final class ProtobufSupport {
         new CompilationArtifacts.Builder()
             .setIntermediateArtifacts(intermediateArtifacts)
             .setPchFile(Optional.<Artifact>absent())
-            .addAdditionalHdrs(getGeneratedProtoOutputs(filteredInputProtos, ".pbobjc.h"))
+            .addAdditionalHdrs(getGeneratedProtoOutputs(filteredInputProtos, HEADER_SUFFIX))
             .addAdditionalHdrs(getProtobufHeaders());
 
     if (isLinkingTarget()) {
-      compilationArtifacts.addNonArcSrcs(getGeneratedProtoOutputs(outputProtoFiles, ".pbobjc.m"));
+      compilationArtifacts.addNonArcSrcs(getProtoSourceFilesForCompilation(outputProtoFiles));
     }
 
     return compilationArtifacts.build();
   }
 
+  private Iterable<Artifact> getProtoSourceFilesForCompilation(
+      Iterable<Artifact> outputProtoFiles) {
+    Iterable<Artifact> filteredOutputs =
+        Iterables.filter(outputProtoFiles, Predicates.not(Predicates.in(dylibHandledProtos)));
+    return getGeneratedProtoOutputs(filteredOutputs, SOURCE_SUFFIX);
+  }
+
   private void registerGenerationAction(
       Iterable<Artifact> outputProtos, Iterable<Artifact> inputProtos, String protoFileSuffix) {
     Artifact protoInputsFile = getProtoInputsFile(protoFileSuffix);
@@ -478,8 +510,8 @@ final class ProtobufSupport {
             .addTransitiveInputs(getPortableProtoFilters())
             .addInput(protoInputsFile)
             .addInputs(inputProtos)
-            .addOutputs(getGeneratedProtoOutputs(outputProtos, ".pbobjc.h"))
-            .addOutputs(getGeneratedProtoOutputs(outputProtos, ".pbobjc.m"))
+            .addOutputs(getGeneratedProtoOutputs(outputProtos, HEADER_SUFFIX))
+            .addOutputs(getProtoSourceFilesForCompilation(outputProtos))
             .setExecutable(attributes.getProtoCompiler().getExecPath())
             .setCommandLine(getGenerationCommandLine(protoInputsFile))
             .build(ruleContext));