Add support for params files for darwin

Clang has supported params files for a while now. This updates the cc toolchain for darwin to use them. The logic for processing response files is mostly copied from rules_swift where similar processing is done. Closes #12265. PiperOrigin-RevId: 340416047
bazelbuild · Nov 3, 2020 · d3fc253 · d3fc253
1 parent b74f160
commit d3fc253
Show file tree

Hide file tree

Showing 15 changed files with 293 additions and 92 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -532,6 +532,10 @@ boolean canSplitCommandLine() {
       case PIC_STATIC_LIBRARY:
       case ALWAYS_LINK_STATIC_LIBRARY:
       case ALWAYS_LINK_PIC_STATIC_LIBRARY:
+      case OBJC_ARCHIVE:
+      case OBJC_FULLY_LINKED_ARCHIVE:
+      case OBJC_EXECUTABLE:
+      case OBJCPP_EXECUTABLE:
         return true;
 
       default:

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java
@@ -291,7 +291,7 @@ private static ExtraLinkArgs getRequiredLinkopts(RuleContext ruleContext)
     switch (binaryType) {
       case LOADABLE_BUNDLE:
         extraLinkArgs.add("-bundle");
-        extraLinkArgs.add("-Wl,-rpath,@loader_path/Frameworks");
+        extraLinkArgs.add("-Xlinker", "-rpath", "-Xlinker", "@loader_path/Frameworks");
         if (didProvideBundleLoader) {
           AppleExecutableBinaryInfo executableProvider =
               ruleContext.getPrerequisite(

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
@@ -1183,7 +1183,7 @@ CompilationSupport registerLinkActions(
                 ruleContext,
                 ruleContext.getLabel(),
                 binaryToLink,
-                ruleContext.getConfiguration(),
+                buildConfiguration,
                 toolchain,
                 toolchain.getFdoContext(),
                 getFeatureConfiguration(ruleContext, toolchain, buildConfiguration),
@@ -1358,7 +1358,7 @@ CompilationSupport registerFullyLinkAction(ObjcProvider objcProvider, Artifact o
                 ruleContext,
                 ruleContext.getLabel(),
                 outputArchive,
-                ruleContext.getConfiguration(),
+                buildConfiguration,
                 toolchain,
                 toolchain.getFdoContext(),
                 getFeatureConfiguration(ruleContext, toolchain, buildConfiguration),

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
@@ -604,6 +604,18 @@ public void testCommandLineSplitting() throws Exception {
     builder.setLinkType(LinkTargetType.STATIC_LIBRARY);
     assertThat(builder.canSplitCommandLine()).isTrue();
 
+    builder.setLinkType(LinkTargetType.OBJC_ARCHIVE);
+    assertThat(builder.canSplitCommandLine()).isTrue();
+
+    builder.setLinkType(LinkTargetType.OBJC_EXECUTABLE);
+    assertThat(builder.canSplitCommandLine()).isTrue();
+
+    builder.setLinkType(LinkTargetType.OBJCPP_EXECUTABLE);
+    assertThat(builder.canSplitCommandLine()).isTrue();
+
+    builder.setLinkType(LinkTargetType.OBJC_FULLY_LINKED_ARCHIVE);
+    assertThat(builder.canSplitCommandLine()).isTrue();
+
     builder.setLinkType(LinkTargetType.NODEPS_DYNAMIC_LIBRARY);
     assertThat(builder.canSplitCommandLine()).isTrue();
 

diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java
@@ -902,7 +902,7 @@ protected void checkBundleLoaderIsCorrectlyPassedToTheLinker(RuleType ruleType)
     assertThat(Joiner.on(" ").join(linkAction.getArguments()))
         .contains("-bundle_loader " + getBinArtifact("bin_lipobin", binTarget).getExecPath());
     assertThat(Joiner.on(" ").join(linkAction.getArguments()))
-        .contains("-Wl,-rpath,@loader_path/Frameworks");
+        .contains("-Xlinker -rpath -Xlinker @loader_path/Frameworks");
   }
 
   protected Action lipoLibAction(String libLabel) throws Exception {

diff --git a/src/test/shell/integration/aquery_test.sh b/src/test/shell/integration/aquery_test.sh
@@ -711,10 +711,6 @@ EOF
 }
 
 function test_aquery_include_param_file_cc_binary() {
-  if is_darwin; then
-    return 0
-  fi
-
   local pkg="${FUNCNAME[0]}"
   mkdir -p "$pkg" || fail "mkdir -p $pkg"
   cat > "$pkg/BUILD" <<'EOF'
@@ -739,10 +735,6 @@ EOF
 }
 
 function test_aquery_include_param_file_starlark_rule() {
-  if is_darwin; then
-    return 0
-  fi
-
   local pkg="${FUNCNAME[0]}"
   mkdir -p "$pkg" || fail "mkdir -p $pkg"
   cat > "$pkg/test_rule.bzl" <<'EOF'
@@ -793,10 +785,6 @@ EOF
 }
 
 function test_aquery_include_param_file_not_enabled_by_default() {
-  if is_darwin; then
-    return 0
-  fi
-
   local pkg="${FUNCNAME[0]}"
   mkdir -p "$pkg" || fail "mkdir -p $pkg"
   cat > "$pkg/BUILD" <<'EOF'

diff --git a/tools/cpp/BUILD.tools b/tools/cpp/BUILD.tools
@@ -277,7 +277,7 @@ cc_toolchain(
     linker_files = ":osx_wrapper",
     objcopy_files = ":empty",
     strip_files = ":empty",
-    supports_param_files = 0,
+    supports_param_files = 1,
     toolchain_config = ":local_darwin",
     toolchain_identifier = "local_darwin",
 )

diff --git a/tools/cpp/BUILD.tpl b/tools/cpp/BUILD.tpl
@@ -66,7 +66,7 @@ cc_toolchain(
     linker_files = ":compiler_deps",
     objcopy_files = ":empty",
     strip_files = ":empty",
-    supports_param_files = %{supports_param_files},
+    supports_param_files = 1,
     module_map = %{modulemap},
 )
 

diff --git a/tools/cpp/osx_cc_wrapper.sh b/tools/cpp/osx_cc_wrapper.sh
@@ -34,20 +34,33 @@ LIBS=
 LIB_DIRS=
 RPATHS=
 OUTPUT=
-# let parse the option list
-for i in "$@"; do
+
+function parse_option() {
+    local -r opt="$1"
     if [[ "${OUTPUT}" = "1" ]]; then
-        OUTPUT=$i
-    elif [[ "$i" =~ ^-l(.*)$ ]]; then
+        OUTPUT=$opt
+    elif [[ "$opt" =~ ^-l(.*)$ ]]; then
         LIBS="${BASH_REMATCH[1]} $LIBS"
-    elif [[ "$i" =~ ^-L(.*)$ ]]; then
+    elif [[ "$opt" =~ ^-L(.*)$ ]]; then
         LIB_DIRS="${BASH_REMATCH[1]} $LIB_DIRS"
-    elif [[ "$i" =~ ^-Wl,-rpath,\@loader_path/(.*)$ ]]; then
+    elif [[ "$opt" =~ ^-Wl,-rpath,\@loader_path/(.*)$ ]]; then
         RPATHS="${BASH_REMATCH[1]} ${RPATHS}"
-    elif [[ "$i" = "-o" ]]; then
+    elif [[ "$opt" = "-o" ]]; then
         # output is coming
         OUTPUT=1
     fi
+}
+
+# let parse the option list
+for i in "$@"; do
+    if [[ "$i" = @* ]]; then
+        while IFS= read -r opt
+        do
+            parse_option "$opt"
+        done < "${i:1}" || exit 1
+    else
+        parse_option "$i"
+    fi
 done
 
 # Call gcc

diff --git a/tools/cpp/osx_cc_wrapper.sh.tpl b/tools/cpp/osx_cc_wrapper.sh.tpl
@@ -33,20 +33,33 @@ LIBS=
 LIB_DIRS=
 RPATHS=
 OUTPUT=
-# let parse the option list
-for i in "$@"; do
+
+function parse_option() {
+    local -r opt="$1"
     if [[ "${OUTPUT}" = "1" ]]; then
-        OUTPUT=$i
-    elif [[ "$i" =~ ^-l(.*)$ ]]; then
+        OUTPUT=$opt
+    elif [[ "$opt" =~ ^-l(.*)$ ]]; then
         LIBS="${BASH_REMATCH[1]} $LIBS"
-    elif [[ "$i" =~ ^-L(.*)$ ]]; then
+    elif [[ "$opt" =~ ^-L(.*)$ ]]; then
         LIB_DIRS="${BASH_REMATCH[1]} $LIB_DIRS"
-    elif [[ "$i" =~ ^-Wl,-rpath,\@loader_path/(.*)$ ]]; then
+    elif [[ "$opt" =~ ^-Wl,-rpath,\@loader_path/(.*)$ ]]; then
         RPATHS="${BASH_REMATCH[1]} ${RPATHS}"
-    elif [[ "$i" = "-o" ]]; then
+    elif [[ "$opt" = "-o" ]]; then
         # output is coming
         OUTPUT=1
     fi
+}
+
+# let parse the option list
+for i in "$@"; do
+    if [[ "$i" = @* ]]; then
+        while IFS= read -r opt
+        do
+            parse_option "$opt"
+        done < "${i:1}" || exit 1
+    else
+        parse_option "$i"
+    fi
 done
 
 # Set-up the environment

diff --git a/tools/cpp/unix_cc_configure.bzl b/tools/cpp/unix_cc_configure.bzl
@@ -447,7 +447,6 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools):
             "%{cc_toolchain_identifier}": cc_toolchain_identifier,
             "%{name}": cpu_value,
             "%{modulemap}": ("\":module.modulemap\"" if is_clang else "None"),
-            "%{supports_param_files}": "0" if darwin else "1",
             "%{cc_compiler_deps}": get_starlark_list([":builtin_include_directory_paths"] + (
                 [":cc_wrapper"] if darwin else []
             )),

diff --git a/tools/osx/crosstool/BUILD.tpl b/tools/osx/crosstool/BUILD.tpl
@@ -70,7 +70,7 @@ cc_toolchain_suite(
         linker_files = ":osx_tools_" + arch,
         objcopy_files = ":empty",
         strip_files = ":osx_tools_" + arch,
-        supports_param_files = 0,
+        supports_param_files = 1,
         toolchain_config = ":" + (
             arch if arch != "armeabi-v7a" else "stub_armeabi-v7a"
         ),

diff --git a/tools/osx/crosstool/cc_toolchain_config.bzl b/tools/osx/crosstool/cc_toolchain_config.bzl
@@ -5198,16 +5198,13 @@ def _impl(ctx):
         name = "linker_param_file",
         flag_sets = [
             flag_set(
-                actions = all_link_actions,
-                flag_groups = [
-                    flag_group(
-                        flags = ["-Wl,@%{linker_param_file}"],
-                        expand_if_available = "linker_param_file",
-                    ),
+                actions = all_link_actions + [
+                    ACTION_NAMES.cpp_link_static_library,
+                    ACTION_NAMES.objc_archive,
+                    ACTION_NAMES.objc_fully_link,
+                    ACTION_NAMES.objc_executable,
+                    ACTION_NAMES.objcpp_executable,
                 ],
-            ),
-            flag_set(
-                actions = [ACTION_NAMES.cpp_link_static_library],
                 flag_groups = [
                     flag_group(
                         flags = ["@%{linker_param_file}"],