Refactor spawnXcrunActionBuilder to take an explicit apple platform.

This both starts to clean up the code (making it clearer which platform each action is targeted to, caller by caller), as well as allows callers to issue
certain actions for platforms which are not implicitly ios.
We'll still need to migrate callers onto the new method signature.

--
MOS_MIGRATED_REVID=117982741

6 files changed, 64 insertions, 28 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
index 5f8b4cac5f..c16a4d4355 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
@@ -133,16 +133,15 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
 
   /**
    * Returns a map of environment variables (derived from configuration) that should be propagated
-   * for actions pertaining to building ios applications. Keys are variable names and values are
+   * for actions pertaining to the given apple platform. Keys are variable names and values are
    * their corresponding values.
    */
-  // TODO(bazel-team): Repurpose for non-ios platforms.
-  public Map<String, String> getEnvironmentForIosAction() {
+  public Map<String, String> getTargetAppleEnvironment(Platform platform) {
     ImmutableMap.Builder<String, String> mapBuilder = ImmutableMap.builder();
-    mapBuilder.putAll(appleTargetPlatformEnv(Platform.forIosArch(getIosCpu())));
+    mapBuilder.putAll(appleTargetPlatformEnv(platform));
     return mapBuilder.build();
   }
-  
+
   /**
    * Returns a map of environment variables that should be propagated for actions that build on an
    * apple host system. These environment variables are needed by the apple toolchain. Keys are
@@ -182,9 +181,25 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
     return builder.build();
   }
 
+  /**
+   * Returns the value of {@code ios_cpu} for this configuration. This is not necessarily the
+   * platform or cpu for all actions spawned in this configuration; it is appropriate for
+   * identifying the target cpu of iOS compile and link actions within this configuration.
+   */
   public String getIosCpu() {
     return iosCpu;
   }
+
+  /**
+   * Returns the {@link Platform} represented by {@code ios_cpu} (see {@link #getIosCpu}.
+   * (For example, {@code i386} maps to {@link Platform#IOS_SIMULATOR}.) Note that this is not
+   * necessarily the effective platform for all ios actions in the current context: This is
+   * typically the correct platform for implicityly-ios compile and link actions in the current
+   * context. For effective platform for bundling actions, see {@link #getBundlingPlatform}.
+   */
+  public Platform getIosCpuPlatform() {
+    return Platform.forIosArch(getIosCpu());
+  }
   
   /**
    * Returns the platform of the configuration for the current bundle, based on configured
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 3123ded3d0..d1b7ab4c32 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
@@ -226,7 +226,7 @@ final class BundleSupport {
           .compiledStoryboardZip(storyboardInput);
 
       ruleContext.registerAction(
-          ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+          ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
               .setMnemonic("StoryboardCompile")
               .setExecutable(attributes.ibtoolWrapper())
               .setCommandLine(ibActionsCommandLine(archiveRoot, zipOutput, storyboardInput))
@@ -264,7 +264,7 @@ final class BundleSupport {
     for (Xcdatamodel datamodel : xcdatamodels) {
       Artifact outputZip = datamodel.getOutputZip();
       ruleContext.registerAction(
-          ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+          ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
               .setMnemonic("MomCompile")
               .setExecutable(attributes.momcWrapper())
               .addOutput(outputZip)
@@ -290,7 +290,7 @@ final class BundleSupport {
           FileSystemUtils.replaceExtension(original.getExecPath(), ".nib"));
 
       ruleContext.registerAction(
-          ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+          ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
               .setMnemonic("XibCompile")
               .setExecutable(attributes.ibtoolWrapper())
               .setCommandLine(ibActionsCommandLine(archiveRoot, zipOutput, original))
@@ -303,7 +303,7 @@ final class BundleSupport {
   private void registerConvertStringsActions(ObjcProvider objcProvider) {
     for (Artifact strings : objcProvider.get(ObjcProvider.STRINGS)) {
       Artifact bundled = bundling.getIntermediateArtifacts().convertedStringsFile(strings);
-      ruleContext.registerAction(ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+      ruleContext.registerAction(ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
           .setMnemonic("ConvertStringsPlist")
           .setExecutable(new PathFragment("/usr/bin/plutil"))
           .setCommandLine(CustomCommandLine.builder()
@@ -380,7 +380,7 @@ final class BundleSupport {
     // zip file will be rooted at the bundle root, and we have to prepend the bundle root to each
     // entry when merging it with the final .ipa file.
     ruleContext.registerAction(
-        ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+        ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
             .setMnemonic("AssetCatalogCompile")
             .setExecutable(attributes.actoolWrapper())
             .addTransitiveInputs(objcProvider.get(ASSET_CATALOG))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
index d26304cdef..fd0c9247a4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
@@ -408,7 +408,7 @@ public final class CompilationSupport {
     }
 
     // TODO(bazel-team): Remote private headers from inputs once they're added to the provider.
-    ruleContext.registerAction(ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+    ruleContext.registerAction(ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
         .setMnemonic("ObjcCompile")
         .setExecutable(xcrunwrapper(ruleContext))
         .setCommandLine(commandLine.build())
@@ -508,7 +508,7 @@ public final class CompilationSupport {
     commandLine.add(commonFrameworkFlags(objcProvider, appleConfiguration));
 
     ruleContext.registerAction(
-        ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+        ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
             .setMnemonic("SwiftCompile")
             .setExecutable(xcrunwrapper(ruleContext))
             .setCommandLine(commandLine.build())
@@ -580,7 +580,7 @@ public final class CompilationSupport {
 
     commandLine.add(commonFrameworkFlags(objcProvider, appleConfiguration));
 
-    ruleContext.registerAction(ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+    ruleContext.registerAction(ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
         .setMnemonic("SwiftModuleMerge")
         .setExecutable(xcrunwrapper(ruleContext))
         .setCommandLine(commandLine.build())
@@ -616,7 +616,7 @@ public final class CompilationSupport {
         Artifact.joinExecPaths("\n", objFiles),
         /*makeExecutable=*/ false));
 
-    actions.add(ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+    actions.add(ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
         .setMnemonic("ObjcLink")
         .setExecutable(xcrunwrapper(ruleContext))
         .setCommandLine(new CustomCommandLine.Builder()
@@ -640,7 +640,7 @@ public final class CompilationSupport {
     Artifact archive = ruleContext.getImplicitOutputArtifact(FULLY_LINKED_LIB);
 
     ImmutableList<Artifact> ccLibraries = ccLibraries(objcProvider);
-    ruleContext.registerAction(ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+    ruleContext.registerAction(ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
         .setMnemonic("ObjcLink")
         .setExecutable(xcrunwrapper(ruleContext))
         .setCommandLine(new CustomCommandLine.Builder()
@@ -810,7 +810,7 @@ public final class CompilationSupport {
     NestedSet<Artifact> bazelBuiltLibraries = Iterables.isEmpty(prunedJ2ObjcArchives)
         ? objcProvider.get(LIBRARY) : substituteJ2ObjcPrunedLibraries(objcProvider);
     ruleContext.registerAction(
-        ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+        ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
             .setMnemonic("ObjcLink")
             .setShellCommand(ImmutableList.of("/bin/bash", "-c"))
             .setCommandLine(
@@ -836,7 +836,7 @@ public final class CompilationSupport {
       Artifact strippedBinary = intermediateArtifacts.strippedSingleArchitectureBinary();
 
       ruleContext.registerAction(
-          ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+          ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
               .setMnemonic("ObjcBinarySymbolStrip")
               .setExecutable(xcrunwrapper(ruleContext))
               .setCommandLine(symbolStripCommandLine(stripArgs, binaryToLink, strippedBinary))
@@ -1085,7 +1085,7 @@ public final class CompilationSupport {
             paramFile,
             commandLine,
             ParameterFile.ParameterFileType.UNQUOTED, ISO_8859_1));
-        ruleContext.registerAction(ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+        ruleContext.registerAction(ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
             .setMnemonic("DummyPruner")
             .setExecutable(pruner)
             .addInput(dummyArchive)
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 84f83867c4..650b8e9700 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
@@ -131,16 +131,37 @@ public class ObjcRuleClasses {
       new SdkFramework("Foundation"), new SdkFramework("UIKit"));
 
   /**
-   * Creates a new spawn action builder that will ultimately use part of the apple toolchain
-   * using the xcrun binary. Such a spawn action is special in that, in order to run, it requires
-   * both a darwin architecture and a collection of environment variables which contain
-   * information about the target and host architectures.
+   * Creates a new spawn action builder with apple environment variables set that are typically
+   * needed by the apple toolchain. This should be used to start to build spawn actions that, in
+   * order to run, require both a darwin architecture and a collection of environment variables
+   * which contain information about the target and host architectures. This implicitly
+   * assumes that this action is targeting ios platforms, and that
+   * {@link AppleConfiguration#getIosCpu()} is the source of truth for their target architecture.
+   * 
+   * @deprecated use {@link #spawnAppleEnvActionBuilder(RuleContext, Platform)} instead
    */
-  static SpawnAction.Builder spawnXcrunActionBuilder(RuleContext ruleContext) {
+  // TODO(cparsons): Refactor callers to use the alternate method. Callers should be aware
+  // of their effective Platform.
+  @Deprecated
+  static SpawnAction.Builder spawnAppleEnvActionBuilder(RuleContext ruleContext) {
+    AppleConfiguration appleConfiguration = ruleContext.getFragment(AppleConfiguration.class);
+
+    return spawnAppleEnvActionBuilder(ruleContext,
+        Platform.forIosArch(appleConfiguration.getIosCpu()));
+  }
+
+  /**
+   * Creates a new spawn action builder with apple environment variables set that are typically
+   * needed by the apple toolchain. This should be used to start to build spawn actions that, in
+   * order to run, require both a darwin architecture and a collection of environment variables
+   * which contain information about the target and host architectures.
+   */
+  static SpawnAction.Builder spawnAppleEnvActionBuilder(RuleContext ruleContext,
+      Platform targetPlatform) {
     AppleConfiguration appleConfiguration = ruleContext.getFragment(AppleConfiguration.class);
 
     ImmutableMap.Builder<String, String> envBuilder = ImmutableMap.<String, String>builder()
-        .putAll(appleConfiguration.getEnvironmentForIosAction())
+        .putAll(appleConfiguration.getTargetAppleEnvironment(targetPlatform))
         .putAll(appleConfiguration.getAppleHostSystemEnv());
 
     return spawnOnDarwinActionBuilder()
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 40c52be628..4f922f728d 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
@@ -358,7 +358,7 @@ public final class ReleaseBundlingSupport {
             configuration.getBundlingPlatform().getLowerCaseNameInPlist(),
             configuration.getIosSdkVersion());
     ruleContext.registerAction(
-        ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+        ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
             .setMnemonic("EnvironmentPlist")
             .setExecutable(attributes.environmentPlist())
             .addArguments("--platform", platformWithVersion)
@@ -730,7 +730,7 @@ public final class ReleaseBundlingSupport {
     Artifact resultingLinkedBinary = intermediateArtifacts.combinedArchitectureBinary();
     NestedSet<Artifact> linkedBinaries = linkedBinaries();
 
-    ruleContext.registerAction(ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+    ruleContext.registerAction(ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
         .setMnemonic("ObjcCombiningArchitectures")
         .addTransitiveInputs(linkedBinaries)
         .addOutput(resultingLinkedBinary)
@@ -949,7 +949,7 @@ public final class ReleaseBundlingSupport {
         .addExecPath("--scan-executable", intermediateArtifacts.combinedArchitectureBinary());
 
     ruleContext.registerAction(
-        ObjcRuleClasses.spawnXcrunActionBuilder(ruleContext)
+        ObjcRuleClasses.spawnAppleEnvActionBuilder(ruleContext)
             .setMnemonic("SwiftStdlibCopy")
             .setExecutable(attributes.swiftStdlibToolWrapper())
             .setCommandLine(commandLine.build())
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java
index b1c5050b64..ba0c5a14c9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java
@@ -255,7 +255,7 @@ public class TestSupport {
 
     ImmutableMap.Builder<String, String> envBuilder = ImmutableMap.builder();
 
-    envBuilder.putAll(configuration.getEnvironmentForIosAction());
+    envBuilder.putAll(configuration.getTargetAppleEnvironment(configuration.getIosCpuPlatform()));
     envBuilder.putAll(configuration.getAppleHostSystemEnv());
 
     if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {