From 3fdef3afeb5beef0af3aa9d91d7e923b02bd839f Mon Sep 17 00:00:00 2001 From: allevato Date: Mon, 1 Apr 2019 13:00:57 -0400 Subject: [PATCH] Remove the need to pass the objc/swift fragments as swift_common API inputs. This information is now propagated via the toolchain. RELNOTES: The `objc_fragment` and `swift_fragment` arguments to `swift_common` compilation APIs are now deprecated. PiperOrigin-RevId: 241345122 --- swift/internal/api.bzl | 71 ++++++++++------------ swift/internal/compiling.bzl | 22 +------ swift/internal/providers.bzl | 9 ++- swift/internal/swift_binary_test.bzl | 18 ------ swift/internal/swift_import.bzl | 1 - swift/internal/swift_library.bzl | 10 --- swift/internal/swift_module_alias.bzl | 5 -- swift/internal/swift_protoc_gen_aspect.bzl | 10 --- swift/internal/swift_toolchain.bzl | 2 + swift/internal/xcode_swift_toolchain.bzl | 24 +++++++- 10 files changed, 66 insertions(+), 106 deletions(-) diff --git a/swift/internal/api.bzl b/swift/internal/api.bzl index 2bd73f7f8..a15ead5f4 100644 --- a/swift/internal/api.bzl +++ b/swift/internal/api.bzl @@ -398,14 +398,13 @@ def _global_module_cache_path(genfiles_dir): # java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java. return paths.join(genfiles_dir.path, "_objc_module_cache") -def _is_wmo(copts, feature_configuration, swift_fragment): +def _is_wmo(copts, feature_configuration): """Returns a value indicating whether a compilation will use whole module optimization. Args: copts: A list of compiler flags to scan for WMO usage. feature_configuration: The Swift feature configuration, as returned from `swift_common.configure_features`. - swift_fragment: The `swift` configuration fragment from Bazel. Returns: True if WMO is enabled in the given list of flags. @@ -425,10 +424,9 @@ def _is_wmo(copts, feature_configuration, swift_fragment): return True # Otherwise, check any explicit copts the user may have set on the target or command line. - explicit_copts = copts + swift_fragment.copts() - return ("-wmo" in explicit_copts or - "-whole-module-optimization" in explicit_copts or - "-force-single-frontend-invocation" in explicit_copts) + return ("-wmo" in copts or + "-whole-module-optimization" in copts or + "-force-single-frontend-invocation" in copts) def _cc_feature_configuration(feature_configuration): """Returns the C++ feature configuration nested inside the given Swift feature configuration. @@ -447,7 +445,6 @@ def _compile_as_objects( arguments, module_name, srcs, - swift_fragment, target_name, toolchain, additional_input_depsets = [], @@ -460,7 +457,8 @@ def _compile_as_objects( deps = [], feature_configuration = None, genfiles_dir = None, - objc_fragment = None): + objc_fragment = None, + swift_fragment = None): """Compiles Swift source files into object files (and optionally a module). Args: @@ -471,7 +469,6 @@ def _compile_as_objects( present and valid; use `swift_common.derive_module_name` to generate a default from the target's label if needed. srcs: The Swift source files to compile. - swift_fragment: The `swift` configuration fragment from Bazel. target_name: The name of the target for which the code is being compiled, which is used to determine unique file paths for the outputs. toolchain: The `SwiftToolchainInfo` provider of the toolchain. @@ -503,9 +500,10 @@ def _compile_as_objects( is added to ClangImporter's header search paths for compatibility with Bazel's C++ and Objective-C rules which support inclusions of generated headers from that location. - objc_fragment: The `objc` configuration fragment from Bazel. This must be - provided if the toolchain supports Objective-C interop; if it does not, - then this argument may be omitted. + objc_fragment: This argument is no longer used and will be removed in a + future version. + swift_fragment: This argument is no longer used and will be removed in a + future version. Returns: A `struct` containing the following fields: @@ -529,7 +527,7 @@ def _compile_as_objects( * `output_objects`: The object (`.o`) files that were produced by the compiler. """ - _ignore = [allow_testing, compilation_mode, configuration] + _ignore = [allow_testing, compilation_mode, configuration, objc_fragment, swift_fragment] # TODO(b/112900284): Make this a required argument. if not feature_configuration: @@ -539,14 +537,14 @@ def _compile_as_objects( # on a Mac Pro for historical reasons. # TODO(b/32571265): Generalize this based on platform and core count when an # API to obtain this is available. - if _is_wmo(copts, feature_configuration, swift_fragment): + if _is_wmo(copts + toolchain.command_line_copts, feature_configuration): # We intentionally don't use `+=` or `extend` here to ensure that a # copy is made instead of extending the original. copts = copts + ["-num-threads", "12"] compile_reqs = declare_compile_outputs( actions = actions, - copts = copts + swift_fragment.copts(), + copts = copts + toolchain.command_line_copts, srcs = srcs, target_name = target_name, index_while_building = swift_common.is_enabled( @@ -588,7 +586,6 @@ def _compile_as_objects( args = compile_args, module_name = module_name, srcs = srcs, - swift_fragment = swift_fragment, toolchain = toolchain, additional_input_depsets = additional_input_depsets, copts = copts, @@ -596,7 +593,6 @@ def _compile_as_objects( deps = deps, feature_configuration = feature_configuration, genfiles_dir = genfiles_dir, - objc_fragment = objc_fragment, ) all_inputs = depset( @@ -668,7 +664,6 @@ def _compile_as_library( label, module_name, srcs, - swift_fragment, toolchain, additional_inputs = [], allow_testing = True, @@ -682,7 +677,8 @@ def _compile_as_library( genfiles_dir = None, library_name = None, linkopts = [], - objc_fragment = None): + objc_fragment = None, + swift_fragment = None): """Compiles Swift source files into static and/or shared libraries. This is a high-level API that wraps the compilation and library creation steps @@ -702,7 +698,6 @@ def _compile_as_library( present and valid; use `swift_common.derive_module_name` to generate a default from the target's label if needed. srcs: The Swift source files to compile. - swift_fragment: The `swift` configuration fragment from Bazel. toolchain: The `SwiftToolchainInfo` provider of the toolchain. additional_inputs: A list of `File`s representing additional inputs that need to be passed to the Swift compile action because they are @@ -738,9 +733,10 @@ def _compile_as_library( used directly by any action registered by this function, but they are added to the `SwiftInfo` provider that it returns so that the linker flags can be propagated to dependent targets. - objc_fragment: The `objc` configuration fragment from Bazel. This must be - provided if the toolchain supports Objective-C interop; if it does not, - then this argument may be omitted. + objc_fragment: This argument is no longer used and will be removed in a + future version. + swift_fragment: This argument is no longer used and will be removed in a + future version. Returns: A `struct` containing the following fields: @@ -763,7 +759,7 @@ def _compile_as_library( rule. This includes the `SwiftInfo` provider, and if Objective-C interop is enabled on the toolchain, an `apple_common.Objc` provider as well. """ - _ignore = [allow_testing, compilation_mode, configuration] + _ignore = [allow_testing, compilation_mode, configuration, objc_fragment, swift_fragment] # TODO(b/112900284): Make this a required argument. if not feature_configuration: @@ -816,7 +812,7 @@ def _compile_as_library( feature_configuration = feature_configuration, feature_name = SWIFT_FEATURE_NO_GENERATED_HEADER, ) - if generates_header and toolchain.supports_objc_interop and objc_fragment: + if generates_header and toolchain.supports_objc_interop: # Generate a Swift bridging header for this library so that it can be # included by Objective-C code that may depend on it. objc_header = derived_files.objc_header(actions, target_name = label.name) @@ -855,14 +851,12 @@ def _compile_as_library( feature_configuration = feature_configuration, module_name = module_name, srcs = srcs, - swift_fragment = swift_fragment, target_name = label.name, toolchain = toolchain, additional_input_depsets = compile_input_depsets, additional_outputs = additional_outputs, deps = deps, genfiles_dir = genfiles_dir, - objc_fragment = objc_fragment, ) # Create an archive that contains the compiled .o files. @@ -900,7 +894,7 @@ def _compile_as_library( # Propagate an `objc` provider if the toolchain supports Objective-C interop, # which allows `objc_library` targets to import `swift_library` targets. - if toolchain.supports_objc_interop and objc_fragment: + if toolchain.supports_objc_interop: providers.append(new_objc_provider( defines = defines, deps = all_deps, @@ -1186,7 +1180,6 @@ def _swiftc_command_line_and_inputs( args, module_name, srcs, - swift_fragment, toolchain, additional_input_depsets = [], allow_testing = True, @@ -1197,7 +1190,8 @@ def _swiftc_command_line_and_inputs( deps = [], feature_configuration = None, genfiles_dir = None, - objc_fragment = None): + objc_fragment = None, + swift_fragment = None): """Computes command line arguments and inputs needed to invoke `swiftc`. The command line arguments computed by this function are any that do *not* @@ -1215,7 +1209,6 @@ def _swiftc_command_line_and_inputs( present and valid; use `swift_common.derive_module_name` to generate a default from the target's label if needed. srcs: The Swift source files to compile. - swift_fragment: The `swift` configuration fragment from Bazel. toolchain: The `SwiftToolchainInfo` provider of the toolchain. additional_input_depsets: A list of `depset`s of `File`s representing additional input files that need to be passed to the Swift compile @@ -1243,16 +1236,17 @@ def _swiftc_command_line_and_inputs( is added to ClangImporter's header search paths for compatibility with Bazel's C++ and Objective-C rules which support inclusions of generated headers from that location. - objc_fragment: The `objc` configuration fragment from Bazel. This must be - provided if the toolchain supports Objective-C interop; if it does not, - then this argument may be omitted. + objc_fragment: This argument is no longer used and will be removed in a + future version. + swift_fragment: This argument is no longer used and will be removed in a + future version. Returns: A `depset` containing the full set of files that need to be passed as inputs of the Bazel action that spawns a tool with the computed command line (i.e., any source files, referenced module maps and headers, and so forth.) """ - _ignore = [allow_testing, compilation_mode, configuration] + _ignore = [allow_testing, compilation_mode, configuration, objc_fragment, swift_fragment] # TODO(b/112900284): Make this a required argument. if not feature_configuration: @@ -1283,7 +1277,7 @@ def _swiftc_command_line_and_inputs( # Do not enable batch mode if the user requested WMO; this silences an "ignoring # '-enable-batch-mode' because '-whole-module-optimization' was also specified" warning. if ( - not _is_wmo(copts, feature_configuration, swift_fragment) and + not _is_wmo(copts + toolchain.command_line_copts, feature_configuration) and swift_common.is_enabled( feature_configuration = feature_configuration, feature_name = SWIFT_FEATURE_ENABLE_BATCH_MODE, @@ -1305,13 +1299,12 @@ def _swiftc_command_line_and_inputs( input_depsets.extend(transitive_inputs) input_depsets.append(depset(direct = srcs)) - if toolchain.supports_objc_interop and objc_fragment: + if toolchain.supports_objc_interop: # Collect any additional inputs and flags needed to pull in Objective-C # dependencies. input_depsets.append(objc_compile_requirements( args = args, deps = all_deps, - objc_fragment = objc_fragment, )) # Add toolchain copts, target copts, and command-line `--swiftcopt` flags, @@ -1319,7 +1312,7 @@ def _swiftc_command_line_and_inputs( # uses if needed. args.add_all(toolchain.swiftc_copts) args.add_all(copts) - args.add_all(swift_fragment.copts()) + args.add_all(toolchain.command_line_copts) args.add_all(srcs) diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index 753a68e4b..ccea1d816 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -314,13 +314,12 @@ def new_objc_provider( return apple_common.new_objc_provider(**objc_provider_args) -def objc_compile_requirements(args, deps, objc_fragment): +def objc_compile_requirements(args, deps): """Collects compilation requirements for Objective-C dependencies. Args: args: An `Args` object to which compile options will be added. deps: The `deps` of the target being built. - objc_fragment: The `objc` configuration fragment. Returns: A `depset` of files that should be included among the inputs of the compile action. @@ -402,9 +401,6 @@ def objc_compile_requirements(args, deps, objc_fragment): # args.add_all(module_maps, before_each = "-Xcc", format_each = "-fmodule-map-file=%s") - # Add any copts required by the `objc` configuration fragment. - args.add_all(_clang_copts(objc_fragment), before_each = "-Xcc") - return depset(transitive = inputs) def register_autolink_extract_action( @@ -486,22 +482,6 @@ def write_objc_header_module_map( output = output, ) -def _clang_copts(objc_fragment): - """Returns copts that should be passed to `clang` from the `objc` fragment. - - Args: - objc_fragment: The `objc` configuration fragment. - - Returns: - A list of `clang` copts. - """ - - # In general, every compilation mode flag from native `objc_*` rules should be passed, but `-g` - # seems to break Clang module compilation. Since this flag does not make much sense for module - # compilation and only touches headers, it's ok to omit. - clang_copts = objc_fragment.copts + objc_fragment.copts_for_current_compilation_mode - return [copt for copt in clang_copts if copt != "-g"] - def _dirname_map_fn(f): """Returns the dir name of a file. diff --git a/swift/internal/providers.bzl b/swift/internal/providers.bzl index 44a0bddea..6d1a93d57 100644 --- a/swift/internal/providers.bzl +++ b/swift/internal/providers.bzl @@ -179,6 +179,12 @@ depends on. """, "clang_executable": """ `String`. The path to the `clang` executable, which is used to link binaries. +""", + "command_line_copts": """ +`List` of `strings`. Flags that were passed to Bazel using the `--swiftcopt` command line flag. +These flags have the highest precedence; they are added to compilation command lines after the +toolchain default flags (`SwiftToolchainInfo.swiftc_copts`) and after flags specified in the +`copts` attributes of Swift targets. """, "cpu": "`String`. The CPU architecture that the toolchain is targeting.", "execution_requirements": """ @@ -229,7 +235,8 @@ stamping enabled. """, "swiftc_copts": """ `List` of `strings`. Additional flags that should be passed to `swiftc` when compiling libraries or -binaries with this toolchain. +binaries with this toolchain. These flags will come first in compilation command lines, allowing +them to be overridden by `copts` attributes and `--swiftcopt` flags. """, "system_name": """ `String`. The name of the operating system that the toolchain is targeting. diff --git a/swift/internal/swift_binary_test.bzl b/swift/internal/swift_binary_test.bzl index d3ed30303..e37456292 100644 --- a/swift/internal/swift_binary_test.bzl +++ b/swift/internal/swift_binary_test.bzl @@ -79,12 +79,6 @@ def _swift_linking_rule_impl( A tuple with two values: the `File` representing the binary that was linked, and a list of providers to be propagated by the target being built. """ - - # Bazel fails the build if you try to query a fragment that hasn't been declared, even - # dynamically with `hasattr`/`getattr`. Thus, we have to use other information to determine - # whether we can access the `objc` configuration. - objc_fragment = (ctx.fragments.objc if toolchain.supports_objc_interop else None) - copts = expand_locations(ctx, ctx.attr.copts, ctx.attr.swiftc_inputs) linkopts = list(linkopts) + expand_locations(ctx, ctx.attr.linkopts, ctx.attr.swiftc_inputs) @@ -114,13 +108,11 @@ def _swift_linking_rule_impl( feature_configuration = feature_configuration, module_name = module_name, srcs = srcs, - swift_fragment = ctx.fragments.swift, target_name = ctx.label.name, toolchain = toolchain, additional_input_depsets = [depset(direct = additional_inputs)], deps = ctx.attr.deps, genfiles_dir = ctx.genfiles_dir, - objc_fragment = objc_fragment, ) link_args.add_all(compile_results.linker_flags) objects_to_link.extend(compile_results.output_objects) @@ -340,11 +332,6 @@ platform-specific application rules in [rules_apple](https://github.com/bazelbui instead of `swift_binary`. """, executable = True, - fragments = [ - "cpp", - "objc", - "swift", - ], implementation = _swift_binary_impl, ) @@ -404,11 +391,6 @@ You can also disable this feature for all the tests in a package by applying it `package()` declaration instead of the individual targets. """, executable = True, - fragments = [ - "cpp", - "objc", - "swift", - ], test = True, implementation = _swift_test_impl, ) diff --git a/swift/internal/swift_import.bzl b/swift/internal/swift_import.bzl index 36b716b33..8d5fae967 100644 --- a/swift/internal/swift_import.bzl +++ b/swift/internal/swift_import.bzl @@ -100,6 +100,5 @@ The list of `.swiftmodule` files provided to Swift targets that depend on this t Allows for the use of precompiled Swift modules as dependencies in other `swift_library` and `swift_binary` targets. """, - fragments = ["objc"], implementation = _swift_import_impl, ) diff --git a/swift/internal/swift_library.bzl b/swift/internal/swift_library.bzl index 72041b4b0..8a3b252b9 100644 --- a/swift/internal/swift_library.bzl +++ b/swift/internal/swift_library.bzl @@ -32,11 +32,7 @@ def _swift_library_impl(ctx): if not module_name: module_name = swift_common.derive_module_name(ctx.label) - # Bazel fails the build if you try to query a fragment that hasn't been declared, even - # dynamically with `hasattr`/`getattr`. Thus, we have to use other information to determine - # whether we can access the `objc` configuration. toolchain = ctx.attr._toolchain[SwiftToolchainInfo] - objc_fragment = (ctx.fragments.objc if toolchain.supports_objc_interop else None) feature_configuration = swift_common.configure_features( requested_features = ctx.features, @@ -50,7 +46,6 @@ def _swift_library_impl(ctx): label = ctx.label, module_name = module_name, srcs = ctx.files.srcs, - swift_fragment = ctx.fragments.swift, toolchain = toolchain, additional_inputs = ctx.files.swiftc_inputs, alwayslink = ctx.attr.alwayslink, @@ -60,7 +55,6 @@ def _swift_library_impl(ctx): feature_configuration = feature_configuration, genfiles_dir = ctx.genfiles_dir, linkopts = linkopts, - objc_fragment = objc_fragment, ) direct_output_files = [ @@ -97,10 +91,6 @@ swift_library = rule( doc = """ Compiles and links Swift code into a static library and Swift module. """, - fragments = [ - "objc", - "swift", - ], outputs = swift_library_output_map, implementation = _swift_library_impl, ) diff --git a/swift/internal/swift_module_alias.bzl b/swift/internal/swift_module_alias.bzl index 30d83e69e..61f490941 100644 --- a/swift/internal/swift_module_alias.bzl +++ b/swift/internal/swift_module_alias.bzl @@ -60,20 +60,16 @@ following dependencies instead:\n\n""".format( unsupported_features = ctx.disabled_features, ) - objc_fragment = (ctx.fragments.objc if toolchain.supports_objc_interop else None) - compile_results = swift_common.compile_as_library( actions = ctx.actions, bin_dir = ctx.bin_dir, label = ctx.label, module_name = module_name, srcs = [reexport_src], - swift_fragment = ctx.fragments.swift, toolchain = ctx.attr._toolchain[SwiftToolchainInfo], deps = ctx.attr.deps, feature_configuration = feature_configuration, genfiles_dir = ctx.genfiles_dir, - objc_fragment = objc_fragment, ) return compile_results.providers + [ @@ -133,6 +129,5 @@ to create "umbrella modules". > `deps` in the new module. You depend on undocumented features at your own > risk, as they may change in a future version of Swift. """, - fragments = ["swift", "objc"], implementation = _swift_module_alias_impl, ) diff --git a/swift/internal/swift_protoc_gen_aspect.bzl b/swift/internal/swift_protoc_gen_aspect.bzl index 8e3143b63..f9af99f81 100644 --- a/swift/internal/swift_protoc_gen_aspect.bzl +++ b/swift/internal/swift_protoc_gen_aspect.bzl @@ -283,7 +283,6 @@ def _swift_protoc_gen_aspect_impl(target, aspect_ctx): label = target.label, module_name = swift_common.derive_module_name(target.label), srcs = pbswift_files, - swift_fragment = aspect_ctx.fragments.swift, toolchain = toolchain, deps = compile_deps, feature_configuration = feature_configuration, @@ -292,11 +291,6 @@ def _swift_protoc_gen_aspect_impl(target, aspect_ctx): # use the `lib{name}.a` pattern. This will produce `lib{name}.swift.a` # instead. library_name = "{}.swift".format(target.label.name), - # The generated protos themselves are not usable in Objective-C, but we - # still need the Objective-C provider that it propagates since it - # carries the static libraries that apple_binary will want to link on - # those platforms. - objc_fragment = aspect_ctx.fragments.objc, ) providers = compile_results.providers else: @@ -364,9 +358,5 @@ provider. Most users should not need to use this aspect directly; it is an implementation detail of the `swift_proto_library` rule. """, - fragments = [ - "objc", - "swift", - ], implementation = _swift_protoc_gen_aspect_impl, ) diff --git a/swift/internal/swift_toolchain.bzl b/swift/internal/swift_toolchain.bzl index 3356068cd..726f3d520 100644 --- a/swift/internal/swift_toolchain.bzl +++ b/swift/internal/swift_toolchain.bzl @@ -207,6 +207,7 @@ def _swift_toolchain_impl(ctx): action_registrars = action_registrars, cc_toolchain_info = cc_toolchain, clang_executable = ctx.attr.clang_executable, + command_line_copts = ctx.fragments.swift.copts(), cpu = ctx.attr.arch, execution_requirements = {}, implicit_deps = [], @@ -269,5 +270,6 @@ The C++ toolchain from which other tools needed by the Swift toolchain (such as ), }), doc = "Represents a Swift compiler toolchain.", + fragments = ["swift"], implementation = _swift_toolchain_impl, ) diff --git a/swift/internal/xcode_swift_toolchain.bzl b/swift/internal/xcode_swift_toolchain.bzl index 88d53031a..19cf2ddd5 100644 --- a/swift/internal/xcode_swift_toolchain.bzl +++ b/swift/internal/xcode_swift_toolchain.bzl @@ -19,6 +19,7 @@ toolchain package. If you are looking for rules to build Swift code using this toolchain, see `swift.bzl`. """ +load("@bazel_skylib//lib:collections.bzl", "collections") load("@bazel_skylib//lib:dicts.bzl", "dicts") load("@bazel_skylib//lib:partial.bzl", "partial") load("@bazel_skylib//lib:types.bzl", "types") @@ -35,6 +36,23 @@ load( load(":providers.bzl", "SwiftToolchainInfo") load(":wrappers.bzl", "SWIFT_TOOL_WRAPPER_ATTRIBUTES") +def _command_line_objc_copts(objc_fragment): + """Returns copts that should be passed to `clang` from the `objc` fragment. + + Args: + objc_fragment: The `objc` configuration fragment. + + Returns: + A list of `clang` copts, each of which is preceded by `-Xcc` so that they can be passed + through `swiftc` to its underlying ClangImporter instance. + """ + + # In general, every compilation mode flag from native `objc_*` rules should be passed, but `-g` + # seems to break Clang module compilation. Since this flag does not make much sense for module + # compilation and only touches headers, it's ok to omit. + clang_copts = objc_fragment.copts + objc_fragment.copts_for_current_compilation_mode + return collections.before_each("-Xcc", [copt for copt in clang_copts if copt != "-g"]) + def _default_linker_opts( apple_fragment, apple_toolchain, @@ -479,12 +497,15 @@ def _xcode_swift_toolchain_impl(ctx): # TODO(#35): Add SWIFT_FEATURE_DEBUG_PREFIX_MAP based on Xcode version. + command_line_copts = _command_line_objc_copts(ctx.fragments.objc) + ctx.fragments.swift.copts() + return [ SwiftToolchainInfo( action_environment = env, action_registrars = action_registrars, cc_toolchain_info = cc_toolchain, clang_executable = None, + command_line_copts = command_line_copts, cpu = cpu, execution_requirements = execution_requirements, implicit_deps = [], @@ -536,7 +557,8 @@ The C++ toolchain from which linking flags and other tools needed by the Swift t doc = "Represents a Swift compiler toolchain provided by Xcode.", fragments = [ "apple", - "cpp", + "objc", + "swift", ], implementation = _xcode_swift_toolchain_impl, )