macOS link actions set the correct system framework include paths

RELNOTES: None.
PiperOrigin-RevId: 163114020

6 files changed, 54 insertions, 14 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java
index 35bd1d7991..a380f52713 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java
@@ -106,6 +106,14 @@ public class AppleToolchain {
   }
 
   /**
+   * Returns the platform frameworks directory inside of Xcode for a given {@link ApplePlatform}.
+   */
+  public static String platformDeveloperFrameworkDir(ApplePlatform platform) {
+    String platformDir = platformDir(platform.getNameInPlist());
+    return platformDir + "/Developer/Library/Frameworks";
+  }
+
+  /**
    * Returns the platform frameworks directory inside of Xcode for a given configuration.
    */
   @SkylarkCallable(
@@ -113,8 +121,7 @@ public class AppleToolchain {
     doc = "Returns the platform frameworks directory inside of Xcode for a given configuration."
   )
   public static String platformDeveloperFrameworkDir(AppleConfiguration configuration) {
-    String platformDir = platformDir(configuration.getSingleArchPlatform().getNameInPlist());
-    return platformDir + "/Developer/Library/Frameworks";
+    return platformDeveloperFrameworkDir(configuration.getSingleArchPlatform());
   }
 
   /** Returns the SDK frameworks directory inside of Xcode for a given configuration. */
@@ -132,8 +139,6 @@ public class AppleToolchain {
         }
         break;
       case MACOS:
-        relativePath = DEVELOPER_FRAMEWORK_PATH;
-        break;
       case WATCHOS_DEVICE:
       case WATCHOS_SIMULATOR:
       case TVOS_DEVICE:
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 7e49b8eea3..299ab73c5b 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
@@ -265,22 +265,21 @@ public abstract class CompilationSupport {
 
   /** Returns a list of framework search path flags for clang actions. */
   static Iterable<String> commonFrameworkFlags(
-      ObjcProvider provider, RuleContext ruleContext) {
-    return Interspersing.beforeEach("-F", commonFrameworkNames(provider, ruleContext));
+      ObjcProvider provider, RuleContext ruleContext, ApplePlatform applePlaform) {
+    return Interspersing.beforeEach("-F",
+        commonFrameworkNames(provider, ruleContext, applePlaform));
   }
 
   /** Returns a list of frameworks for clang actions. */
   static Iterable<String> commonFrameworkNames(
-      ObjcProvider provider, RuleContext ruleContext) {
-    AppleConfiguration appleConfiguration = ruleContext.getFragment(AppleConfiguration.class);
-    ApplePlatform platform = appleConfiguration.getSingleArchPlatform();
+      ObjcProvider provider, RuleContext ruleContext, ApplePlatform platform) {
 
     ImmutableList.Builder<String> frameworkNames =
         new ImmutableList.Builder<String>()
             .add(AppleToolchain.sdkFrameworkDir(platform, ruleContext));
-    if (platform.getType() == PlatformType.IOS) {
-      // As of sdk8.1, XCTest is in a base Framework dir
-      frameworkNames.add(AppleToolchain.platformDeveloperFrameworkDir(appleConfiguration));
+    // As of sdk8.1, XCTest is in a base Framework dir.
+    if (platform.getType() != PlatformType.WATCHOS) { // WatchOS does not have this directory.
+      frameworkNames.add(AppleToolchain.platformDeveloperFrameworkDir(platform));
     }
     return frameworkNames
         // Add custom (non-SDK) framework search paths. For each framework foo/bar.framework,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java
index bfcf03b992..570487a49f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java
@@ -864,7 +864,7 @@ public class LegacyCompilationSupport extends CompilationSupport {
         .add("-arch", appleConfiguration.getSingleArchitecture())
         .add("-isysroot", AppleToolchain.sdkDir())
         // TODO(bazel-team): Pass framework search paths to Xcodegen.
-        .addAll(commonFrameworkFlags(provider, ruleContext))
+        .addAll(commonFrameworkFlags(provider, ruleContext, platform))
         .build();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java
index 452b2606ea..e9b421c170 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java
@@ -25,6 +25,8 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
+import com.google.devtools.build.lib.rules.apple.ApplePlatform;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.StringSequenceBuilder;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension;
@@ -153,9 +155,11 @@ class ObjcVariablesExtension implements VariablesExtension {
   }
 
   private void addFrameworkVariables(CcToolchainFeatures.Variables.Builder builder) {
+    ApplePlatform applePlatform =
+        buildConfiguration.getFragment(AppleConfiguration.class).getSingleArchPlatform();
     StringSequenceBuilder frameworkSequence = new StringSequenceBuilder();
     for (String framework :
-        CompilationSupport.commonFrameworkNames(objcProvider, ruleContext)) {
+        CompilationSupport.commonFrameworkNames(objcProvider, ruleContext, applePlatform)) {
       frameworkSequence.addValue(framework);
     }
     builder.addCustomBuiltVariable(FRAMEWORKS_VARIABLE_NAME, frameworkSequence);
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java
index a359149170..c58c64972c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java
@@ -28,6 +28,9 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
 import com.google.devtools.build.lib.rules.apple.AppleConfiguration.ConfigurationDistinguisher;
+import com.google.devtools.build.lib.rules.apple.ApplePlatform;
+import com.google.devtools.build.lib.rules.apple.ApplePlatform.PlatformType;
+import com.google.devtools.build.lib.rules.apple.AppleToolchain;
 import com.google.devtools.build.lib.rules.objc.AppleBinary.BinaryType;
 import com.google.devtools.build.lib.rules.objc.CompilationSupport.ExtraLinkArgs;
 import com.google.devtools.build.lib.rules.objc.ObjcCommandLineOptions.ObjcCrosstoolMode;
@@ -1444,4 +1447,27 @@ public class AppleBinaryTest extends ObjcRuleTestCase {
   public void testMinimumOsDifferentTargets() throws Exception {
     checkMinimumOsDifferentTargets(RULE_TYPE, "_lipobin", "_bin");
   }
+
+  @Test
+  public void testMacosFrameworkDirectories() throws Exception {
+    scratch.file(
+        "test/BUILD",
+        "apple_binary(",
+        "    name = 'test',",
+        "    srcs = ['test.m'],",
+        "    platform_type = 'macos',",
+        ")");
+
+    CommandAction linkAction = linkAction("//test:test");
+    ImmutableList<String> expectedCommandLineFragments =
+        ImmutableList.<String>builder()
+            .add(AppleToolchain.sdkDir() + AppleToolchain.SYSTEM_FRAMEWORK_PATH)
+            .add(frameworkDir(ApplePlatform.forTarget(PlatformType.MACOS, "x86_64")))
+            .build();
+
+    String linkArgs = Joiner.on(" ").join(linkAction.getArguments());
+    for (String expectedCommandLineFragment : expectedCommandLineFragments) {
+      assertThat(linkArgs).contains(expectedCommandLineFragment);
+    }
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/LegacyAppleBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/LegacyAppleBinaryTest.java
index 7f919ceee3..c89b00fde0 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/LegacyAppleBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/LegacyAppleBinaryTest.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.rules.objc;
 
 import com.google.devtools.build.lib.rules.objc.ObjcCommandLineOptions.ObjcCrosstoolMode;
+import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
@@ -28,4 +29,9 @@ public class LegacyAppleBinaryTest extends AppleBinaryTest {
     return ObjcCrosstoolMode.OFF;
   }
 
+  @Test
+  @Override
+  public void testMacosFrameworkDirectories() throws Exception {
+    // This AppleBinaryTest fails due to lack of support in legacy mode.
+  }
 }