Add back uses of -disable-autolink-framework for static frameworks (#…

…941)

This was removed a while back because it appeared to be unused. Since
then this rules_apple change:

bazelbuild/rules_apple@8d84134
causes it to be slightly necessary again. The situation is that we
compile Swift files with the framework search paths, but then we now
link by passing the static framework binary directly to the linker, as
opposed to using `-F` / `-framework`, which means any `LC_LINKER_OPTION`
`-framework` uses pointing to these static frameworks are invalid.
Invalid `LC_LINKER_OPTION`s are ignored by ld64 _unless_ the entire link
fails, but lld and mold do not implement this workaround, so it fails
the link. This also potentially helps with cases of including undeclared
dependencies. Ideally we could disable autolinking all together to
ensure dependencies were strict, but that requires us to manually link
the various Swift support dylibs.

This discovers framework names based on their search paths similar to
how bazel did before the rules_apple change
https://github.com/bazelbuild/bazel/blob/8bd2d97fd143b4b8a6fc96ce300820e6e422c03a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java#L689-L697

We cannot revert the rules_apple change since theoretically that field
or provider will be removed entirely in the future.

This reverts commit b37a717.

swift/internal/compiling.bzl

@@ -14,6 +14,7 @@
 
 """Implementation of compilation logic for Swift."""
 
+load("@bazel_skylib//lib:collections.bzl", "collections")
 load("@bazel_skylib//lib:partial.bzl", "partial")
 load("@bazel_skylib//lib:paths.bzl", "paths")
 load("@bazel_skylib//lib:sets.bzl", "sets")
@@ -1111,6 +1112,12 @@ def compile_action_configs(
             ],
             configurators = [_conditional_compilation_flag_configurator],
         ),
+
+        # Disable auto-linking for prebuilt static frameworks.
+        swift_toolchain_config.action_config(
+            actions = [swift_action_names.COMPILE],
+            configurators = [_static_frameworks_disable_autolink_configurator],
+        ),
     ]
 
     # NOTE: The positions of these action configs in the list are important,
@@ -1607,6 +1614,19 @@ def _framework_search_paths_configurator(prerequisites, args, is_swift):
         before_each = "-Xcc",
     )
 
+def _static_frameworks_disable_autolink_configurator(prerequisites, args):
+    """Add flags to disable auto-linking for static prebuilt frameworks.
+
+    This disables the `LC_LINKER_OPTION` load commands for auto-linking when
+    importing a static framework. This is needed to avoid potential linker
+    errors since when linking the framework it will be passed directly as a
+    library.
+    """
+    args.add_all(
+        prerequisites.objc_info.imported_library,
+        map_each = _disable_autolink_framework_copts,
+    )
+
 def _dependencies_swiftmodules_configurator(prerequisites, args):
     """Adds `.swiftmodule` files from deps to search paths and action inputs."""
     args.add_all(
@@ -3011,6 +3031,27 @@ def _swift_module_search_path_map_fn(module):
     else:
         return None
 
+def _disable_autolink_framework_copts(library_path):
+    """A `map_each` helper that potentially disables autolinking for the given library.
+
+    Args:
+        library_path: The path to an imported library that is potentially a static framework.
+
+    Returns:
+        The list of `swiftc` flags needed to disable autolinking for the given
+        framework.
+    """
+    if not library_path.dirname.endswith(".framework"):
+        return []
+
+    return collections.before_each(
+        "-Xfrontend",
+        [
+            "-disable-autolink-framework",
+            library_path.basename,
+        ],
+    )
+
 def _find_num_threads_flag_value(user_compile_flags):
     """Finds the value of the `-num-threads` flag.
 

test/BUILD

@@ -4,7 +4,7 @@ load(":coverage_settings_tests.bzl", "coverage_settings_test_suite")
 load(":debug_settings_tests.bzl", "debug_settings_test_suite")
 load(":features_tests.bzl", "features_test_suite")
 load(":generated_header_tests.bzl", "generated_header_test_suite")
-load(":linking_tests.bzl", "linking_test_suite")
+load(":imported_framework_tests.bzl", "imported_framework_test_suite")
 load(":mainattr_tests.bzl", "mainattr_test_suite")
 load(":module_cache_settings_tests.bzl", "module_cache_settings_test_suite")
 load(":output_file_map_tests.bzl", "output_file_map_test_suite")
@@ -27,7 +27,7 @@ features_test_suite(name = "features")
 
 generated_header_test_suite(name = "generated_header")
 
-linking_test_suite(name = "linking")
+imported_framework_test_suite(name = "imported_framework")
 
 mainattr_test_suite(name = "mainattr")
 

test/fixtures/linking/fake_framework.bzl

@@ -1,12 +1,13 @@
 """Simple rule to emulate apple_static_framework_import"""
 
 def _impl(ctx):
-    binary1 = ctx.actions.declare_file("framework1.framework/framework")
+    binary1 = ctx.actions.declare_file("framework1.framework/framework1")
     ctx.actions.write(binary1, "empty")
-    binary2 = ctx.actions.declare_file("framework2.framework/framework")
+    binary2 = ctx.actions.declare_file("framework2.framework/framework2")
     ctx.actions.write(binary2, "empty")
     return apple_common.new_objc_provider(
         static_framework_file = depset([binary1, binary2]),
+        imported_library = depset([binary1, binary2]),
     )
 
 fake_framework = rule(

test/imported_framework_tests.bzl

@@ -0,0 +1,34 @@
+"""Tests for validating linking behavior"""
+
+load(
+    "@build_bazel_rules_swift//test/rules:action_command_line_test.bzl",
+    "action_command_line_test",
+)
+
+def imported_framework_test_suite(name):
+    action_command_line_test(
+        name = "{}_disable_autolink_framework_test".format(name),
+        expected_argv = [
+            "-Xfrontend -disable-autolink-framework -Xfrontend framework1",
+            "-Xfrontend -disable-autolink-framework -Xfrontend framework2",
+        ],
+        mnemonic = "SwiftCompile",
+        tags = [name],
+        target_under_test = "@build_bazel_rules_swift//test/fixtures/linking:bin",
+    )
+
+    action_command_line_test(
+        name = "{}_duplicate_linking_args".format(name),
+        expected_argv = [
+            "-framework framework1",
+            "-framework framework2",
+        ],
+        mnemonic = "CppLink",
+        tags = [name],
+        target_under_test = "@build_bazel_rules_swift//test/fixtures/linking:bin",
+    )
+
+    native.test_suite(
+        name = name,
+        tags = [name],
+    )