From 154be9c02830801ba3bd8767a349a9667751a301 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Mon, 8 Jan 2024 19:28:29 -0500 Subject: [PATCH] Fix compatibility with --incompatible_objc_provider_remove_linking_info (#1143) Fixes https://github.com/bazelbuild/rules_swift/issues/1142 --- swift/internal/compiling.bzl | 27 +++++++++--- swift/internal/linking.bzl | 54 +++++++++++++++--------- swift/internal/xcode_swift_toolchain.bzl | 10 ++++- test/fixtures/linking/fake_framework.bzl | 53 ++++++++++++++++++++--- test/imported_framework_tests.bzl | 11 ----- 5 files changed, 113 insertions(+), 42 deletions(-) diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index 57661e893..3844a3b6d 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -1709,11 +1709,27 @@ def _frameworks_disable_autolink_configurator(prerequisites, args): errors since when linking the framework it will be passed directly as a library. """ - args.add_all( - # TODO: This needs to come from CcInfo at some point - depset(transitive = [prerequisites.objc_info.imported_library, prerequisites.objc_info.dynamic_framework_file]), - map_each = _disable_autolink_framework_copts, - ) + if hasattr(prerequisites.objc_info, "dynamic_framework_file"): + args.add_all( + depset(transitive = [prerequisites.objc_info.imported_library, prerequisites.objc_info.dynamic_framework_file]), + map_each = _disable_autolink_framework_copts, + ) + else: + libraries = [] + inputs = prerequisites.cc_linking_context.linker_inputs.to_list() + for linker_input in inputs: + for library in linker_input.libraries: + if library.dynamic_library: + libraries.append(library.dynamic_library) + if library.static_library: + libraries.append(library.static_library) + if library.pic_static_library: + libraries.append(library.pic_static_library) + + args.add_all( + depset(transitive = [depset(libraries)]), + map_each = _disable_autolink_framework_copts, + ) def _dependencies_swiftmodules_configurator(prerequisites, args): """Adds `.swiftmodule` files from deps to search paths and action inputs.""" @@ -2460,6 +2476,7 @@ def compile( additional_inputs = additional_inputs, bin_dir = feature_configuration._bin_dir, cc_compilation_context = merged_providers.cc_info.compilation_context, + cc_linking_context = merged_providers.cc_info.linking_context, defines = sets.to_list(defines_set), explicit_swift_module_map_file = explicit_swift_module_map_file, developer_dirs = swift_toolchain.developer_dirs, diff --git a/swift/internal/linking.bzl b/swift/internal/linking.bzl index 7ee820e7e..e714d8df0 100644 --- a/swift/internal/linking.bzl +++ b/swift/internal/linking.bzl @@ -45,6 +45,9 @@ load(":providers.bzl", "SwiftToolchainInfo") load(":swift_clang_module_aspect.bzl", "swift_clang_module_aspect") load(":utils.bzl", "get_providers") +# TODO: Remove once we drop bazel 7.x +_OBJC_PROVIDER_LINKING = hasattr(apple_common.new_objc_provider(), "linkopt") + def binary_rule_attrs( *, additional_deps_providers = [], @@ -439,23 +442,29 @@ def new_objc_provider( ): extra_linkopts.append("-ObjC") - return apple_common.new_objc_provider( - force_load_library = depset( - force_load_libraries, - order = "topological", - ), - library = depset( - direct_libraries, - transitive = transitive_cc_libs, - order = "topological", - ), - link_inputs = depset(additional_link_inputs + debug_link_inputs), - linkopt = depset(user_link_flags + extra_linkopts), - providers = get_providers( + kwargs = { + "providers": get_providers( deps, apple_common.Objc, ) + additional_objc_infos, - ) + } + + if _OBJC_PROVIDER_LINKING: + kwargs = dicts.add(kwargs, { + "force_load_library": depset( + force_load_libraries, + order = "topological", + ), + "library": depset( + direct_libraries, + transitive = transitive_cc_libs, + order = "topological", + ), + "link_inputs": depset(additional_link_inputs + debug_link_inputs), + "linkopt": depset(user_link_flags + extra_linkopts), + }) + + return apple_common.new_objc_provider(**kwargs) def register_link_binary_action( actions, @@ -514,32 +523,37 @@ def register_link_binary_action( if apple_common.Objc in dep: objc = dep[apple_common.Objc] + def get_objc_list(objc, attr): + return getattr(objc, attr, depset([])).to_list() + # We don't need to handle the `objc.sdk_framework` field here # because those values have also been put into the user link flags # of a CcInfo, but the others don't seem to have been. dep_link_flags = [ "-l{}".format(dylib) - for dylib in objc.sdk_dylib.to_list() + for dylib in get_objc_list(objc, "sdk_dylib") ] dep_link_flags.extend([ "-F{}".format(path) - for path in objc.dynamic_framework_paths.to_list() + for path in get_objc_list(objc, "dynamic_framework_paths") ]) dep_link_flags.extend(collections.before_each( "-framework", - objc.dynamic_framework_names.to_list(), + get_objc_list(objc, "dynamic_framework_names"), )) dep_link_flags.extend([ "-F{}".format(path) - for path in objc.static_framework_paths.to_list() + for path in get_objc_list(objc, "static_framework_paths") ]) dep_link_flags.extend(collections.before_each( "-framework", - objc.static_framework_names.to_list(), + get_objc_list(objc, "static_framework_names"), )) is_bazel_6 = hasattr(apple_common, "link_multi_arch_static_library") - if is_bazel_6: + if not hasattr(objc, "static_framework_file"): + additional_inputs = depset([]) + elif is_bazel_6: additional_inputs = objc.static_framework_file else: additional_inputs = depset( diff --git a/swift/internal/xcode_swift_toolchain.bzl b/swift/internal/xcode_swift_toolchain.bzl index eb59d5424..8441fe9d6 100644 --- a/swift/internal/xcode_swift_toolchain.bzl +++ b/swift/internal/xcode_swift_toolchain.bzl @@ -67,6 +67,9 @@ load( "resolve_optional_tool", ) +# TODO: Remove once we drop bazel 7.x +_OBJC_PROVIDER_LINKING = hasattr(apple_common.new_objc_provider(), "linkopt") + # Maps (operating system, environment) pairs from target triples to the legacy # Bazel core `apple_common.platform` values, since we still use some APIs that # require these. @@ -243,6 +246,11 @@ def _swift_linkopts_providers( "-Wl,-rpath,/usr/lib/swift", ]) + if _OBJC_PROVIDER_LINKING: + objc_info = apple_common.new_objc_provider(linkopt = depset(linkopts)) + else: + objc_info = apple_common.new_objc_provider() + return struct( cc_info = CcInfo( linking_context = cc_common.create_linking_context( @@ -254,7 +262,7 @@ def _swift_linkopts_providers( ]), ), ), - objc_info = apple_common.new_objc_provider(linkopt = depset(linkopts)), + objc_info = objc_info, ) def _resource_directory_configurator(developer_dir, _prerequisites, args): diff --git a/test/fixtures/linking/fake_framework.bzl b/test/fixtures/linking/fake_framework.bzl index 69cbbab58..6781c78b0 100644 --- a/test/fixtures/linking/fake_framework.bzl +++ b/test/fixtures/linking/fake_framework.bzl @@ -1,16 +1,59 @@ """Simple rule to emulate apple_static_framework_import""" +load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain", "use_cpp_toolchain") + def _impl(ctx): binary1 = ctx.actions.declare_file("framework1.framework/framework1") ctx.actions.write(binary1, "empty") binary2 = ctx.actions.declare_file("framework2.framework/framework2") ctx.actions.write(binary2, "empty") - return apple_common.new_objc_provider( - static_framework_file = depset([binary1]), - imported_library = depset([binary1]), - dynamic_framework_file = depset([binary2]), - ) + if hasattr(apple_common.new_objc_provider(), "static_framework_file"): + return apple_common.new_objc_provider( + static_framework_file = depset([binary1]), + imported_library = depset([binary1]), + dynamic_framework_file = depset([binary2]), + ) + else: + cc_toolchain = find_cpp_toolchain(ctx) + feature_configuration = cc_common.configure_features( + ctx = ctx, + cc_toolchain = cc_toolchain, + language = "objc", + requested_features = ctx.features, + unsupported_features = ctx.disabled_features, + ) + return CcInfo( + linking_context = cc_common.create_linking_context( + linker_inputs = depset([ + cc_common.create_linker_input( + owner = ctx.label, + libraries = depset([ + cc_common.create_library_to_link( + actions = ctx.actions, + cc_toolchain = cc_toolchain, + feature_configuration = feature_configuration, + static_library = binary1, + ), + cc_common.create_library_to_link( + actions = ctx.actions, + cc_toolchain = cc_toolchain, + dynamic_library = binary2, + feature_configuration = feature_configuration, + ), + ]), + ), + ]), + ), + ) fake_framework = rule( implementation = _impl, + attrs = { + "_cc_toolchain": attr.label( + default = "@bazel_tools//tools/cpp:current_cc_toolchain", + doc = "The C++ toolchain to use.", + ), + }, + toolchains = use_cpp_toolchain(), + fragments = ["cpp"], ) diff --git a/test/imported_framework_tests.bzl b/test/imported_framework_tests.bzl index 0ecd356ac..375867367 100644 --- a/test/imported_framework_tests.bzl +++ b/test/imported_framework_tests.bzl @@ -17,17 +17,6 @@ def imported_framework_test_suite(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],