From 665ea7576e0794dc2d72cc7302714056ba143211 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Tue, 6 Apr 2021 11:11:55 -0700 Subject: [PATCH] Simplify and cleanup how implicit toolchain dependencies are managed. - When propagating implicit deps in the toolchain, merge and propagate the providers instead of the `Target` object itself. The `Target` object is fairly heavyweight and not intended to be propagated around the build graph. - Remove the `swift.minimal_deps` feature and the distinction between "required" and "optional" implicit deps. PiperOrigin-RevId: 367044294 --- swift/internal/compiling.bzl | 91 +++++++++++------------- swift/internal/feature_names.bzl | 7 -- swift/internal/providers.bzl | 80 ++++++++++++++------- swift/internal/swift_binary_test.bzl | 13 ++-- swift/internal/swift_common.bzl | 2 - swift/internal/swift_library.bzl | 15 ++-- swift/internal/swift_toolchain.bzl | 15 ++-- swift/internal/utils.bzl | 37 ++++++++++ swift/internal/xcode_swift_toolchain.bzl | 34 ++++----- 9 files changed, 175 insertions(+), 119 deletions(-) diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index 7dbf64fe2..583b381ad 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -42,7 +42,6 @@ load( "SWIFT_FEATURE_FASTBUILD", "SWIFT_FEATURE_FULL_DEBUG_INFO", "SWIFT_FEATURE_INDEX_WHILE_BUILDING", - "SWIFT_FEATURE_MINIMAL_DEPS", "SWIFT_FEATURE_MODULE_MAP_HOME_IS_CWD", "SWIFT_FEATURE_NO_EMBED_DEBUG_MODULE", "SWIFT_FEATURE_NO_GENERATED_MODULE_MAP", @@ -1298,16 +1297,26 @@ def compile( * `swiftmodule`: The `.swiftmodule` file that was produced by the compiler. """ - generated_module_deps = ( - deps + swift_toolchain.generated_header_module_implicit_deps + + # Collect the `SwiftInfo` providers that represent the dependencies of the + # Objective-C generated header module -- this includes the dependencies of + # the Swift module, plus any additional dependencies that the toolchain says + # are required for all generated header modules. These are used immediately + # below to write the module map for the header's module (to provide the + # `use` declarations), and later in this function when precompiling the + # module. + generated_module_deps_swift_infos = ( + get_providers(deps, SwiftInfo) + + swift_toolchain.generated_header_module_implicit_deps_providers.swift_infos ) + compile_outputs, other_outputs = _declare_compile_outputs( + srcs = srcs, actions = actions, feature_configuration = feature_configuration, generated_header_name = generated_header_name, - generated_module_deps = generated_module_deps, + generated_module_deps_swift_infos = generated_module_deps_swift_infos, module_name = module_name, - srcs = srcs, target_name = target_name, user_compile_flags = copts, ) @@ -1328,11 +1337,9 @@ def compile( # into the action prerequisites so that configurators have easy access to # the full set of values and inputs through a single accessor. merged_providers = _merge_targets_providers( + implicit_deps_providers = swift_toolchain.implicit_deps_providers, supports_objc_interop = swift_toolchain.supports_objc_interop, - targets = deps + private_deps + get_implicit_deps( - feature_configuration = feature_configuration, - swift_toolchain = swift_toolchain, - ), + targets = deps + private_deps, ) # Flattening this `depset` is necessary because we need to extract the @@ -1427,11 +1434,7 @@ def compile( module_map_file = compile_outputs.generated_module_map_file, module_name = module_name, swift_info = create_swift_info( - swift_infos = [ - dep[SwiftInfo] - for dep in generated_module_deps - if SwiftInfo in dep - ], + swift_infos = generated_module_deps_swift_infos, ), swift_toolchain = swift_toolchain, target_name = target_name, @@ -1568,32 +1571,12 @@ def precompile_clang_module( return precompiled_module -def get_implicit_deps(feature_configuration, swift_toolchain): - """Gets the list of implicit dependencies from the toolchain. - - Args: - feature_configuration: The feature configuration, which determines - whether optional implicit dependencies are included. - swift_toolchain: The Swift toolchain. - - Returns: - A list of targets that should be treated as implicit dependencies of - the toolchain under the given feature configuration. - """ - deps = list(swift_toolchain.required_implicit_deps) - if not is_feature_enabled( - feature_configuration = feature_configuration, - feature_name = SWIFT_FEATURE_MINIMAL_DEPS, - ): - deps.extend(swift_toolchain.optional_implicit_deps) - return deps - def _declare_compile_outputs( *, actions, feature_configuration, generated_header_name, - generated_module_deps, + generated_module_deps_swift_infos, module_name, srcs, target_name, @@ -1606,8 +1589,9 @@ def _declare_compile_outputs( `swift_common.configure_features`. generated_header_name: The desired name of the generated header for this module, or `None` if no header should be generated. - generated_module_deps: Dependencies of the module for the generated - header of the target being compiled. + generated_module_deps_swift_infos: `SwiftInfo` providers from + dependencies of the module for the generated header of the target + being compiled. module_name: The name of the Swift module being compiled. srcs: The list of source files that will be compiled. target_name: The name (excluding package path) of the target being @@ -1676,11 +1660,10 @@ def _declare_compile_outputs( # Collect the names of Clang modules that the module being built # directly depends on. dependent_module_names = sets.make() - for dep in generated_module_deps: - if SwiftInfo in dep: - for module in dep[SwiftInfo].direct_modules: - if module.clang: - sets.insert(dependent_module_names, module.name) + for swift_info in generated_module_deps_swift_infos: + for module in swift_info.direct_modules: + if module.clang: + sets.insert(dependent_module_names, module.name) generated_module_map = derived_files.module_map( actions = actions, @@ -1868,7 +1851,10 @@ def _declare_validated_generated_header(actions, generated_header_name): return actions.declare_file(generated_header_name) -def _merge_targets_providers(supports_objc_interop, targets): +def _merge_targets_providers( + implicit_deps_providers, + supports_objc_interop, + targets): """Merges the compilation-related providers for the given targets. This function merges the `CcInfo`, `SwiftInfo`, and `apple_common.Objc` @@ -1878,6 +1864,8 @@ def _merge_targets_providers(supports_objc_interop, targets): their data. Args: + implicit_deps_providers: The implicit deps providers `struct` from the + Swift toolchain. supports_objc_interop: `True` if the current toolchain supports Objective-C interop and the `apple_common.Objc` providers should also be used to determine compilation flags and inputs. If `False`, @@ -1895,9 +1883,9 @@ def _merge_targets_providers(supports_objc_interop, targets): * `objc_info`: The merged `apple_common.Objc` provider of the targets. * `swift_info`: The merged `SwiftInfo` provider of the targets. """ - cc_infos = [] - objc_infos = [] - swift_infos = [] + cc_infos = list(implicit_deps_providers.cc_infos) + objc_infos = list(implicit_deps_providers.objc_infos) + swift_infos = list(implicit_deps_providers.swift_infos) # TODO(b/146575101): This is only being used to preserve the current # behavior of strict Objective-C include paths being propagated one more @@ -2018,7 +2006,8 @@ def new_objc_provider( module_map, static_archives, swiftmodules, - objc_header = None): + objc_header = None, + objc_providers = []): """Creates an `apple_common.Objc` provider for a Swift target. Args: @@ -2038,15 +2027,17 @@ def new_objc_provider( `None`, no headers will be propagated. This header is only needed for Swift code that defines classes that should be exposed to Objective-C. + objc_providers: Additional `apple_common.Objc` providers from transitive + dependencies not provided by the `deps` argument. Returns: An `apple_common.Objc` provider that should be returned by the calling rule. """ - objc_providers = get_providers(deps, apple_common.Objc) + all_objc_providers = get_providers(deps, apple_common.Objc) + objc_providers objc_provider_args = { "link_inputs": depset(direct = swiftmodules + link_inputs), - "providers": objc_providers, + "providers": all_objc_providers, "uses_swift": True, } @@ -2086,7 +2077,7 @@ def new_objc_provider( # direct deps' Objective-C module maps to dependents, because those Swift # modules still need to see them. We need to construct a new transitive objc # provider to get the correct strict propagation behavior. - transitive_objc_provider_args = {"providers": objc_providers} + transitive_objc_provider_args = {"providers": all_objc_providers} if module_map: transitive_objc_provider_args["module_map"] = depset( direct = [module_map], diff --git a/swift/internal/feature_names.bzl b/swift/internal/feature_names.bzl index 52edf0bf2..f17930229 100644 --- a/swift/internal/feature_names.bzl +++ b/swift/internal/feature_names.bzl @@ -88,13 +88,6 @@ SWIFT_FEATURE_FULL_DEBUG_INFO = "swift.full_debug_info" # If enabled, the compilation action for a target will produce an index store. SWIFT_FEATURE_INDEX_WHILE_BUILDING = "swift.index_while_building" -# If enabled, Swift libraries, binaries, and tests will only have automatic -# dependencies on the targets provided by the toolchain's -# `required_implicit_deps` field but not those in the `optional_implicit_deps` -# field. Users may still explicitly list the latter in the `deps` of their -# targets if they are needed. -SWIFT_FEATURE_MINIMAL_DEPS = "swift.minimal_deps" - # If enabled, compilation actions and module map generation will assume that the # header paths in module maps are relative to the current working directory # (i.e., the workspace root); if disabled, header paths in module maps are diff --git a/swift/internal/providers.bzl b/swift/internal/providers.bzl index c6561d357..c6f147515 100644 --- a/swift/internal/providers.bzl +++ b/swift/internal/providers.bzl @@ -73,12 +73,38 @@ Swift toolchain depends on. "cpu": """\ `String`. The CPU architecture that the toolchain is targeting. """, - "generated_header_module_implicit_deps": """\ -`List` of `Target`s. Targets whose `SwiftInfo` providers should be treated as -compile-time inputs to actions that precompile the explicit module for the -generated Objective-C header of a Swift module. This is used to provide modular -dependencies for the fixed inclusions (Darwin, Foundation) that are -unconditionally emitted in those files. + "generated_header_module_implicit_deps_providers": """\ +A `struct` with the following fields, which are providers from targets that +should be treated as compile-time inputs to actions that precompile the explicit +module for the generated Objective-C header of a Swift module: + +* `cc_infos`: A list of `CcInfo` providers from targets specified as the + toolchain's implicit dependencies. +* `objc_infos`: A list of `apple_common.Objc` providers from targets specified + as the toolchain's implicit dependencies. +* `swift_infos`: A list of `SwiftInfo` providers from targets specified as the + toolchain's implicit dependencies. + +This is used to provide modular dependencies for the fixed inclusions (Darwin, +Foundation) that are unconditionally emitted in those files. + +For ease of use, this field is never `None`; it will always be a valid `struct` +containing the fields described above, even if those lists are empty. +""", + "implicit_deps_providers": """\ +A `struct` with the following fields, which represent providers from targets +that should be added as implicit dependencies of any Swift compilation or +linking target (but not to precompiled explicit C/Objective-C modules): + +* `cc_infos`: A list of `CcInfo` providers from targets specified as the + toolchain's implicit dependencies. +* `objc_infos`: A list of `apple_common.Objc` providers from targets specified + as the toolchain's implicit dependencies. +* `swift_infos`: A list of `SwiftInfo` providers from targets specified as the + toolchain's implicit dependencies. + +For ease of use, this field is never `None`; it will always be a valid `struct` +containing the fields described above, even if those lists are empty. """, "linker_opts_producer": """\ Skylib `partial`. A partial function that returns the flags that should be @@ -96,11 +122,6 @@ The partial should be called with two arguments: "object_format": """\ `String`. The object file format of the platform that the toolchain is targeting. The currently supported values are `"elf"` and `"macho"`. -""", - "optional_implicit_deps": """\ -`List` of `Target`s. Library targets that should be added as implicit -dependencies of any `swift_library`, `swift_binary`, or `swift_test` target that -does not have the feature `swift.minimal_deps` applied. """, "requested_features": """\ `List` of `string`s. Features that should be implicitly enabled by default for @@ -111,11 +132,6 @@ their negation in the `features` attribute of a target/package or in the These features determine various compilation and debugging behaviors of the Swift build rules, and they are also passed to the C++ APIs used when linking (so features defined in CROSSTOOL may be used here). -""", - "required_implicit_deps": """\ -`List` of `Target`s. Library targets that should be unconditionally added as -implicit dependencies of any `swift_library`, `swift_binary`, or `swift_test` -target. """, "root_dir": """\ `String`. The workspace-relative root directory of the toolchain. @@ -283,6 +299,7 @@ def create_swift_module( def create_swift_info( *, + direct_swift_infos = [], modules = [], swift_infos = []): """Creates a new `SwiftInfo` provider with the given values. @@ -293,25 +310,38 @@ def create_swift_info( are set consistently. This function can also be used to do a simple merge of `SwiftInfo` - providers, by leaving all of the arguments except for `swift_infos` as their - empty defaults. In that case, the returned provider will not represent a - true Swift module; it is merely a "collector" for other dependencies. + providers, by leaving the `modules` argument unspecified. In that case, the + returned provider will not represent a true Swift module; it is merely a + "collector" for other dependencies. Args: + direct_swift_infos: A list of `SwiftInfo` providers from dependencies + whose direct modules should be treated as direct modules in the + resulting provider, in addition to their transitive modules being + merged. modules: A list of values (as returned by `swift_common.create_module`) that represent Clang and/or Swift module artifacts that are direct outputs of the target being built. - swift_infos: A list of `SwiftInfo` providers from dependencies, whose - transitive fields should be merged into the new one. If omitted, no - transitive data is collected. + swift_infos: A list of `SwiftInfo` providers from dependencies whose + transitive modules should be merged into the resulting provider. Returns: A new `SwiftInfo` provider with the given values. """ - transitive_modules = [x.transitive_modules for x in swift_infos] + direct_modules = modules + [ + provider.modules + for provider in direct_swift_infos + ] + transitive_modules = [ + provider.transitive_modules + for provider in direct_swift_infos + swift_infos + ] return SwiftInfo( - direct_modules = modules, - transitive_modules = depset(modules, transitive = transitive_modules), + direct_modules = direct_modules, + transitive_modules = depset( + direct_modules, + transitive = transitive_modules, + ), ) diff --git a/swift/internal/swift_binary_test.bzl b/swift/internal/swift_binary_test.bzl index d0ab1e884..598454080 100644 --- a/swift/internal/swift_binary_test.bzl +++ b/swift/internal/swift_binary_test.bzl @@ -217,13 +217,10 @@ def _swift_linking_rule_impl( ) user_link_flags.extend(toolchain_linker_flags) - # Collect the dependencies of the target being linked as well as the - # toolchain's implicit dependencies (depending on the current feature - # configuration). - deps_to_link = ctx.attr.deps + swift_common.get_implicit_deps( - feature_configuration = feature_configuration, - swift_toolchain = swift_toolchain, - ) + # Collect linking contexts from any of the toolchain's implicit + # dependencies. + for cc_info in swift_toolchain.implicit_deps_providers.cc_infos: + additional_linking_contexts.append(cc_info.linking_context) # If a custom malloc implementation has been provided, pass that to the # linker as well. @@ -244,7 +241,7 @@ def _swift_linking_rule_impl( additional_inputs = additional_inputs_to_linker, additional_linking_contexts = additional_linking_contexts, cc_feature_configuration = cc_feature_configuration, - deps = deps_to_link, + deps = ctx.attr.deps, grep_includes = ctx.file._grep_includes, name = ctx.label.name, objects = objects_to_link, diff --git a/swift/internal/swift_common.bzl b/swift/internal/swift_common.bzl index 09dffc4d5..98ebd736c 100644 --- a/swift/internal/swift_common.bzl +++ b/swift/internal/swift_common.bzl @@ -33,7 +33,6 @@ load( ":compiling.bzl", "compile", "derive_module_name", - "get_implicit_deps", "precompile_clang_module", ) load( @@ -64,7 +63,6 @@ swift_common = struct( create_swift_info = create_swift_info, create_swift_module = create_swift_module, derive_module_name = derive_module_name, - get_implicit_deps = get_implicit_deps, is_enabled = is_feature_enabled, library_rule_attrs = swift_library_rule_attrs, precompile_clang_module = precompile_clang_module, diff --git a/swift/internal/swift_library.bzl b/swift/internal/swift_library.bzl index 97408d334..acacd8183 100644 --- a/swift/internal/swift_library.bzl +++ b/swift/internal/swift_library.bzl @@ -120,10 +120,6 @@ def _swift_library_impl(ctx): unsupported_features = ctx.disabled_features, ) - implicit_deps = swift_common.get_implicit_deps( - feature_configuration = feature_configuration, - swift_toolchain = swift_toolchain, - ) if swift_common.is_enabled( feature_configuration = feature_configuration, feature_name = SWIFT_FEATURE_SUPPORTS_PRIVATE_DEPS, @@ -134,8 +130,8 @@ def _swift_library_impl(ctx): # deps in "deps", either, so we need to make sure not to pass them in to # `_check_deps_are_disjoint`. deps = ctx.attr.deps - private_deps = ctx.attr.private_deps + implicit_deps - _check_deps_are_disjoint(ctx.label, deps, ctx.attr.private_deps) + private_deps = ctx.attr.private_deps + _check_deps_are_disjoint(ctx.label, deps, private_deps) elif ctx.attr.private_deps: fail( ("In target '{}', 'private_deps' cannot be used because this " + @@ -217,6 +213,7 @@ def _swift_library_impl(ctx): library_to_link.pic_static_library, ]) + implicit_deps_providers = swift_toolchain.implicit_deps_providers providers = [ DefaultInfo( files = depset(direct_output_files), @@ -235,7 +232,10 @@ def _swift_library_impl(ctx): defines = ctx.attr.defines, includes = [ctx.bin_dir.path], linker_inputs = [linker_input], - private_cc_infos = get_providers(private_deps, CcInfo), + private_cc_infos = ( + get_providers(private_deps, CcInfo) + + implicit_deps_providers.cc_infos + ), ), coverage_common.instrumented_files_info( ctx, @@ -278,6 +278,7 @@ def _swift_library_impl(ctx): static_archives = compact([library_to_link.pic_static_library]), swiftmodules = [compilation_outputs.swiftmodule], objc_header = compilation_outputs.generated_header, + objc_providers = implicit_deps_providers.objc_infos, )) return providers diff --git a/swift/internal/swift_toolchain.bzl b/swift/internal/swift_toolchain.bzl index 91b4c0885..07192e0eb 100644 --- a/swift/internal/swift_toolchain.bzl +++ b/swift/internal/swift_toolchain.bzl @@ -36,7 +36,11 @@ load( load(":features.bzl", "features_for_build_modes") load(":providers.bzl", "SwiftToolchainInfo") load(":toolchain_config.bzl", "swift_toolchain_config") -load(":utils.bzl", "get_swift_executable_for_toolchain") +load( + ":utils.bzl", + "collect_implicit_deps_providers", + "get_swift_executable_for_toolchain", +) def _all_tool_configs( swift_executable, @@ -209,12 +213,15 @@ def _swift_toolchain_impl(ctx): all_files = depset(all_files), cc_toolchain_info = cc_toolchain, cpu = ctx.attr.arch, - generated_header_module_implicit_deps = [], + generated_header_module_implicit_deps_providers = ( + collect_implicit_deps_providers([]) + ), + implicit_deps_providers = ( + collect_implicit_deps_providers([]) + ), linker_opts_producer = linker_opts_producer, object_format = "elf", - optional_implicit_deps = [], requested_features = requested_features, - required_implicit_deps = [], root_dir = toolchain_root, supports_objc_interop = False, swift_worker = ctx.executable._worker, diff --git a/swift/internal/utils.bzl b/swift/internal/utils.bzl index d6d776635..9c8bb6320 100644 --- a/swift/internal/utils.bzl +++ b/swift/internal/utils.bzl @@ -14,6 +14,7 @@ """Common utility definitions used by various BUILD rules.""" +load(":providers.bzl", "SwiftInfo") load("@bazel_skylib//lib:paths.bzl", "paths") def collect_cc_libraries( @@ -58,6 +59,42 @@ def collect_cc_libraries( return libraries +def collect_implicit_deps_providers(targets): + """Returns a struct with important providers from a list of implicit deps. + + Note that the relationship between each provider in the list and the target + it originated from is no longer retained. + + Args: + targets: A list (possibly empty) of `Target`s. + + Returns: + A `struct` containing three fields: + + * `cc_infos`: The merged `CcInfo` provider from the given targets. + * `objc_infos`: The merged `apple_common.Objc` provider from the given + targets. + * `swift_infos`: The merged `SwiftInfo` provider from the given + targets. + """ + cc_infos = [] + objc_infos = [] + swift_infos = [] + + for target in targets: + if CcInfo in target: + cc_infos.append(target[CcInfo]) + if apple_common.Objc in target: + objc_infos.append(target[apple_common.Objc]) + if SwiftInfo in target: + swift_infos.append(target[SwiftInfo]) + + return struct( + cc_infos = cc_infos, + objc_infos = objc_infos, + swift_infos = swift_infos, + ) + def compact(sequence): """Returns a copy of the sequence with any `None` items removed. diff --git a/swift/internal/xcode_swift_toolchain.bzl b/swift/internal/xcode_swift_toolchain.bzl index 8b6e5686b..7614b5479 100644 --- a/swift/internal/xcode_swift_toolchain.bzl +++ b/swift/internal/xcode_swift_toolchain.bzl @@ -42,7 +42,12 @@ load( load(":features.bzl", "features_for_build_modes") load(":toolchain_config.bzl", "swift_toolchain_config") load(":providers.bzl", "SwiftInfo", "SwiftToolchainInfo") -load(":utils.bzl", "compact", "get_swift_executable_for_toolchain") +load( + ":utils.bzl", + "collect_implicit_deps_providers", + "compact", + "get_swift_executable_for_toolchain", +) def _swift_developer_lib_dir(platform_framework_dir): """Returns the directory containing extra Swift developer libraries. @@ -699,14 +704,17 @@ def _xcode_swift_toolchain_impl(ctx): all_files = depset(all_files), cc_toolchain_info = cc_toolchain, cpu = cpu, - generated_header_module_implicit_deps = ( - ctx.attr.generated_header_module_implicit_deps + generated_header_module_implicit_deps_providers = ( + collect_implicit_deps_providers( + ctx.attr.generated_header_module_implicit_deps, + ) + ), + implicit_deps_providers = collect_implicit_deps_providers( + ctx.attr.implicit_deps, ), linker_opts_producer = linker_opts_producer, object_format = "macho", - optional_implicit_deps = ctx.attr.optional_implicit_deps, requested_features = requested_features, - required_implicit_deps = ctx.attr.required_implicit_deps, supports_objc_interop = True, swift_worker = ctx.executable._worker, system_name = "darwin", @@ -733,22 +741,16 @@ of a Swift module. """, providers = [[SwiftInfo]], ), - "optional_implicit_deps": attr.label_list( - allow_files = True, - doc = """\ -A list of labels to library targets that should be added as implicit -dependencies of any Swift compilation or linking target that does not have the -`swift.minimal_deps` feature applied. -""", - providers = [[CcInfo], [SwiftInfo]], - ), - "required_implicit_deps": attr.label_list( + "implicit_deps": attr.label_list( allow_files = True, doc = """\ A list of labels to library targets that should be unconditionally added as implicit dependencies of any Swift compilation or linking target. """, - providers = [[CcInfo], [SwiftInfo]], + providers = [ + [CcInfo], + [SwiftInfo], + ], ), "_cc_toolchain": attr.label( default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"),