From aff1754b064898fde731a64622840535f8dd7165 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Fri, 6 Nov 2020 16:56:35 -0600 Subject: [PATCH] Migrate to the modern Starlark linker input API. (#512) See https://github.com/bazelbuild/bazel/issues/10860. --- swift/internal/linking.bzl | 40 +++++++++++++----- swift/internal/swift_binary_test.bzl | 1 + swift/internal/swift_grpc_library.bzl | 11 +++-- swift/internal/swift_import.bzl | 22 ++++++---- swift/internal/swift_library.bzl | 13 +++--- swift/internal/swift_module_alias.bzl | 12 +++--- swift/internal/swift_protoc_gen_aspect.bzl | 11 +++-- swift/internal/utils.bzl | 49 +++++++--------------- test/private_deps_tests.bzl | 2 +- test/rules/provider_test.bzl | 9 +++- test/split_derived_files_tests.bzl | 6 +-- 11 files changed, 100 insertions(+), 76 deletions(-) diff --git a/swift/internal/linking.bzl b/swift/internal/linking.bzl index 613c77ff2..ac8a87c27 100644 --- a/swift/internal/linking.bzl +++ b/swift/internal/linking.bzl @@ -95,6 +95,7 @@ def _register_static_library_link_action( ) def register_libraries_to_link( + owning_label, actions, alwayslink, cc_feature_configuration, @@ -102,10 +103,13 @@ def register_libraries_to_link( is_static, library_name, objects, - swift_toolchain): + swift_toolchain, + user_link_flags, + additional_inputs): """Declares the requested libraries and registers actions to link them. Args: + owning_label: Label executing rule (i.e., ctx.label). actions: The object used to register actions. alwayslink: If True, create a static library that should be always-linked (having a `.lo` extension instead of `.a`). This @@ -120,9 +124,11 @@ def register_libraries_to_link( linked. swift_toolchain: The Swift toolchain provider to use when constructing the action. + user_link_flags: Extra link flags to be passed with the library. + additional_inputs: Extra inputs for a link action involving the library. Returns: - A `LibraryToLink` object containing the libraries that were created. + A `LinkerInput` object containing the libraries that were created. """ dynamic_library = None if is_dynamic: @@ -145,16 +151,24 @@ def register_libraries_to_link( else: static_library = None - return cc_common.create_library_to_link( - actions = actions, - alwayslink = alwayslink, - cc_toolchain = swift_toolchain.cc_toolchain_info, - feature_configuration = cc_feature_configuration, - pic_static_library = static_library, - dynamic_library = dynamic_library, + return cc_common.create_linker_input( + owner = owning_label, + libraries = depset(direct = [ + cc_common.create_library_to_link( + actions = actions, + alwayslink = alwayslink, + cc_toolchain = swift_toolchain.cc_toolchain_info, + feature_configuration = cc_feature_configuration, + pic_static_library = static_library, + dynamic_library = dynamic_library, + ), + ]), + additional_inputs = depset(direct = additional_inputs), + user_link_flags = depset(direct = user_link_flags), ) def register_link_binary_action( + owning_label, actions, additional_inputs, additional_linking_contexts, @@ -170,6 +184,7 @@ def register_link_binary_action( """Registers an action that invokes the linker to produce a binary. Args: + owning_label: Label of the rule creating the link action. actions: The object used to register actions. additional_inputs: A list of additional inputs to the link action, such as those used in `$(location ...)` substitution, linker @@ -231,7 +246,12 @@ def register_link_binary_action( linking_contexts.append( cc_common.create_linking_context( - user_link_flags = dep_link_flags, + linker_inputs = depset(direct = [ + cc_common.create_linker_input( + owner = owning_label, + user_link_flags = depset(direct = dep_link_flags), + ), + ]) ), ) diff --git a/swift/internal/swift_binary_test.bzl b/swift/internal/swift_binary_test.bzl index 6d0d544d3..bfbc80d93 100644 --- a/swift/internal/swift_binary_test.bzl +++ b/swift/internal/swift_binary_test.bzl @@ -237,6 +237,7 @@ def _swift_linking_rule_impl( user_link_flags.extend(ctx.fragments.cpp.linkopts) linking_outputs = register_link_binary_action( + owning_label = ctx.label, actions = ctx.actions, additional_inputs = additional_inputs_to_linker, additional_linking_contexts = additional_linking_contexts, diff --git a/swift/internal/swift_grpc_library.bzl b/swift/internal/swift_grpc_library.bzl index 5688ac18c..29ffa6f46 100644 --- a/swift/internal/swift_grpc_library.bzl +++ b/swift/internal/swift_grpc_library.bzl @@ -293,7 +293,8 @@ def _swift_grpc_library_impl(ctx): target_name = ctx.label.name, ) - library_to_link = register_libraries_to_link( + linker_input = register_libraries_to_link( + owning_label = ctx.label, actions = ctx.actions, alwayslink = False, cc_feature_configuration = swift_common.cc_feature_configuration( @@ -304,6 +305,8 @@ def _swift_grpc_library_impl(ctx): library_name = ctx.label.name, objects = compilation_outputs.object_files, swift_toolchain = swift_toolchain, + additional_inputs = compilation_outputs.linker_inputs, + user_link_flags = compilation_outputs.linker_flags, ) providers = [ @@ -311,7 +314,7 @@ def _swift_grpc_library_impl(ctx): files = depset(direct = generated_files + compact([ compilation_outputs.swiftdoc, compilation_outputs.swiftmodule, - library_to_link.pic_static_library, + linker_input.libraries[0].pic_static_library, ])), ), OutputGroupInfo(**output_groups_from_compilation_outputs( @@ -320,7 +323,7 @@ def _swift_grpc_library_impl(ctx): create_cc_info( cc_infos = get_providers(compile_deps, CcInfo), compilation_outputs = compilation_outputs, - libraries_to_link = [library_to_link], + linker_inputs = [linker_input], ), deps[0][SwiftProtoInfo], swift_common.create_swift_info( @@ -346,7 +349,7 @@ def _swift_grpc_library_impl(ctx): link_inputs = compilation_outputs.linker_inputs, linkopts = compilation_outputs.linker_flags, module_map = compilation_outputs.generated_module_map, - static_archives = compact([library_to_link.pic_static_library]), + static_archives = compact([linker_input.libraries[0].pic_static_library]), swiftmodules = [compilation_outputs.swiftmodule], objc_header = compilation_outputs.generated_header, )) diff --git a/swift/internal/swift_import.bzl b/swift/internal/swift_import.bzl index 7fdfd3f31..10fc0c7ec 100644 --- a/swift/internal/swift_import.bzl +++ b/swift/internal/swift_import.bzl @@ -38,14 +38,18 @@ def _swift_import_impl(ctx): unsupported_features = ctx.disabled_features, ) - libraries_to_link = [ - cc_common.create_library_to_link( - actions = ctx.actions, - cc_toolchain = cc_toolchain, - feature_configuration = cc_feature_configuration, - static_library = archive, - ) - for archive in archives + linker_inputs = [ + cc_common.create_linker_input( + libraries = depset(direct = [ + cc_common.create_library_to_link( + actions = ctx.actions, + cc_toolchain = cc_toolchain, + feature_configuration = cc_feature_configuration, + static_library = archive, + ) + for archive in archives + ]), + ), ] providers = [ @@ -59,7 +63,7 @@ def _swift_import_impl(ctx): ), create_cc_info( cc_infos = get_providers(deps, CcInfo), - libraries_to_link = libraries_to_link, + linker_inputs = linker_inputs, ), # Propagate an `Objc` provider so that Apple-specific rules like # apple_binary` will link the imported library properly. Typically we'd diff --git a/swift/internal/swift_library.bzl b/swift/internal/swift_library.bzl index 374b5184e..706eb6a1b 100644 --- a/swift/internal/swift_library.bzl +++ b/swift/internal/swift_library.bzl @@ -179,7 +179,8 @@ def _swift_library_impl(ctx): else: clang_module = None - library_to_link = register_libraries_to_link( + linker_input = register_libraries_to_link( + owning_label = ctx.label, actions = ctx.actions, alwayslink = ctx.attr.alwayslink, cc_feature_configuration = swift_common.cc_feature_configuration( @@ -190,6 +191,8 @@ def _swift_library_impl(ctx): library_name = ctx.label.name, objects = compilation_outputs.object_files, swift_toolchain = swift_toolchain, + user_link_flags = linkopts, + additional_inputs = compilation_outputs.linker_inputs, ) direct_output_files = compact([ @@ -197,7 +200,7 @@ def _swift_library_impl(ctx): compilation_outputs.swiftdoc, compilation_outputs.swiftinterface, compilation_outputs.swiftmodule, - library_to_link.pic_static_library, + linker_input.libraries[0].pic_static_library, ]) providers = [ @@ -213,14 +216,12 @@ def _swift_library_impl(ctx): compilation_outputs = compilation_outputs, )), create_cc_info( - additional_inputs = additional_inputs, cc_infos = get_providers(deps, CcInfo), compilation_outputs = compilation_outputs, defines = ctx.attr.defines, includes = [ctx.bin_dir.path], - libraries_to_link = [library_to_link], + linker_inputs = [linker_input], private_cc_infos = get_providers(private_deps, CcInfo), - user_link_flags = linkopts, ), coverage_common.instrumented_files_info( ctx, @@ -261,7 +262,7 @@ def _swift_library_impl(ctx): link_inputs = compilation_outputs.linker_inputs + additional_inputs, linkopts = compilation_outputs.linker_flags + linkopts, module_map = compilation_outputs.generated_module_map, - static_archives = compact([library_to_link.pic_static_library]), + static_archives = compact([linker_input.libraries[0].pic_static_library]), swiftmodules = [compilation_outputs.swiftmodule], objc_header = compilation_outputs.generated_header, )) diff --git a/swift/internal/swift_module_alias.bzl b/swift/internal/swift_module_alias.bzl index 55a57bf7c..217b7798c 100644 --- a/swift/internal/swift_module_alias.bzl +++ b/swift/internal/swift_module_alias.bzl @@ -72,7 +72,8 @@ def _swift_module_alias_impl(ctx): target_name = ctx.label.name, ) - library_to_link = register_libraries_to_link( + linker_input = register_libraries_to_link( + owning_label = ctx.label, actions = ctx.actions, alwayslink = False, cc_feature_configuration = swift_common.cc_feature_configuration( @@ -83,6 +84,7 @@ def _swift_module_alias_impl(ctx): library_name = ctx.label.name, objects = compilation_outputs.object_files, swift_toolchain = swift_toolchain, + additional_inputs = compilation_outputs.linker_inputs, ) providers = [ @@ -90,8 +92,8 @@ def _swift_module_alias_impl(ctx): files = depset(compact([ compilation_outputs.swiftdoc, compilation_outputs.swiftmodule, - library_to_link.dynamic_library, - library_to_link.pic_static_library, + linker_input.libraries[0].dynamic_library, + linker_input.libraries[0].pic_static_library, ])), ), OutputGroupInfo(**output_groups_from_compilation_outputs( @@ -105,7 +107,7 @@ def _swift_module_alias_impl(ctx): cc_infos = get_providers(deps, CcInfo), compilation_outputs = compilation_outputs, includes = [ctx.bin_dir.path], - libraries_to_link = [library_to_link], + linker_inputs = [linker_input], ), swift_common.create_swift_info( modules = [ @@ -130,7 +132,7 @@ def _swift_module_alias_impl(ctx): link_inputs = compilation_outputs.linker_inputs, linkopts = compilation_outputs.linker_flags, module_map = compilation_outputs.generated_module_map, - static_archives = compact([library_to_link.pic_static_library]), + static_archives = compact([linker_input.libraries[0].pic_static_library]), swiftmodules = [compilation_outputs.swiftmodule], objc_header = compilation_outputs.generated_header, )) diff --git a/swift/internal/swift_protoc_gen_aspect.bzl b/swift/internal/swift_protoc_gen_aspect.bzl index 091013a9e..45f78837a 100644 --- a/swift/internal/swift_protoc_gen_aspect.bzl +++ b/swift/internal/swift_protoc_gen_aspect.bzl @@ -428,7 +428,8 @@ def _swift_protoc_gen_aspect_impl(target, aspect_ctx): target_name = target.label.name, ) - library_to_link = register_libraries_to_link( + linker_input = register_libraries_to_link( + owning_label = aspect_ctx.label, actions = aspect_ctx.actions, alwayslink = False, cc_feature_configuration = swift_common.cc_feature_configuration( @@ -442,6 +443,8 @@ def _swift_protoc_gen_aspect_impl(target, aspect_ctx): library_name = "{}.swift".format(target.label.name), objects = compilation_outputs.object_files, swift_toolchain = swift_toolchain, + additional_inputs = compilation_outputs.linker_inputs, + user_link_flags = compilation_outputs.linker_flags, ) # It's bad practice to attach providers you don't own to other targets, @@ -477,9 +480,9 @@ def _swift_protoc_gen_aspect_impl(target, aspect_ctx): objc_info_args["header"] = depset([ compilation_outputs.generated_header, ]) - if library_to_link.pic_static_library: + if linker_input.libraries[0].pic_static_library: objc_info_args["library"] = depset( - [library_to_link.pic_static_library], + [linker_input.libraries[0].pic_static_library], order = "topological", ) if compilation_outputs.linker_flags: @@ -516,7 +519,7 @@ def _swift_protoc_gen_aspect_impl(target, aspect_ctx): cc_infos = cc_infos, compilation_outputs = compilation_outputs, includes = includes, - libraries_to_link = [library_to_link], + linker_inputs = [linker_input], ), objc_info = objc_info, ), diff --git a/swift/internal/utils.bzl b/swift/internal/utils.bzl index b4e41d85f..fcda609af 100644 --- a/swift/internal/utils.bzl +++ b/swift/internal/utils.bzl @@ -41,25 +41,20 @@ def collect_cc_libraries( """ libraries = [] - # TODO(https://github.com/bazelbuild/bazel/issues/8118): Remove once flag is - # flipped. - libraries_to_link = cc_info.linking_context.libraries_to_link - if hasattr(libraries_to_link, "to_list"): - libraries_to_link = libraries_to_link.to_list() - - for library in libraries_to_link: - if include_pic_static: - if library.pic_static_library: - libraries.append(library.pic_static_library) - elif library.static_library: + for li in cc_info.linking_context.linker_inputs.to_list(): + for library in li.libraries: + if include_pic_static: + if library.pic_static_library: + libraries.append(library.pic_static_library) + elif library.static_library: + libraries.append(library.static_library) + elif include_static and library.static_library: libraries.append(library.static_library) - elif include_static and library.static_library: - libraries.append(library.static_library) - if include_dynamic and library.dynamic_library: - libraries.append(library.dynamic_library) - if include_interface and library.interface_library: - libraries.append(library.interface_library) + if include_dynamic and library.dynamic_library: + libraries.append(library.dynamic_library) + if include_interface and library.interface_library: + libraries.append(library.interface_library) return libraries @@ -74,19 +69,15 @@ def compact(sequence): return [item for item in sequence if item != None] def create_cc_info( - additional_inputs = [], cc_infos = [], compilation_outputs = None, defines = [], includes = [], - libraries_to_link = [], - private_cc_infos = [], - user_link_flags = []): + linker_inputs = [], + private_cc_infos = []): """Creates a `CcInfo` provider from Swift compilation info and deps. Args: - additional_inputs: A list of additional files that should be passed as - inputs to the final link action. cc_infos: A list of `CcInfo` providers from public dependencies, whose compilation and linking contexts should both be merged into the new provider. @@ -96,32 +87,24 @@ def create_cc_info( context. includes: The list of include paths to insert into the compilation context. - libraries_to_link: A list of `LibraryToLink` objects that represent the + linker_inputs: A list of `LinkerInput` objects that represent the libraries that should be linked into the final binary. private_cc_infos: A list of `CcInfo` providers from private (implementation-only) dependencies, whose linking contexts should be merged into the new provider but whose compilation contexts should be excluded. - user_link_flags: A list of flags that should be passed to the final link - action. Returns: A new `CcInfo`. """ - all_additional_inputs = list(additional_inputs) - all_user_link_flags = list(user_link_flags) all_headers = [] if compilation_outputs: - all_additional_inputs.extend(compilation_outputs.linker_inputs) - all_user_link_flags.extend(compilation_outputs.linker_flags) all_headers = compact([compilation_outputs.generated_header]) local_cc_infos = [ CcInfo( linking_context = cc_common.create_linking_context( - additional_inputs = all_additional_inputs, - libraries_to_link = libraries_to_link, - user_link_flags = all_user_link_flags, + linker_inputs = depset(direct = linker_inputs), ), compilation_context = cc_common.create_compilation_context( defines = depset(defines), diff --git a/test/private_deps_tests.bzl b/test/private_deps_tests.bzl index 1afd6b10e..148777462 100644 --- a/test/private_deps_tests.bzl +++ b/test/private_deps_tests.bzl @@ -115,7 +115,7 @@ def private_deps_test_suite(name = "private_deps"): # dependencies, which we need to ignore. "*", ], - field = "linking_context.libraries_to_link.static_library!", + field = "linking_context.linker_inputs.libraries.static_library!", provider = "CcInfo", tags = [name], target_under_test = "@build_bazel_rules_swift//test/fixtures/private_deps:client_cc_deps", diff --git a/test/rules/provider_test.bzl b/test/rules/provider_test.bzl index 42799fa79..b9cf0b762 100644 --- a/test/rules/provider_test.bzl +++ b/test/rules/provider_test.bzl @@ -85,7 +85,14 @@ def _evaluate_field(env, source, field): ) return _EVALUATE_FIELD_FAILED - source = [getattr(item, component, None) for item in source] + expanded = [] + for item in source: + item = _normalize_collection(item) + if types.is_list(item): + expanded.extend(item) + else: + expanded.append(item) + source = [getattr(item, component, None) for item in expanded] if filter_nones: source = [item for item in source if item != None] else: diff --git a/test/split_derived_files_tests.bzl b/test/split_derived_files_tests.bzl index 2eeb17664..34839ebae 100644 --- a/test/split_derived_files_tests.bzl +++ b/test/split_derived_files_tests.bzl @@ -114,7 +114,7 @@ def split_derived_files_test_suite(name = "split_derived_files"): expected_files = [ "libsimple.a", ], - field = "linking_context.libraries_to_link.pic_static_library!", + field = "linking_context.linker_inputs.libraries.pic_static_library!", provider = "CcInfo", tags = [name], target_under_test = "@build_bazel_rules_swift//test/fixtures/debug_settings:simple", @@ -162,7 +162,7 @@ def split_derived_files_test_suite(name = "split_derived_files"): expected_files = [ "libsimple.a", ], - field = "linking_context.libraries_to_link.pic_static_library!", + field = "linking_context.linker_inputs.libraries.pic_static_library!", provider = "CcInfo", tags = [name], target_under_test = "@build_bazel_rules_swift//test/fixtures/debug_settings:simple", @@ -212,7 +212,7 @@ def split_derived_files_test_suite(name = "split_derived_files"): expected_files = [ "libsimple.a", ], - field = "linking_context.libraries_to_link.pic_static_library!", + field = "linking_context.linker_inputs.libraries.pic_static_library!", provider = "CcInfo", tags = [name], target_under_test = "@build_bazel_rules_swift//test/fixtures/debug_settings:simple",