Execute the plist merge action if there is a bundle id to stamp onto the plist file.

RELNOTES: Set the bundle id on plist files referenced by XCode application target.

--
MOS_MIGRATED_REVID=92378112

4 files changed, 64 insertions, 32 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BundleSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/BundleSupport.java
index 4b586c1e6a..826ea260ad 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/BundleSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/BundleSupport.java
@@ -68,12 +68,18 @@ final class BundleSupport {
    *    rule's
    * @param optionsProvider provider containing options and plist settings for this rule and its
    *    dependencies
+   * @param primaryBundleId used to set the bundle identifier or override the existing one from
+   *     plist file, can be null
+   * @param fallbackBundleId used to set the bundle identifier if it is not set by plist file or
+   *     primary identifier, can be null
    * @param extraMergePlists additional plist files to merge
    */
   static InfoplistMerging infoPlistMerging(
       RuleContext ruleContext,
       ObjcProvider objcProvider,
       OptionsProvider optionsProvider,
+      String primaryBundleId,
+      String fallbackBundleId,
       ExtraMergePlists extraMergePlists) {
     IntermediateArtifacts intermediateArtifacts =
         ObjcRuleClasses.intermediateArtifacts(ruleContext);
@@ -86,6 +92,7 @@ final class BundleSupport {
             .addAll(extraMergePlists)
             .build())
         .setPlmerge(ruleContext.getExecutablePrerequisite("$plmerge", Mode.HOST))
+        .setBundleIdentifiers(primaryBundleId, fallbackBundleId)
         .build();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/InfoplistMerging.java b/src/main/java/com/google/devtools/build/lib/rules/objc/InfoplistMerging.java
index 017e15fbe9..3caf1b266c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/InfoplistMerging.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/InfoplistMerging.java
@@ -16,13 +16,13 @@ package com.google.devtools.build.lib.rules.objc;
 
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
 import com.google.devtools.build.lib.analysis.actions.CommandLine;
+import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 
@@ -31,9 +31,9 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
  * <ul>
  *   <li>the Info.plist which contains the fields from every source. If there is only one source
  *       plist, this is that plist.
- *   <li>the action to merge all the Infoplists into a single one. This is present even if there is
- *       only one Infoplist, to prevent a Bazel error when an Artifact does not have a generating
- *       action.
+ *   <li>the action to merge all the Infoplists into a single one and stamp the bundle ID on it.
+ *       This action is present if there is more than one plist file or there is a non-null bundle
+ *       ID to stamp on the merged plist file.
  * </ul>
  */
 class InfoplistMerging {
@@ -42,6 +42,8 @@ class InfoplistMerging {
     private NestedSet<Artifact> inputPlists;
     private FilesToRunProvider plmerge;
     private IntermediateArtifacts intermediateArtifacts;
+    private String primaryBundleId;
+    private String fallbackBundleId;
 
     public Builder(ActionConstructionContext context) {
       this.context = Preconditions.checkNotNull(context);
@@ -65,21 +67,38 @@ class InfoplistMerging {
     }
 
     /**
+     * Sets the potential bundle identifiers to stamp on the merged plist file.
+     *
+     * @param primaryBundleId used to set the bundle identifier or override the existing one from
+     *     plist file, can be null
+     * @param fallbackBundleId used to set the bundle identifier if it is not set by plist file or
+     *     primary identifier, can be null
+     */
+    public Builder setBundleIdentifiers(String primaryBundleId, String fallbackBundleId) {
+      this.primaryBundleId = primaryBundleId;
+      this.fallbackBundleId = fallbackBundleId;
+      return this;
+    }
+
+    /**
      * This static factory method prevents retention of the outer {@link Builder} class reference by
      * the anonymous {@link CommandLine} instance.
      */
-    private static CommandLine mergeCommandLine(
-        final NestedSet<Artifact> inputPlists, final Artifact mergedInfoplist) {
-      return new CommandLine() {
-        @Override
-        public Iterable<String> arguments() {
-          return new ImmutableList.Builder<String>()
-              .addAll(Interspersing.beforeEach(
-                  "--source_file", Artifact.toExecPaths(inputPlists)))
-              .add("--out_file", mergedInfoplist.getExecPathString())
-              .build();
-        }
-      };
+    private static CommandLine mergeCommandLine(NestedSet<Artifact> inputPlists,
+        Artifact mergedInfoplist, String primaryBundleId,  String fallbackBundleId) {
+      CustomCommandLine.Builder argBuilder = CustomCommandLine.builder()
+          .addBeforeEachExecPath("--source_file", inputPlists)
+          .addExecPath("--out_file", mergedInfoplist);
+
+      if (primaryBundleId != null) {
+        argBuilder.add("--primary_bundle_id").add(primaryBundleId);
+      }
+
+      if (fallbackBundleId != null) {
+        argBuilder.add("--fallback_bundle_id").add(fallbackBundleId);
+      }
+
+      return argBuilder.build();
     }
 
     public InfoplistMerging build() {
@@ -88,20 +107,23 @@ class InfoplistMerging {
       Optional<Artifact> plistWithEverything = Optional.absent();
       Action[] mergeActions = new Action[0];
 
-      int inputs = Iterables.size(inputPlists);
-      if (inputs == 1) {
-        plistWithEverything = Optional.of(Iterables.getOnlyElement(inputPlists));
-      } else if (inputs > 1) {
-        Artifact merged = intermediateArtifacts.mergedInfoplist();
-
-        plistWithEverything = Optional.of(merged);
-        mergeActions = new SpawnAction.Builder()
-            .setMnemonic("MergeInfoPlistFiles")
-            .setExecutable(plmerge)
-            .setCommandLine(mergeCommandLine(inputPlists, merged))
-            .addTransitiveInputs(inputPlists)
-            .addOutput(merged)
-            .build(context);
+      if (!inputPlists.isEmpty()) {
+        int inputs = Iterables.size(inputPlists);
+        if (inputs == 1 && primaryBundleId == null && fallbackBundleId == null) {
+          plistWithEverything = Optional.of(Iterables.getOnlyElement(inputPlists));
+        } else {
+          Artifact merged = intermediateArtifacts.mergedInfoplist();
+
+          plistWithEverything = Optional.of(merged);
+          mergeActions = new SpawnAction.Builder()
+              .setMnemonic("MergeInfoPlistFiles")
+              .setExecutable(plmerge)
+              .setCommandLine(
+                  mergeCommandLine(inputPlists, merged, primaryBundleId, fallbackBundleId))
+              .addTransitiveInputs(inputPlists)
+              .addOutput(merged)
+              .build(context);
+        }
       }
 
       return new InfoplistMerging(plistWithEverything, mergeActions, inputPlists);
@@ -120,8 +142,9 @@ class InfoplistMerging {
   }
 
   /**
-   * Creates action to merge multiple Info.plist files of a binary into a single Info.plist. No
-   * action is necessary if there is only one source.
+   * Creates action to merge multiple Info.plist files of a binary into a single Info.plist. The
+   * merge action is necessary if there are more than one input plist files or we have a bundle ID
+   * to stamp on the merged plist.
    */
   public Action[] getMergeAction() {
     return mergeActions;
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 005eb67a36..4506e6e1ae 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
@@ -90,6 +90,7 @@ public class ObjcBundleLibrary implements RuleConfiguredTargetFactory {
         .setObjcProvider(common.getObjcProvider())
         .setInfoplistMerging(
             BundleSupport.infoPlistMerging(ruleContext, common.getObjcProvider(), optionsProvider,
+                /*primaryBundleId=*/null, /*fallbackBundleId=*/null,
                 new BundleSupport.ExtraMergePlists()))
         .setIntermediateArtifacts(intermediateArtifacts)
         .setMinimumOsVersion(objcConfiguration.getMinimumOs())
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 da0c7abd8e..a51e4b787e 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
@@ -419,6 +419,7 @@ public final class ReleaseBundlingSupport {
         .setObjcProvider(objcProvider)
         .setInfoplistMerging(
             BundleSupport.infoPlistMerging(ruleContext, objcProvider, optionsProvider,
+                primaryBundleId, fallbackBundleId,
                 new BundleSupport.ExtraMergePlists(getGeneratedVersionPlist())))
         .setIntermediateArtifacts(ObjcRuleClasses.intermediateArtifacts(ruleContext))
         .setPrimaryBundleId(primaryBundleId)