From f51e68960fca1e0e6d594f3d7b519917ec4f988b Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Wed, 20 Nov 2019 15:38:45 -0800 Subject: [PATCH] Refactor slightly how actions are registered in the Swift rules. Swift toolchains now have a `swift_executable` attribute that lets the configurator replace the Swift driver used by the rules with a custom executable in their workspace. This can be useful for testing the build rules alongside compiler development, and since that file is part of the workspace and a proper input of actions, it also works correctly across remote build environments. Also remove the no-longer-used `SwiftToolchainInfo.clang_executable` field, now that linking has migrated to the `cc_common.link` API. RELNOTES: None. PiperOrigin-RevId: 281620086 --- swift/internal/actions.bzl | 71 ++++++++++++++++------ swift/internal/api.bzl | 13 +--- swift/internal/compiling.bzl | 7 +-- swift/internal/debugging.bzl | 4 +- swift/internal/providers.bzl | 11 +++- swift/internal/swift_autoconfiguration.bzl | 3 - swift/internal/swift_toolchain.bzl | 28 ++++++--- swift/internal/xcode_swift_toolchain.bzl | 37 +++++++++-- 8 files changed, 119 insertions(+), 55 deletions(-) diff --git a/swift/internal/actions.bzl b/swift/internal/actions.bzl index 5f051f884..a635b3077 100644 --- a/swift/internal/actions.bzl +++ b/swift/internal/actions.bzl @@ -18,27 +18,49 @@ load("@bazel_skylib//lib:dicts.bzl", "dicts") load("@bazel_skylib//lib:paths.bzl", "paths") load("@bazel_skylib//lib:types.bzl", "types") -def get_swift_tool(swift_toolchain, tool): - """Gets the path to the given Swift toolchain tool. +def _get_swift_driver_mode_args(driver_mode, swift_toolchain): + """Gets the arguments to pass to the worker to invoke the Swift driver. Args: + driver_mode: The mode in which to invoke the Swift driver. In other + words, this is the name of the executable of symlink that you want + to execute (e.g., `swift`, `swiftc`, `swift-autolink-extract`). swift_toolchain: The Swift toolchain being used to register actions. - tool: The name of a tool in the Swift toolchain (i.e., in the `bin` - directory). Returns: - The path to the tool. If the toolchain provides a root directory, then - the path will include the `bin` directory of that toolchain. If the - toolchain does not provide a root, then it is assumed that the tool will - be available by invoking just its name (e.g., found on the system path - or by another delegating tool like `xcrun` from Xcode). + A list of values that can be added to an `Args` object and passed to the + worker to invoke the command. + + This method implements three kinds of "dispatch": + + 1. If the toolchain provides a custom driver executable, it is invoked + with the requested mode passed via the `--driver_mode` argument. + 2. If the toolchain provides a root directory, then the returned list + will be the path to the executable with the same name as the driver + mode in the `bin` directory of that toolchain. + 3. If the toolchain does not provide a root, then it is assumed that + the tool will be available by invoking just the driver mode by name + (e.g., found on the system path or by another delegating tool like + `xcrun` from Xcode). """ - if ("/" not in tool and swift_toolchain.root_dir): - return paths.join(swift_toolchain.root_dir, "bin", tool) - return tool + if swift_toolchain.swift_executable: + return [ + swift_toolchain.swift_executable, + "--driver-mode={}".format(driver_mode), + ] + + if swift_toolchain.root_dir: + return [paths.join(swift_toolchain.root_dir, "bin", driver_mode)] -def run_swift_action(actions, swift_toolchain, **kwargs): - """Executes a Swift toolchain tool using the worker. + return [driver_mode] + +def run_swift_action( + actions, + arguments, + driver_mode, + swift_toolchain, + **kwargs): + """Executes the Swift driver using the worker. This function applies the toolchain's environment and execution requirements and wraps the invocation in the worker tool that handles platform-specific @@ -48,18 +70,24 @@ def run_swift_action(actions, swift_toolchain, **kwargs): Since this function uses the worker as the `executable` of the underlying action, it is an error to pass `executable` into this function. Instead, the - tool to run should be the first item in the `arguments` list (or in the - first `Args` object). This tool should be obtained using `get_swift_tool` in - order to correctly handle toolchains with explicit root directories. + `driver_mode` argument should be used to specify which Swift tool should be + invoked (`swift`, `swiftc`, `swift-autolink-extract`, etc.). This lets the + rules correctly handle the case where a custom driver executable is provided + by passing the `--driver-mode` flag that overrides its internal `argv[0]` + handling. Args: actions: The `Actions` object with which to register actions. + arguments: The arguments to pass to the invoked action. + driver_mode: The mode in which to invoke the Swift driver. In other + words, this is the name of the executable of symlink that you want + to execute (e.g., `swift`, `swiftc`, `swift-autolink-extract`). swift_toolchain: The Swift toolchain being used to register actions. **kwargs: Additional arguments to `actions.run`. """ if "executable" in kwargs: fail("run_swift_action does not support 'executable'. " + - "The tool to run should be the first item in 'arguments'.") + "Use 'driver_mode' instead.") remaining_args = dict(kwargs) @@ -86,7 +114,14 @@ def run_swift_action(actions, swift_toolchain, **kwargs): else: tools = toolchain_files + driver_mode_args = actions.args() + driver_mode_args.add_all(_get_swift_driver_mode_args( + driver_mode = driver_mode, + swift_toolchain = swift_toolchain, + )) + actions.run( + arguments = [driver_mode_args] + arguments, env = env, executable = swift_toolchain.swift_worker, execution_requirements = execution_requirements, diff --git a/swift/internal/api.bzl b/swift/internal/api.bzl index 23eacf30b..d245ffb5f 100644 --- a/swift/internal/api.bzl +++ b/swift/internal/api.bzl @@ -29,7 +29,7 @@ load("@bazel_skylib//lib:new_sets.bzl", "sets") load("@bazel_skylib//lib:partial.bzl", "partial") load("@bazel_skylib//lib:paths.bzl", "paths") load("@bazel_skylib//lib:types.bzl", "types") -load(":actions.bzl", "get_swift_tool", "run_swift_action") +load(":actions.bzl", "run_swift_action") load(":attrs.bzl", "swift_common_rule_attrs") load( ":compiling.bzl", @@ -600,14 +600,6 @@ def _compile( swiftdoc = derived_files.swiftdoc(actions, module_name = module_name) additional_outputs = [] - # Since all actions go through the worker (whether in persistent mode or - # not), the actual tool we want to run (swiftc) should be the first - # "argument". - tool_args = actions.args() - tool_args.add( - get_swift_tool(swift_toolchain = swift_toolchain, tool = "swiftc"), - ) - args = actions.args() if _is_enabled( feature_configuration = feature_configuration, @@ -758,7 +750,8 @@ def _compile( run_swift_action( actions = actions, - arguments = [tool_args, args], + arguments = [args], + driver_mode = "swiftc", execution_requirements = execution_requirements, inputs = all_inputs, mnemonic = "SwiftCompile", diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index 39be73985..f5cfa7e40 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -16,7 +16,7 @@ load("@bazel_skylib//lib:collections.bzl", "collections") load("@bazel_skylib//lib:paths.bzl", "paths") -load(":actions.bzl", "get_swift_tool", "run_swift_action") +load(":actions.bzl", "run_swift_action") load(":derived_files.bzl", "derived_files") load(":providers.bzl", "SwiftInfo") load(":utils.bzl", "collect_cc_libraries", "get_providers") @@ -525,16 +525,13 @@ def register_autolink_extract_action( toolchain: The `SwiftToolchainInfo` provider of the toolchain. """ args = actions.args() - args.add(get_swift_tool( - swift_toolchain = toolchain, - tool = "swift-autolink-extract", - )) args.add_all(objects) args.add("-o", output) run_swift_action( actions = actions, arguments = [args], + driver_mode = "swift-autolink-extract", inputs = objects, mnemonic = "SwiftAutolinkExtract", outputs = [output], diff --git a/swift/internal/debugging.bzl b/swift/internal/debugging.bzl index 525dd93b3..e34378498 100644 --- a/swift/internal/debugging.bzl +++ b/swift/internal/debugging.bzl @@ -14,7 +14,7 @@ """Functions relating to debugging support during compilation and linking.""" -load(":actions.bzl", "get_swift_tool", "run_swift_action") +load(":actions.bzl", "run_swift_action") load(":derived_files.bzl", "derived_files") def ensure_swiftmodule_is_embedded( @@ -97,7 +97,6 @@ def _register_modulewrap_action( toolchain: The `SwiftToolchainInfo` provider of the toolchain. """ args = actions.args() - args.add(get_swift_tool(swift_toolchain = toolchain, tool = "swift")) args.add("-modulewrap") args.add(swiftmodule) @@ -110,6 +109,7 @@ def _register_modulewrap_action( run_swift_action( actions = actions, arguments = [args], + driver_mode = "swift", inputs = [swiftmodule], mnemonic = "SwiftModuleWrap", outputs = [object], diff --git a/swift/internal/providers.bzl b/swift/internal/providers.bzl index 522681791..529b7fef9 100644 --- a/swift/internal/providers.bzl +++ b/swift/internal/providers.bzl @@ -111,9 +111,6 @@ using this toolchain. "cc_toolchain_info": """\ The `cc_common.CcToolchainInfo` provider from the Bazel C++ toolchain that this Swift toolchain 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` @@ -204,6 +201,14 @@ linked into the binary. compiling libraries or binaries with this toolchain. These flags will come first in compilation command lines, allowing them to be overridden by `copts` attributes and `--swiftcopt` flags. +""", + "swift_executable": """\ +A replacement Swift driver executable. + +If this is `None`, the default Swift driver in the toolchain will be used. +Otherwise, this binary will be used and `--driver-mode` will be passed to ensure +that it is invoked in the correct mode (i.e., `swift`, `swiftc`, +`swift-autolink-extract`, etc.). """, "swift_worker": """\ `File`. The executable representing the worker executable used to invoke the diff --git a/swift/internal/swift_autoconfiguration.bzl b/swift/internal/swift_autoconfiguration.bzl index eae71af2a..ec5e66ae0 100644 --- a/swift/internal/swift_autoconfiguration.bzl +++ b/swift/internal/swift_autoconfiguration.bzl @@ -180,7 +180,6 @@ def _create_linux_toolchain(repository_ctx): "in your environment before invoking Bazel.") path_to_swiftc = repository_ctx.which("swiftc") - path_to_clang = repository_ctx.which("clang") root = path_to_swiftc.dirname.dirname feature_values = _compute_feature_values(repository_ctx, path_to_swiftc) @@ -197,7 +196,6 @@ package(default_visibility = ["//visibility:public"]) swift_toolchain( name = "toolchain", arch = "x86_64", - clang_executable = "{path_to_clang}", features = [{feature_list}], os = "linux", root = "{root}", @@ -207,7 +205,6 @@ swift_toolchain( '"{}"'.format(feature) for feature in feature_values ]), - path_to_clang = path_to_clang, root = root, ), ) diff --git a/swift/internal/swift_toolchain.bzl b/swift/internal/swift_toolchain.bzl index 25dc1d3bd..31ad49399 100644 --- a/swift/internal/swift_toolchain.bzl +++ b/swift/internal/swift_toolchain.bzl @@ -106,6 +106,11 @@ def _swift_toolchain_impl(ctx): requested_features.extend(ctx.features) requested_features.append(SWIFT_FEATURE_AUTOLINK_EXTRACT) + all_files = [] + swift_executable = ctx.file.swift_executable + if swift_executable: + all_files.append(swift_executable) + # TODO(allevato): Move some of the remaining hardcoded values, like object # format and Obj-C interop support, to attributes so that we can remove the # assumptions that are only valid on Linux. @@ -114,9 +119,8 @@ def _swift_toolchain_impl(ctx): action_environment = {}, # Swift.org toolchains assume everything is just available on the # PATH and we don't try to pass the toolchain contents here. - all_files = depset(), + all_files = depset(all_files), cc_toolchain_info = cc_toolchain, - clang_executable = ctx.attr.clang_executable, command_line_copts = ctx.fragments.swift.copts(), cpu = ctx.attr.arch, execution_requirements = {}, @@ -129,6 +133,7 @@ def _swift_toolchain_impl(ctx): stamp_producer = None, supports_objc_interop = False, swiftc_copts = [], + swift_executable = swift_executable, swift_worker = ctx.executable._worker, system_name = ctx.attr.os, unsupported_features = ctx.disabled_features + [ @@ -145,12 +150,6 @@ The name of the architecture that this toolchain targets. This name should match the name used in the toolchain's directory layout for architecture-specific content, such as "x86_64" in "lib/swift/linux/x86_64". -""", - mandatory = True, - ), - "clang_executable": attr.string( - doc = """\ -The path to the `clang` executable, which is used for linking. """, mandatory = True, ), @@ -166,6 +165,19 @@ platform-specific content, such as "linux" in "lib/swift/linux". "root": attr.string( mandatory = True, ), + "swift_executable": attr.label( + # TODO(allevato): Use a label-typed build setting to allow this to + # have a default that is overridden from the command line. + allow_single_file = True, + doc = """\ +A replacement Swift driver executable. + +If this is empty, the default Swift driver in the toolchain will be used. +Otherwise, this binary will be used and `--driver-mode` will be passed to ensure +that it is invoked in the correct mode (i.e., `swift`, `swiftc`, +`swift-autolink-extract`, etc.). +""", + ), "_cc_toolchain": attr.label( default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"), doc = """\ diff --git a/swift/internal/xcode_swift_toolchain.bzl b/swift/internal/xcode_swift_toolchain.bzl index 8791a57fe..fe8f10da6 100644 --- a/swift/internal/xcode_swift_toolchain.bzl +++ b/swift/internal/xcode_swift_toolchain.bzl @@ -182,6 +182,7 @@ def _default_linker_opts( def _default_swiftc_copts( apple_fragment, apple_toolchain, + swift_executable, target, toolchain_root): """Returns options that should be passed by default to `swiftc`. @@ -189,6 +190,8 @@ def _default_swiftc_copts( Args: apple_fragment: The `apple` configuration fragment. apple_toolchain: The `apple_common.apple_toolchain()` object. + swift_executable: A `File` representing a custom Swift driver to be used + by the toolchain, or `None` to use the default driver. target: The target triple. toolchain_root: The optional toolchain root, if specified by the `--define=SWIFT_USE_TOOLCHAIN_ROOT=` flag. @@ -213,10 +216,10 @@ def _default_swiftc_copts( # If we have a custom "toolchain root" (meaning a bin/ dir with a custom # compiler that we want to use in place of the original, but not a *full* - # toolchain, make sure we use the resource dir of the *original* toolchain - # so that libraries are still found (otherwise, by default, the compiler - # will look in its parent directory for them). - if toolchain_root: + # toolchain) or a custom Swift driver, make sure we use the resource dir of + # the *original* toolchain so that libraries are still found (otherwise, by + # default, the driver will look in its parent directory for them). + if swift_executable or toolchain_root: copts.extend([ "-resource-dir", ("{developer_dir}/Toolchains/{toolchain}.xctoolchain/" + @@ -343,8 +346,12 @@ def _xcode_swift_toolchain_impl(ctx): # structure, meaning that the path passed here must contain a `bin` # directory and that directory contains the `swift` and `swiftc` files. # + # TODO(allevato): Retire this feature in favor of the `swift_executable` + # attribute, which supports remote builds. + # # To use a "standard" custom toolchain built using the full Swift build # script, use `--define=SWIFT_CUSTOM_TOOLCHAIN=` as shown below. + swift_executable = ctx.file.swift_executable toolchain_root = ctx.var.get("SWIFT_USE_TOOLCHAIN_ROOT") custom_toolchain = ctx.var.get("SWIFT_CUSTOM_TOOLCHAIN") @@ -355,6 +362,7 @@ def _xcode_swift_toolchain_impl(ctx): swiftc_copts = _default_swiftc_copts( apple_fragment, apple_toolchain, + swift_executable, target, toolchain_root, ) @@ -398,14 +406,17 @@ def _xcode_swift_toolchain_impl(ctx): ctx.fragments.swift.copts() ) + all_files = [] + if swift_executable: + all_files.append(swift_executable) + return [ SwiftToolchainInfo( action_environment = env, # Xcode toolchains don't pass any files explicitly here because # they're just available as part of the Xcode bundle. - all_files = depset(), + all_files = depset(all_files), cc_toolchain_info = cc_toolchain, - clang_executable = None, command_line_copts = command_line_copts, cpu = cpu, execution_requirements = execution_requirements, @@ -418,6 +429,7 @@ def _xcode_swift_toolchain_impl(ctx): stamp_producer = None, supports_objc_interop = True, swiftc_copts = swiftc_copts, + swift_executable = swift_executable, swift_worker = ctx.executable._worker, system_name = "darwin", unsupported_features = ctx.disabled_features + [ @@ -429,6 +441,19 @@ def _xcode_swift_toolchain_impl(ctx): xcode_swift_toolchain = rule( attrs = dicts.add({ + "swift_executable": attr.label( + # TODO(allevato): Use a label-typed build setting to allow this to + # have a default that is overridden from the command line. + allow_single_file = True, + doc = """\ +A replacement Swift driver executable. + +If this is empty, the default Swift driver in the toolchain will be used. +Otherwise, this binary will be used and `--driver-mode` will be passed to ensure +that it is invoked in the correct mode (i.e., `swift`, `swiftc`, +`swift-autolink-extract`, etc.). +""", + ), "_cc_toolchain": attr.label( default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"), doc = """\