Only use a single copy of a nested bundle with ios_multi_cpus.

We generate a nested bundle for each architecture specified in ios_multi_cpus
which would cause any artifacts generated by these rules (such as info plists
or compiled resource files) to clash in the final application bundle (where
the bundle directory only exists once). For now just pick one architecture at
random to store the bundle in - unfortunately we still generate the bundle for
all architectures.

--
MOS_MIGRATED_REVID=89152199

7 files changed, 48 insertions, 22 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BundleMergeControlBytes.java b/src/main/java/com/google/devtools/build/lib/rules/objc/BundleMergeControlBytes.java
index db2ad424ff..24c5f58660 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/BundleMergeControlBytes.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/BundleMergeControlBytes.java
@@ -111,7 +111,9 @@ final class BundleMergeControlBytes extends ByteSource {
     }
 
     for (Bundling nestedBundling : bundling.getObjcProvider().get(NESTED_BUNDLE)) {
-      control.addNestedBundle(control(mergeZipPrefix, nestedBundling));
+      if (nestedBundling.getArchitecture().equals(bundling.getArchitecture())) {
+        control.addNestedBundle(control(mergeZipPrefix, nestedBundling));
+      }
     }
     
     if (bundling.getPrimaryBundleId()  != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/Bundling.java b/src/main/java/com/google/devtools/build/lib/rules/objc/Bundling.java
index 3174567e8e..acf40675d8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/Bundling.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/Bundling.java
@@ -49,12 +49,22 @@ final class Bundling {
     private IntermediateArtifacts intermediateArtifacts;
     private String primaryBundleId;
     private String fallbackBundleId;
+    private String architecture;
 
     public Builder setName(String name) {
       this.name = name;
       return this;
     }
 
+    /**
+     * Sets the CPU architecture this bundling was constructed for. Legal value are any that may be
+     * set on {@link ObjcConfiguration#getIosCpu()}.
+     */
+    public Builder setArchitecture(String architecture) {
+      this.architecture = architecture;
+      return this;
+    }
+
     public Builder setBundleDirFormat(String bundleDirFormat) {
       this.bundleDirFormat = bundleDirFormat;
       return this;
@@ -129,11 +139,12 @@ final class Bundling {
 
       return new Bundling(name, bundleDirFormat, combinedArchitectureBinary, extraBundleFiles,
           objcProvider, infoplistMerging, actoolzipOutput, bundleContentArtifacts, mergeZips, 
-          primaryBundleId, fallbackBundleId);
+          primaryBundleId, fallbackBundleId, architecture);
     }
   }
 
   private final String name;
+  private final String architecture;
   private final String bundleDirFormat;
   private final Optional<Artifact> combinedArchitectureBinary;
   private final ImmutableList<BundleableFile> extraBundleFiles;
@@ -156,7 +167,8 @@ final class Bundling {
       NestedSet<Artifact> bundleContentArtifacts,
       NestedSet<Artifact> mergeZips,
       String primaryBundleId,
-      String fallbackBundleId) {
+      String fallbackBundleId,
+      String architecture) {
     this.name = Preconditions.checkNotNull(name);
     this.bundleDirFormat = Preconditions.checkNotNull(bundleDirFormat);
     this.combinedArchitectureBinary = Preconditions.checkNotNull(combinedArchitectureBinary);
@@ -166,8 +178,9 @@ final class Bundling {
     this.actoolzipOutput = Preconditions.checkNotNull(actoolzipOutput);
     this.bundleContentArtifacts = Preconditions.checkNotNull(bundleContentArtifacts);
     this.mergeZips = Preconditions.checkNotNull(mergeZips);
-    this.primaryBundleId = primaryBundleId;
     this.fallbackBundleId = fallbackBundleId;
+    this.primaryBundleId = primaryBundleId;
+    this.architecture = Preconditions.checkNotNull(architecture);
   }
 
   /**
@@ -272,4 +285,11 @@ final class Bundling {
   public String getFallbackBundleId() {
     return fallbackBundleId;
   }
+
+  /**
+   * Returns the iOS CPU architecture this bundle was constructed for.
+   */
+  public String getArchitecture() {
+    return architecture;
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcBundleLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcBundleLibrary.java
index 1d44401f3b..1de38f13a5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcBundleLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcBundleLibrary.java
@@ -84,6 +84,7 @@ public class ObjcBundleLibrary implements RuleConfiguredTargetFactory {
         ObjcRuleClasses.intermediateArtifacts(ruleContext);
     return new Bundling.Builder()
         .setName(ruleContext.getLabel().getName())
+        .setArchitecture(ObjcRuleClasses.objcConfiguration(ruleContext).getIosCpu())
         .setBundleDirFormat("%s.bundle")
         .setObjcProvider(common.getObjcProvider())
         .setInfoplistMerging(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java
index b75199d286..50290d5e48 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java
@@ -56,7 +56,7 @@ public class
   public String iosSimulatorDevice;
 
   @Option(name = "ios_cpu",
-      defaultValue = "i386",
+      defaultValue = DEFAULT_IOS_CPU,
       category = "build",
       help = "Specifies to target CPU of iOS compilation.")
   public String iosCpu;
@@ -110,6 +110,7 @@ public class
   public String iosSplitCpu;
 
   @VisibleForTesting static final String DEFAULT_MINIMUM_IOS = "7.0";
+  @VisibleForTesting static final String DEFAULT_IOS_CPU = "i386";
 
   @Override
   public void addAllLabels(Multimap<String, Label> labelMap) {}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
index 61b81d3de4..d5d1b98828 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
@@ -200,6 +200,22 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment {
     return iosMultiCpus;
   }
 
+  /**
+   * Returns the architecture for which we keep dependencies that should be present only once (in a
+   * single architecture).
+   *
+   * <p>When building with multiple architectures there are some dependencies we want to avoid
+   * duplicating: they would show up more than once in the same location in the final application
+   * bundle which is illegal. Instead we pick one architecture for which to keep all dependencies
+   * and discard any others.
+   */
+  public String getDependencySingleArchitecture() {
+    if (!getIosMultiCpus().isEmpty()) {
+      return getIosMultiCpus().get(0);
+    }
+    return getIosCpu();
+  }
+
   @Override
   public String getName() {
     return "Objective-C";
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java
index 76a44ba317..636eac4bab 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java
@@ -379,6 +379,8 @@ public final class ReleaseBundlingSupport {
 
     return new Bundling.Builder()
         .setName(ruleContext.getLabel().getName())
+        // Architecture that determines which nested bundles are kept.
+        .setArchitecture(objcConfiguration.getDependencySingleArchitecture())
         .setBundleDirFormat(bundleDirFormat)
         .setExtraBundleFiles(extraBundleFiles)
         .setObjcProvider(objcProvider)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/XcodeProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/XcodeProvider.java
index 5bdaf865b2..ac20c8ea8a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/XcodeProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/XcodeProvider.java
@@ -135,7 +135,7 @@ public final class XcodeProvider implements TransitiveInfoProvider {
      */
     public Builder addDependencies(
         Iterable<XcodeProvider> dependencies, ObjcConfiguration configuration) {
-      String architecture = getDependencyArchitecture(configuration);
+      String architecture = configuration.getDependencySingleArchitecture();
       for (XcodeProvider dependency : dependencies) {
         // TODO(bazel-team): This is messy. Maybe we should make XcodeProvider be able to specify
         // how to depend on it rather than require this method to choose based on the dependency's
@@ -153,22 +153,6 @@ public final class XcodeProvider implements TransitiveInfoProvider {
     }
 
     /**
-     * Returns the architecture for which we keep dependencies.
-     *
-     * <p>When building with multiple architectures we want to avoid duplicating the same target for
-     * each architecture. Instead we pick one architecture for which to keep all dependencies and
-     * discard any others.
-     */
-    private String getDependencyArchitecture(ObjcConfiguration configuration) {
-      List<String> iosMultiCpus = configuration.getIosMultiCpus();
-      String architecture = configuration.getIosCpu();
-      if (!iosMultiCpus.isEmpty()) {
-        architecture = iosMultiCpus.get(0);
-      }
-      return architecture;
-    }
-
-    /**
      * Adds additional build settings of this target.
      */
     public Builder addXcodeprojBuildSettings(