Propagate apple platform environment variables for CppCompileAction

--
MOS_MIGRATED_REVID=108182745

4 files changed, 99 insertions, 8 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java
index e4268a2976..bddfc81a53 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java
@@ -59,6 +59,7 @@ 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.packages.TriState;
+import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
 import com.google.devtools.build.lib.rules.cpp.CppRuleClasses;
@@ -609,7 +610,7 @@ public class BazelCppRuleClasses {
     @Override
     public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
       return builder
-          .requiresConfigurationFragments(CppConfiguration.class)
+          .requiresConfigurationFragments(CppConfiguration.class, AppleConfiguration.class)
           /*<!-- #BLAZE_RULE(cc_binary).IMPLICIT_OUTPUTS -->
           <ul>
           <li><code><var>name</var>.stripped</code> (only built if explicitly requested): A stripped
@@ -755,7 +756,7 @@ public class BazelCppRuleClasses {
           // TODO: Google cc_library overrides documentation for:
           // deps, data, linkopts, defines, srcs; override here too?
 
-          .requiresConfigurationFragments(CppConfiguration.class)
+          .requiresConfigurationFragments(CppConfiguration.class, AppleConfiguration.class)
           /*<!-- #BLAZE_RULE(cc_library).ATTRIBUTE(alwayslink) -->
           If 1, any binary that depends (directly or indirectly) on this C++
           library will link in all the object files for the files listed in
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 783adddebc..b751a13f6f 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
@@ -83,17 +83,45 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
    * for actions pertaining to building ios applications. Keys are variable names and values are
    * their corresponding values.
    */
-  // TODO(bazel-team): Repurpose for non-ios platforms.
+  // TODO(bazel-team): Separate host system and target platform environment
   public Map<String, String> getEnvironmentForIosAction() {
+    ImmutableMap.Builder<String, String> mapBuilder = ImmutableMap.builder();
+    mapBuilder.putAll(appleTargetPlatformEnv(Platform.forIosArch(getIosCpu())));
+    mapBuilder.putAll(appleHostSystemEnv());
+    return mapBuilder.build();
+  }
+
+  /**
+   * Returns a map of environment variables (derived from configuration) that should be propagated
+   * for actions that build on an apple host system. These environment variables are needed to
+   * by apple toolchain. Keys are variable names and values are their corresponding values.
+   */
+  public Map<String, String> appleHostSystemEnv() {
     ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
-    if (xcodeVersionOverride.isPresent()) {
-      builder.put(XCODE_VERSION_ENV_NAME, xcodeVersionOverride.get().toString());
+    if (getXcodeVersionOverride().isPresent()) {
+      builder.put(AppleConfiguration.XCODE_VERSION_ENV_NAME,
+          getXcodeVersionOverride().get().toString());
     }
-    builder.put(APPLE_SDK_VERSION_ENV_NAME, iosSdkVersion.toString());
-    builder.put(APPLE_SDK_PLATFORM_ENV_NAME, Platform.forIosArch(getIosCpu()).getNameInPlist());
     return builder.build();
   }
   
+  /**
+   * Returns a map of environment variables (derived from configuration) that should be propagated
+   * for actions pertaining to building applications for apple platforms. These environment
+   * variables are needed to use apple toolkits. Keys are variable names and values are their
+   * corresponding values.
+   */
+  public Map<String, String> appleTargetPlatformEnv(Platform platform) {
+    ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
+
+    // TODO(bazel-team): Handle non-ios platforms.
+    if (platform == Platform.IOS_DEVICE || platform == Platform.IOS_SIMULATOR) {
+      builder.put(AppleConfiguration.APPLE_SDK_VERSION_ENV_NAME, getIosSdkVersion().toString())
+          .put(AppleConfiguration.APPLE_SDK_PLATFORM_ENV_NAME, platform.getNameInPlist());
+    }
+    return builder.build();
+  }
+
   public String getIosCpu() {
     return iosCpu;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/Platform.java b/src/main/java/com/google/devtools/build/lib/rules/apple/Platform.java
index e5b447e327..ad75b11856 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/Platform.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/Platform.java
@@ -20,15 +20,26 @@ import com.google.common.collect.ImmutableSet;
 import java.util.Locale;
 import java.util.Set;
 
+import javax.annotation.Nullable;
+
 /**
  * An enum that can be used to distinguish between various apple platforms.
  */
 public enum Platform {
-  IOS_DEVICE("iPhoneOS"), IOS_SIMULATOR("iPhoneSimulator");
+  IOS_DEVICE("iPhoneOS"),
+  IOS_SIMULATOR("iPhoneSimulator"),
+  MACOSX("MacOSX");
 
   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 static final Set<String> IOS_SIMULATOR_TARGET_CPUS =
+      ImmutableSet.of("ios_x86_64", "ios_i386");
+  private static final Set<String> IOS_DEVICE_TARGET_CPUS =
+      ImmutableSet.of("ios_armv7", "ios_arm64");
+  private static final Set<String> MACOSX_TARGET_CPUS =
+      ImmutableSet.of("darwin_x86_64");
 
   private final String nameInPlist;
 
@@ -70,4 +81,39 @@ public enum Platform {
           "No supported ios platform registered for architecture " + arch);
     }
   }
+  
+  @Nullable
+  private static Platform forTargetCpuNullable(String targetCpu) {
+    if (IOS_SIMULATOR_TARGET_CPUS.contains(targetCpu)) {
+      return IOS_SIMULATOR;
+    } else if (IOS_DEVICE_TARGET_CPUS.contains(targetCpu)) {
+      return IOS_DEVICE;
+    } else if (MACOSX_TARGET_CPUS.contains(targetCpu)) {
+      return MACOSX;
+    } else {
+      return null;
+    }
+  }
+
+  /**
+   * Returns the platform for the given target cpu.
+   * 
+   * @throws IllegalArgumentException if there is no valid apple platform for the given target cpu
+   */
+  public static Platform forTargetCpu(String targetCpu) {
+    Platform platform = forTargetCpuNullable(targetCpu);
+    if (platform != null) {
+      return platform; 
+    } else {
+      throw new IllegalArgumentException(
+          "No supported apple platform registered for target cpu " + targetCpu);
+    }
+  }
+  
+  /**
+   * Returns true if the given target cpu is an apple platform.
+   */
+  public static boolean isApplePlatform(String targetCpu) {
+    return forTargetCpuNullable(targetCpu) != null;
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index d486975c93..792e036a39 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -50,6 +50,8 @@ import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.events.EventKind;
 import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
+import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
+import com.google.devtools.build.lib.rules.apple.Platform;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppCompileActionContext.Reply;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
@@ -596,6 +598,20 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
     if (configuration.isCodeCoverageEnabled()) {
       environment.put("PWD", "/proc/self/cwd");
     }
+
+    // TODO(bazel-team): Handle at the level of crosstool (feature) templates instead of in this
+    // compile action. This will also prevent the need for apple host system and target platform
+    // evaluation here.
+    AppleConfiguration appleConfiguration = configuration.getFragment(AppleConfiguration.class);
+    if (CppConfiguration.MAC_SYSTEM_NAME.equals(getHostSystemName())) {
+      environment.putAll(appleConfiguration.appleHostSystemEnv());
+    }
+    if (Platform.isApplePlatform(cppConfiguration.getTargetCpu())) {
+      environment.putAll(appleConfiguration.appleTargetPlatformEnv(
+          Platform.forTargetCpu(cppConfiguration.getTargetCpu())));
+    }
+
+    // TODO(bazel-team): Check (crosstool) host system name instead of using OS.getCurrent.
     if (OS.getCurrent() == OS.WINDOWS) {
       // TODO(bazel-team): Both GCC and clang rely on their execution directories being on
       // PATH, otherwise they fail to find dependent DLLs (and they fail silently...). On