Ensure that the plist inside an .ipa bundle produced by blaze and the adjacent plist read by xcode are identical.

To do this, we use the output of plmerge as the single plist for the bundle.  Automatic entries and variable substitutions are both computed in blaze and passed into plmerge.  The output of plmerge is passed into bundlemerge to be placed directly into the final bundle.

--
MOS_MIGRATED_REVID=110770779

6 files changed, 210 insertions, 64 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BUILD b/src/main/java/com/google/devtools/build/lib/rules/objc/BUILD
index 26da7d0188..c746977eff 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/BUILD
@@ -25,9 +25,11 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/rules/cpp",
         "//src/main/java/com/google/devtools/common/options",
         "//src/main/protobuf:bundlemerge_proto",
+        "//src/main/protobuf:plmerge_proto",
         "//src/main/protobuf:xcodegen_proto",
         "//third_party:guava",
         "//third_party:jsr305",
+        "//third_party/java/dd_plist",
     ],
 )
 
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 4327b926e3..40a0b84479 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
@@ -22,16 +22,14 @@ import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.xcode.bundlemerge.proto.BundleMergeProtos;
 import com.google.devtools.build.xcode.bundlemerge.proto.BundleMergeProtos.Control;
 import com.google.devtools.build.xcode.bundlemerge.proto.BundleMergeProtos.MergeZip;
-import com.google.devtools.build.xcode.bundlemerge.proto.BundleMergeProtos.VariableSubstitution;
 
 import java.io.InputStream;
-import java.util.Map;
 
 /**
- * A byte source that can be used to generate a control file for the tool:
- * {@code //java/com/google/devtools/build/xcode/bundlemerge}. Note that this generates the control
- * proto and bytes on-the-fly rather than eagerly. This is to prevent a copy of the bundle files and
- * .xcdatamodels from being stored for each {@code objc_binary} (or any bundle) being built.
+ * A byte source that can be used to generate a control file for the tool bundlemerge . 
+ * Note that this generates the control proto and bytes on-the-fly rather than eagerly. 
+ * This is to prevent a copy of the bundle files and .xcdatamodels from being stored for 
+ * each {@code objc_binary} (or any bundle) being built.
  */
 // TODO(bazel-team): Move the logic in this class to Bundling (as a .toControl method).
 final class BundleMergeControlBytes extends ByteSource {
@@ -62,17 +60,16 @@ final class BundleMergeControlBytes extends ByteSource {
     BundleMergeProtos.Control.Builder control =
         BundleMergeProtos.Control.newBuilder()
             .addAllBundleFile(BundleableFile.toBundleFiles(bundling.getBundleFiles()))
-            // TODO(bazel-team): This should really be bundling.getBundleInfoplistInputs since
-            // (most of) those are editable, whereas this is usually the programatically merged
-            // plist. If we pass the sources here though, any synthetic data (generated plists with
-            // blaze-derived values) should be passed as well.
-            .addAllSourcePlistFile(Artifact.toExecPaths(bundling.getBundleInfoplist().asSet()))
             // TODO(bazel-team): Add rule attribute for specifying targeted device family
             .setMinimumOsVersion(bundling.getMinimumOsVersion().toString())
             .setSdkVersion(appleConfiguration.getIosSdkVersion().toString())
             .setPlatform(appleConfiguration.getBundlingPlatform().name())
             .setBundleRoot(bundling.getBundleDir());
 
+    if (bundling.getBundleInfoplist().isPresent()) {
+      control.setBundleInfoPlistFile((bundling.getBundleInfoplist().get().getExecPathString()));
+    }
+    
     for (Artifact mergeZip : bundling.getMergeZips()) {
       control.addMergeZip(MergeZip.newBuilder()
           .setEntryNamePrefix(mergeZipPrefix)
@@ -84,24 +81,15 @@ final class BundleMergeControlBytes extends ByteSource {
       control.addTargetDeviceFamily(targetDeviceFamily.name());
     }
 
-    Map<String, String> variableSubstitutions = bundling.variableSubstitutions();
-    for (String variable : variableSubstitutions.keySet()) {
-      control.addVariableSubstitution(VariableSubstitution.newBuilder()
-          .setName(variable)
-          .setValue(variableSubstitutions.get(variable))
-          .build());
-    }
-
     control.setOutFile(mergedIpa.getExecPathString());
 
     for (Artifact linkedBinary : bundling.getCombinedArchitectureBinary().asSet()) {
-      control
-          .addBundleFile(BundleMergeProtos.BundleFile.newBuilder()
+      control.addBundleFile(
+          BundleMergeProtos.BundleFile.newBuilder()
               .setSourceFile(linkedBinary.getExecPathString())
               .setBundlePath(bundling.getName())
               .setExternalFileAttribute(BundleableFile.EXECUTABLE_EXTERNAL_FILE_ATTRIBUTE)
-              .build())
-          .setExecutableName(bundling.getName());
+              .build());
     }
 
     for (Bundling nestedBundling : bundling.getNestedBundlings()) {
@@ -110,7 +98,7 @@ final class BundleMergeControlBytes extends ByteSource {
       }
     }
     
-    if (bundling.getPrimaryBundleId()  != null) {
+    if (bundling.getPrimaryBundleId() != null) {
       control.setPrimaryBundleIdentifier(bundling.getPrimaryBundleId());
     }
     
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 11ec5e59a4..9e1d8e0bb5 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
@@ -27,6 +27,7 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.actions.BinaryFileWriteAction;
 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;
@@ -98,7 +99,7 @@ final class BundleSupport {
     this.bundling = bundling;
     this.attributes = new Attributes(ruleContext);
   }
-  
+
   /**
    * Registers actions required for constructing this bundle, namely merging all involved {@code
    * Info.plist} files and generating asset catalogues.
@@ -111,9 +112,14 @@ final class BundleSupport {
     registerConvertXibsActions(objcProvider);
     registerMomczipActions(objcProvider);
     registerInterfaceBuilderActions(objcProvider);
-    registerMergeInfoplistAction();
     registerActoolActionIfNecessary(objcProvider);
 
+    if (bundling.needsToMergeInfoplist()) {
+      NestedSet<Artifact> mergingContentArtifacts = bundling.getMergingContentArtifacts();
+      Artifact mergedPlist = bundling.getBundleInfoplist().get();
+      PlMergeControlBytes plMergeControlBytes = new PlMergeControlBytes(bundling, mergedPlist);
+      registerMergeInfoplistAction(mergingContentArtifacts, plMergeControlBytes);
+    }
     return this;
   }
 
@@ -342,34 +348,31 @@ final class BundleSupport {
    * 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.
    */
-  private void registerMergeInfoplistAction() {
+  private void registerMergeInfoplistAction(
+      NestedSet<Artifact> mergingContentArtifacts, PlMergeControlBytes controlBytes) {
     if (!bundling.needsToMergeInfoplist()) {
       return; // Nothing to do here.
     }
+    
+    Artifact plMergeControlArtifact =
+        ObjcRuleClasses.artifactByAppendingToBaseName(ruleContext, ".plmerge-control");
 
-    ruleContext.registerAction(new SpawnAction.Builder()
-        .setMnemonic("MergeInfoPlistFiles")
-        .setExecutable(attributes.plmerge())
-        .setCommandLine(mergeCommandLine())
-        .addInputs(bundling.getBundleInfoplistInputs())
-        .addOutput(ObjcRuleClasses.intermediateArtifacts(ruleContext).mergedInfoplist())
-        .build(ruleContext));
-  }
-
-  private CommandLine mergeCommandLine() {
-    CustomCommandLine.Builder argBuilder = CustomCommandLine.builder()
-        .addBeforeEachExecPath("--source_file", bundling.getBundleInfoplistInputs())
-        .addExecPath(
-            "--out_file", ObjcRuleClasses.intermediateArtifacts(ruleContext).mergedInfoplist());
-
-    if (bundling.getPrimaryBundleId() != null) {
-      argBuilder.add("--primary_bundle_id").add(bundling.getPrimaryBundleId());
-    }
-    if (bundling.getFallbackBundleId() != null) {
-      argBuilder.add("--fallback_bundle_id").add(bundling.getFallbackBundleId());
-    }
+    ruleContext.registerAction(
+        new BinaryFileWriteAction(
+            ruleContext.getActionOwner(),
+            plMergeControlArtifact,
+            controlBytes,
+            /*makeExecutable=*/ false));
 
-    return argBuilder.build();
+    ruleContext.registerAction(
+        new SpawnAction.Builder()
+            .setMnemonic("MergeInfoPlistFiles")
+            .setExecutable(attributes.plmerge())
+            .addArgument("--control")
+            .addInputArgument(plMergeControlArtifact)
+            .addTransitiveInputs(mergingContentArtifacts)
+            .addOutput(ObjcRuleClasses.intermediateArtifacts(ruleContext).mergedInfoplist())
+            .build(ruleContext));
   }
 
   private void registerActoolActionIfNecessary(ObjcProvider objcProvider) {
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 7fd7664d5c..da4843444f 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
@@ -56,7 +56,8 @@ final class Bundling {
     private String bundleDirFormat;
     private ImmutableList.Builder<BundleableFile> bundleFilesBuilder = ImmutableList.builder();
     private ObjcProvider objcProvider;
-    private NestedSetBuilder<Artifact> infoplists = NestedSetBuilder.stableOrder();
+    private NestedSetBuilder<Artifact> infoplistInputs = NestedSetBuilder.stableOrder();
+    private Artifact automaticEntriesInfoplistInput;
     private IntermediateArtifacts intermediateArtifacts;
     private String primaryBundleId;
     private String fallbackBundleId;
@@ -94,15 +95,24 @@ final class Bundling {
 
     /**
      * Adds an artifact representing an {@code Info.plist} as an input to this bundle's
-     * {@code Info.plist} (which is merged from any such added plists plus some additional
-     * information).
+     * {@code Info.plist} (which is merged from any such added plists plus the generated
+     * automatic entries plist).
      */
     public Builder addInfoplistInput(Artifact infoplist) {
-      this.infoplists.add(infoplist);
+      this.infoplistInputs.add(infoplist);
       return this;
     }
 
     /**
+     * Adds an artifact representing an {@code Info.plist} that contains automatic entries
+     * generated by xcode.
+     */
+    public Builder setAutomaticEntriesInfoplistInput(Artifact automaticEntriesInfoplist) {
+      this.automaticEntriesInfoplistInput = automaticEntriesInfoplist;
+      return this;
+    }
+    
+    /**
      * Adds any info plists specified in the given rule's {@code infoplist} attribute as well as
      * from its {@code options} as inputs to this bundle's {@code Info.plist} (which is merged from
      * any such added plists plus some additional information).
@@ -112,12 +122,12 @@ final class Bundling {
         OptionsProvider optionsProvider = ruleContext
             .getPrerequisite("options", Mode.TARGET, OptionsProvider.class);
         if (optionsProvider != null) {
-          infoplists.addAll(optionsProvider.getInfoplists());
+          infoplistInputs.addAll(optionsProvider.getInfoplists());
         }
       }
       Artifact infoplist = ruleContext.getPrerequisiteArtifact("infoplist", Mode.TARGET);
       if (infoplist != null) {
-        infoplists.add(infoplist);
+        infoplistInputs.add(infoplist);
       }
       return this;
     }
@@ -179,9 +189,9 @@ final class Bundling {
 
     private NestedSet<Artifact> bundleInfoplistInputs() {
       if (objcProvider.hasAssetCatalogs()) {
-        infoplists.add(intermediateArtifacts.actoolPartialInfoplist());
+        infoplistInputs.add(intermediateArtifacts.actoolPartialInfoplist());
       }
-      return infoplists.build();
+      return infoplistInputs.build();
     }
 
     private Optional<Artifact> bundleInfoplist(NestedSet<Artifact> bundleInfoplistInputs) {
@@ -283,6 +293,7 @@ final class Bundling {
           architecture,
           minimumOsVersion,
           bundleInfoplistInputs,
+          automaticEntriesInfoplistInput,
           objcProvider.get(NESTED_BUNDLE));
     }
   }
@@ -305,8 +316,9 @@ final class Bundling {
   private final String primaryBundleId;
   private final String fallbackBundleId;
   private final DottedVersion minimumOsVersion;
-  private final NestedSet<Artifact> bundleInfoplistInputs;
+  private final NestedSet<Artifact> infoplistInputs;
   private final NestedSet<Bundling> nestedBundlings;
+  private Artifact automaticEntriesInfoplistInput;
 
   private Bundling(
       String name,
@@ -321,7 +333,8 @@ final class Bundling {
       String fallbackBundleId,
       String architecture,
       DottedVersion minimumOsVersion,
-      NestedSet<Artifact> bundleInfoplistInputs,
+      NestedSet<Artifact> infoplistInputs,
+      Artifact automaticEntriesInfoplistInput,
       NestedSet<Bundling> nestedBundlings) {
     this.nestedBundlings = Preconditions.checkNotNull(nestedBundlings);
     this.name = Preconditions.checkNotNull(name);
@@ -336,7 +349,8 @@ final class Bundling {
     this.primaryBundleId = primaryBundleId;
     this.architecture = Preconditions.checkNotNull(architecture);
     this.minimumOsVersion = Preconditions.checkNotNull(minimumOsVersion);
-    this.bundleInfoplistInputs = Preconditions.checkNotNull(bundleInfoplistInputs);
+    this.infoplistInputs = Preconditions.checkNotNull(infoplistInputs);
+    this.automaticEntriesInfoplistInput = automaticEntriesInfoplistInput;
   }
 
   /**
@@ -386,21 +400,40 @@ final class Bundling {
   public Optional<Artifact> getBundleInfoplist() {
     return bundleInfoplist;
   }
-
+  
   /**
    * Returns all info plists that need to be merged into this bundle's {@link #getBundleInfoplist()
-   * info plist}.
+   * info plist}, other than that plist that contains blaze-generated automatic entires.
    */
   public NestedSet<Artifact> getBundleInfoplistInputs() {
-    return bundleInfoplistInputs;
+    return infoplistInputs;
+  }
+
+  /**
+   * Returns an artifact representing a plist containing automatic entries generated by bazel.
+   */
+  public Artifact getAutomaticInfoPlist() {
+    return automaticEntriesInfoplistInput;
   }
 
   /**
+   * Returns all artifacts that are required as input to the merging of the final plist.
+   */
+  public NestedSet<Artifact> getMergingContentArtifacts() {
+    NestedSetBuilder<Artifact> result = NestedSetBuilder.stableOrder();
+    result.addTransitive(infoplistInputs);
+    if (automaticEntriesInfoplistInput != null) {
+      result.add(automaticEntriesInfoplistInput);
+    }
+    return result.build();
+  }
+  
+  /**
    * Returns {@code true} if this bundle requires merging of its {@link #getBundleInfoplist() info
    * plist}.
    */
   public boolean needsToMergeInfoplist() {
-    return needsToMerge(bundleInfoplistInputs, primaryBundleId, fallbackBundleId);
+    return needsToMerge(infoplistInputs, primaryBundleId, fallbackBundleId);
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/PlMergeControlBytes.java b/src/main/java/com/google/devtools/build/lib/rules/objc/PlMergeControlBytes.java
new file mode 100644
index 0000000000..4f2e0c15cf
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/PlMergeControlBytes.java
@@ -0,0 +1,74 @@
+// Copyright 2015 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.rules.objc;
+
+import com.google.common.io.ByteSource;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.xcode.plmerge.proto.PlMergeProtos;
+import com.google.devtools.build.xcode.plmerge.proto.PlMergeProtos.Control;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * A byte source that can be used the generate a control file for the tool plmerge.
+ */
+public final class PlMergeControlBytes extends ByteSource {
+
+  private final Bundling bundling;
+  private final Artifact mergedPlist;
+
+  /**
+   * @param bundling  the {@code Bundling} instance describing the bundle
+   *     for which to create a merged plist
+   * @param mergedPlist the merged plist that should be bundled as Info.plist
+   */
+  public PlMergeControlBytes(Bundling bundling, Artifact mergedPlist) {
+    this.bundling = bundling;
+    this.mergedPlist = mergedPlist;
+  }
+
+  @Override
+  public InputStream openStream() throws IOException {
+    return control(bundling).toByteString().newInput();
+  }
+
+  private Control control(Bundling bundling) {
+    PlMergeProtos.Control.Builder control =
+        PlMergeProtos.Control.newBuilder()
+            .addAllSourceFile(Artifact.toExecPaths(bundling.getBundleInfoplistInputs()))
+            .setOutFile(mergedPlist.getExecPathString());
+
+    if (bundling.getAutomaticInfoPlist() != null) {
+      control.addImmutableSourceFile(bundling.getAutomaticInfoPlist().getExecPathString());
+    }
+
+    if (bundling.getPrimaryBundleId() != null) {
+      control.setPrimaryBundleId(bundling.getPrimaryBundleId());
+    }
+
+    if (bundling.getFallbackBundleId() != null) {
+      control.setFallbackBundleId(bundling.getFallbackBundleId());
+    }
+
+    if (bundling.variableSubstitutions() != null) {
+      control.putAllVariableSubstitutionMap(bundling.variableSubstitutions());
+    }
+
+    control.setExecutableName(bundling.getName());
+
+    return control.build();
+  }
+}
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 84ee62b00b..7b5acfa3b8 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
@@ -58,6 +58,10 @@ import com.google.devtools.build.lib.shell.ShellUtils;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.xcode.xcodegen.proto.XcodeGenProtos.XcodeprojBuildSetting;
 
+import com.dd.plist.NSArray;
+import com.dd.plist.NSDictionary;
+import com.dd.plist.NSObject;
+
 import java.util.List;
 import java.util.Map.Entry;
 
@@ -295,6 +299,7 @@ public final class ReleaseBundlingSupport {
 
     registerEmbedLabelPlistAction();
     registerEnvironmentPlistAction();
+    registerAutomaticPlistAction();
 
     if (ObjcRuleClasses.useLaunchStoryboard(ruleContext)) {
       registerLaunchStoryboardPlistAction();
@@ -388,6 +393,41 @@ public final class ReleaseBundlingSupport {
             .addOutput(getGeneratedEnvironmentPlist())
             .build(ruleContext));
   }
+  
+  private void registerAutomaticPlistAction() {
+    ruleContext.registerAction(
+        new FileWriteAction(
+            ruleContext.getActionOwner(),
+            getGeneratedAutomaticPlist(),
+            automaticEntries().toASCIIPropertyList(),
+            /*makeExecutable=*/ false));
+  }
+
+  /**
+   * Returns a map containing entries that should be added to the merged plist. These are usually
+   * generated by Xcode automatically during the build process.
+   */
+  private NSDictionary automaticEntries() {
+    List<Integer> uiDeviceFamily =
+        TargetDeviceFamily.UI_DEVICE_FAMILY_VALUES.get(bundleSupport.targetDeviceFamilies());
+    AppleConfiguration appleConfiguration = ruleContext.getFragment(AppleConfiguration.class);
+    Platform platform = appleConfiguration.getBundlingPlatform();
+    ObjcConfiguration objcConfiguration = ObjcRuleClasses.objcConfiguration(ruleContext);
+
+    NSDictionary result = new NSDictionary();
+
+    if (uiDeviceFamily != null) {
+      result.put("UIDeviceFamily", NSObject.wrap(uiDeviceFamily.toArray()));
+    }
+    result.put("DTPlatformName", NSObject.wrap(platform.getLowerCaseNameInPlist()));
+    result.put(
+        "DTSDKName",
+        NSObject.wrap(platform.getLowerCaseNameInPlist() + appleConfiguration.getIosSdkVersion()));
+    result.put("CFBundleSupportedPlatforms", new NSArray(NSObject.wrap(platform.getNameInPlist())));
+    result.put("MinimumOSVersion", NSObject.wrap(objcConfiguration.getMinimumOs().toString()));
+
+    return result;
+  }
 
   private Artifact registerBundleSigningActions(Artifact ipaOutput) throws InterruptedException {
     IntermediateArtifacts intermediateArtifacts =
@@ -570,6 +610,7 @@ public final class ReleaseBundlingSupport {
             .addInfoplistInputFromRule(ruleContext)
             .addInfoplistInput(getGeneratedVersionPlist())
             .addInfoplistInput(getGeneratedEnvironmentPlist())
+            .setAutomaticEntriesInfoplistInput(getGeneratedAutomaticPlist())
             .setIntermediateArtifacts(ObjcRuleClasses.intermediateArtifacts(ruleContext))
             .setPrimaryBundleId(primaryBundleId)
             .setFallbackBundleId(fallbackBundleId)
@@ -888,6 +929,11 @@ public final class ReleaseBundlingSupport {
     return ruleContext.getRelatedArtifact(
         ruleContext.getUniqueDirectory("plists"), "-environment.plist");
   }
+  
+  private Artifact getGeneratedAutomaticPlist() {
+    return ruleContext.getRelatedArtifact(
+        ruleContext.getUniqueDirectory("plists"), "-automatic.plist");
+  }
 
   private Artifact getLaunchStoryboardPlist() {
     return ruleContext.getRelatedArtifact(