Support multiarchitecture objc libraries for both simulator and device architectures in a single build, refining the multiarchitecture device restriction only to rules which require bundling.

--
MOS_MIGRATED_REVID=103016981

4 files changed, 35 insertions, 29 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 6fcb2f2073..890d4a8b1d 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
@@ -95,7 +95,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.
@@ -126,13 +126,22 @@ final class BundleSupport {
     return this;
   }
 
-  /**
-   * Validates that resources defined in this rule and its dependencies and written to this bundle
-   * are legal (for example that they are not mapped to the same bundle location).
-   *
-   * @return this bundle support
-   */
-  BundleSupport validateResources(ObjcProvider objcProvider) {
+  private void validatePlatform() {
+    ObjcConfiguration objcConfiguration = ObjcRuleClasses.objcConfiguration(ruleContext);
+    Platform platform = null;
+    for (String architecture : objcConfiguration.getIosMultiCpus()) {
+      if (platform == null) {
+        platform = Platform.forArch(architecture);
+      } else if (platform != Platform.forArch(architecture)) {
+        ruleContext.ruleError(
+            String.format("In builds which require bundling, --ios_multi_cpus does not currently "
+                + "allow values for both simulator and device builds. Flag was %s",
+            objcConfiguration.getIosMultiCpus()));
+      }
+    }
+  }
+
+  private void validateResources(ObjcProvider objcProvider) {
     Map<String, Artifact> bundlePathToFile = new HashMap<>();
     NestedSet<Artifact> artifacts = objcProvider.get(STRINGS);
 
@@ -177,6 +186,22 @@ final class BundleSupport {
 
     // TODO(bazel-team): Do the same validation for storyboards and datamodels which could also be
     // generated by genrules or doubly defined.
+  }
+
+  /**
+   * Validates bundle support.
+   * <ul>
+   * <li>Validates that resources defined in this rule and its dependencies and written to this
+   *     bundle are legal (for example that they are not mapped to the same bundle location)
+   * <li>Validates the platform for this build is either simulator or device, and does not
+   *     contain architectures for both platforms
+   * </ul>
+   *
+   * @return this bundle support
+   */
+  BundleSupport validate(ObjcProvider objcProvider) {
+    validatePlatform();
+    validateResources(objcProvider);
 
     return this;
   }
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 acb0e87982..808bd3dd74 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
@@ -25,7 +25,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes;
 
-
 /**
  * Implementation for {@code objc_bundle_library}.
  */
@@ -49,7 +48,7 @@ public class ObjcBundleLibrary implements RuleConfiguredTargetFactory {
 
     new BundleSupport(ruleContext, bundling)
         .registerActions(common.getObjcProvider())
-        .validateResources(common.getObjcProvider())
+        .validate(common.getObjcProvider())
         .addXcodeSettings(xcodeProviderBuilder);
 
     if (ruleContext.hasErrors()) {
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 299bc290f8..8f073a18ad 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
@@ -22,10 +22,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
-import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.CompilationMode;
-import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.rules.objc.ReleaseBundlingSupport.SplitArchTransition.ConfigurationDistinguisher;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.lib.vfs.Path;
@@ -290,21 +287,6 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment {
     return Joiner.on('-').join(components);
   }
 
-  @Override
-  public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOptions) {
-    // TODO(bazel-team): Remove this constraint once getBundlingPlatform can return multiple values.
-    Platform platform = null;
-    for (String architecture : iosMultiCpus) {
-      if (platform == null) {
-        platform = Platform.forArch(architecture);
-      } else if (platform != Platform.forArch(architecture)) {
-        reporter.handle(Event.error(
-            String.format("--ios_multi_cpus does not currently allow values for both simulator and "
-                + "device builds but was %s", iosMultiCpus)));
-      }
-    }
-  }
-
   /**
    * @return whether to add include path entries for every proto file's containing directory.
    */
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 a7a476b9ff..f6a3af6eb0 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
@@ -218,7 +218,7 @@ public final class ReleaseBundlingSupport {
    * @return this release bundling support
    */
   ReleaseBundlingSupport validateResources() {
-    bundleSupport.validateResources(objcProvider);
+    bundleSupport.validate(objcProvider);
     return this;
   }