Move bundle_id storage from BundleMergeControlBytes to Bundling to fix a case of nested bundles.

RELNOTES: objc: Fixes a case where nested bundle would pick up parent bundle_id value

--
MOS_MIGRATED_REVID=88372062

3 files changed, 52 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 a03acd46ff..047a102a53 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
@@ -42,19 +42,14 @@ final class BundleMergeControlBytes extends ByteSource {
   private final Artifact mergedIpa;
   private final ObjcConfiguration objcConfiguration;
   private final ImmutableSet<TargetDeviceFamily> families;
-  private final String primaryBundleIdentifier;
-  private final String fallbackBundleIdentifier;
 
   public BundleMergeControlBytes(
       Bundling rootBundling, Artifact mergedIpa, ObjcConfiguration objcConfiguration,
-      ImmutableSet<TargetDeviceFamily> families, 
-      String primaryBundleIdentifer, String fallbackBundleIdentifier) {
+      ImmutableSet<TargetDeviceFamily> families) {
     this.rootBundling = Preconditions.checkNotNull(rootBundling);
     this.mergedIpa = Preconditions.checkNotNull(mergedIpa);
     this.objcConfiguration = Preconditions.checkNotNull(objcConfiguration);
     this.families = Preconditions.checkNotNull(families);
-    this.primaryBundleIdentifier = primaryBundleIdentifer;
-    this.fallbackBundleIdentifier = fallbackBundleIdentifier;
   }
 
   @Override
@@ -120,12 +115,12 @@ final class BundleMergeControlBytes extends ByteSource {
       control.addNestedBundle(control(mergeZipPrefix, nestedBundling));
     }
     
-    if (primaryBundleIdentifier != null) {
-      control.setPrimaryBundleIdentifier(primaryBundleIdentifier);
+    if (bundling.getPrimaryBundleId()  != null) {
+      control.setPrimaryBundleIdentifier(bundling.getPrimaryBundleId());
     }
     
-    if (fallbackBundleIdentifier != null) {
-      control.setFallbackBundleIdentifier(fallbackBundleIdentifier);
+    if (bundling.getFallbackBundleId() != null) {
+      control.setFallbackBundleIdentifier(bundling.getFallbackBundleId());
     }
     
     return control.build();
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 7ba2f6ea7e..3174567e8e 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
@@ -47,6 +47,8 @@ final class Bundling {
     private ObjcProvider objcProvider;
     private InfoplistMerging infoplistMerging;
     private IntermediateArtifacts intermediateArtifacts;
+    private String primaryBundleId;
+    private String fallbackBundleId;
 
     public Builder setName(String name) {
       this.name = name;
@@ -77,6 +79,16 @@ final class Bundling {
       this.intermediateArtifacts = intermediateArtifacts;
       return this;
     }
+    
+    public Builder setPrimaryBundleId(String primaryId) {
+      this.primaryBundleId = primaryId;
+      return this;
+    }
+    
+    public Builder setFallbackBundleId(String fallbackId) {
+      this.fallbackBundleId = fallbackId;
+      return this;
+    }
 
     private static NestedSet<Artifact> nestedBundleContentArtifacts(Iterable<Bundling> bundles) {
       NestedSetBuilder<Artifact> artifacts = NestedSetBuilder.stableOrder();
@@ -116,7 +128,8 @@ final class Bundling {
           .build();
 
       return new Bundling(name, bundleDirFormat, combinedArchitectureBinary, extraBundleFiles,
-          objcProvider, infoplistMerging, actoolzipOutput, bundleContentArtifacts, mergeZips);
+          objcProvider, infoplistMerging, actoolzipOutput, bundleContentArtifacts, mergeZips, 
+          primaryBundleId, fallbackBundleId);
     }
   }
 
@@ -129,6 +142,8 @@ final class Bundling {
   private final Optional<Artifact> actoolzipOutput;
   private final NestedSet<Artifact> bundleContentArtifacts;
   private final NestedSet<Artifact> mergeZips;
+  private final String primaryBundleId;
+  private final String fallbackBundleId;
 
   private Bundling(
       String name,
@@ -139,7 +154,9 @@ final class Bundling {
       InfoplistMerging infoplistMerging,
       Optional<Artifact> actoolzipOutput,
       NestedSet<Artifact> bundleContentArtifacts,
-      NestedSet<Artifact> mergeZips) {
+      NestedSet<Artifact> mergeZips,
+      String primaryBundleId,
+      String fallbackBundleId) {
     this.name = Preconditions.checkNotNull(name);
     this.bundleDirFormat = Preconditions.checkNotNull(bundleDirFormat);
     this.combinedArchitectureBinary = Preconditions.checkNotNull(combinedArchitectureBinary);
@@ -149,6 +166,8 @@ final class Bundling {
     this.actoolzipOutput = Preconditions.checkNotNull(actoolzipOutput);
     this.bundleContentArtifacts = Preconditions.checkNotNull(bundleContentArtifacts);
     this.mergeZips = Preconditions.checkNotNull(mergeZips);
+    this.primaryBundleId = primaryBundleId;
+    this.fallbackBundleId = fallbackBundleId;
   }
 
   /**
@@ -239,4 +258,18 @@ final class Bundling {
   public NestedSet<Artifact> getBundleContentArtifacts() {
     return bundleContentArtifacts;
   }
+  
+  /**
+   * Returns primary bundle ID to use, can be null.
+   */
+  public String getPrimaryBundleId() {
+    return primaryBundleId;
+  }
+  
+  /**
+   * Returns fallback bundle ID to use when primary isn't set.
+   */
+  public String getFallbackBundleId() {
+    return fallbackBundleId;
+  }
 }
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 e00a98d1a2..8e571c3c77 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
@@ -191,17 +191,8 @@ public final class ReleaseBundlingSupport {
       maybeSignedIpa = registerBundleSigningActions(ipaOutput);
     }
     
-    String primaryBundleId = null; 
-    String fallbackBundleId = null;
-
-    if (ruleContext.attributes().isAttributeValueExplicitlySpecified("bundle_id")) {
-      primaryBundleId = attributes.bundleId();
-    } else {
-      fallbackBundleId = attributes.bundleId();
-    }
-    
     BundleMergeControlBytes bundleMergeControlBytes = new BundleMergeControlBytes(
-        bundling, maybeSignedIpa, objcConfiguration, families, primaryBundleId, fallbackBundleId);
+        bundling, maybeSignedIpa, objcConfiguration, families);
     registerBundleMergeActions(
         maybeSignedIpa, bundling.getBundleContentArtifacts(), bundleMergeControlBytes);
 
@@ -343,6 +334,15 @@ public final class ReleaseBundlingSupport {
     } else {
       extraBundleFiles = ImmutableList.of();
     }
+    
+    String primaryBundleId = null; 
+    String fallbackBundleId = null;
+
+    if (ruleContext.attributes().isAttributeValueExplicitlySpecified("bundle_id")) {
+      primaryBundleId = ruleContext.attributes().get("bundle_id", Type.STRING);
+    } else {
+      fallbackBundleId = ruleContext.attributes().get("bundle_id", Type.STRING);
+    }
 
     return new Bundling.Builder()
         .setName(ruleContext.getLabel().getName())
@@ -352,6 +352,8 @@ public final class ReleaseBundlingSupport {
         .setInfoplistMerging(
             BundleSupport.infoPlistMerging(ruleContext, objcProvider, optionsProvider))
         .setIntermediateArtifacts(ObjcRuleClasses.intermediateArtifacts(ruleContext))
+        .setPrimaryBundleId(primaryBundleId)
+        .setFallbackBundleId(fallbackBundleId)
         .build();
   }