Strict matching of architecture name to ios platform. Also refactor Platform into a new .apple package, as it is not relevant for solely objc rules.

--
MOS_MIGRATED_REVID=106709486

11 files changed, 74 insertions, 28 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/Platform.java b/src/main/java/com/google/devtools/build/lib/rules/apple/Platform.java
index 4d5be27eaf..4871fd8875 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/Platform.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/Platform.java
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.devtools.build.lib.rules.objc;
+package com.google.devtools.build.lib.rules.apple;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
@@ -21,12 +21,14 @@ import java.util.Locale;
 import java.util.Set;
 
 /**
- * An enum that can be used to distinguish between an iOS simulator and device.
+ * An enum that can be used to distinguish between various apple platforms.
  */
 public enum Platform {
-  DEVICE("iPhoneOS"), SIMULATOR("iPhoneSimulator");
+  IOS_DEVICE("iPhoneOS"), IOS_SIMULATOR("iPhoneSimulator");
 
-  private static final Set<String> SIMULATOR_ARCHS = ImmutableSet.of("i386", "x86_64");
+  private static final Set<String> IOS_SIMULATOR_ARCHS = ImmutableSet.of("i386", "x86_64");
+  private static final Set<String> IOS_DEVICE_ARCHS =
+      ImmutableSet.of("armv6", "armv7", "armv7s", "arm64");
 
   private final String nameInPlist;
 
@@ -50,9 +52,19 @@ public enum Platform {
   }
 
   /**
-   * Returns the platform for the arch.
+   * Returns the platform for the architecture.
+   * 
+   * @throws IllegalArgumentException if there is no valid apple platform for the given
+   *     architecture
    */
   public static Platform forArch(String arch) {
-    return SIMULATOR_ARCHS.contains(arch) ? SIMULATOR : DEVICE;
+    if (IOS_SIMULATOR_ARCHS.contains(arch)) {
+      return IOS_SIMULATOR;
+    } else if (IOS_DEVICE_ARCHS.contains(arch)) {
+      return IOS_DEVICE;
+    } else {
+      throw new IllegalArgumentException(
+          "No supported apple platform registered for architecture " + arch);
+    }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java b/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
index 683577a539..7da8aa13b7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
@@ -29,6 +29,7 @@ import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.RunfilesSupport;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
+import com.google.devtools.build.lib.rules.apple.Platform;
 import com.google.devtools.build.lib.rules.cpp.CcLinkParamsProvider;
 import com.google.devtools.build.lib.rules.cpp.CppCompilationContext;
 import com.google.devtools.build.lib.rules.objc.CompilationSupport.ExtraLinkArgs;
@@ -126,7 +127,7 @@ abstract class BinaryLinkingTargetFactory implements RuleConfiguredTargetFactory
             .validateAttributes();
 
         xcTestAppProvider = Optional.of(releaseBundlingSupport.xcTestAppProvider());
-        if (objcConfiguration.getBundlingPlatform() == Platform.SIMULATOR) {
+        if (objcConfiguration.getBundlingPlatform() == Platform.IOS_SIMULATOR) {
           Artifact runnerScript = intermediateArtifacts.runnerScript();
           Artifact ipaFile = ruleContext.getImplicitOutputArtifact(ReleaseBundlingSupport.IPA);
           releaseBundlingSupport.registerGenerateRunnerScriptAction(runnerScript, ipaFile);
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 50a387c9b7..3e192aeacc 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
@@ -32,6 +32,7 @@ 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;
+import com.google.devtools.build.lib.rules.apple.Platform;
 import com.google.devtools.build.lib.rules.objc.TargetDeviceFamily.InvalidFamilyNameException;
 import com.google.devtools.build.lib.rules.objc.TargetDeviceFamily.RepeatedFamilyNameException;
 import com.google.devtools.build.lib.rules.objc.XcodeProvider.Builder;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IosApplication.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IosApplication.java
index b97804ba63..13a46f4f51 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IosApplication.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IosApplication.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
+import com.google.devtools.build.lib.rules.apple.Platform;
 import com.google.devtools.build.lib.rules.objc.ReleaseBundlingSupport.SplitArchTransition;
 import com.google.devtools.build.lib.rules.objc.ReleaseBundlingSupport.SplitArchTransition.ConfigurationDistinguisher;
 
@@ -52,7 +53,7 @@ public class IosApplication extends ReleaseBundlingTargetFactory {
       ReleaseBundlingSupport releaseBundlingSupport) throws InterruptedException {
     // If this is an application built for the simulator, make it runnable.
     ObjcConfiguration objcConfiguration = ObjcRuleClasses.objcConfiguration(ruleContext);
-    if (objcConfiguration.getBundlingPlatform() == Platform.SIMULATOR) {
+    if (objcConfiguration.getBundlingPlatform() == Platform.IOS_SIMULATOR) {
       Artifact runnerScript = ObjcRuleClasses.intermediateArtifacts(ruleContext).runnerScript();
       Artifact ipaFile = ruleContext.getImplicitOutputArtifact(ReleaseBundlingSupport.IPA);
       releaseBundlingSupport.registerGenerateRunnerScriptAction(runnerScript, ipaFile);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IosSdkCommands.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IosSdkCommands.java
index c5635ceffe..73be677afc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IosSdkCommands.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IosSdkCommands.java
@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.rules.apple.Platform;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.xcode.xcodegen.proto.XcodeGenProtos.XcodeprojBuildSetting;
 
@@ -118,7 +119,7 @@ public class IosSdkCommands {
   public static List<String> commonLinkAndCompileFlagsForClang(
       ObjcProvider provider, ObjcConfiguration configuration) {
     ImmutableList.Builder<String> builder = new ImmutableList.Builder<>();
-    if (Platform.forArch(configuration.getIosCpu()) == Platform.SIMULATOR) {
+    if (Platform.forArch(configuration.getIosCpu()) == Platform.IOS_SIMULATOR) {
       builder.add("-mios-simulator-version-min=" + configuration.getMinimumOs());
     } else {
       builder.add("-miphoneos-version-min=" + configuration.getMinimumOs());
@@ -154,9 +155,9 @@ public class IosSdkCommands {
   private static List<String> platformSpecificCompileFlagsForClang(
       ObjcConfiguration configuration) {
     switch (Platform.forArch(configuration.getIosCpu())) {
-      case DEVICE:
+      case IOS_DEVICE:
         return ImmutableList.of();
-      case SIMULATOR:
+      case IOS_SIMULATOR:
         // These are added by Xcode when building, because the simulator is built on OSX
         // frameworks so we aim compile to match the OSX objc runtime.
         return ImmutableList.of(
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 fd9a9ec103..6db3d16c96 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
@@ -23,6 +23,7 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
+import com.google.devtools.build.lib.rules.apple.Platform;
 import com.google.devtools.build.lib.rules.objc.ReleaseBundlingSupport.SplitArchTransition.ConfigurationDistinguisher;
 import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter;
 import com.google.devtools.common.options.EnumConverter;
@@ -224,15 +225,15 @@ public class ObjcCommandLineOptions extends FragmentOptions {
       labelMap.put("dump_syms", dumpSyms);
     }
 
-    if (getPlatform() == Platform.DEVICE) {
+    if (getPlatform() == Platform.IOS_DEVICE) {
       labelMap.put("default_provisioning_profile", defaultProvisioningProfile);
     }
   }
 
   private Platform getPlatform() {
     for (String architecture : iosMultiCpus) {
-      if (Platform.forArch(architecture) == Platform.DEVICE) {
-        return Platform.DEVICE;
+      if (Platform.forArch(architecture) == Platform.IOS_DEVICE) {
+        return Platform.IOS_DEVICE;
       }
     }
     return Platform.forArch(iosCpu);
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 abeccdc0fe..c5fe6c614a 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
@@ -24,6 +24,7 @@ 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.CompilationMode;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.rules.apple.Platform;
 import com.google.devtools.build.lib.rules.objc.ReleaseBundlingSupport.SplitArchTransition.ConfigurationDistinguisher;
 import com.google.devtools.build.lib.vfs.Path;
 
@@ -168,9 +169,9 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment {
 
   /**
    * Returns the platform of the configuration for the current bundle, based on configured
-   * architectures (for example, {@code i386} maps to {@link Platform#SIMULATOR}).
+   * architectures (for example, {@code i386} maps to {@link Platform#IOS_SIMULATOR}).
    *
-   * <p>If {@link #getIosMultiCpus()} is set, returns {@link Platform#DEVICE} if any of the
+   * <p>If {@link #getIosMultiCpus()} is set, returns {@link Platform#IOS_DEVICE} if any of the
    * architectures matches it, otherwise returns the mapping for {@link #getIosCpu()}.
    *
    * <p>Note that this method should not be used to determine the platform for code compilation.
@@ -180,8 +181,8 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment {
   // (in particular actool, bundlemerge, momc) have been upgraded to support multiple values.
   public Platform getBundlingPlatform() {
     for (String architecture : getIosMultiCpus()) {
-      if (Platform.forArch(architecture) == Platform.DEVICE) {
-        return Platform.DEVICE;
+      if (Platform.forArch(architecture) == Platform.IOS_DEVICE) {
+        return Platform.IOS_DEVICE;
       }
     }
     return Platform.forArch(getIosCpu());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
index 4173d6dd0b..febfe93c2b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
@@ -48,6 +48,7 @@ import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.rules.apple.Platform;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.util.FileTypeSet;
@@ -951,7 +952,7 @@ public class ObjcRuleClasses {
                 public Label getDefault(Rule rule, BuildConfiguration configuration) {
                   ObjcConfiguration objcConfiguration =
                       configuration.getFragment(ObjcConfiguration.class);
-                  if (objcConfiguration.getBundlingPlatform() != Platform.DEVICE) {
+                  if (objcConfiguration.getBundlingPlatform() != Platform.IOS_DEVICE) {
                     return null;
                   }
                   if (rule.isAttributeValueExplicitlySpecified("provisioning_profile")) {
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 51366a0363..09dd16ea81 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
@@ -47,6 +47,7 @@ import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction;
+import com.google.devtools.build.lib.rules.apple.Platform;
 import com.google.devtools.build.lib.rules.objc.BundleSupport.ExtraActoolArgs;
 import com.google.devtools.build.lib.shell.ShellUtils;
 import com.google.devtools.build.lib.syntax.Type;
@@ -244,7 +245,7 @@ public final class ReleaseBundlingSupport {
     Artifact ipaOutput = ruleContext.getImplicitOutputArtifact(IPA);
 
     Artifact maybeSignedIpa;
-    if (objcConfiguration.getBundlingPlatform() == Platform.SIMULATOR) {
+    if (objcConfiguration.getBundlingPlatform() == Platform.IOS_SIMULATOR) {
       maybeSignedIpa = ipaOutput;
     } else if (attributes.provisioningProfile() == null) {
       throw new IllegalStateException(DEVICE_NO_PROVISIONING_PROFILE);
@@ -466,7 +467,7 @@ public final class ReleaseBundlingSupport {
       String bundleDirFormat, String bundleName, String minimumOsVersion) {
     ImmutableList<BundleableFile> extraBundleFiles;
     ObjcConfiguration objcConfiguration = ObjcRuleClasses.objcConfiguration(ruleContext);
-    if (objcConfiguration.getBundlingPlatform() == Platform.DEVICE) {
+    if (objcConfiguration.getBundlingPlatform() == Platform.IOS_DEVICE) {
       extraBundleFiles = ImmutableList.of(new BundleableFile(
           new Attributes(ruleContext).provisioningProfile(),
           PROVISIONING_PROFILE_BUNDLE_FILE));
diff --git a/src/objc_tools/bundlemerge/java/com/google/devtools/build/xcode/bundlemerge/BundleMerging.java b/src/objc_tools/bundlemerge/java/com/google/devtools/build/xcode/bundlemerge/BundleMerging.java
index 4523db2d26..c402f60a96 100644
--- a/src/objc_tools/bundlemerge/java/com/google/devtools/build/xcode/bundlemerge/BundleMerging.java
+++ b/src/objc_tools/bundlemerge/java/com/google/devtools/build/xcode/bundlemerge/BundleMerging.java
@@ -88,6 +88,21 @@ public final class BundleMerging {
   private static final String PKGINFO_FILENAME = "PkgInfo";
 
   /**
+   * A hack needed briefly to maintain backwards compatibility during rename of {@link Platform}
+   * enums. Except for backwards-compatible names, falls back to usage of {@link Platform#valueOf}.
+   */
+  // TODO(bazel-team): Remove this hack.
+  private static Platform platformFromName(String platformName) {
+    if ("SIMULATOR".equals(platformName)) {
+      return Platform.IOS_SIMULATOR;
+    } else if ("DEVICE".equals(platformName)) {
+      return Platform.IOS_DEVICE;
+    } else {
+      return Platform.valueOf(platformName);
+    }
+  }
+
+  /**
    * Adds merge artifacts from the given {@code control} into builders that collect merge zips and
    * individual files. {@code bundleRoot} is prepended to each path, except the paths in the merge
    * zips.
@@ -113,7 +128,7 @@ public final class BundleMerging {
         sourcePlistFiles,
         PlistMerging.automaticEntries(
             control.getTargetDeviceFamilyList(),
-            Platform.valueOf(control.getPlatform()),
+            platformFromName(control.getPlatform()),
             control.getSdkVersion(),
             control.getMinimumOsVersion()),
         substitutionMap.build(),
diff --git a/src/tools/xcode-common/java/com/google/devtools/build/xcode/common/Platform.java b/src/tools/xcode-common/java/com/google/devtools/build/xcode/common/Platform.java
index b680497e42..e17ad12c74 100644
--- a/src/tools/xcode-common/java/com/google/devtools/build/xcode/common/Platform.java
+++ b/src/tools/xcode-common/java/com/google/devtools/build/xcode/common/Platform.java
@@ -16,18 +16,19 @@ package com.google.devtools.build.xcode.common;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.xcode.util.Containing;
 
 import java.util.Locale;
 import java.util.Set;
 
 /**
- * An enum that can be used to distinguish between an iOS simulator and device.
+ * An enum that can be used to distinguish between various apple platforms.
  */
 public enum Platform {
-  DEVICE("iPhoneOS"), SIMULATOR("iPhoneSimulator");
+  IOS_DEVICE("iPhoneOS"), IOS_SIMULATOR("iPhoneSimulator");
 
-  private static final Set<String> SIMULATOR_ARCHS = ImmutableSet.of("i386", "x86_64");
+  private static final Set<String> IOS_SIMULATOR_ARCHS = ImmutableSet.of("i386", "x86_64");
+  private static final Set<String> IOS_DEVICE_ARCHS =
+      ImmutableSet.of("armv6", "armv7", "armv7s", "arm64");
 
   private final String nameInPlist;
 
@@ -51,9 +52,19 @@ public enum Platform {
   }
 
   /**
-   * Returns the platform for the arch.
+   * Returns the platform for the architecture.
+   * 
+   * @throws IllegalArgumentException if there is no valid apple platform for the given
+   *     architecture.
    */
   public static Platform forArch(String arch) {
-    return Containing.item(SIMULATOR_ARCHS, arch) ? SIMULATOR : DEVICE;
+    if (IOS_SIMULATOR_ARCHS.contains(arch)) {
+      return IOS_SIMULATOR;
+    } else if (IOS_DEVICE_ARCHS.contains(arch)) {
+      return IOS_DEVICE;
+    } else {
+      throw new IllegalArgumentException(
+          "No supported apple platform registered for architecture " + arch);
+    }
   }
 }