From afb1a68eb419da255d95504f9aef15a20b88f17c Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Tue, 23 Sep 2025 00:56:58 -0400 Subject: [PATCH 01/13] Remove support for incremental compilation mode --- doc/providers.md | 2 +- swift/providers.bzl | 3 +- swift/toolchains/swift_toolchain.bzl | 3 +- swift/toolchains/xcode_swift_toolchain.bzl | 3 +- tools/worker/compile_with_worker.cc | 51 ----- tools/worker/output_file_map.cc | 162 +------------- tools/worker/output_file_map.h | 44 +--- tools/worker/swift_runner.cc | 16 +- tools/worker/work_processor.cc | 234 +-------------------- 9 files changed, 25 insertions(+), 493 deletions(-) diff --git a/doc/providers.md b/doc/providers.md index cc8e6f712..86ae55e09 100644 --- a/doc/providers.md +++ b/doc/providers.md @@ -140,7 +140,7 @@ that use the toolchain. | package_configurations | A list of `SwiftPackageConfigurationInfo` providers that specify additional compilation configuration options that are applied to targets on a per-package basis. | | requested_features | `List` of `string`s. Features that should be implicitly enabled by default for targets built using this toolchain, unless overridden by the user by listing their negation in the `features` attribute of a target/package or in the `--features` command line flag.

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). | | root_dir | `String`. The workspace-relative root directory of the toolchain. | -| swift_worker | `File`. The executable representing the worker executable used to invoke the compiler and other Swift tools (for both incremental and non-incremental compiles). | +| swift_worker | `File`. The executable representing the worker executable used to invoke the compiler and other Swift tools. | | test_configuration | `Struct` containing the following fields:

* `binary_name`: A template string used to compute the name of the output binary for `swift_test` rules. Any occurrences of the string `"{name}"` will be substituted by the name of the target.

* `env`: A `dict` of environment variables to be set when running tests that were built with this toolchain.

* `execution_requirements`: A `dict` of execution requirements for tests that were built with this toolchain.

* `objc_test_discovery`: A Boolean value indicating whether test targets should discover tests dynamically using the Objective-C runtime.

* `test_linking_contexts`: A list of `CcLinkingContext`s that provide additional flags to use when linking test binaries.

This is used, for example, with Xcode-based toolchains to ensure that the `xctest` helper and coverage tools are found in the correct developer directory when running tests. | | tool_configs | This field is an internal implementation detail of the build rules. | | unsupported_features | `List` of `string`s. Features that should be implicitly disabled by default for targets built using this toolchain, unless overridden by the user by listing them in the `features` attribute of a target/package or in the `--features` command line flag.

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). | diff --git a/swift/providers.bzl b/swift/providers.bzl index f7ab88f9e..0d11e9a55 100644 --- a/swift/providers.bzl +++ b/swift/providers.bzl @@ -443,8 +443,7 @@ Swift build rules, and they are also passed to the C++ APIs used when linking """, "swift_worker": """\ `File`. The executable representing the worker executable used to invoke the -compiler and other Swift tools (for both incremental and non-incremental -compiles). +compiler and other Swift tools. """, "test_configuration": """\ `Struct` containing the following fields: diff --git a/swift/toolchains/swift_toolchain.bzl b/swift/toolchains/swift_toolchain.bzl index e879ce7bf..e989c9cc0 100644 --- a/swift/toolchains/swift_toolchain.bzl +++ b/swift/toolchains/swift_toolchain.bzl @@ -662,8 +662,7 @@ to the compiler for exec transition builds. allow_files = True, default = Label("//tools/worker"), doc = """\ -An executable that wraps Swift compiler invocations and also provides support -for incremental compilation using a persistent mode. +An executable that wraps Swift compiler invocations using a persistent mode. """, executable = True, ), diff --git a/swift/toolchains/xcode_swift_toolchain.bzl b/swift/toolchains/xcode_swift_toolchain.bzl index a7cdb456b..9bb9e809f 100644 --- a/swift/toolchains/xcode_swift_toolchain.bzl +++ b/swift/toolchains/xcode_swift_toolchain.bzl @@ -972,8 +972,7 @@ to the compiler for exec transition builds. allow_files = True, default = Label("//tools/worker:worker_wrapper"), doc = """\ -An executable that wraps Swift compiler invocations and also provides support -for incremental compilation using a persistent mode. +An executable that wraps Swift compiler invocations using a persistent mode. """, executable = True, ), diff --git a/tools/worker/compile_with_worker.cc b/tools/worker/compile_with_worker.cc index 4b4139580..9efd184eb 100644 --- a/tools/worker/compile_with_worker.cc +++ b/tools/worker/compile_with_worker.cc @@ -20,57 +20,6 @@ #include "tools/worker/work_processor.h" #include "tools/worker/worker_protocol.h" -// How Swift Incremental Compilation Works -// ======================================= -// When a Swift module is compiled, the output file map (a JSON file mapping -// source files to outputs) tells the compiler where to write the object (.o) -// files and partial .swiftmodule files. For incremental mode to work, the -// output file map must also contain "swift-dependencies" entries; these files -// contain compiler-internal data that describes how the sources in the module -// are interrelated. Once all of these outputs exist on the file system, future -// invocations of the compiler will use them to detect which source files -// actually need to be recompiled if any of them change. -// -// This compilation model doesn't interact well with Bazel, which expects builds -// to be hermetic (not affected by each other). In other words, outputs of build -// N are traditionally not available as inputs to build N+1; the action -// declaration model does not allow this. -// -// One could disable the sandbox to hack around this, but this should not be a -// requirement of a well-designed build rule implementation. -// -// Bazel provides "persistent workers" to address this. A persistent worker is a -// long-running "server" that waits for requests, which it can then handle -// in-process or by spawning other commands (we do the latter). The important -// feature here is that this worker can manage a separate file store that allows -// state to persist across multiple builds. -// -// However, there are still some caveats that we have to address: -// -// - The "SwiftCompile" actions registered by the build rules must declare the -// object files and partial .swiftmodules as outputs, because later actions -// need those files as inputs (e.g., archiving a static library or linking a -// dynamic library or executable). -// -// - Because those files are declared action outputs, Bazel will delete them or -// otherwise make them unavailable before the action executes, which destroys -// our persistent state. -// -// - We could avoid declaring those individual outputs if we had the persistent -// worker also link them, but this is infeasible: static archiving uses -// platform-dependent logic and will eventually be migrated to actions from -// the C++ toolchain, and linking a dynamic library or executable also uses -// the C++ toolchain. Furthermore, we may want to stop propagating .a files -// for linking and instead propagate the .o files directly, avoiding an -// archiving step when it isn't explicitly requested. -// -// So to make this work, we redirect the compiler to write its outputs to an -// alternate location that isn't declared by any Bazel action -- this prevents -// the files from being deleted between builds so the compiler can find them. -// (We still use a descendant of `bazel-bin` so that it *will* be removed by a -// `bazel clean`, as the user would expect.) Then, after the compiler is done, -// we copy those outputs into the locations where Bazel declared them, so that -// it can find them as well. int CompileWithWorker(const std::vector &args, std::string index_import_path) { diff --git a/tools/worker/output_file_map.cc b/tools/worker/output_file_map.cc index 5a4e9faef..68dcd85b7 100644 --- a/tools/worker/output_file_map.cc +++ b/tools/worker/output_file_map.cc @@ -17,170 +17,24 @@ #include #include #include -#include #include #include +#include -namespace { - -// Returns the given path transformed to point to the incremental storage area. -// For example, "bazel-out/config/{genfiles,bin}/path" becomes -// "bazel-out/config/{genfiles,bin}/_swift_incremental/path". -// When split compiling we need different directories, as the various swiftdeps -// and priors files conflict. -static std::string MakeIncrementalOutputPath(std::string path, - bool is_derived) { - auto bin_index = path.find("/bin/"); - if (bin_index != std::string::npos) { - if (is_derived) { - path.replace(bin_index, 5, "/bin/_swift_incremental_derived/"); - } else { - path.replace(bin_index, 5, "/bin/_swift_incremental/"); - } - return path; - } - auto genfiles_index = path.find("/genfiles/"); - if (genfiles_index != std::string::npos) { - if (is_derived) { - path.replace(genfiles_index, 10, "/genfiles/_swift_incremental_derived/"); - } else { - path.replace(genfiles_index, 10, "/genfiles/_swift_incremental/"); - } - return path; - } - return path; -} - -}; // end namespace - -void OutputFileMap::ReadFromPath(const std::string &path, - const std::string &emit_module_path, - const std::string &emit_objc_header_path) { +void OutputFileMap::ReadFromPath(const std::string &path) { std::ifstream stream(path); stream >> json_; - UpdateForIncremental(path, emit_module_path, emit_objc_header_path); } -void OutputFileMap::WriteToPath(const std::string &path) { - std::ofstream stream(path); - stream << json_; -} - -void OutputFileMap::UpdateForIncremental( - const std::string &path, const std::string &emit_module_path, - const std::string &emit_objc_header_path) { - bool derived = - path.find(".derived_output_file_map.json") != std::string::npos; - - nlohmann::json new_output_file_map; - std::map incremental_outputs; - std::map incremental_inputs; - std::vector incremental_cleanup_outputs; - - // The empty string key is used to represent outputs that are for the whole - // module, rather than for a particular source file. - nlohmann::json module_map; - // Derive the swiftdeps file name from the .output-file-map.json name. - std::string new_path = - std::filesystem::path(path).replace_extension(".swiftdeps").string(); - auto swiftdeps_path = MakeIncrementalOutputPath(new_path, derived); - module_map["swift-dependencies"] = swiftdeps_path; - new_output_file_map[""] = module_map; +std::vector OutputFileMap::get_outputs_by_type(const std::string& type) const { + std::vector result; for (auto &element : json_.items()) { - auto src = element.key(); auto outputs = element.value(); - - nlohmann::json src_map; - std::string swiftdeps_path; - - // Process the outputs for the current source file. - for (auto &output : outputs.items()) { - auto kind = output.key(); - auto path = output.value().get(); - - if (kind == "object" || kind == "const-values") { - // If the file kind is "object" or "const-values", we want to update the path to point to - // the incremental storage area and then add a "swift-dependencies" - // in the same location. - auto new_path = MakeIncrementalOutputPath(path, derived); - src_map[kind] = new_path; - incremental_outputs[path] = new_path; - - if (swiftdeps_path.empty()) { - swiftdeps_path = std::filesystem::path(new_path) - .replace_extension(".swiftdeps") - .string(); - } - - incremental_cleanup_outputs.push_back(swiftdeps_path); - } else if (kind == "swiftdoc" || kind == "swiftinterface" || - kind == "swiftmodule" || kind == "swiftsourceinfo") { - // Module/interface outputs should be moved to the incremental storage - // area without additional processing. - auto new_path = MakeIncrementalOutputPath(path, derived); - src_map[kind] = new_path; - incremental_outputs[path] = new_path; - - if (swiftdeps_path.empty()) { - swiftdeps_path = std::filesystem::path(new_path) - .replace_extension(".swiftdeps") - .string(); - } - - incremental_cleanup_outputs.push_back(swiftdeps_path); - } else if (kind == "swift-dependencies") { - // If there was already a "swift-dependencies" entry present, ignore it. - // (This shouldn't happen because the build rules won't do this, but - // check just in case.) - std::cerr << "There was a 'swift-dependencies' entry for " << src - << ", but the build rules should not have done this; " - << "ignoring it.\n"; - } else { - // Otherwise, just copy the mapping over verbatim. - src_map[kind] = path; - } + if (outputs.is_object() && outputs.contains(type)) { + result.push_back(outputs[type].get()); } - - // When split compiling both output_file_maps need src level swiftdeps - if (!swiftdeps_path.empty()) { - src_map["swift-dependencies"] = swiftdeps_path; - } - - new_output_file_map[src] = src_map; } - // If we don't generate a swiftmodule, don't try to copy those files - if (!emit_module_path.empty()) { - auto swiftmodule_path = emit_module_path; - auto copied_swiftmodule_path = - MakeIncrementalOutputPath(swiftmodule_path, derived); - incremental_inputs[swiftmodule_path] = copied_swiftmodule_path; - - std::string swiftdoc_path = std::filesystem::path(swiftmodule_path) - .replace_extension(".swiftdoc") - .string(); - auto copied_swiftdoc_path = - MakeIncrementalOutputPath(swiftdoc_path, derived); - incremental_inputs[swiftdoc_path] = copied_swiftdoc_path; - - std::string swiftsourceinfo_path = - std::filesystem::path(swiftmodule_path) - .replace_extension(".swiftsourceinfo") - .string(); - auto copied_swiftsourceinfo_path = - MakeIncrementalOutputPath(swiftsourceinfo_path, derived); - incremental_inputs[swiftsourceinfo_path] = copied_swiftsourceinfo_path; - } - - if (!emit_objc_header_path.empty()) { - auto copied_objc_header_path = - MakeIncrementalOutputPath(emit_objc_header_path, derived); - incremental_inputs[emit_objc_header_path] = copied_objc_header_path; - } - - json_ = new_output_file_map; - incremental_outputs_ = incremental_outputs; - incremental_inputs_ = incremental_inputs; - incremental_cleanup_outputs_ = incremental_cleanup_outputs; -} + return result; +} \ No newline at end of file diff --git a/tools/worker/output_file_map.h b/tools/worker/output_file_map.h index b3b684c8c..aeacc49f4 100644 --- a/tools/worker/output_file_map.h +++ b/tools/worker/output_file_map.h @@ -19,8 +19,7 @@ #include #include -// Supports loading and rewriting a `swiftc` output file map to support -// incremental compilation. +// Supports loading a `swiftc` output file map. // // See // https://github.com/apple/swift/blob/master/docs/Driver.md#output-file-maps @@ -32,47 +31,14 @@ class OutputFileMap { // The in-memory JSON-based representation of the output file map. const nlohmann::json &json() const { return json_; } - // A map containing expected output files that will be generated in the - // incremental storage area. The key is the original object path; the - // corresponding value is its location in the incremental storage area. - const std::map incremental_outputs() const { - return incremental_outputs_; - } + // Get output files of a specific kind from the map + std::vector get_outputs_by_type(const std::string& type) const; - // A map containing expected output files that will be generated in the - // non-incremental storage area, but need to be copied back at the start of - // the next compile. The key is the original object path; the corresponding - // value is its location in the incremental storage area. - const std::map incremental_inputs() const { - return incremental_inputs_; - } - - // A list of output files that will be generated in the incremental storage - // area, and need to be cleaned up if a corrupt module is detected. - const std::vector incremental_cleanup_outputs() const { - return incremental_cleanup_outputs_; - } - - // Reads the output file map from the JSON file at the given path, and updates - // it to support incremental builds. - void ReadFromPath(const std::string &path, - const std::string &emit_module_path, - const std::string &emit_objc_header_path); - - // Writes the output file map as JSON to the file at the given path. - void WriteToPath(const std::string &path); + // Reads the output file map from the JSON file at the given path. + void ReadFromPath(const std::string &path); private: - // Modifies the output file map's JSON structure in-place to replace file - // paths with equivalents in the incremental storage area. - void UpdateForIncremental(const std::string &path, - const std::string &emit_module_path, - const std::string &emit_objc_header_path); - nlohmann::json json_; - std::map incremental_outputs_; - std::map incremental_inputs_; - std::vector incremental_cleanup_outputs_; }; #endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_OUTPUT_FILE_MAP_H_ diff --git a/tools/worker/swift_runner.cc b/tools/worker/swift_runner.cc index 4788c331f..bf863be55 100644 --- a/tools/worker/swift_runner.cc +++ b/tools/worker/swift_runner.cc @@ -159,10 +159,9 @@ int SwiftRunner::Run(std::ostream *stderr_stream, bool stdout_to_stderr) { } OutputFileMap output_file_map; - output_file_map.ReadFromPath(output_file_map_path_, "", ""); + output_file_map.ReadFromPath(output_file_map_path_); - auto outputs = output_file_map.incremental_outputs(); - std::map::iterator it; + auto object_files = output_file_map.get_outputs_by_type("object"); std::vector ii_args; ii_args.push_back(index_import_path_); @@ -172,14 +171,9 @@ int SwiftRunner::Run(std::ostream *stderr_stream, bool stdout_to_stderr) { ii_args.push_back(std::filesystem::current_path().string() + "=."); } - for (it = outputs.begin(); it != outputs.end(); it++) { - // Need the actual output paths of the compiler - not bazel - auto output_path = it->first; - auto file_type = output_path.substr(output_path.find_last_of(".") + 1); - if (file_type == "o") { - ii_args.push_back("-import-output-file"); - ii_args.push_back(output_path); - } + for (const auto& obj_file : object_files) { + ii_args.push_back("-import-output-file"); + ii_args.push_back(obj_file); } const std::filesystem::path &exec_root = std::filesystem::current_path(); diff --git a/tools/worker/work_processor.cc b/tools/worker/work_processor.cc index ac35aaa86..d98755c66 100644 --- a/tools/worker/work_processor.cc +++ b/tools/worker/work_processor.cc @@ -14,40 +14,17 @@ #include "tools/worker/work_processor.h" -#if defined(__APPLE__) -#include -#endif -#include - #include #include -#include -#include #include #include #include "tools/common/temp_file.h" -#include "tools/worker/output_file_map.h" #include "tools/worker/swift_runner.h" #include "tools/worker/worker_protocol.h" namespace { -bool copy_file(const std::filesystem::path &from, - const std::filesystem::path &to, std::error_code &ec) noexcept { -#if defined(__APPLE__) - if (copyfile(from.string().c_str(), to.string().c_str(), nullptr, - COPYFILE_ALL | COPYFILE_CLONE) < 0) { - ec = std::error_code(errno, std::system_category()); - return false; - } - ec = std::error_code(); - return true; -#else - return std::filesystem::copy_file(from, to, ec); -#endif -} - static void FinalizeWorkRequest( const bazel_rules_swift::worker_protocol::WorkRequest &request, bazel_rules_swift::worker_protocol::WorkResponse &response, int exit_code, @@ -80,73 +57,9 @@ void WorkProcessor::ProcessWorkRequest( auto params_file = TempFile::Create("swiftc_params.XXXXXX"); std::ofstream params_file_stream(params_file->GetPath()); - OutputFileMap output_file_map; - std::string output_file_map_path; - std::string emit_module_path; - std::string emit_objc_header_path; - bool is_wmo = false; - bool is_dump_ast = false; - bool emit_swift_source_info = false; - - std::string prev_arg; - for (std::string arg : request.arguments) { - std::string original_arg = arg; - - // Handle arguments, in some cases we rewrite the argument entirely and in others we simply use it to determine - // specific behavior. - if (arg == "-output-file-map") { - // Peel off the `-output-file-map` argument, so we can rewrite it if necessary later. - arg.clear(); - } else if (arg == "-dump-ast") { - is_dump_ast = true; - } else if (prev_arg == "-output-file-map") { - // Peel off the `-output-file-map` argument, so we can rewrite it if necessary later. - output_file_map_path = arg; - arg.clear(); - } else if (prev_arg == "-emit-module-path") { - emit_module_path = arg; - } else if (prev_arg == "-emit-objc-header-path") { - emit_objc_header_path = arg; - } else if (ArgumentEnablesWMO(arg)) { - is_wmo = true; - } else if (prev_arg == "-Xwrapped-swift=-emit-swiftsourceinfo") { - emit_swift_source_info = true; - } - - if (!arg.empty()) { - params_file_stream << arg << '\n'; - } - - prev_arg = original_arg; - } - - bool is_incremental = !is_wmo && !is_dump_ast; - - if (!output_file_map_path.empty()) { - if (is_incremental) { - output_file_map.ReadFromPath(output_file_map_path, emit_module_path, - emit_objc_header_path); - - // Rewrite the output file map to use the incremental storage area and - // pass the compiler the path to the rewritten file. - std::string new_path = std::filesystem::path(output_file_map_path) - .replace_extension(".incremental.json") - .string(); - output_file_map.WriteToPath(new_path); - - params_file_stream << "-output-file-map\n"; - params_file_stream << new_path << '\n'; - - // Pass the incremental flags only if WMO is disabled. WMO would overrule - // incremental mode anyway, but since we control the passing of this flag, - // there's no reason to pass it when it's a no-op. - params_file_stream << "-incremental\n"; - } else { - // If WMO or -dump-ast is forcing us out of incremental mode, just put the - // original output file map back so the outputs end up where they should. - params_file_stream << "-output-file-map\n"; - params_file_stream << output_file_map_path << '\n'; - } + // Simply pass all arguments through to the compiler without modification + for (const std::string &arg : request.arguments) { + params_file_stream << arg << '\n'; } processed_args.push_back("@" + params_file->GetPath()); @@ -154,150 +67,9 @@ void WorkProcessor::ProcessWorkRequest( std::ostringstream stderr_stream; - if (is_incremental) { - std::set dir_paths; - - for (const auto &expected_object_pair : - output_file_map.incremental_inputs()) { - - const auto expected_object_path = std::filesystem::path(expected_object_pair.second); - - // In rules_swift < 3.x the .swiftsourceinfo files are unconditionally written to the module path. - // In rules_swift >= 3.x these same files are no longer tracked by Bazel unless explicitly requested. - // When using non-sandboxed mode, previous builds will contain these files and cause build failures - // when Swift tries to use them, in order to work around this compatibility issue, we remove them if they are - // present but not requested. - if (!emit_swift_source_info && expected_object_path.extension() == ".swiftsourceinfo") { - std::filesystem::remove(expected_object_path); - } - - // Bazel creates the intermediate directories for the files declared at - // analysis time, but not any any deeper directories, like one can have - // with -emit-objc-header-path, so we need to create those. - const std::string dir_path = - expected_object_path - .parent_path() - .string(); - dir_paths.insert(dir_path); - } - - for (const auto &expected_object_pair : - output_file_map.incremental_outputs()) { - // Bazel creates the intermediate directories for the files declared at - // analysis time, but we need to manually create the ones for the - // incremental storage area. - const std::string dir_path = - std::filesystem::path(expected_object_pair.second) - .parent_path() - .string(); - dir_paths.insert(dir_path); - } - - for (const auto &dir_path : dir_paths) { - std::error_code ec; - std::filesystem::create_directories(dir_path, ec); - if (ec) { - stderr_stream << "swift_worker: Could not create directory " << dir_path - << " (" << ec.message() << ")\n"; - FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); - return; - } - } - - // Copy some input files from the incremental storage area to the locations - // where Bazel will generate them. swiftc expects all or none of them exist - // otherwise the next invocation may not produce all the files. We also need - // to remove some files that exist in the incremental storage area. - auto inputs = output_file_map.incremental_inputs(); - bool all_inputs_exist = std::all_of( - inputs.cbegin(), inputs.cend(), [](const auto &expected_object_pair) { - return std::filesystem::exists(expected_object_pair.second); - }); - - if (all_inputs_exist) { - for (const auto &expected_object_pair : inputs) { - std::error_code ec; - copy_file(expected_object_pair.second, expected_object_pair.first, ec); - if (ec) { - stderr_stream << "swift_worker: Could not copy " - << expected_object_pair.second << " to " - << expected_object_pair.first << " (" << ec.message() - << ")\n"; - FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); - return; - } - } - } else { - auto cleanup_outputs = output_file_map.incremental_cleanup_outputs(); - for (const auto &cleanup_output : cleanup_outputs) { - if (!std::filesystem::exists(cleanup_output)) { - continue; - } - - std::error_code ec; - std::filesystem::remove(cleanup_output, ec); - if (ec) { - stderr_stream << "swift_worker: Could not remove " << cleanup_output - << " (" << ec.message() << ")\n"; - FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); - return; - } - } - } - } - SwiftRunner swift_runner(processed_args, index_import_path_, /*force_response_file=*/true); int exit_code = swift_runner.Run(&stderr_stream, /*stdout_to_stderr=*/true); - if (exit_code != 0) { - FinalizeWorkRequest(request, response, exit_code, stderr_stream); - return; - } - - if (is_incremental) { - // Copy the output files from the incremental storage area back to the - // locations where Bazel declared the files. - for (const auto &expected_object_pair : - output_file_map.incremental_outputs()) { - std::error_code ec; - copy_file(expected_object_pair.second, expected_object_pair.first, ec); - if (ec) { - stderr_stream << "swift_worker: Could not copy " - << expected_object_pair.second << " to " - << expected_object_pair.first << " (" << ec.message() - << ")\n"; - FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); - return; - } - } - - // Copy the replaced input files back to the incremental storage for the - // next run. - for (const auto &expected_object_pair : - output_file_map.incremental_inputs()) { - if (std::filesystem::exists(expected_object_pair.first)) { - if (std::filesystem::exists(expected_object_pair.second)) { - // CopyFile fails if the file already exists - std::filesystem::remove(expected_object_pair.second); - } - std::error_code ec; - copy_file(expected_object_pair.first, expected_object_pair.second, ec); - if (ec) { - stderr_stream << "swift_worker: Could not copy " - << expected_object_pair.first << " to " - << expected_object_pair.second << " (" << ec.message() - << ")\n"; - FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); - return; - } - } else if (exit_code == 0) { - stderr_stream << "Failed to copy " << expected_object_pair.first - << " for incremental builds, maybe it wasn't produced?\n"; - FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); - return; - } - } - } FinalizeWorkRequest(request, response, exit_code, stderr_stream); } From 58820ddbe0bd5400a1f20aa8de8fa21a80a6cdd5 Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Tue, 23 Sep 2025 01:19:46 -0400 Subject: [PATCH 02/13] Remove support for persistent worker --- doc/providers.md | 2 +- swift/providers.bzl | 3 +- swift/toolchains/swift_toolchain.bzl | 4 +- swift/toolchains/xcode_swift_toolchain.bzl | 4 +- tools/worker/BUILD | 86 ++--------------- tools/worker/compile_with_worker.cc | 48 ---------- tools/worker/compile_with_worker.h | 26 ------ tools/worker/compile_without_worker.cc | 27 ------ tools/worker/compile_without_worker.h | 26 ------ tools/worker/{worker_main.cc => main.cc} | 28 +++--- tools/worker/work_processor.cc | 75 --------------- tools/worker/work_processor.h | 44 --------- tools/worker/worker_protocol.cc | 76 --------------- tools/worker/worker_protocol.h | 103 --------------------- 14 files changed, 25 insertions(+), 527 deletions(-) delete mode 100644 tools/worker/compile_with_worker.cc delete mode 100644 tools/worker/compile_with_worker.h delete mode 100644 tools/worker/compile_without_worker.cc delete mode 100644 tools/worker/compile_without_worker.h rename tools/worker/{worker_main.cc => main.cc} (67%) delete mode 100644 tools/worker/work_processor.cc delete mode 100644 tools/worker/work_processor.h delete mode 100644 tools/worker/worker_protocol.cc delete mode 100644 tools/worker/worker_protocol.h diff --git a/doc/providers.md b/doc/providers.md index 86ae55e09..95e6f97b2 100644 --- a/doc/providers.md +++ b/doc/providers.md @@ -140,7 +140,7 @@ that use the toolchain. | package_configurations | A list of `SwiftPackageConfigurationInfo` providers that specify additional compilation configuration options that are applied to targets on a per-package basis. | | requested_features | `List` of `string`s. Features that should be implicitly enabled by default for targets built using this toolchain, unless overridden by the user by listing their negation in the `features` attribute of a target/package or in the `--features` command line flag.

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). | | root_dir | `String`. The workspace-relative root directory of the toolchain. | -| swift_worker | `File`. The executable representing the worker executable used to invoke the compiler and other Swift tools. | +| swift_worker | `File`. The executable that wraps Swift compiler invocations. | | test_configuration | `Struct` containing the following fields:

* `binary_name`: A template string used to compute the name of the output binary for `swift_test` rules. Any occurrences of the string `"{name}"` will be substituted by the name of the target.

* `env`: A `dict` of environment variables to be set when running tests that were built with this toolchain.

* `execution_requirements`: A `dict` of execution requirements for tests that were built with this toolchain.

* `objc_test_discovery`: A Boolean value indicating whether test targets should discover tests dynamically using the Objective-C runtime.

* `test_linking_contexts`: A list of `CcLinkingContext`s that provide additional flags to use when linking test binaries.

This is used, for example, with Xcode-based toolchains to ensure that the `xctest` helper and coverage tools are found in the correct developer directory when running tests. | | tool_configs | This field is an internal implementation detail of the build rules. | | unsupported_features | `List` of `string`s. Features that should be implicitly disabled by default for targets built using this toolchain, unless overridden by the user by listing them in the `features` attribute of a target/package or in the `--features` command line flag.

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). | diff --git a/swift/providers.bzl b/swift/providers.bzl index 0d11e9a55..15aee1ea8 100644 --- a/swift/providers.bzl +++ b/swift/providers.bzl @@ -442,8 +442,7 @@ Swift build rules, and they are also passed to the C++ APIs used when linking `String`. The workspace-relative root directory of the toolchain. """, "swift_worker": """\ -`File`. The executable representing the worker executable used to invoke the -compiler and other Swift tools. +`File`. The executable that wraps Swift compiler invocations. """, "test_configuration": """\ `Struct` containing the following fields: diff --git a/swift/toolchains/swift_toolchain.bzl b/swift/toolchains/swift_toolchain.bzl index e989c9cc0..0f3b21f29 100644 --- a/swift/toolchains/swift_toolchain.bzl +++ b/swift/toolchains/swift_toolchain.bzl @@ -139,7 +139,7 @@ def _all_tool_configs( driver_config = _driver_config(mode = "swiftc"), resource_set = _swift_compile_resource_set, use_param_file = True, - worker_mode = "persistent", + worker_mode = "wrap", env = env, ) @@ -662,7 +662,7 @@ to the compiler for exec transition builds. allow_files = True, default = Label("//tools/worker"), doc = """\ -An executable that wraps Swift compiler invocations using a persistent mode. +An executable that wraps Swift compiler invocations. """, executable = True, ), diff --git a/swift/toolchains/xcode_swift_toolchain.bzl b/swift/toolchains/xcode_swift_toolchain.bzl index 9bb9e809f..498c6f4d2 100644 --- a/swift/toolchains/xcode_swift_toolchain.bzl +++ b/swift/toolchains/xcode_swift_toolchain.bzl @@ -504,7 +504,7 @@ def _all_tool_configs( execution_requirements = execution_requirements, resource_set = _swift_compile_resource_set, use_param_file = True, - worker_mode = "persistent", + worker_mode = "wrap", ) tool_configs = { @@ -972,7 +972,7 @@ to the compiler for exec transition builds. allow_files = True, default = Label("//tools/worker:worker_wrapper"), doc = """\ -An executable that wraps Swift compiler invocations using a persistent mode. +An executable that wraps Swift compiler invocations. """, executable = True, ), diff --git a/tools/worker/BUILD b/tools/worker/BUILD index 342893139..1542041b5 100644 --- a/tools/worker/BUILD +++ b/tools/worker/BUILD @@ -3,64 +3,16 @@ load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library") licenses(["notice"]) -cc_library( - name = "compile_with_worker", - srcs = [ - "compile_with_worker.cc", - "work_processor.cc", - "work_processor.h", - ], - hdrs = ["compile_with_worker.h"], - copts = select({ - "//tools:clang-cl": [ - "-Xclang", - "-fno-split-cold-code", - "/std:c++17", - ], - "//tools:msvc": [ - "/std:c++17", - ], - "//conditions:default": [ - "-std=c++17", - ], - }), - deps = [ - ":swift_runner", - ":worker_protocol", - "//tools/common:temp_file", - ], -) - -cc_library( - name = "compile_without_worker", - srcs = ["compile_without_worker.cc"], - hdrs = ["compile_without_worker.h"], - copts = select({ - "//tools:clang-cl": [ - "-Xclang", - "-fno-split-cold-code", - "/std:c++17", - ], - "//tools:msvc": [ - "/std:c++17", - ], - "//conditions:default": [ - "-std=c++17", - ], - }), - deps = [ - ":swift_runner", - ], -) - cc_library( name = "swift_runner", srcs = [ "output_file_map.cc", - "output_file_map.h", "swift_runner.cc", ], - hdrs = ["swift_runner.h"], + hdrs = [ + "output_file_map.h", + "swift_runner.h", + ], copts = select({ "//tools:clang-cl": [ "-Xclang", @@ -89,35 +41,13 @@ cc_library( ], ) -cc_library( - name = "worker_protocol", - srcs = ["worker_protocol.cc"], - hdrs = ["worker_protocol.h"], - copts = select({ - "//tools:clang-cl": [ - "-Xclang", - "-fno-split-cold-code", - "/std:c++17", - ], - "//tools:msvc": [ - "/std:c++17", - ], - "//conditions:default": [ - "-std=c++17", - ], - }), - deps = [ - "@com_github_nlohmann_json//:json", - ], -) - cc_binary( name = "worker", - srcs = ["worker_main.cc"], + srcs = ["main.cc"], visibility = ["//visibility:public"], deps = [ - ":compile_with_worker", - ":compile_without_worker", + ":swift_runner", + "//tools/common:process", "@bazel_tools//tools/cpp/runfiles", ], ) @@ -145,4 +75,4 @@ filegroup( visibility = [ "//tools:__pkg__", ], -) +) \ No newline at end of file diff --git a/tools/worker/compile_with_worker.cc b/tools/worker/compile_with_worker.cc deleted file mode 100644 index 9efd184eb..000000000 --- a/tools/worker/compile_with_worker.cc +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2019 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "tools/worker/compile_with_worker.h" - -#include -#include - -#include "tools/worker/work_processor.h" -#include "tools/worker/worker_protocol.h" - - -int CompileWithWorker(const std::vector &args, - std::string index_import_path) { - // Pass the "universal arguments" to the Swift work processor. They will be - // rewritten to replace any placeholders if necessary, and then passed at the - // beginning of any process invocation. Note that these arguments include the - // tool itself (i.e., "swiftc"). - WorkProcessor swift_worker(args, index_import_path); - - while (true) { - std::optional request = - bazel_rules_swift::worker_protocol::ReadWorkRequest(std::cin); - if (!request) { - std::cerr << "Could not read WorkRequest from stdin. Killing worker " - << "process.\n"; - return 254; - } - - bazel_rules_swift::worker_protocol::WorkResponse response; - swift_worker.ProcessWorkRequest(*request, response); - - bazel_rules_swift::worker_protocol::WriteWorkResponse(response, std::cout); - } - - return 0; -} diff --git a/tools/worker/compile_with_worker.h b/tools/worker/compile_with_worker.h deleted file mode 100644 index 1e5f52c62..000000000 --- a/tools/worker/compile_with_worker.h +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2019 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_COMPILE_WITH_WORKER_H_ -#define BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_COMPILE_WITH_WORKER_H_ - -#include -#include - -// Starts the worker processing loop and listens to stdin for work requests from -// Bazel. -int CompileWithWorker(const std::vector &args, - std::string index_import_path); - -#endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_COMPILE_WITH_WORKER_H_ diff --git a/tools/worker/compile_without_worker.cc b/tools/worker/compile_without_worker.cc deleted file mode 100644 index c55a210c4..000000000 --- a/tools/worker/compile_without_worker.cc +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2019 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "tools/worker/compile_without_worker.h" - -#include -#include -#include - -#include "tools/worker/swift_runner.h" - -int CompileWithoutWorker(const std::vector &args, - std::string index_import_path) { - return SwiftRunner(args, index_import_path) - .Run(&std::cerr, /*stdout_to_stderr=*/false); -} diff --git a/tools/worker/compile_without_worker.h b/tools/worker/compile_without_worker.h deleted file mode 100644 index 14fde0455..000000000 --- a/tools/worker/compile_without_worker.h +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2019 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_COMPILE_WITHOUT_WORKER_H_ -#define BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_COMPILE_WITHOUT_WORKER_H_ - -#include -#include - -// Directly invokes the Swift compiler and exits, without performing any -// worker-related processing. -int CompileWithoutWorker(const std::vector &args, - std::string index_import_path); - -#endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_COMPILE_WITHOUT_WORKER_H_ diff --git a/tools/worker/worker_main.cc b/tools/worker/main.cc similarity index 67% rename from tools/worker/worker_main.cc rename to tools/worker/main.cc index 004ac6807..34cb3cb09 100644 --- a/tools/worker/worker_main.cc +++ b/tools/worker/main.cc @@ -19,18 +19,20 @@ #include "tools/common/process.h" #include "tools/cpp/runfiles/runfiles.h" -#include "tools/worker/compile_with_worker.h" -#include "tools/worker/compile_without_worker.h" +#include "tools/worker/swift_runner.h" using bazel::tools::cpp::runfiles::Runfiles; int main(int argc, char *argv[]) { std::string index_import_path; + + // Find the index-import tool from runfiles if available #ifdef BAZEL_CURRENT_REPOSITORY std::unique_ptr runfiles(Runfiles::Create(argv[0], BAZEL_CURRENT_REPOSITORY)); #else std::unique_ptr runfiles(Runfiles::Create(argv[0])); - #endif // BAZEL_CURRENT_REPOSITORY + #endif + if (runfiles != nullptr) { // TODO: Remove once we drop support for Xcode 16.x. // Determine which version of index-import to use based on the environment @@ -46,18 +48,10 @@ int main(int argc, char *argv[]) { auto args = std::vector(argv + 1, argv + argc); - // When Bazel invokes a tool in persistent worker mode, it includes the flag - // "--persistent_worker" on the command line (typically the first argument, - // but we don't want to rely on that). Since this "worker" tool also supports - // a non-worker mode, we can detect the mode based on the presence of this - // flag. - auto persistent_worker_it = - std::find(args.begin(), args.end(), "--persistent_worker"); - if (persistent_worker_it == args.end()) { - return CompileWithoutWorker(args, index_import_path); - } + // Filter out the --persistent_worker flag if present (no longer supported) + args.erase(std::remove(args.begin(), args.end(), "--persistent_worker"), args.end()); - // Remove the special flag before starting the worker processing loop. - args.erase(persistent_worker_it); - return CompileWithWorker(args, index_import_path); -} + // Run the Swift compiler with the provided arguments + return SwiftRunner(args, index_import_path) + .Run(&std::cerr, /*stdout_to_stderr=*/false); +} \ No newline at end of file diff --git a/tools/worker/work_processor.cc b/tools/worker/work_processor.cc deleted file mode 100644 index d98755c66..000000000 --- a/tools/worker/work_processor.cc +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright 2019 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "tools/worker/work_processor.h" - -#include -#include -#include -#include - -#include "tools/common/temp_file.h" -#include "tools/worker/swift_runner.h" -#include "tools/worker/worker_protocol.h" - -namespace { - -static void FinalizeWorkRequest( - const bazel_rules_swift::worker_protocol::WorkRequest &request, - bazel_rules_swift::worker_protocol::WorkResponse &response, int exit_code, - const std::ostringstream &output) { - response.exit_code = exit_code; - response.output = output.str(); - response.request_id = request.request_id; - response.was_cancelled = false; -} - -}; // end namespace - -WorkProcessor::WorkProcessor(const std::vector &args, - std::string index_import_path) - : index_import_path_(index_import_path) { - universal_args_.insert(universal_args_.end(), args.begin(), args.end()); -} - -void WorkProcessor::ProcessWorkRequest( - const bazel_rules_swift::worker_protocol::WorkRequest &request, - bazel_rules_swift::worker_protocol::WorkResponse &response) { - std::vector processed_args(universal_args_); - - // Bazel's worker spawning strategy reads the arguments from the params file - // and inserts them into the proto. This means that if we just try to pass - // them verbatim to swiftc, we might end up with a command line that's too - // long. Rather than try to figure out these limits (which is very - // OS-specific and easy to get wrong), we unconditionally write the processed - // arguments out to a params file. - auto params_file = TempFile::Create("swiftc_params.XXXXXX"); - std::ofstream params_file_stream(params_file->GetPath()); - - // Simply pass all arguments through to the compiler without modification - for (const std::string &arg : request.arguments) { - params_file_stream << arg << '\n'; - } - - processed_args.push_back("@" + params_file->GetPath()); - params_file_stream.close(); - - std::ostringstream stderr_stream; - - SwiftRunner swift_runner(processed_args, index_import_path_, - /*force_response_file=*/true); - int exit_code = swift_runner.Run(&stderr_stream, /*stdout_to_stderr=*/true); - - FinalizeWorkRequest(request, response, exit_code, stderr_stream); -} diff --git a/tools/worker/work_processor.h b/tools/worker/work_processor.h deleted file mode 100644 index b36ebf101..000000000 --- a/tools/worker/work_processor.h +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2019 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_WORK_PROCESSOR_H -#define BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_WORK_PROCESSOR_H - -#include -#include -#include - -#include "tools/worker/worker_protocol.h" - -// Manages persistent global state for the Swift worker and processes individual -// work requests. -class WorkProcessor { - public: - // Initializes a new work processor with the given universal arguments from - // the job invocation. - WorkProcessor(const std::vector &args, - std::string index_import_path); - - // Processes the given work request and writes its exit code and stderr output - // (if any) into the given response. - void ProcessWorkRequest( - const bazel_rules_swift::worker_protocol::WorkRequest &request, - bazel_rules_swift::worker_protocol::WorkResponse &response); - - private: - std::vector universal_args_; - std::string index_import_path_; -}; - -#endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_WORK_PROCESSOR_H diff --git a/tools/worker/worker_protocol.cc b/tools/worker/worker_protocol.cc deleted file mode 100644 index 70f3fd519..000000000 --- a/tools/worker/worker_protocol.cc +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright 2022 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "tools/worker/worker_protocol.h" - -#include - -namespace bazel_rules_swift::worker_protocol { - -// Populates an `Input` parsed from JSON. This function satisfies an API -// requirement of the JSON library, allowing it to automatically parse `Input` -// values from nested JSON objects. -void from_json(const ::nlohmann::json &j, Input &input) { - // As with the protobuf messages from which these types originate, we supply - // default values if any keys are not present. - input.path = j.value("path", ""); - input.digest = j.value("digest", ""); -} - -// Populates an `WorkRequest` parsed from JSON. This function satisfies an API -// requirement of the JSON library (although `WorkRequest` is a top-level object -// in our schema so we only call it directly). -void from_json(const ::nlohmann::json &j, WorkRequest &work_request) { - // As with the protobuf messages from which these types originate, we supply - // default values if any keys are not present. - work_request.arguments = j.value("arguments", std::vector()); - work_request.inputs = j.value("inputs", std::vector()); - work_request.request_id = j.value("requestId", 0); - work_request.cancel = j.value("cancel", false); - work_request.verbosity = j.value("verbosity", 0); - work_request.sandbox_dir = j.value("sandboxDir", ""); -} - -// Populates a JSON object with values from an `WorkResponse`. This function -// satisfies an API requirement of the JSON library (although `WorkResponse` is -// a top-level object in our schema so we only call it directly). -void to_json(::nlohmann::json &j, const WorkResponse &work_response) { - j = ::nlohmann::json{{"exitCode", work_response.exit_code}, - {"output", work_response.output}, - {"requestId", work_response.request_id}, - {"wasCancelled", work_response.was_cancelled}}; -} - -std::optional ReadWorkRequest(std::istream &stream) { - std::string line; - if (!std::getline(stream, line)) { - return std::nullopt; - } - - WorkRequest request; - from_json(::nlohmann::json::parse(line), request); - return request; -} - -void WriteWorkResponse(const WorkResponse &response, std::ostream &stream) { - ::nlohmann::json response_json; - to_json(response_json, response); - - // Use `dump` with default arguments to get the most compact representation - // of the response, and flush stdout after writing to ensure that Bazel - // doesn't hang waiting for the response due to buffering. - stream << response_json.dump() << std::flush; -} - -} // namespace bazel_rules_swift::worker_protocol diff --git a/tools/worker/worker_protocol.h b/tools/worker/worker_protocol.h deleted file mode 100644 index da27ad0fc..000000000 --- a/tools/worker/worker_protocol.h +++ /dev/null @@ -1,103 +0,0 @@ -// Copyright 2022 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_WORKER_PROTOCOL_H_ -#define BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_WORKER_PROTOCOL_H_ - -#include - -#include -#include -#include -#include - -namespace bazel_rules_swift::worker_protocol { - -// An input file passed into a work request. -// -// This struct corresponds to the `blaze.worker.Input` message defined in -// https://github.com/bazelbuild/bazel/blob/master/src/main/protobuf/worker_protocol.proto. -struct Input { - // The path in the file system from which the file should be read. - std::string path; - - // An opaque token representing a hash of the file's contents. - std::string digest; -}; - -// A single work unit that Bazel sent to the worker. -// -// This struct corresponds to the `blaze.worker.WorkRequest` message defined in -// https://github.com/bazelbuild/bazel/blob/master/src/main/protobuf/worker_protocol.proto. -struct WorkRequest { - // The command line arguments of the action. - std::vector arguments; - - // The inputs that the worker is allowed to read during execution of this - // request. - std::vector inputs; - - // If 0, this request must be processed alone; otherwise, it is the unique - // identifier of a request that can be processed in parallel with other - // requests. - int request_id; - - // If true, a previously sent `WorkRequest` with the same request ID should be - // cancelled. - bool cancel; - - // If greater than zero, the worker may output extra debug information to the - // worker log via stderr. - int verbosity; - - // For multiplex workers, this is the relative path inside the worker's - // current working directory where the worker can place inputs and outputs. - // This is empty for singleplex workers, which use their current working - // directory directly. - std::string sandbox_dir; -}; - -// A message sent from the worker back to Bazel when it has finished its work on -// a request. -// -// This struct corresponds to the `blaze.worker.WorkResponse` message defined in -// https://github.com/bazelbuild/bazel/blob/master/src/main/protobuf/worker_protocol.proto. -struct WorkResponse { - // The exit status to report for the action. - int exit_code; - - // Text printed to the user after the response has been received (for example, - // compiler warnings/errors). - std::string output; - - // The ID of the `WorkRequest` that this response is associated with. - int request_id; - - // Indicates that the corresponding request was cancelled. - bool was_cancelled; -}; - -// Parses and returns the next `WorkRequest` from the given stream. The format -// of the stream must be newline-delimited JSON (i.e., each line of the input is -// a complete JSON object). This function returns `nullopt` if the request could -// not be read (for example, because the JSON was malformed, or the stream was -// closed). -std::optional ReadWorkRequest(std::istream &stream); - -// Writes the given `WorkResponse` as compact JSON to the given stream. -void WriteWorkResponse(const WorkResponse &response, std::ostream &stream); - -} // namespace bazel_rules_swift::worker_protocol - -#endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_WORKER_PROTOCOL_H_ From 16ac29c89d57b26cee0e0724ef786593ea315d53 Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Tue, 23 Sep 2025 01:41:35 -0400 Subject: [PATCH 03/13] Sync swift_runner code with upstream --- MODULE.bazel | 2 + tools/common/BUILD | 93 ++ tools/common/bazel_substitutions.cc | 130 +-- tools/common/bazel_substitutions.h | 60 +- tools/common/color.h | 85 ++ tools/common/file_system.cc | 165 ++++ tools/common/file_system.h | 40 + tools/common/path_utils.cc | 87 ++ tools/common/path_utils.h | 47 + tools/common/process.cc | 572 +++++------- tools/common/process.h | 88 +- tools/common/status.cc | 69 ++ tools/common/status.h | 27 + tools/common/target_triple.h | 97 +++ tools/common/temp_file.h | 157 ++-- tools/worker/BUILD | 14 +- tools/worker/main.cc | 39 +- tools/worker/swift_runner.cc | 1260 ++++++++++++++++++++------- tools/worker/swift_runner.h | 158 +++- 19 files changed, 2221 insertions(+), 969 deletions(-) create mode 100644 tools/common/color.h create mode 100644 tools/common/file_system.cc create mode 100644 tools/common/file_system.h create mode 100644 tools/common/path_utils.cc create mode 100644 tools/common/path_utils.h create mode 100644 tools/common/status.cc create mode 100644 tools/common/status.h create mode 100644 tools/common/target_triple.h diff --git a/MODULE.bazel b/MODULE.bazel index 2f15a5e49..6346672d8 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -16,6 +16,8 @@ bazel_dep(name = "rules_shell", version = "0.3.0") bazel_dep(name = "platforms", version = "0.0.9") bazel_dep(name = "protobuf", version = "27.0", repo_name = "com_google_protobuf") bazel_dep(name = "nlohmann_json", version = "3.6.1", repo_name = "com_github_nlohmann_json") +bazel_dep(name = "abseil-cpp", version = "20250127.0") +bazel_dep(name = "re2", version = "2024-07-02") bazel_dep( name = "swift_argument_parser", version = "1.3.1.2", diff --git a/tools/common/BUILD b/tools/common/BUILD index bdd8dcf69..35ba99b61 100644 --- a/tools/common/BUILD +++ b/tools/common/BUILD @@ -39,6 +39,11 @@ cc_library( "-std=c++17", ], }), + deps = [ + ":temp_file", + "@abseil-cpp//absl/container:flat_hash_map", + "@abseil-cpp//absl/status:statusor", + ], ) cc_library( @@ -52,6 +57,94 @@ cc_library( "-std=c++17", ], }), + deps = [ + "@abseil-cpp//absl/strings", + ], +) + +cc_library( + name = "color", + hdrs = ["color.h"], + copts = selects.with_or({ + ("//tools:clang-cl", "//tools:msvc"): [ + "/std:c++17", + ], + "//conditions:default": [ + "-std=c++17", + ], + }), +) + +cc_library( + name = "file_system", + srcs = ["file_system.cc"], + hdrs = ["file_system.h"], + copts = selects.with_or({ + ("//tools:clang-cl", "//tools:msvc"): [ + "/std:c++17", + ], + "//conditions:default": [ + "-std=c++17", + ], + }), + deps = [ + ":path_utils", + ":status", + "@abseil-cpp//absl/cleanup", + "@abseil-cpp//absl/status:statusor", + "@abseil-cpp//absl/strings", + ], +) + +cc_library( + name = "path_utils", + srcs = ["path_utils.cc"], + hdrs = ["path_utils.h"], + copts = selects.with_or({ + ("//tools:clang-cl", "//tools:msvc"): [ + "/std:c++17", + ], + "//conditions:default": [ + "-std=c++17", + ], + }), + deps = [ + "@abseil-cpp//absl/strings", + ], +) + +cc_library( + name = "status", + srcs = ["status.cc"], + hdrs = ["status.h"], + copts = selects.with_or({ + ("//tools:clang-cl", "//tools:msvc"): [ + "/std:c++17", + ], + "//conditions:default": [ + "-std=c++17", + ], + }), + deps = [ + "@abseil-cpp//absl/status", + "@abseil-cpp//absl/strings", + ], +) + +cc_library( + name = "target_triple", + hdrs = ["target_triple.h"], + copts = selects.with_or({ + ("//tools:clang-cl", "//tools:msvc"): [ + "/std:c++17", + ], + "//conditions:default": [ + "-std=c++17", + ], + }), + deps = [ + "@abseil-cpp//absl/strings", + ], ) # Consumed by Bazel integration tests. diff --git a/tools/common/bazel_substitutions.cc b/tools/common/bazel_substitutions.cc index cf71de995..a85023607 100644 --- a/tools/common/bazel_substitutions.cc +++ b/tools/common/bazel_substitutions.cc @@ -15,133 +15,53 @@ #include "tools/common/bazel_substitutions.h" #include -#include #include -#include -#include #include -#include "tools/common/process.h" +#include "absl/container/flat_hash_map.h" +#include "absl/strings/str_replace.h" +#include "absl/strings/string_view.h" namespace bazel_rules_swift { namespace { -// The placeholder string used by Bazel that should be replaced by -// `DEVELOPER_DIR` at runtime. -static const char kBazelXcodeDeveloperDir[] = "__BAZEL_XCODE_DEVELOPER_DIR__"; - -// The placeholder string used by Bazel that should be replaced by `SDKROOT` -// at runtime. -static const char kBazelXcodeSdkRoot[] = "__BAZEL_XCODE_SDKROOT__"; - -// The placeholder string used by the Apple and Swift rules to be replaced with -// the absolute path to the custom toolchain being used -static const char kBazelToolchainPath[] = - "__BAZEL_CUSTOM_XCODE_TOOLCHAIN_PATH__"; - // Returns the value of the given environment variable, or the empty string if // it wasn't set. -std::string GetAppleEnvironmentVariable(const char *name) { -#if !defined(__APPLE__) - return ""; -#endif - - char *env_value = getenv(name); +std::string GetEnvironmentVariable(absl::string_view name) { + std::string null_terminated_name(name.data(), name.length()); + char *env_value = getenv(null_terminated_name.c_str()); if (env_value == nullptr) { - std::cerr << "error: required Apple environment variable '" << name << "' was not set. Please file an issue on bazelbuild/rules_swift.\n"; - exit(EXIT_FAILURE); - } - return env_value; -} - -std::string GetToolchainPath() { -#if !defined(__APPLE__) - return ""; -#endif - - char *toolchain_id = getenv("TOOLCHAINS"); - if (toolchain_id == nullptr) { return ""; } - - std::ostringstream output_stream; - int exit_code = - RunSubProcess({"/usr/bin/xcrun", "--find", "clang", "--toolchain", toolchain_id}, - /*env=*/nullptr, &output_stream, /*stdout_to_stderr=*/true); - if (exit_code != 0) { - std::cerr << output_stream.str() << "Error: TOOLCHAINS was set to '" - << toolchain_id << "' but xcrun failed when searching for that ID" - << std::endl; - exit(EXIT_FAILURE); - } - - if (output_stream.str().empty()) { - std::cerr << "Error: TOOLCHAINS was set to '" << toolchain_id - << "' but no toolchain with that ID was found" << std::endl; - exit(EXIT_FAILURE); - } else if (output_stream.str().find("XcodeDefault.xctoolchain") != - std::string::npos) { - // NOTE: Ideally xcrun would fail if the toolchain we asked for didn't exist - // but it falls back to the DEVELOPER_DIR instead, so we have to check the - // output ourselves. - std::cerr << "Error: TOOLCHAINS was set to '" << toolchain_id - << "' but the default toolchain was found, that likely means a " - "matching " - << "toolchain isn't installed" << std::endl; - exit(EXIT_FAILURE); - } - - std::filesystem::path toolchain_path(output_stream.str()); - // Remove usr/bin/clang components to get the root of the custom toolchain - return toolchain_path.parent_path().parent_path().parent_path().string(); + return env_value; } } // namespace BazelPlaceholderSubstitutions::BazelPlaceholderSubstitutions() { // When targeting Apple platforms, replace the magic Bazel placeholders with - // the path in the corresponding environment variable. These should be set by - // the build rules; only attempt to retrieve them if they're actually seen in - // the argument list. - placeholder_resolvers_ = { - {kBazelXcodeDeveloperDir, PlaceholderResolver([]() { - return GetAppleEnvironmentVariable("DEVELOPER_DIR"); - })}, - {kBazelXcodeSdkRoot, PlaceholderResolver([]() { - return GetAppleEnvironmentVariable("SDKROOT"); - })}, - {kBazelToolchainPath, - PlaceholderResolver([]() { return GetToolchainPath(); })}, - }; -} - -bool BazelPlaceholderSubstitutions::Apply(std::string &arg) { - bool changed = false; - - // Replace placeholders in the string with their actual values. - for (auto &pair : placeholder_resolvers_) { - changed |= FindAndReplace(pair.first, pair.second, arg); + // the path in the corresponding environment variable, which should be set by + // the build rules. If the variable isn't set, we don't store a substitution; + // if it was needed then the eventual replacement will be a no-op and the + // command will presumably fail later. + if (std::string developer_dir = GetEnvironmentVariable("DEVELOPER_DIR"); + !developer_dir.empty()) { + substitutions_[kBazelXcodeDeveloperDir] = developer_dir; + } + if (std::string sdk_root = GetEnvironmentVariable("SDKROOT"); + !sdk_root.empty()) { + substitutions_[kBazelXcodeSdkRoot] = sdk_root; } +} - return changed; +BazelPlaceholderSubstitutions::BazelPlaceholderSubstitutions( + absl::string_view developer_dir, absl::string_view sdk_root) { + substitutions_[kBazelXcodeDeveloperDir] = std::string(developer_dir); + substitutions_[kBazelXcodeSdkRoot] = std::string(sdk_root); } -bool BazelPlaceholderSubstitutions::FindAndReplace( - const std::string &placeholder, - BazelPlaceholderSubstitutions::PlaceholderResolver &resolver, - std::string &str) { - int start = 0; - bool changed = false; - while ((start = str.find(placeholder, start)) != std::string::npos) { - std::string resolved_value = resolver.get(); - if (resolved_value.empty()) { - return false; - } - changed = true; - str.replace(start, placeholder.length(), resolved_value); - start += resolved_value.length(); - } - return changed; +bool BazelPlaceholderSubstitutions::Apply(std::string &arg) { + return absl::StrReplaceAll(substitutions_, &arg) > 0; } } // namespace bazel_rules_swift diff --git a/tools/common/bazel_substitutions.h b/tools/common/bazel_substitutions.h index 8ba54586b..e7b4177eb 100644 --- a/tools/common/bazel_substitutions.h +++ b/tools/common/bazel_substitutions.h @@ -15,10 +15,11 @@ #ifndef BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_BAZEL_SUBSTITUTIONS_H_ #define BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_BAZEL_SUBSTITUTIONS_H_ -#include -#include #include +#include "absl/container/flat_hash_map.h" +#include "absl/strings/string_view.h" + namespace bazel_rules_swift { // Manages the substitution of special Bazel placeholder strings in command line @@ -27,57 +28,32 @@ namespace bazel_rules_swift { class BazelPlaceholderSubstitutions { public: // Initializes the substitutions by looking them up in the process's - // environment when they are first requested. + // environment. BazelPlaceholderSubstitutions(); // Initializes the substitutions with the given fixed strings. Intended to be // used for testing. - BazelPlaceholderSubstitutions(const std::string &developer_dir, - const std::string &sdk_root); + BazelPlaceholderSubstitutions(absl::string_view developer_dir, + absl::string_view sdk_root); // Applies any necessary substitutions to `arg` and returns true if this // caused the string to change. bool Apply(std::string &arg); - private: - // A resolver for a Bazel placeholder string that retrieves and caches the - // value the first time it is requested. - class PlaceholderResolver { - public: - explicit PlaceholderResolver(std::function fn) - : function_(fn), initialized_(false) {} - - // Returns the requested placeholder value, caching it for future - // retrievals. - std::string get() { - if (!initialized_) { - value_ = function_(); - initialized_ = true; - } - return value_; - } - - private: - // The function that returns the value of the placeholder, or the empty - // string if the placeholder should not be replaced. - std::function function_; + // The placeholder string used by Bazel that should be replaced by + // `DEVELOPER_DIR` at runtime. + inline static constexpr absl::string_view kBazelXcodeDeveloperDir = + "__BAZEL_XCODE_DEVELOPER_DIR__"; - // Indicates whether the value of the placeholder has been requested yet and - // and is therefore initialized. - bool initialized_; + // The placeholder string used by Bazel that should be replaced by `SDKROOT` + // at runtime. + inline static constexpr absl::string_view kBazelXcodeSdkRoot = + "__BAZEL_XCODE_SDKROOT__"; - // The cached value of the placeholder if `initialized_` is true. - std::string value_; - }; - - // Finds and replaces all instances of `placeholder` with the value provided - // by `resolver`, in-place on `str`. Returns true if the string was changed. - bool FindAndReplace(const std::string &placeholder, - PlaceholderResolver &resolver, std::string &str); - - // A mapping from Bazel placeholder strings to resolvers that provide their - // values. - std::map placeholder_resolvers_; + private: + // A mapping from Bazel placeholder strings to the values that should be + // substituted for them. + absl::flat_hash_map substitutions_; }; } // namespace bazel_rules_swift diff --git a/tools/common/color.h b/tools/common/color.h new file mode 100644 index 000000000..ad7197b8f --- /dev/null +++ b/tools/common/color.h @@ -0,0 +1,85 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_COLOR_H_ +#define BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_COLOR_H_ + +#include + +#include "absl/strings/string_view.h" + +namespace bazel_rules_swift { + +// A color that can be passed to the constructor of `ColorStream` when wrapping +// an `ostream`. +class Color { + public: + static const Color kBold; + static const Color kBoldRed; + static const Color kBoldGreen; + static const Color kBoldMagenta; + static const Color kBoldWhite; + static const Color kReset; + + friend std::ostream &operator<<(std::ostream &stream, Color color) { + return stream << "\x1b[" << color.code_ << "m"; + } + + private: + constexpr explicit Color(absl::string_view code) : code_(code) {} + + // The ANSI code for the color. + absl::string_view code_; +}; + +inline constexpr const Color Color::kBold = Color("1"); +inline constexpr const Color Color::kBoldRed = Color("1;31"); +inline constexpr const Color Color::kBoldGreen = Color("1;32"); +inline constexpr const Color Color::kBoldMagenta = Color("1;35"); +inline constexpr const Color Color::kBoldWhite = Color("1;37"); +inline constexpr const Color Color::kReset = Color("0"); + +// An RAII-style wrapper for an `std::ostream` that prints the ANSI code for a +// color when initialized and prints the reset code on destruction. +// +// Modeled loosely after the `llvm::WithColor` support class. +class WithColor { + public: + // Wraps the given `ostream` so that its output is in `color` for the duration + // of the wrapper's lifetime. + WithColor(std::ostream &stream, Color color) : stream_(stream) { + stream << color; + } + + ~WithColor() { stream_ << Color::kReset; } + + template + WithColor &operator<<(const T &value) { + stream_ << value; + return *this; + } + + WithColor &operator<<(std::ostream &(*modifier)(std::ostream &)) { + modifier(stream_); + return *this; + } + + private: + // The wrapped `ostream`. + std::ostream &stream_; +}; + +} // namespace bazel_rules_swift + +#endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_COLOR_H_ diff --git a/tools/common/file_system.cc b/tools/common/file_system.cc new file mode 100644 index 000000000..ffd7123d0 --- /dev/null +++ b/tools/common/file_system.cc @@ -0,0 +1,165 @@ +// Copyright 2019 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "tools/common/file_system.h" + +#include +#include +#include + +#include + +#ifdef __APPLE__ +#include +#else +#include +#include +#endif + +#include "absl/cleanup/cleanup.h" +#include "absl/status/status.h" +#include "absl/strings/string_view.h" +#include "absl/strings/substitute.h" +#include "tools/common/path_utils.h" +#include "tools/common/status.h" + +namespace bazel_rules_swift { + +std::string GetCurrentDirectory() { + // Passing null,0 causes getcwd to allocate the buffer of the correct size. + char *buffer = getcwd(nullptr, 0); + std::string cwd(buffer); + free(buffer); + return cwd; +} + +absl::Status CopyFile(absl::string_view src, absl::string_view dest) { + // `string_view`s are not required to be null-terminated, so get explicit + // null-terminated strings that we can pass to the C functions below. + std::string null_terminated_src(src.data(), src.length()); + std::string null_terminated_dest(dest.data(), dest.length()); + +#ifdef __APPLE__ + // The `copyfile` function with `COPYFILE_ALL` mode preserves permissions and + // modification time. + if (copyfile(null_terminated_src.c_str(), null_terminated_dest.c_str(), + nullptr, COPYFILE_ALL | COPYFILE_CLONE) == 0) { + return absl::OkStatus(); + } + return bazel_rules_swift::MakeStatusFromErrno( + absl::Substitute("Could not copy $0 to $1", src, dest)); +#elif __unix__ + // On Linux, we can use `sendfile` to copy it more easily than calling + // `read`/`write` in a loop. + auto MakeFailingStatus = [src, dest](absl::string_view reason) { + return bazel_rules_swift::MakeStatusFromErrno( + absl::Substitute("Could not copy $0 to $1; $2", src, dest, reason)); + }; + + int src_fd = open(null_terminated_src.c_str(), O_RDONLY); + if (!src_fd) { + return MakeFailingStatus("could not open source for reading"); + } + + absl::Cleanup src_closer = [src_fd] { close(src_fd); }; + + struct stat stat_buf; + if (fstat(src_fd, &stat_buf) == -1) { + return MakeFailingStatus("could not stat source file"); + } + + int dest_fd = + open(null_terminated_dest.c_str(), O_WRONLY | O_CREAT, stat_buf.st_mode); + if (!dest_fd) { + return MakeFailingStatus("could not open destination for writing"); + } + + absl::Cleanup dest_closer = [dest_fd] { close(dest_fd); }; + + off_t offset = 0; + if (sendfile(dest_fd, src_fd, &offset, stat_buf.st_size) == -1) { + return MakeFailingStatus("could not copy file data"); + } + + struct timespec timespecs[2] = {stat_buf.st_atim, stat_buf.st_mtim}; + if (futimens(dest_fd, timespecs) == -1) { + return MakeFailingStatus("could not update destination timestamps"); + } + + return absl::OkStatus(); +#else +#error Only macOS and Unix are supported. +#endif +} + +absl::Status MakeDirs(absl::string_view path, int mode) { + auto MakeFailingStatus = [path](absl::string_view reason) { + return bazel_rules_swift::MakeStatusFromErrno( + absl::Substitute("Could not create directory $0; $1", path, reason)); + }; + + // If we got an empty string, we've recursed past the first segment in the + // path. Assume it exists (if it doesn't, we'll fail when we try to create a + // directory inside it). + if (path.empty()) { + return absl::OkStatus(); + } + + // `string_view`s are not required to be null-terminated, so get an explicit + // null-terminated string that we can pass to the C functions below. + std::string null_terminated_path(path.data(), path.length()); + + struct stat dir_stats; + if (stat(null_terminated_path.c_str(), &dir_stats) == 0) { + // Return true if the directory already exists. + if (S_ISDIR(dir_stats.st_mode)) { + return absl::OkStatus(); + } + + return MakeFailingStatus("path already exists but is not a directory"); + } + + // Recurse to create the parent directory. + if (absl::Status parent_status = MakeDirs(Dirname(path), mode); + !parent_status.ok()) { + return parent_status; + } + + // Create the directory that was requested. + if (mkdir(null_terminated_path.c_str(), mode) == 0) { + return absl::OkStatus(); + } + + // Race condition: The above call to `mkdir` could fail if there are multiple + // calls to `MakeDirs` running at the same time with overlapping paths, so + // check again to see if the directory exists despite the call failing. If it + // does, that's ok. + if (errno == EEXIST && stat(null_terminated_path.c_str(), &dir_stats) == 0) { + if (S_ISDIR(dir_stats.st_mode)) { + return absl::OkStatus(); + } + + return MakeFailingStatus("path already exists but is not a directory"); + } + + return MakeFailingStatus("unexpected error"); +} + +bool PathExists(absl::string_view path) { + struct stat stats; + std::string null_terminated_path{path}; + return stat(null_terminated_path.c_str(), &stats) == 0; +} + +} // namespace bazel_rules_swift diff --git a/tools/common/file_system.h b/tools/common/file_system.h new file mode 100644 index 000000000..eb121188b --- /dev/null +++ b/tools/common/file_system.h @@ -0,0 +1,40 @@ +// Copyright 2019 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_FILE_SYSTEM_H_ +#define BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_FILE_SYSTEM_H_ + +#include + +#include "absl/status/status.h" +#include "absl/strings/string_view.h" + +namespace bazel_rules_swift { + +// Gets the path to the current working directory. +std::string GetCurrentDirectory(); + +// Copies the file at src to dest. Returns true if successful. +absl::Status CopyFile(absl::string_view src, absl::string_view dest); + +// Creates a directory at the given path, along with any parent directories that +// don't already exist. Returns true if successful. +absl::Status MakeDirs(absl::string_view path, int mode); + +// Returns true if the given path exists. +bool PathExists(absl::string_view path); + +} // namespace bazel_rules_swift + +#endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_FILE_SYSTEM_H_ diff --git a/tools/common/path_utils.cc b/tools/common/path_utils.cc new file mode 100644 index 000000000..53f2fa993 --- /dev/null +++ b/tools/common/path_utils.cc @@ -0,0 +1,87 @@ +// Copyright 2019 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "tools/common/path_utils.h" + +#include + +#include "absl/strings/str_cat.h" + +namespace bazel_rules_swift { + +namespace { + +size_t ExtensionStartPosition(absl::string_view path, bool all_extensions) { + size_t last_slash = path.rfind('/'); + size_t dot; + + if (all_extensions) { + // Find the first dot, signifying the first of all extensions. + if (last_slash != absl::string_view::npos) { + dot = path.find('.', last_slash); + } else { + dot = path.find('.'); + } + } else { + // Find the last extension only. + dot = path.rfind('.'); + if (dot < last_slash) { + // If the dot was part of a previous path segment, treat it as if it + // wasn't found (it's not an extension of the filename). + dot = absl::string_view::npos; + } + } + + return dot; +} + +} // namespace + +absl::string_view Basename(absl::string_view path) { + if (size_t last_slash = path.rfind('/'); + last_slash != absl::string_view::npos) { + return path.substr(last_slash + 1); + } + return path; +} + +absl::string_view Dirname(absl::string_view path) { + if (size_t last_slash = path.rfind('/'); + last_slash != absl::string_view::npos) { + return path.substr(0, last_slash); + } + return absl::string_view(); +} + +absl::string_view GetExtension(absl::string_view path, bool all_extensions) { + if (size_t dot = ExtensionStartPosition(path, all_extensions); + dot != absl::string_view::npos) { + return path.substr(dot); + } + return ""; +} + +std::string ReplaceExtension(absl::string_view path, + absl::string_view new_extension, + bool all_extensions) { + if (size_t dot = ExtensionStartPosition(path, all_extensions); + dot != absl::string_view::npos) { + return absl::StrCat(path.substr(0, dot), new_extension); + } + + // If there was no dot append the extension to the path. + return absl::StrCat(path, new_extension); +} + +} // namespace bazel_rules_swift diff --git a/tools/common/path_utils.h b/tools/common/path_utils.h new file mode 100644 index 000000000..7625295fc --- /dev/null +++ b/tools/common/path_utils.h @@ -0,0 +1,47 @@ +// Copyright 2019 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_PATH_UTILS_H_ +#define BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_PATH_UTILS_H_ + +#include + +#include "absl/strings/string_view.h" + +namespace bazel_rules_swift { + +// Returns the base name of the given filepath. For example, given +// "/foo/bar/baz.txt", returns "baz.txt". +absl::string_view Basename(absl::string_view path); + +// Returns the directory name of the given filepath. For example, given +// "/foo/bar/baz.txt", returns "/foo/bar". +absl::string_view Dirname(absl::string_view path); + +// Returns the extension of the file specified by `path`. If the file has no +// extension, the empty string is returned. +absl::string_view GetExtension(absl::string_view path, + bool all_extensions = false); + +// Replaces the file extension of path with new_extension. It is assumed that +// new_extension starts with a dot if it is desired for a dot to precede the new +// extension in the returned path. If the path does not have a file extension, +// then new_extension is appended to it. +std::string ReplaceExtension(absl::string_view path, + absl::string_view new_extension, + bool all_extensions = false); + +} // namespace bazel_rules_swift + +#endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_PATH_UTILS_H_ diff --git a/tools/common/process.cc b/tools/common/process.cc index e546a3756..5186ae9b5 100644 --- a/tools/common/process.cc +++ b/tools/common/process.cc @@ -12,241 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "process.h" +#include "tools/common/process.h" -#include -#include -#include -#include -#include -#include -#include -#include - -#if defined(_WIN32) -#define WIN32_LEAN_AND_MEAN -#include -#include -#else -extern "C" { - extern char **environ; -} -#endif - -std::map GetCurrentEnvironment() { - std::map result; - char **envp = environ; - for (int i = 0; envp[i] != nullptr; ++i) { - std::string envString(envp[i]); - size_t equalsPos = envString.find('='); - if (equalsPos != std::string::npos) { - std::string key = envString.substr(0, equalsPos); - std::string value = envString.substr(equalsPos + 1); - result[key] = value; - } - } - return result; -} - -#if defined(_WIN32) - -namespace { -class WindowsIORedirector { - enum { In, Out }; - enum { Rd, Wr }; - - HANDLE hIO_[2][2]; - - explicit WindowsIORedirector(HANDLE hIO[2][2], bool include_stdout) { - std::memcpy(hIO_, hIO, sizeof(hIO_)); - assert(hIO_[In][Rd] == INVALID_HANDLE_VALUE); - assert(hIO_[In][Wr] == INVALID_HANDLE_VALUE); - - ZeroMemory(&siStartInfo, sizeof(siStartInfo)); - siStartInfo.cb = sizeof(siStartInfo); - siStartInfo.dwFlags = STARTF_USESTDHANDLES; - siStartInfo.hStdInput = INVALID_HANDLE_VALUE; - siStartInfo.hStdOutput = - include_stdout ? hIO_[Out][Wr] : INVALID_HANDLE_VALUE; - siStartInfo.hStdError = hIO_[Out][Wr]; - } - - public: - STARTUPINFOA siStartInfo; - - WindowsIORedirector(const WindowsIORedirector &) = delete; - WindowsIORedirector &operator=(const WindowsIORedirector &) = delete; - - WindowsIORedirector(WindowsIORedirector &&) = default; - WindowsIORedirector &operator=(WindowsIORedirector &&) = default; - - ~WindowsIORedirector() { - assert(hIO_[In][Rd] == INVALID_HANDLE_VALUE); - assert(hIO_[In][Wr] == INVALID_HANDLE_VALUE); - CloseHandle(hIO_[Out][Rd]); - CloseHandle(hIO_[Out][Wr]); - } - - static std::unique_ptr Create(bool include_stdout, - std::error_code &ec) { - HANDLE hIO[2][2] = { - {INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE}, - {INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE}, - }; - - SECURITY_ATTRIBUTES saAttr; - ZeroMemory(&saAttr, sizeof(saAttr)); - saAttr.nLength = sizeof(saAttr); - saAttr.lpSecurityDescriptor = nullptr; - saAttr.bInheritHandle = TRUE; - - if (!CreatePipe(&hIO[Out][Rd], &hIO[Out][Wr], &saAttr, 0)) { - ec = std::error_code(GetLastError(), std::system_category()); - return nullptr; - } - - // The read handle for stdout should not be inheritted by the child. - if (!SetHandleInformation(hIO[Out][Rd], HANDLE_FLAG_INHERIT, FALSE)) { - ec = std::error_code(GetLastError(), std::system_category()); - CloseHandle(hIO[Out][Rd]); - CloseHandle(hIO[Out][Wr]); - return nullptr; - } - - return std::unique_ptr( - new WindowsIORedirector(hIO, include_stdout)); - } - - void ConsumeAllSubprocessOutput(std::ostream *stderr_stream); -}; - -void WindowsIORedirector::ConsumeAllSubprocessOutput( - std::ostream *stderr_stream) { - CloseHandle(hIO_[Out][Wr]); - hIO_[Out][Wr] = INVALID_HANDLE_VALUE; - - char stderr_buffer[1024]; - DWORD dwNumberOfBytesRead; - while (ReadFile(hIO_[Out][Rd], stderr_buffer, sizeof(stderr_buffer), - &dwNumberOfBytesRead, nullptr)) { - if (dwNumberOfBytesRead) - stderr_stream->write(stderr_buffer, dwNumberOfBytesRead); - } - if (dwNumberOfBytesRead) - stderr_stream->write(stderr_buffer, dwNumberOfBytesRead); -} - -std::string GetCommandLine(const std::vector &arguments) { - // To escape the command line, we surround the argument with quotes. - // However, the complication comes due to how the Windows command line - // parser treats backslashes (\) and quotes ("). - // - // - \ is normally treated as a literal backslash - // e.g. alpha\beta\gamma => alpha\beta\gamma - // - The sequence \" is treated as a literal " - // e.g. alpha\"beta => alpha"beta - // - // But then what if we are given a path that ends with a \? - // - // Surrounding alpha\beta\ with " would be "alpha\beta\" which would be - // an unterminated string since it ends on a literal quote. To allow - // this case the parser treats: - // - // - \\" as \ followed by the " metacharacter - // - \\\" as \ followed by a literal " - // - // In general: - // - 2n \ followed by " => n \ followed by the " metacharacter - // - 2n + 1 \ followed by " => n \ followed by a literal " - auto quote = [](const std::string &argument) -> std::string { - if (argument.find_first_of(" \t\n\"") == std::string::npos) return argument; - - std::ostringstream buffer; - - buffer << '\"'; - std::string::const_iterator cur = std::begin(argument); - std::string::const_iterator end = std::end(argument); - while (cur < end) { - std::string::size_type offset = std::distance(std::begin(argument), cur); - - std::string::size_type start = argument.find_first_not_of('\\', offset); - if (start == std::string::npos) { - // String ends with a backslash (e.g. first\second\), escape all - // the backslashes then add the metacharacter ". - buffer << std::string(2 * (argument.length() - offset), '\\'); - break; - } - - std::string::size_type count = start - offset; - // If this is a string of \ followed by a " (e.g. first\"second). - // Escape the backslashes and the quote, otherwise these are just literal - // backslashes. - buffer << std::string(argument.at(start) == '\"' ? 2 * count + 1 : count, - '\\') - << argument.at(start); - // Drop the backslashes and the following character. - std::advance(cur, count + 1); - } - buffer << '\"'; - - return buffer.str(); - }; - - std::ostringstream quoted; - std::transform(std::begin(arguments), std::end(arguments), - std::ostream_iterator(quoted, " "), quote); - return quoted.str(); -} -} // namespace - -int RunSubProcess(const std::vector &args, - std::map *env, - std::ostream *stderr_stream, bool stdout_to_stderr) { - std::error_code ec; - std::unique_ptr redirector = - WindowsIORedirector::Create(stdout_to_stderr, ec); - if (!redirector) { - (*stderr_stream) << "unable to create stderr pipe: " << ec.message() - << '\n'; - return 254; - } - - PROCESS_INFORMATION piProcess = {0}; - if (!CreateProcessA(NULL, GetCommandLine(args).data(), nullptr, nullptr, TRUE, - 0, nullptr, nullptr, &redirector->siStartInfo, - &piProcess)) { - DWORD dwLastError = GetLastError(); - (*stderr_stream) << "unable to create process (error " << dwLastError - << ")\n"; - return dwLastError; - } - - CloseHandle(piProcess.hThread); - - redirector->ConsumeAllSubprocessOutput(stderr_stream); - - if (WaitForSingleObject(piProcess.hProcess, INFINITE) == WAIT_FAILED) { - DWORD dwLastError = GetLastError(); - (*stderr_stream) << "wait for process failure (error " << dwLastError - << ")\n"; - CloseHandle(piProcess.hProcess); - return dwLastError; - } - - DWORD dwExitCode; - if (!GetExitCodeProcess(piProcess.hProcess, &dwExitCode)) { - DWORD dwLastError = GetLastError(); - (*stderr_stream) << "unable to get exit code (error " << dwLastError - << ")\n"; - CloseHandle(piProcess.hProcess); - return dwLastError; - } - - CloseHandle(piProcess.hProcess); - return dwExitCode; -} - -#else #include #include #include @@ -254,28 +21,52 @@ int RunSubProcess(const std::vector &args, #include #include +#include #include -#include +#include // NOLINT +#include #include +#include +#include +#include +#include + +#include "absl/container/flat_hash_map.h" +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_split.h" +#include "absl/strings/string_view.h" +#include "tools/common/temp_file.h" + +extern char **environ; + +namespace bazel_rules_swift { namespace { +// Converts an array of string arguments to char *arguments, terminated by a +// nullptr. +// It is the responsibility of the caller to free the elements of the returned +// vector. +std::vector ConvertToCArgs(const std::vector &args) { + std::vector c_args; + c_args.reserve(args.size() + 1); + for (int i = 0; i < args.size(); i++) { + c_args.push_back(strdup(args[i].c_str())); + } + c_args.push_back(nullptr); + return c_args; +} + // An RAII class that manages the pipes and posix_spawn state needed to redirect -// subprocess I/O. Currently only supports stderr, but can be extended to handle -// stdin and stdout if needed. +// subprocess I/O. Currently only supports stdout and stderr, but can be +// extended to handle stdin if needed. class PosixSpawnIORedirector { public: // Create an I/O redirector that can be used with posix_spawn to capture // stderr. - static std::unique_ptr Create(bool stdoutToStderr) { - int stderr_pipe[2]; - if (pipe(stderr_pipe) != 0) { - return nullptr; - } - - return std::unique_ptr( - new PosixSpawnIORedirector(stderr_pipe, stdoutToStderr)); - } + static std::unique_ptr Create(); // Explicitly make PosixSpawnIORedirector non-copyable and movable. PosixSpawnIORedirector(const PosixSpawnIORedirector &) = delete; @@ -283,114 +74,198 @@ class PosixSpawnIORedirector { PosixSpawnIORedirector(PosixSpawnIORedirector &&) = default; PosixSpawnIORedirector &operator=(PosixSpawnIORedirector &&) = default; - ~PosixSpawnIORedirector() { - SafeClose(&stderr_pipe_[0]); - SafeClose(&stderr_pipe_[1]); - posix_spawn_file_actions_destroy(&file_actions_); - } + ~PosixSpawnIORedirector(); // Returns the pointer to a posix_spawn_file_actions_t value that should be // passed to posix_spawn to enable this redirection. posix_spawn_file_actions_t *PosixSpawnFileActions() { return &file_actions_; } + // Returns a pointer to the two-element file descriptor array for the stdout + // pipe. + int *StdoutPipe() { return stdout_pipe_; } + // Returns a pointer to the two-element file descriptor array for the stderr // pipe. int *StderrPipe() { return stderr_pipe_; } - // Consumes all the data output to stderr by the subprocess and writes it to - // the given output stream. - void ConsumeAllSubprocessOutput(std::ostream *stderr_stream); + // Consumes all the data output to stdout and stderr by the subprocess and + // writes it to the given output stream. + void ConsumeAllSubprocessOutput(std::ostream &stdout_stream, + std::ostream &stderr_stream); private: - explicit PosixSpawnIORedirector(int stderr_pipe[], bool stdoutToStderr) { - memcpy(stderr_pipe_, stderr_pipe, sizeof(int) * 2); - - posix_spawn_file_actions_init(&file_actions_); - posix_spawn_file_actions_addclose(&file_actions_, stderr_pipe_[0]); - if (stdoutToStderr) { - posix_spawn_file_actions_adddup2(&file_actions_, stderr_pipe_[1], - STDOUT_FILENO); - } - posix_spawn_file_actions_adddup2(&file_actions_, stderr_pipe_[1], - STDERR_FILENO); - posix_spawn_file_actions_addclose(&file_actions_, stderr_pipe_[1]); - } + PosixSpawnIORedirector(int stdout_pipe[], int stderr_pipe[]); // Closes a file descriptor only if it hasn't already been closed. - void SafeClose(int *fd) { - if (*fd >= 0) { - close(*fd); - *fd = -1; + void SafeClose(int &fd) { + if (fd >= 0) { + close(fd); + fd = -1; } } + int stdout_pipe_[2]; int stderr_pipe_[2]; posix_spawn_file_actions_t file_actions_; }; +PosixSpawnIORedirector::PosixSpawnIORedirector(int stdout_pipe[], + int stderr_pipe[]) { + memcpy(stdout_pipe_, stdout_pipe, sizeof(int) * 2); + memcpy(stderr_pipe_, stderr_pipe, sizeof(int) * 2); + + posix_spawn_file_actions_init(&file_actions_); + posix_spawn_file_actions_addclose(&file_actions_, stdout_pipe_[0]); + posix_spawn_file_actions_addclose(&file_actions_, stderr_pipe_[0]); + posix_spawn_file_actions_adddup2(&file_actions_, stdout_pipe_[1], + STDOUT_FILENO); + posix_spawn_file_actions_adddup2(&file_actions_, stderr_pipe_[1], + STDERR_FILENO); + posix_spawn_file_actions_addclose(&file_actions_, stdout_pipe_[1]); + posix_spawn_file_actions_addclose(&file_actions_, stderr_pipe_[1]); +} + +PosixSpawnIORedirector::~PosixSpawnIORedirector() { + SafeClose(stdout_pipe_[1]); + SafeClose(stderr_pipe_[1]); + posix_spawn_file_actions_destroy(&file_actions_); +} + +std::unique_ptr PosixSpawnIORedirector::Create() { + int stdout_pipe[2]; + int stderr_pipe[2]; + if (pipe(stdout_pipe) != 0 || pipe(stderr_pipe) != 0) { + return nullptr; + } + + return std::unique_ptr( + new PosixSpawnIORedirector(stdout_pipe, stderr_pipe)); +} + void PosixSpawnIORedirector::ConsumeAllSubprocessOutput( - std::ostream *stderr_stream) { - SafeClose(&stderr_pipe_[1]); + std::ostream &stdout_stream, std::ostream &stderr_stream) { + SafeClose(stdout_pipe_[1]); + SafeClose(stderr_pipe_[1]); + char stdout_buffer[1024]; char stderr_buffer[1024]; - pollfd stderr_poll = {stderr_pipe_[0], POLLIN}; - int status; - while ((status = poll(&stderr_poll, 1, -1)) > 0) { - if (stderr_poll.revents) { + + std::vector poll_list{ + {stdout_pipe_[0], POLLIN}, + {stderr_pipe_[0], POLLIN}, + }; + + bool active_events = true; + while (active_events) { + active_events = false; + while (poll(&poll_list.front(), poll_list.size(), -1) == -1) { + int err = errno; + if (err == EAGAIN || err == EINTR) { + continue; + } + } + if (poll_list[0].revents & POLLIN) { + int bytes_read = + read(stdout_pipe_[0], stdout_buffer, sizeof(stdout_buffer)); + if (bytes_read > 0) { + stdout_stream.write(stdout_buffer, bytes_read); + active_events = true; + } + } + if (poll_list[1].revents & POLLIN) { int bytes_read = read(stderr_pipe_[0], stderr_buffer, sizeof(stderr_buffer)); - if (bytes_read == 0) { - break; + if (bytes_read > 0) { + stderr_stream.write(stderr_buffer, bytes_read); + active_events = true; } - stderr_stream->write(stderr_buffer, bytes_read); } } } -// Converts an array of string arguments to char *arguments. -// The first arg is reduced to its basename as per execve conventions. -// Note that the lifetime of the char* arguments in the returned array -// are controlled by the lifetime of the strings in args. -std::vector ConvertToCArgs(const std::vector &args) { - std::vector c_args; - std::string filename = std::filesystem::path(args[0]).filename().string(); - c_args.push_back(&*std::next(args[0].rbegin(), filename.length() - 1)); - for (int i = 1; i < args.size(); i++) { - c_args.push_back(args[i].c_str()); +} // namespace + +absl::flat_hash_map GetCurrentEnvironment() { + absl::flat_hash_map result; + if (environ == nullptr) { + return result; } - c_args.push_back(nullptr); - return c_args; + for (char **envp = environ; *envp != nullptr; ++envp) { + std::pair key_value = + absl::StrSplit(*envp, absl::MaxSplits('=', 1)); + result[key_value.first] = std::string(key_value.second); + } + return result; } -} // namespace - int RunSubProcess(const std::vector &args, - std::map *env, - std::ostream *stderr_stream, bool stdout_to_stderr) { - std::vector exec_argv = ConvertToCArgs(args); + const absl::flat_hash_map *env, + std::ostream &stdout_stream, std::ostream &stderr_stream) { + absl::StatusOr> process = + AsyncProcess::Spawn(args, nullptr, env); + if (!process.ok()) { + stderr_stream << "error spawning subprocess: " << process.status() << "\n"; + return 254; + } + absl::StatusOr result = + (*process)->WaitForTermination(); + if (!result.ok()) { + stderr_stream << "error waiting for subprocess: " << result.status() + << "\n"; + return 254; + } + stdout_stream << result->stdout; + stderr_stream << result->stderr; + return result->exit_code; +} + +AsyncProcess::AsyncProcess( + pid_t pid, std::vector argv, + std::unique_ptr response_file, + std::vector allocated_environ, + std::future> output) + : pid_(pid), + argv_(argv), + response_file_(std::move(response_file)), + allocated_environ_(std::move(allocated_environ)), + output_(std::move(output)) {} + +AsyncProcess::~AsyncProcess() { + for (char *arg : argv_) { + free(arg); + } + for (char *envp : allocated_environ_) { + free(envp); + } +} - // Set up a pipe to redirect stderr from the child process so that we can - // capture it and return it in the response message. +absl::StatusOr> AsyncProcess::Spawn( + const std::vector &normal_args, + std::unique_ptr response_file, + const absl::flat_hash_map *env) { + // Set up a pipe to redirect stdout and stderr from the child process so that + // we can redirect them to the given streams. std::unique_ptr redirector = - PosixSpawnIORedirector::Create(stdout_to_stderr); + PosixSpawnIORedirector::Create(); if (!redirector) { - (*stderr_stream) << "Error creating stderr pipe for child process.\n"; - return 254; + return absl::UnknownError("Failed to create pipes for child process"); + } + + std::vector exec_argv = ConvertToCArgs(normal_args); + if (response_file) { + exec_argv.back() = + strdup(absl::StrCat("@", response_file->GetPath()).c_str()); + exec_argv.push_back(nullptr); } char **envp; std::vector new_environ; - if (env) { // Copy the environment as an array of C strings, with guaranteed cleanup // below whenever we exit. for (const auto &[key, value] : *env) { - std::string pair = key + "=" + value; - char* c_str = new char[pair.length() + 1]; - std::strcpy(c_str, pair.c_str()); - new_environ.push_back(c_str); + new_environ.push_back(strdup(absl::StrCat(key, "=", value).c_str())); } - new_environ.push_back(nullptr); envp = new_environ.data(); } else { @@ -399,43 +274,54 @@ int RunSubProcess(const std::vector &args, } pid_t pid; - int status = - posix_spawn(&pid, args[0].c_str(), redirector->PosixSpawnFileActions(), - nullptr, const_cast(exec_argv.data()), envp); - redirector->ConsumeAllSubprocessOutput(stderr_stream); - - for (char* envp : new_environ) { - if (envp) { - delete[] envp; - } + int result = + posix_spawn(&pid, exec_argv[0], redirector->PosixSpawnFileActions(), + nullptr, exec_argv.data(), envp); + if (result != 0) { + return absl::Status(absl::ErrnoToStatusCode(errno), + "Failed to spawn child process"); } - if (status == 0) { - int wait_status; - do { - wait_status = waitpid(pid, &status, 0); - } while ((wait_status == -1) && (errno == EINTR)); - - if (wait_status < 0) { - (*stderr_stream) << "error: waiting on child process '" << args[0] - << "'. " << strerror(errno) << "\n"; - return wait_status; - } - - if (WIFEXITED(status)) { - return WEXITSTATUS(status); - } - - if (WIFSIGNALED(status)) { - return WTERMSIG(status); - } + // Start an asynchronous task in the background that reads the output from the + // stdout/stderr pipes while the process is running. + std::future> output = + std::async(std::launch::async, [r = std::move(redirector)]() { + std::ostringstream stdout_output; + std::ostringstream stderr_output; + r->ConsumeAllSubprocessOutput(stdout_output, stderr_output); + return std::make_pair(stdout_output.str(), stderr_output.str()); + }); + return std::unique_ptr( + new AsyncProcess(pid, exec_argv, std::move(response_file), new_environ, + std::move(output))); +} - // Unhandled case, if we hit this we should handle it above. - return 42; - } else { - (*stderr_stream) << "error: forking process failed '" << args[0] << "'. " - << strerror(status) << "\n"; - return status; +absl::StatusOr AsyncProcess::WaitForTermination() { + int status; + int wait_status; + do { + wait_status = waitpid(pid_, &status, 0); + } while ((wait_status == -1) && (errno == EINTR)); + + // Once the process has terminated, wait for the output to be read and prepare + // the result. + std::pair output = output_.get(); + Result result{0, std::move(output.first), std::move(output.second)}; + + if (wait_status < 0) { + return absl::Status(absl::ErrnoToStatusCode(errno), + "error waiting on child process"); + } + if (WIFEXITED(status)) { + result.exit_code = WEXITSTATUS(status); + return result; } + if (WIFSIGNALED(status)) { + result.exit_code = WTERMSIG(status); + return result; + } + // If we get here, we should add a case to handle it above instead. + return absl::UnknownError(absl::StrCat("Unexpected wait status: ", status)); } -#endif + +} // namespace bazel_rules_swift diff --git a/tools/common/process.h b/tools/common/process.h index c9c57d745..48eb53c94 100644 --- a/tools/common/process.h +++ b/tools/common/process.h @@ -15,19 +15,93 @@ #ifndef BUILD_BAZEL_RULES_SWIFT_TOOLS_WRAPPERS_PROCESS_H #define BUILD_BAZEL_RULES_SWIFT_TOOLS_WRAPPERS_PROCESS_H -#include +#include + +#include // NOLINT +#include +#include #include +#include #include +#include "absl/container/flat_hash_map.h" +#include "absl/status/statusor.h" +#include "tools/common/temp_file.h" + +namespace bazel_rules_swift { + // Spawns a subprocess for given arguments args and waits for it to terminate. -// The first argument is used for the executable path. If stdout_to_stderr is -// set, then stdout is redirected to the stderr stream as well. Returns the exit -// code of the spawned process. +// The first element in args is used for the executable path. If env is nullptr, +// then the current process's environment is used; otherwise, the new +// environment is used. Returns the exit code of the spawned process. It is a +// convenience wrapper around `AsyncProcess::Spawn` and +// `AsyncProcess::WaitForTermination`. int RunSubProcess(const std::vector &args, - std::map *env, - std::ostream *stderr_stream, bool stdout_to_stderr = false); + const absl::flat_hash_map *env, + std::ostream &stdout_stream, std::ostream &stderr_stream); // Returns a hash map containing the current process's environment. -std::map GetCurrentEnvironment(); +absl::flat_hash_map GetCurrentEnvironment(); + +// A wrapper around a subprocess that, when spawned, runs and reads stdout and +// stderr asynchronously. +class AsyncProcess { + public: + // A value containing the result of the subprocess's execution. + struct Result { + int exit_code; + std::string stdout; + std::string stderr; + }; + + // Spawns a subprocess with the given arguments, an optional response file + // containing additional arguments, and an optional environment. If the + // response file is provided, the `AsyncProcess` will take ownership of it and + // ensure that it is not deleted until the lifetime of the `AsyncSubprocess` + // has ended. If `env == nullptr`, the current process's environment is + // inherited. + static absl::StatusOr> Spawn( + const std::vector &normal_args, + std::unique_ptr response_file, + const absl::flat_hash_map *env); + + // Explicitly make `AsyncProcess` non-copyable and movable. + AsyncProcess(const AsyncProcess &) = delete; + AsyncProcess &operator=(const AsyncProcess &) = delete; + AsyncProcess(AsyncProcess &&) = default; + AsyncProcess &operator=(AsyncProcess &&) = default; + + ~AsyncProcess(); + + // Waits for the subprocess to terminate and returns its exit code. + absl::StatusOr WaitForTermination(); + + private: + // Constructs a new AsyncProcess; only used by the `Spawn` factory function. + AsyncProcess(pid_t pid, std::vector argv, + std::unique_ptr response_file, + std::vector allocated_environ, + std::future> output); + + // The pid of the spawned subprocess. + pid_t pid_; + + // The arguments to the subprocess, which must remain valid for the lifetime + // of the process. + std::vector argv_; + + // The response file containing additional arguments to pass to the + // subprocess, which must remain valid for the lifetime of the process. + std::unique_ptr response_file_; + + // The environment variables to set when launching the subprocess, which must + // remain valid for the lifetime of the process. + std::vector allocated_environ_; + + // The I/O redirector that captures the subprocess's stdout and stderr. + std::future> output_; +}; + +} // namespace bazel_rules_swift #endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_WRAPPERS_PROCESS_H diff --git a/tools/common/status.cc b/tools/common/status.cc new file mode 100644 index 000000000..3d1cc641f --- /dev/null +++ b/tools/common/status.cc @@ -0,0 +1,69 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "tools/common/status.h" + +#include + +#include "absl/strings/str_format.h" +#include "absl/strings/string_view.h" + +namespace bazel_rules_swift { + +absl::Status MakeStatusFromErrno(absl::string_view message) { + absl::StatusCode status_code; + int preserved_errno = errno; + + switch (preserved_errno) { + case ECANCELED: + status_code = absl::StatusCode::kCancelled; + break; + case EINVAL: + status_code = absl::StatusCode::kInvalidArgument; + break; + case ETIMEDOUT: + status_code = absl::StatusCode::kDeadlineExceeded; + break; + case ENOENT: + status_code = absl::StatusCode::kNotFound; + break; + case EEXIST: + status_code = absl::StatusCode::kAlreadyExists; + break; + case EACCES: + status_code = absl::StatusCode::kPermissionDenied; + break; + case ENOMEM: + status_code = absl::StatusCode::kResourceExhausted; + break; + case ENOTSUP: + status_code = absl::StatusCode::kFailedPrecondition; + break; + case ERANGE: + status_code = absl::StatusCode::kOutOfRange; + break; + default: + status_code = absl::StatusCode::kUnknown; + } + + const int kErrBufferSize = 512; + char err_buffer[kErrBufferSize] = {0}; + strerror_r(preserved_errno, err_buffer, kErrBufferSize); + + return absl::Status(status_code, + absl::StrFormat("%s (errno %d: %s)", message, + preserved_errno, err_buffer)); +} + +} // namespace bazel_rules_swift diff --git a/tools/common/status.h b/tools/common/status.h new file mode 100644 index 000000000..d2b58d5cd --- /dev/null +++ b/tools/common/status.h @@ -0,0 +1,27 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_STATUS_H_ +#define BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_STATUS_H_ + +#include "absl/status/status.h" + +namespace bazel_rules_swift { + +// Returns a `Status` value based on the current value of `errno`. +absl::Status MakeStatusFromErrno(absl::string_view message); + +} // namespace bazel_rules_swift + +#endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_STATUS_H_ diff --git a/tools/common/target_triple.h b/tools/common/target_triple.h new file mode 100644 index 000000000..c42aea4ba --- /dev/null +++ b/tools/common/target_triple.h @@ -0,0 +1,97 @@ +// Copyright 2025 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_TARGET_TRIPLE_H +#define BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_TARGET_TRIPLE_H + +#include +#include +#include +#include + +#include "absl/strings/str_cat.h" +#include "absl/strings/str_join.h" +#include "absl/strings/str_split.h" +#include "absl/strings/string_view.h" + +namespace bazel_rules_swift { + +// Represents a target triple as used by LLVM/Swift and provides operations to +// query and modify it. +class TargetTriple { + public: + // Creates a new target triple from the given components. + TargetTriple(absl::string_view arch, absl::string_view vendor, + absl::string_view os, absl::string_view environment) + : arch_(arch), + vendor_(vendor), + os_(os), + environment_(environment) {} + + // Parses the given target triple string into its component parts. + static std::optional Parse(absl::string_view target_triple) { + std::vector components = + absl::StrSplit(target_triple, '-'); + if (components.size() < 3) { + return std::nullopt; + } + return TargetTriple(components[0], components[1], components[2], + components.size() > 3 ? components[3] : ""); + } + + // Returns the architecture component of the target triple. + std::string Arch() const { return arch_; } + + // Returns the vendor component of the target triple. + std::string Vendor() const { return vendor_; } + + // Returns the OS component of the target triple. + std::string OS() const { return os_; } + + // Returns the environment component of the target triple. + std::string Environment() const { return environment_; } + + // Returns this target triple as a string. + std::string TripleString() const { + std::string result = absl::StrJoin({arch_, vendor_, os_}, "-"); + if (!environment_.empty()) { + absl::StrAppend(&result, "-", environment_); + } + return result; + } + + // Returns a copy of this target triple with the version number removed from + // the OS component (if any). + TargetTriple WithoutOSVersion() const { + std::pair os_and_version = + absl::StrSplit(os_, absl::MaxSplits(absl::ByAnyChar("0123456789"), 1)); + return TargetTriple(arch_, vendor_, os_and_version.first, environment_); + } + + // Returns a copy of this target triple, replacing its architecture with the + // given value. + TargetTriple WithArch(absl::string_view arch) const { + return TargetTriple(arch, vendor_, os_, environment_); + } + + private: + std::string arch_; + std::string vendor_; + std::string os_; + std::string environment_; +}; + +} // namespace bazel_rules_swift + +#endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_TARGET_TRIPLE_H diff --git a/tools/common/temp_file.h b/tools/common/temp_file.h index cade09496..325c11ad8 100644 --- a/tools/common/temp_file.h +++ b/tools/common/temp_file.h @@ -15,71 +15,56 @@ #ifndef BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_TEMP_FILE_H #define BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_TEMP_FILE_H -#include +#include +#include +#include + #include -#include #include -#include -#include #include #include #include -#if defined(_WIN32) -#include -#include -#endif -#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) -#include -#endif +#include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" + +namespace bazel_rules_swift { // An RAII temporary file. class TempFile { public: - // Explicitly make TempFile non-copyable and movable. - TempFile(const TempFile &) = delete; - TempFile &operator=(const TempFile &) = delete; - TempFile(TempFile &&) = default; - TempFile &operator=(TempFile &&) = default; - // Create a new temporary file using the given path template string (the same // form used by `mkstemp`). The file will automatically be deleted when the // object goes out of scope. - static std::unique_ptr Create(const std::string &path_template) { - std::error_code ec; - std::filesystem::path temporary{std::filesystem::temp_directory_path(ec) / path_template}; - if (ec) - return nullptr; - - std::string path = temporary.string(); - -#if defined(_WIN32) - if (errno_t error = _mktemp_s(path.data(), path.size() + 1)) { - std::cerr << "Failed to create temporary file '" << temporary - << "': " << strerror(error) << "\n"; - return nullptr; + static std::unique_ptr Create(absl::string_view path_template) { + absl::string_view tmp_dir; + if (const char *env_value = getenv("TMPDIR")) { + tmp_dir = env_value; + } else { + tmp_dir = "/tmp"; } -#else - if (::mkstemp(path.data()) < 0) { - std::cerr << "Failed to create temporary file '" << temporary - << "': " << strerror(errno) << "\n"; + std::string path = absl::StrCat(tmp_dir, "/", path_template); + if (mkstemp(const_cast(path.c_str())) == -1) { + std::cerr << "Failed to create temporary file '" << path + << "': " << strerror(errno) << std::endl; return nullptr; } -#endif - return std::unique_ptr(new TempFile(path)); } - ~TempFile() { - std::error_code ec; - std::filesystem::remove(path_, ec); - } + // Explicitly make TempFile non-copyable and movable. + TempFile(const TempFile &) = delete; + TempFile &operator=(const TempFile &) = delete; + TempFile(TempFile &&) = default; + TempFile &operator=(TempFile &&) = default; + + ~TempFile() { remove(path_.c_str()); } // Gets the path to the temporary file. - std::string GetPath() const { return path_; } + absl::string_view GetPath() const { return path_; } private: - explicit TempFile(const std::string &path) : path_(path) {} + explicit TempFile(absl::string_view path) : path_(path) {} std::string path_; }; @@ -87,69 +72,67 @@ class TempFile { // An RAII temporary directory that is recursively deleted. class TempDirectory { public: - // Explicitly make TempDirectory non-copyable and movable. - TempDirectory(const TempDirectory &) = delete; - TempDirectory &operator=(const TempDirectory &) = delete; - TempDirectory(TempDirectory &&) = default; - TempDirectory &operator=(TempDirectory &&) = default; - // Create a new temporary directory using the given path template string (the // same form used by `mkdtemp`). The file will automatically be deleted when // the object goes out of scope. static std::unique_ptr Create( - const std::string &path_template) { - std::error_code ec; - std::filesystem::path temporary{std::filesystem::temp_directory_path(ec) / path_template}; - if (ec) - return nullptr; - - std::string path = temporary.string(); - -#if defined(_WIN32) - if (memcmp(path.data() + path.length() - 6, "XXXXXX", 6)) { - std::cerr << "Failed to create temporary directory '" << temporary - << "': invalid parameter\n"; - return nullptr; - } - - auto randname = [](char *buffer) { - static const char kAlphabet[] = - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ01123456789"; - constexpr size_t kAlphabetSize = sizeof(kAlphabet) - 1; - - for (unsigned index = 0; index < 6; ++index) - buffer[index] = kAlphabet[rand() % kAlphabetSize]; - }; - - srand(reinterpret_cast(path.data())); - for (unsigned retry = 256; retry; --retry) { - randname(path.data() + path.length() - 6); - if (!_mkdir(path.c_str())) - break; + absl::string_view path_template) { + absl::string_view tmp_dir; + if (const char *env_value = getenv("TMPDIR")) { + tmp_dir = env_value; + } else { + tmp_dir = "/tmp"; } -#else - if (::mkdtemp(path.data()) == nullptr) { - std::cerr << "Failed to create temporary directory '" << temporary - << "': " << strerror(errno) << "\n"; + std::string path = absl::StrCat(tmp_dir, "/", path_template); + if (mkdtemp(const_cast(path.c_str())) == nullptr) { + std::cerr << "Failed to create temporary directory '" << path + << "': " << strerror(errno) << std::endl; return nullptr; } -#endif - return std::unique_ptr(new TempDirectory(path)); } + // Explicitly make TempDirectory non-copyable and movable. + TempDirectory(const TempDirectory &) = delete; + TempDirectory &operator=(const TempDirectory &) = delete; + TempDirectory(TempDirectory &&) = default; + TempDirectory &operator=(TempDirectory &&) = default; + ~TempDirectory() { - std::error_code ec; - std::filesystem::remove_all(path_, ec); + char *files[] = {(char *)path_.c_str(), nullptr}; + // Don't have the walk change directories, don't traverse symlinks, and + // don't cross devices. + auto fts_handle = + fts_open(files, FTS_NOCHDIR | FTS_PHYSICAL | FTS_XDEV, nullptr); + if (!fts_handle) { + return; + } + + FTSENT *entry; + while ((entry = fts_read(fts_handle))) { + switch (entry->fts_info) { + case FTS_F: // regular file + case FTS_SL: // symlink + case FTS_SLNONE: // symlink without target + case FTS_DP: // directory, post-order (after traversing children) + case FTS_DEFAULT: // other non-error conditions + remove(entry->fts_accpath); + break; + } + } + + fts_close(fts_handle); } // Gets the path to the temporary directory. - std::string GetPath() const { return path_; } + absl::string_view GetPath() const { return path_; } private: - explicit TempDirectory(const std::string &path) : path_(path) {} + explicit TempDirectory(absl::string_view path) : path_(path) {} std::string path_; }; +} // namespace bazel_rules_swift + #endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_COMMON_TEMP_FILE_H diff --git a/tools/worker/BUILD b/tools/worker/BUILD index 1542041b5..8d71cb86f 100644 --- a/tools/worker/BUILD +++ b/tools/worker/BUILD @@ -35,9 +35,21 @@ cc_library( }), deps = [ "//tools/common:bazel_substitutions", + "//tools/common:color", + "//tools/common:file_system", + "//tools/common:path_utils", "//tools/common:process", + "//tools/common:target_triple", "//tools/common:temp_file", + "@abseil-cpp//absl/base:nullability", + "@abseil-cpp//absl/container:btree", + "@abseil-cpp//absl/container:flat_hash_map", + "@abseil-cpp//absl/container:flat_hash_set", + "@abseil-cpp//absl/status", + "@abseil-cpp//absl/status:statusor", + "@abseil-cpp//absl/strings", "@com_github_nlohmann_json//:json", + "@re2", ], ) @@ -75,4 +87,4 @@ filegroup( visibility = [ "//tools:__pkg__", ], -) \ No newline at end of file +) diff --git a/tools/worker/main.cc b/tools/worker/main.cc index 34cb3cb09..39c5f055d 100644 --- a/tools/worker/main.cc +++ b/tools/worker/main.cc @@ -12,46 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include #include #include #include -#include "tools/common/process.h" -#include "tools/cpp/runfiles/runfiles.h" #include "tools/worker/swift_runner.h" -using bazel::tools::cpp::runfiles::Runfiles; - int main(int argc, char *argv[]) { - std::string index_import_path; - - // Find the index-import tool from runfiles if available - #ifdef BAZEL_CURRENT_REPOSITORY - std::unique_ptr runfiles(Runfiles::Create(argv[0], BAZEL_CURRENT_REPOSITORY)); - #else - std::unique_ptr runfiles(Runfiles::Create(argv[0])); - #endif - - if (runfiles != nullptr) { - // TODO: Remove once we drop support for Xcode 16.x. - // Determine which version of index-import to use based on the environment - auto env = GetCurrentEnvironment(); - if (env.find("__RULES_SWIFT_USE_LEGACY_INDEX_IMPORT") != env.end()) { - index_import_path = runfiles->Rlocation( - "build_bazel_rules_swift_index_import_5_8/index-import"); - } else { - index_import_path = runfiles->Rlocation( - "build_bazel_rules_swift_index_import_6_1/index-import"); - } - } - - auto args = std::vector(argv + 1, argv + argc); - - // Filter out the --persistent_worker flag if present (no longer supported) - args.erase(std::remove(args.begin(), args.end(), "--persistent_worker"), args.end()); - - // Run the Swift compiler with the provided arguments - return SwiftRunner(args, index_import_path) - .Run(&std::cerr, /*stdout_to_stderr=*/false); -} \ No newline at end of file + std::vector args(argv + 1, argv + argc); + return bazel_rules_swift::SwiftRunner(args).Run(std::cout, std::cerr); +} diff --git a/tools/worker/swift_runner.cc b/tools/worker/swift_runner.cc index bf863be55..4f3609085 100644 --- a/tools/worker/swift_runner.cc +++ b/tools/worker/swift_runner.cc @@ -14,33 +14,166 @@ #include "tools/worker/swift_runner.h" -#include +#include + +#include #include +#include +#include +#include +#include +#include +#include +#include +#include +#include -#include "tools/common/bazel_substitutions.h" +#include "absl/base/nullability.h" +#include "absl/container/btree_set.h" +#include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_set.h" +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "absl/strings/match.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/str_split.h" +#include "absl/strings/string_view.h" +#include "absl/strings/strip.h" +#include "absl/strings/substitute.h" +#include "tools/common/color.h" +#include "tools/common/file_system.h" +#include "tools/common/path_utils.h" #include "tools/common/process.h" +#include "tools/common/target_triple.h" #include "tools/common/temp_file.h" -#include "tools/worker/output_file_map.h" +#include "nlohmann/json.hpp" +#include "re2/re2.h" -bool ArgumentEnablesWMO(const std::string &arg) { - return arg == "-wmo" || arg == "-whole-module-optimization" || - arg == "-force-single-frontend-invocation"; -} +namespace bazel_rules_swift { namespace { +// Extracts frontend command lines from the driver output and groups them into +// buckets that can be run based on the incoming `-compile-step` flag. +class CompilationPlan { + public: + // Creates a new compilation plan by parsing the given driver output. + CompilationPlan(absl::string_view print_jobs_output); + + // Returns the list of module jobs extracted from the plan. Each job is a + // command line that should be invoked to emit some module-wide output. + const std::vector &ModuleJobs() const { return module_jobs_; } + + // Returns the codegen job that is associated with the given output file, or + // `nullopt` if none was found. The job is a command line that should be + // invoked to emit some codegen-related output. + std::vector CodegenJobsForOutputs( + std::vector outputs) const; + + private: + // The command lines of any frontend jobs that emit a module or other + // module-wide outputs, executed when the compilation step is + // `SwiftCompileModule`. These are executed in sequence. + std::vector module_jobs_; + + // The command lines of any frontend jobs that emit codegen output, like + // object files. These are mapped to the output path by + // `codegen_job_indices_by_output_`. + std::vector codegen_jobs_; + + // The indices into `codegen_jobs_` of the command lines of any frontend jobs + // that emit codegen output for some given output path. + absl::flat_hash_map codegen_job_indices_by_output_; +}; + +CompilationPlan::CompilationPlan(absl::string_view print_jobs_output) { + // Looks for the `-o` flags in the command line and captures the path to that + // file. This captures both regular paths (group 2) and single-quoted paths + // (group 1). + RE2 output_pattern("\\s-o\\s+(?:'((?:\\'|[^'])*)'|(\\S+))"); + for (absl::string_view command_line : + absl::StrSplit(print_jobs_output, '\n')) { + if (command_line.empty()) { + continue; + } + + // If the driver created a response file for the frontend invocation, then + // it prints the actual arguments with a shell comment-like notation. This + // is good for job scanning because we don't have to read the response files + // to find the invocations for various output files, but when we invoke it + // we need to strip that off because we aren't spawning like a shell; it + // would interpret the `#` and everything that follows as regular arguments. + // If the comment marker isn't there, then this logic also works because + // `first` will be the same as the original string. + std::pair possible_response_file = + absl::StrSplit(command_line, absl::MaxSplits(" # ", 1)); + absl::string_view command_line_without_expansions = + possible_response_file.first; + + if (absl::StrContains(command_line, " -c ")) { + int index = codegen_jobs_.size(); + codegen_jobs_.push_back(std::string(command_line_without_expansions)); + + // When threaded WMO is enabled, a single invocation might emit multiple + // object files. Associate them with the same command line so that they + // are deduplicated. + std::string quoted_path, normal_path; + absl::string_view anchor = command_line; + while (RE2::FindAndConsume(&anchor, output_pattern, "ed_path, + &normal_path)) { + codegen_job_indices_by_output_[!quoted_path.empty() ? quoted_path + : normal_path] = + index; + } + } else { + module_jobs_.push_back(std::string(command_line_without_expansions)); + } + } +} + +std::vector CompilationPlan::CodegenJobsForOutputs( + std::vector outputs) const { + // Fast-path: If there is only one batch, there's no reason to iterate over + // all of these. The build rules use an empty string to represent this case. + if (outputs.empty()) { + return codegen_jobs_; + } + + absl::btree_set indices; + for (absl::string_view desired_output : outputs) { + for (const auto &[output, index] : codegen_job_indices_by_output_) { + // We need to do a suffix search here because the driver may have + // realpath-ed the output argument, giving us something like + // `/bazel-out/...` when we're just expecting + // `bazel-out/...`. + if (absl::EndsWith(output, desired_output)) { + indices.insert(index); + break; + } + } + } + + std::vector jobs; + jobs.reserve(indices.size()); + for (int index : indices) { + jobs.push_back(codegen_jobs_[index]); + } + return jobs; +} + // Creates a temporary file and writes the given arguments to it, one per line. static std::unique_ptr WriteResponseFile( const std::vector &args) { - auto response_file = TempFile::Create("swiftc_params.XXXXXX"); - std::ofstream response_file_stream(response_file->GetPath()); + std::unique_ptr response_file = + TempFile::Create("swiftc_params.XXXXXX"); + std::ofstream response_file_stream(std::string(response_file->GetPath())); - for (const auto &arg : args) { + for (absl::string_view arg : args) { // When Clang/Swift write out a response file to communicate from driver to // frontend, they just quote every argument to be safe; we duplicate that // instead of trying to be "smarter" and only quoting when necessary. response_file_stream << '"'; - for (auto ch : arg) { + for (char ch : arg) { if (ch == '"' || ch == '\\') { response_file_stream << '\\'; } @@ -53,30 +186,64 @@ static std::unique_ptr WriteResponseFile( return response_file; } -// Unescape and unquote an argument read from a line of a response file. -static std::string Unescape(const std::string &arg) { +// Creates a temporary file and writes the given command line string to it +// without any additional processing. +static std::unique_ptr WriteDirectResponseFile( + absl::string_view args) { + std::unique_ptr response_file = + TempFile::Create("swiftc_params.XXXXXX"); + std::ofstream response_file_stream(std::string(response_file->GetPath())); + response_file_stream << args; + response_file_stream.close(); + return response_file; +} + +// Consumes and returns a single argument from the given command line (skipping +// any leading whitespace and also handling quoted/escaped arguments), advancing +// the view to the end of the argument in a similar fashion to +// `absl::ConsumePrefix()`. +static std::optional ConsumeArg( + absl::Nonnull line) { + size_t whitespace_count = 0; + size_t length = line->size(); + while (whitespace_count < length && (*line)[whitespace_count] == ' ') { + whitespace_count++; + } + if (whitespace_count > 0) { + line->remove_prefix(whitespace_count); + } + + if (line->empty()) { + return std::nullopt; + } + std::string result; - auto length = arg.size(); - for (size_t i = 0; i < length; ++i) { - auto ch = arg[i]; + length = line->size(); + size_t i = 0; + for (i = 0; i < length; ++i) { + char ch = (*line)[i]; + + if (ch == ' ') { + break; + } // If it's a backslash, consume it and append the character that follows. if (ch == '\\' && i + 1 < length) { ++i; - result.push_back(arg[i]); + result.push_back((*line)[i]); continue; } // If it's a quote, process everything up to the matching quote, unescaping // backslashed characters as needed. if (ch == '"' || ch == '\'') { - auto quote = ch; + char quote = ch; ++i; - while (i != length && arg[i] != quote) { - if (arg[i] == '\\' && i + 1 < length) { + while (i != length && (*line)[i] != quote) { + if ((*line)[i] == '\\' && i + 1 < length) { ++i; } - result.push_back(arg[i]); + result.push_back((*line)[i]); ++i; } if (i == length) { @@ -89,375 +256,868 @@ static std::string Unescape(const std::string &arg) { result.push_back(ch); } + line->remove_prefix(i); return result; } -// If `str` starts with `prefix`, `str` is mutated to remove `prefix` and the -// function returns true. Otherwise, `str` is left unmodified and the function -// returns `false`. -static bool StripPrefix(const std::string &prefix, std::string &str) { - if (str.find(prefix) != 0) { - return false; +// Unescape and unquote an argument read from a line of a response file. +static std::string Unescape(absl::string_view arg) { + return ConsumeArg(&arg).value_or(""); +} + +// Reads the list of module names that are direct dependencies of the code being +// compiled. +absl::btree_set ReadDepsModules(absl::string_view path) { + absl::btree_set deps_modules; + std::ifstream deps_file_stream(std::string(path.data(), path.size())); + std::string line; + while (std::getline(deps_file_stream, line)) { + deps_modules.insert(std::string(line)); } - str.erase(0, prefix.size()); - return true; + return deps_modules; } -} // namespace +#if __APPLE__ +// Returns true if the given argument list starts with an invocation of `xcrun`. +bool StartsWithXcrun(const std::vector &args) { + return !args.empty() && Basename(args[0]) == "xcrun"; +} +#endif -SwiftRunner::SwiftRunner(const std::vector &args, - std::string index_import_path, bool force_response_file) - : job_env_(GetCurrentEnvironment()), - index_import_path_(index_import_path), - force_response_file_(force_response_file), - is_dump_ast_(false), - file_prefix_pwd_is_dot_(false) { - args_ = ProcessArguments(args); +// Spawns an executable, constructing the command line by writing `args` to a +// response file and concatenating that after `tool_args` (which are passed +// outside the response file). +int SpawnJob(const std::vector &tool_args, + const std::vector &args, + const absl::flat_hash_map *env, + std::ostream &stdout_stream, std::ostream &stderr_stream) { + std::unique_ptr response_file = WriteResponseFile(args); + + std::vector spawn_args(tool_args); + spawn_args.push_back(absl::StrCat("@", response_file->GetPath())); + return RunSubProcess(spawn_args, env, stdout_stream, stderr_stream); } -int SwiftRunner::Run(std::ostream *stderr_stream, bool stdout_to_stderr) { - // In rules_swift < 3.x the .swiftsourceinfo files are unconditionally written to the module path. - // In rules_swift >= 3.x these same files are no longer tracked by Bazel unless explicitly requested. - // When using non-sandboxed mode, previous builds will contain these files and cause build failures - // when Swift tries to use them, in order to work around this compatibility issue, we check the module path for - // the presence of .swiftsourceinfo files and if they are present but not requested, we remove them. - if (swift_source_info_path_ != "" && !emit_swift_source_info_) { - std::filesystem::remove(swift_source_info_path_); +// Logs an internal error message that occurred during compilation planning and +// provides users with a workaround. +void LogCompilePlanError(std::ostream &stderr_stream, + absl::string_view message) { + WithColor(stderr_stream, Color::kBoldRed) << "Internal planning error: "; + WithColor(stderr_stream, Color::kBold) << message << std::endl; + WithColor(stderr_stream, Color::kBold) + << "You can work around this bug by adding `features = " + "[\"-swift.compile_in_parallel\"] to the affected target until the " + "bug is fixed." + << std::endl; +} + +// Executes the module-wide jobs in a compilation plan. +int SpawnCompileModuleStep( + const CompilationPlan &plan, CompileStep compile_step, + const absl::flat_hash_map *env, + std::ostream &stdout_stream, std::ostream &stderr_stream) { + // If we're trying to execute a SwiftCompileModule step but there aren't any + // module jobs, then there was a bug in the planning phase. + if (plan.ModuleJobs().empty()) { + LogCompilePlanError(stderr_stream, + "Attempting to execute a SwiftCompileModule step but " + "there are no module-wide jobs."); + return 1; } - int exit_code = RunSubProcess( - args_, &job_env_, stderr_stream, stdout_to_stderr); + // Run module jobs sequentially, in case later ones have dependencies on the + // outputs of earlier ones. + for (absl::string_view job : plan.ModuleJobs()) { + std::pair tool_and_args = + absl::StrSplit(job, absl::MaxSplits(' ', 1)); + std::vector step_args{std::string(tool_and_args.first)}; + // We can write the rest of the string out to a response file directly; + // there is no need to split it into individual arguments (and in fact, + // doing so would need to be quotation-aware, since the driver will quote + // arguments that contain spaces). + std::unique_ptr response_file = + WriteDirectResponseFile(tool_and_args.second); + step_args.push_back(absl::StrCat("@", response_file->GetPath())); + int exit_code = RunSubProcess(step_args, env, stdout_stream, stderr_stream); + if (exit_code != 0) { + return exit_code; + } + } + return 0; +} + +// Executes the codegen jobs in a compilation plan. +int SpawnCompileCodegenStep( + const CompilationPlan &plan, CompileStep compile_step, + const absl::flat_hash_map *env, + std::ostream &stdout_stream, std::ostream &stderr_stream) { + // Run codegen jobs in parallel, since they should be independent of each + // other and they are slower so they benefit more from parallelism. + std::vector> processes; + std::vector jobs = plan.CodegenJobsForOutputs( + // Work around awkward legacy behavior in absl::StrSplit() that causes an + // empty string to be split into a single empty string instead of an empty + // array. + compile_step.output.empty() ? std::vector() + : absl::StrSplit(compile_step.output, ',')); + if (jobs.empty()) { + LogCompilePlanError( + stderr_stream, + absl::Substitute("Could not find the frontend command for action $0 " + "for some requested output in $1.", + compile_step.action, compile_step.output)); + return 1; + } + for (absl::string_view job : jobs) { + std::pair tool_and_args = + absl::StrSplit(job, absl::MaxSplits(' ', 1)); + std::vector step_args{std::string(tool_and_args.first)}; + + // We can write the rest of the string out to a response file directly; + // there is no need to split it into individual arguments (and in fact, + // doing so would need to be quotation-aware, since the driver will quote + // arguments that contain spaces). + std::unique_ptr response_file = + WriteDirectResponseFile(absl::StrCat(tool_and_args.second)); + absl::StatusOr> process = + AsyncProcess::Spawn(step_args, std::move(response_file), env); + if (!process.ok()) { + LogCompilePlanError( + stderr_stream, + absl::Substitute("Could not spawn subprocess: $0.", + process.status().ToString( + absl::StatusToStringMode::kWithEverything))); + return 1; + } + processes.emplace_back(std::move(*process)); + } + + int any_failing_exit_code = 0; + for (std::unique_ptr &process : processes) { + absl::StatusOr result = process->WaitForTermination(); + if (!result.ok()) { + LogCompilePlanError( + stderr_stream, + absl::Substitute("Error waiting for subprocess: $0.", + result.status().ToString( + absl::StatusToStringMode::kWithEverything))); + return 1; + } + stdout_stream << result->stdout; + stderr_stream << result->stderr; + if (result->exit_code != 0) { + // Don't return early if the job failed; if we have multiple jobs in the + // batch, we want the user to see possible diagnostics from all of them. + any_failing_exit_code = result->exit_code; + } + } + return any_failing_exit_code; +} + +// Spawns a single step in a parallelized compilation by getting a list of +// frontend jobs that the driver would normally spawn and then running the one +// that emits the output file for the requested plan step. +int SpawnPlanStep(const std::vector &tool_args, + const std::vector &args, + const absl::flat_hash_map *env, + CompileStep compile_step, std::ostream &stdout_stream, + std::ostream &stderr_stream) { + // Add `-driver-print-jobs` to the command line, which will cause the driver + // to print the command lines of the frontend jobs it would normally spawn and + // then exit without running them. + std::vector print_jobs_args(args); + print_jobs_args.push_back("-driver-print-jobs"); + // Ensure that the default TMPDIR is used by the driver for this job, not the + // one used to write macro expansions (which may not be accessible when that + // directory is not a declared output of the action in Bazel). + absl::flat_hash_map print_jobs_env(*env); + print_jobs_env.erase("TMPDIR"); + std::ostringstream captured_stdout_stream; + int exit_code = SpawnJob(tool_args, print_jobs_args, &print_jobs_env, + captured_stdout_stream, stderr_stream); if (exit_code != 0) { return exit_code; } - if (!generated_header_rewriter_path_.empty()) { -#if __APPLE__ - // Skip the `xcrun` argument that's added when running on Apple platforms. - int initial_args_to_skip = 1; -#else - int initial_args_to_skip = 0; -#endif + CompilationPlan plan(captured_stdout_stream.str()); + if (compile_step.action == "SwiftCompileModule") { + return SpawnCompileModuleStep(plan, compile_step, env, stdout_stream, + stderr_stream); + } + if (compile_step.action == "SwiftCompileCodegen") { + return SpawnCompileCodegenStep(plan, compile_step, env, stdout_stream, + stderr_stream); + } + + LogCompilePlanError( + stderr_stream, + absl::Substitute("Unrecognized plan step $0 with output $1.", + compile_step.action, compile_step.output)); + return 1; +} - std::vector rewriter_args; - rewriter_args.reserve(args_.size() + 2 - initial_args_to_skip); - rewriter_args.push_back(generated_header_rewriter_path_); - rewriter_args.push_back("--"); - rewriter_args.insert(rewriter_args.end(), - args_.begin() + initial_args_to_skip, args_.end()); +// Indicates which driver mode we are invoking as an additional invocation, +// which may require special handling of arguments. +enum class IncompatibleArgMode { + kLayeringCheck, + kJsonAst, +}; - exit_code = RunSubProcess( - rewriter_args, /*env=*/nullptr, stderr_stream, stdout_to_stderr); +// Returns a value indicating whether an argument on the Swift command line +// should be skipped because it is incompatible with specific driver modes that +// we invoke as additional invocations. The given iterator is also advanced if +// necessary past any additional flags (e.g., a path following a flag). +bool SkipIncompatibleArgs(IncompatibleArgMode mode, + std::vector::iterator &it) { + if (*it == "-emit-module" || *it == "-emit-module-interface" || + *it == "-emit-object" || *it == "-emit-objc-header" || + *it == "-emit-const-values" || *it == "-warnings-as-errors" || + *it == "-wmo" || *it == "-whole-module-optimization") { + // Skip just this argument. + return true; + } + if (*it == "-o" || *it == "-emit-module-path" || + *it == "-emit-module-interface-path" || *it == "-emit-objc-header-path" || + *it == "-emit-clang-header-path" || *it == "-emit-const-values-path" || + *it == "-num-threads") { + // This flag has a value after it that we also need to skip. + ++it; + return true; } - auto enable_global_index_store = global_index_store_import_path_ != ""; - if (enable_global_index_store) { - if (index_import_path_.empty()) { - (*stderr_stream) << "Failed to find index-import path from runfiles\n"; - return EXIT_FAILURE; + // Some flags we only want to skip when doing a layering check, but we still + // need them for JSON AST dumps. + if (mode == IncompatibleArgMode::kLayeringCheck) { + if (*it == "-output-file-map") { + // This flag has a value after it that we also need to skip. + ++it; + return true; } + } - OutputFileMap output_file_map; - output_file_map.ReadFromPath(output_file_map_path_); + // Don't skip the flag. + return false; +} - auto object_files = output_file_map.get_outputs_by_type("object"); +// Modules that can be imported without an explicit dependency. Specifically, +// the standard library is always provided, along with other modules that are +// distributed as part of the standard library even though they are separate +// modules. +static const absl::flat_hash_set + kModulesIgnorableForLayeringCheck = { + "Builtin", "Swift", "SwiftOnoneSupport", + "_Backtracing", "_Concurrency", "_StringProcessing", +}; - std::vector ii_args; - ii_args.push_back(index_import_path_); +// Returns true if the module can be ignored for the purposes of layering check +// (that is, it does not need to be in `deps` even if imported). +bool IsModuleIgnorableForLayeringCheck(absl::string_view module_name) { + return kModulesIgnorableForLayeringCheck.contains(module_name); +} - if (file_prefix_pwd_is_dot_) { - ii_args.push_back("-file-prefix-map"); - ii_args.push_back(std::filesystem::current_path().string() + "=."); +// Infers the path to the `.swiftinterface` file inside a `.swiftmodule` +// directory based on the given target triple. +// +// This roughly mirrors the logic in the Swift frontend's +// `forEachTargetModuleBasename` function at +// https://github.com/swiftlang/swift/blob/d36b06747a54689a09ca6771b04798fc42b3e701/lib/Serialization/SerializedModuleLoader.cpp#L55. +std::optional InferInterfacePath( + absl::string_view module_path, absl::string_view target_triple_string) { + std::optional parsed_triple = + TargetTriple::Parse(target_triple_string); + if (!parsed_triple.has_value()) { + return std::nullopt; + } + + // The target triple passed to us by the build rules has already been + // normalized (e.g., `macos` instead of `macosx`), so we don't have to do as + // much work here as the frontend normally would. + TargetTriple normalized_triple = parsed_triple->WithoutOSVersion(); + + // First, try the triple we were given. + std::string attempt = absl::Substitute("$0/$1.swiftinterface", module_path, + normalized_triple.TripleString()); + if (PathExists(attempt)) { + return attempt; + } + + // Next, if the target triple is `arm64`, we can also load an `arm64e` + // interface, so try that. + if (normalized_triple.Arch() == "arm64") { + TargetTriple arm64e_triple = normalized_triple.WithArch("arm64e"); + attempt = absl::Substitute("$0/$1.swiftinterface", module_path, + arm64e_triple.TripleString()); + if (PathExists(attempt)) { + return attempt; } + } + + return std::nullopt; +} - for (const auto& obj_file : object_files) { - ii_args.push_back("-import-output-file"); - ii_args.push_back(obj_file); +// Extracts flags from the given `.swiftinterface` file and passes them to the +// given consumer. +void ExtractFlagsFromInterfaceFile( + absl::string_view module_or_interface_path, absl::string_view target_triple, + std::function consumer) { + std::string interface_path; + if (absl::EndsWith(module_or_interface_path, ".swiftinterface")) { + interface_path = std::string(module_or_interface_path); + } else { + std::optional inferred_path = + InferInterfacePath(module_or_interface_path, target_triple); + if (!inferred_path.has_value()) { + return; } + interface_path = *inferred_path; + } + + // Add the path to the interface file as a source file argument, then extract + // the flags from it and add them as well. + consumer(interface_path); - const std::filesystem::path &exec_root = std::filesystem::current_path(); - // Copy back from the global index store to bazel's index store - ii_args.push_back((exec_root / global_index_store_import_path_).string()); - ii_args.push_back((exec_root / index_store_path_).string()); - exit_code = RunSubProcess( - ii_args, /*env=*/nullptr, stderr_stream, /*stdout_to_stderr=*/true); + std::ifstream interface_file{std::string(interface_path)}; + std::string line; + while (std::getline(interface_file, line)) { + absl::string_view line_view = line; + if (absl::ConsumePrefix(&line_view, "// swift-module-flags: ")) { + bool skip_next = false; + while (std::optional flag = ConsumeArg(&line_view)) { + if (skip_next) { + skip_next = false; + } else if (*flag == "-target") { + // We have to skip the target triple in the interface file because it + // might be slightly different from the one the rest of our + // dependencies were compiled with. For example, if we are targeting + // `arm64-apple-macos`, that is the architecture that any Clang + // module dependencies will have used. If the module uses + // `arm64e-apple-macos` instead, then it will not be compatible with + // those Clang modules. + skip_next = true; + } else { + consumer(*flag); + } + } + return; + } } - return exit_code; } -// Marker for end of iteration -class StreamIteratorEnd {}; +} // namespace -// Basic iterator over an ifstream -class StreamIterator { - public: - StreamIterator(std::ifstream &file) : file_{file} { next(); } +SwiftRunner::SwiftRunner(const std::vector &args, + bool force_response_file) + : job_env_(GetCurrentEnvironment()), + force_response_file_(force_response_file), + last_flag_was_module_name_(false), + last_flag_was_tools_directory_(false), + emit_json_ast_(false) { + ProcessArguments(args); +} - const std::string &operator*() const { return str_; } +int SwiftRunner::Run(std::ostream &stdout_stream, std::ostream &stderr_stream) { + int exit_code = 0; - StreamIterator &operator++() { - next(); - return *this; + // Do the layering check before compilation. This gives a better error message + // in the event a Swift module is being imported that depends on a Clang + // module that isn't already in the transitive closure, because that will fail + // to compile ("cannot load underlying module for '...'"). + // + // Note that this also means we have to do the layering check for all + // compilation actions (module and codegen). Otherwise, since they can be + // scheduled in either order, doing it only in one could cause error messages + // to differ if there are layering violations. + if (!deps_modules_path_.empty()) { + exit_code = PerformLayeringCheck(stdout_stream, stderr_stream); + if (exit_code != 0) { + return exit_code; + } } - bool operator!=(StreamIteratorEnd) const { return !!file_; } + bool should_rewrite_header = false; - private: - void next() { std::getline(file_, str_); } - - std::ifstream &file_; - std::string str_; -}; + // Spawn the originally requested job with its full argument list. Capture + // stderr in a string stream, which we post-process to upgrade warnings to + // errors if requested. + if (compile_step_.has_value()) { + std::ostringstream captured_stderr_stream; + exit_code = SpawnPlanStep(tool_args_, args_, &job_env_, *compile_step_, + stdout_stream, captured_stderr_stream); + ProcessDiagnostics(captured_stderr_stream.str(), stderr_stream, exit_code); + if (exit_code != 0) { + return exit_code; + } -class ArgsFile { - public: - ArgsFile(std::ifstream &file) : file_(file) {} + // Handle post-processing for specific kinds of actions. + if (compile_step_->action == "SwiftCompileModule") { + should_rewrite_header = true; + } + } else { + std::ostringstream captured_stderr_stream; + exit_code = SpawnJob(tool_args_, args_, &job_env_, stdout_stream, + captured_stderr_stream); + ProcessDiagnostics(captured_stderr_stream.str(), stderr_stream, exit_code); + if (exit_code != 0) { + return exit_code; + } + should_rewrite_header = true; + } - StreamIterator begin() { return StreamIterator{file_}; } + if (should_rewrite_header && !generated_header_rewriter_path_.empty()) { + exit_code = PerformGeneratedHeaderRewriting(stdout_stream, stderr_stream); + if (exit_code != 0) { + return exit_code; + } + } - StreamIteratorEnd end() { return StreamIteratorEnd{}; } + if (emit_json_ast_) { + PerformJsonAstDump(stdout_stream, stderr_stream); + } - private: - std::ifstream &file_; -}; + return exit_code; +} bool SwiftRunner::ProcessPossibleResponseFile( - const std::string &arg, std::function consumer) { - auto path = arg.substr(1); - std::ifstream original_file(path); - ArgsFile args_file(original_file); - + absl::string_view arg, std::function consumer) { + absl::string_view path = arg.substr(1); + std::ifstream original_file((std::string(path))); // If we couldn't open it, maybe it's not a file; maybe it's just some other - // argument that starts with "@" such as "@loader_path/..." + // argument that starts with "@". (Unlikely, but it's safer to check.) if (!original_file.good()) { consumer(arg); return false; } - // Read the file to a vector to prevent double I/O - auto args = ParseArguments(args_file); - // If we're forcing response files, process and send the arguments from this // file directly to the consumer; they'll all get written to the same response // file at the end of processing all the arguments. if (force_response_file_) { - for (auto it = args.begin(); it != args.end(); ++it) { + std::string arg_from_file; + while (std::getline(original_file, arg_from_file)) { // Arguments in response files might be quoted/escaped, so we need to // unescape them ourselves. - ProcessArgument(it, Unescape(*it), consumer); + ProcessArgument(Unescape(arg_from_file), consumer); } return true; } - // Otherwise, open the file, process the arguments, and rewrite it if any of - // them have changed. + // Otherwise, open the file and process the arguments. bool changed = false; std::string arg_from_file; - std::vector new_args; - for (auto it = args.begin(); it != args.end(); ++it) { - changed |= ProcessArgument(it, *it, [&](const std::string &processed_arg) { - new_args.push_back(processed_arg); - }); - } - if (changed) { - auto new_file = WriteResponseFile(new_args); - consumer("@" + new_file->GetPath()); - temp_files_.push_back(std::move(new_file)); - } else { - // If none of the arguments changed, just keep the original response file - // argument. - consumer(arg); + while (std::getline(original_file, arg_from_file)) { + changed |= ProcessArgument(arg_from_file, consumer); } return changed; } -template bool SwiftRunner::ProcessArgument( - Iterator &itr, const std::string &arg, - std::function consumer) { - bool changed = false; + absl::string_view arg, std::function consumer) { if (arg[0] == '@') { - changed = ProcessPossibleResponseFile(arg, consumer); - } else { - std::string new_arg = arg; - if (StripPrefix("-Xwrapped-swift=", new_arg)) { - if (new_arg == "-debug-prefix-pwd-is-dot") { - // Replace the $PWD with . to make the paths relative to the workspace - // without breaking hermiticity. - consumer("-debug-prefix-map"); - consumer(std::filesystem::current_path().string() + "=."); - changed = true; - } else if (new_arg == "-coverage-prefix-pwd-is-dot") { - // Replace the $PWD with . to make the paths relative to the workspace - // without breaking hermiticity. - consumer("-coverage-prefix-map"); - consumer(std::filesystem::current_path().string() + "=."); - changed = true; - } else if (new_arg == "-file-prefix-pwd-is-dot") { - // Replace the $PWD with . to make the paths relative to the workspace - // without breaking hermiticity. - consumer("-file-prefix-map"); - consumer(std::filesystem::current_path().string() + "=."); - changed = true; - } else if (StripPrefix("-macro-expansion-dir=", new_arg)) { - changed = true; - std::filesystem::create_directories(new_arg); + return ProcessPossibleResponseFile(arg, consumer); + } + + // Helper function for adding path remapping flags that depend on information + // only known at execution time. + auto add_prefix_map_flags = [&](absl::string_view flag) { + // Get the actual current working directory (the execution root), which + // we didn't know at analysis time. + consumer(flag); + consumer(absl::StrCat(GetCurrentDirectory(), "=.")); + #if __APPLE__ - job_env_["TMPDIR"] = new_arg; -#else - // TEMPDIR is read by C++ but not Swift. Swift requires the temprorary - // directory to be an absolute path and otherwise fails (or ignores it - // silently on macOS) so we need to set one that Swift does not read. - // C++ prioritizes TMPDIR over TEMPDIR so we need to wipe out the other - // one. The downside is that anything else reading TMPDIR will not use - // the one potentially set by the user. - job_env_["TEMPDIR"] = new_arg; - job_env_.erase("TMPDIR"); + std::string developer_dir = "__BAZEL_XCODE_DEVELOPER_DIR__"; + if (bazel_placeholder_substitutions_.Apply(developer_dir)) { + consumer(flag); + consumer(absl::StrCat(developer_dir, "=/DEVELOPER_DIR")); + } #endif - } else if (new_arg == "-ephemeral-module-cache") { - // Create a temporary directory to hold the module cache, which will be - // deleted after compilation is finished. - auto module_cache_dir = - TempDirectory::Create("swift_module_cache.XXXXXX"); - consumer("-module-cache-path"); - consumer(module_cache_dir->GetPath()); - temp_directories_.push_back(std::move(module_cache_dir)); - changed = true; - } else if (StripPrefix("-generated-header-rewriter=", new_arg)) { - changed = true; - } else if (StripPrefix("-bazel-target-label=", new_arg)) { - changed = true; - } else if (StripPrefix("-global-index-store-import-path=", new_arg)) { - changed = true; - } else { - // TODO(allevato): Report that an unknown wrapper arg was found and give - // the caller a way to exit gracefully. - changed = true; - } - } else { - // Process default arguments - if (arg == "-index-store-path") { - consumer("-index-store-path"); - ++itr; - - // If there was a global index store set, pass that to swiftc. - // Otherwise, pass the users. We later copy index data onto the users. - if (global_index_store_import_path_ != "") { - new_arg = global_index_store_import_path_; - } else { - new_arg = index_store_path_; - } - changed = true; - } else if (arg == "-output-file-map") { - // Save the output file map to the value proceeding - // `-output-file-map` - consumer("-output-file-map"); - ++itr; - new_arg = output_file_map_path_; - changed = true; - } else if (is_dump_ast_ && ArgumentEnablesWMO(arg)) { - // WMO is invalid for -dump-ast, - // so omit the argument that enables WMO - return true; // return to avoid consuming the arg - } + }; + + absl::string_view trimmed_arg = arg; + if (last_flag_was_module_name_) { + module_name_ = std::string(trimmed_arg); + last_flag_was_module_name_ = false; + } else if (last_flag_was_tools_directory_) { + // Make the value of `-tools-directory` absolute, otherwise swift-driver + // will ignore it. + std::string tools_directory = std::string(trimmed_arg); + consumer(absl::StrCat(GetCurrentDirectory(), "/", tools_directory)); + last_flag_was_tools_directory_ = false; + return true; + } else if (last_flag_was_target_) { + target_triple_ = std::string(trimmed_arg); + last_flag_was_target_ = false; + } else if (last_flag_was_module_alias_) { + std::pair source_and_alias = + absl::StrSplit(trimmed_arg, '='); + alias_to_source_mapping_[source_and_alias.second] = source_and_alias.first; + last_flag_was_module_alias_ = false; + } else if (trimmed_arg == "-module-name") { + last_flag_was_module_name_ = true; + } else if (trimmed_arg == "-tools-directory") { + last_flag_was_tools_directory_ = true; + } else if (trimmed_arg == "-target") { + last_flag_was_target_ = true; + } else if (trimmed_arg == "-module-alias") { + last_flag_was_module_alias_ = true; + } else if (absl::ConsumePrefix(&trimmed_arg, "-Xwrapped-swift=")) { + if (trimmed_arg == "-debug-prefix-pwd-is-dot") { + add_prefix_map_flags("-debug-prefix-map"); + return true; + } - // Apply any other text substitutions needed in the argument (i.e., for - // Apple toolchains). + if (trimmed_arg == "-file-prefix-pwd-is-dot") { + add_prefix_map_flags("-file-prefix-map"); + return true; + } + + if (absl::ConsumePrefix(&trimmed_arg, "-macro-expansion-dir=")) { + std::string temp_dir = std::string(trimmed_arg); + + // We don't have a clean way to report an error out of this function. If + // If creating the directory fails, then the compiler will fail later + // anyway. + MakeDirs(temp_dir, S_IRWXU).IgnoreError(); + + // By default, the compiler creates a directory under the system temp + // directory to hold macro expansions. The underlying LLVM API lets us + // customize this location by setting `TMPDIR` in the environment, so this + // lets us redirect those files to a deterministic location. A pull + // request like https://github.com/apple/swift/pull/67184 would let us do + // the same thing without this trick, but it hasn't been merged. // - // Bazel doesn't quote arguments in multi-line params files, so we need - // to ensure that our defensive quoting kicks in if an argument contains - // a space, even if no other changes would have been made. - changed = bazel_placeholder_substitutions_.Apply(new_arg) || - new_arg.find_first_of(' ') != std::string::npos; - consumer(new_arg); + // For now, this is the only major use of `TMPDIR` by the compiler, so we + // can do this without other stuff that we don't want moving there. We may + // need to revisit this logic if that changes. + job_env_["TMPDIR"] = absl::StrCat(GetCurrentDirectory(), "/", temp_dir); + return true; } - } - return changed; -} + if (trimmed_arg == "-ephemeral-module-cache") { + // Create a temporary directory to hold the module cache, which will be + // deleted after compilation is finished. + std::unique_ptr module_cache_dir = + TempDirectory::Create("swift_module_cache.XXXXXX"); + consumer("-module-cache-path"); + consumer(module_cache_dir->GetPath()); + temp_directories_.push_back(std::move(module_cache_dir)); + return true; + } -template -std::vector SwiftRunner::ParseArguments(Iterator itr) { - std::vector out_args; - for (auto it = itr.begin(); it != itr.end(); ++it) { - auto arg = *it; - out_args.push_back(arg); - - if (StripPrefix("-Xwrapped-swift=", arg)) { - if (StripPrefix("-global-index-store-import-path=", arg)) { - global_index_store_import_path_ = arg; - } else if (StripPrefix("-generated-header-rewriter=", arg)) { - generated_header_rewriter_path_ = arg; - } else if (StripPrefix("-bazel-target-label=", arg)) { - target_label_ = arg; - } else if (arg == "-file-prefix-pwd-is-dot") { - file_prefix_pwd_is_dot_ = true; - } else if (arg == "-emit-swiftsourceinfo") { - emit_swift_source_info_ = true; - } - } else { - if (arg == "-output-file-map") { - ++it; - arg = *it; - output_file_map_path_ = arg; - out_args.push_back(arg); - } else if (arg == "-index-store-path") { - ++it; - arg = *it; - index_store_path_ = arg; - out_args.push_back(arg); - } else if (arg == "-dump-ast") { - is_dump_ast_ = true; - } else if (arg == "-emit-module-path") { - ++it; - arg = *it; - std::filesystem::path module_path(arg); - swift_source_info_path_ = module_path.replace_extension(".swiftsourceinfo").string(); - out_args.push_back(arg); - } + if (absl::ConsumePrefix(&trimmed_arg, "-generated-header-rewriter=")) { + generated_header_rewriter_path_ = std::string(trimmed_arg); + return true; + } + + if (absl::ConsumePrefix(&trimmed_arg, "-tool-arg=")) { + std::pair arg_and_value = + absl::StrSplit(trimmed_arg, absl::MaxSplits('=', 1)); + passthrough_tool_args_[arg_and_value.first].push_back( + std::string(arg_and_value.second)); + return true; + } + + if (absl::ConsumePrefix(&trimmed_arg, "-bazel-target-label=")) { + target_label_ = std::string(trimmed_arg); + return true; } + + if (absl::ConsumePrefix(&trimmed_arg, "-layering-check-deps-modules=")) { + deps_modules_path_ = std::string(trimmed_arg); + return true; + } + + if (absl::ConsumePrefix(&trimmed_arg, "-warning-as-error=")) { + warnings_as_errors_.insert(std::string(trimmed_arg)); + return true; + } + + if (absl::ConsumePrefix(&trimmed_arg, "-compile-step=")) { + std::pair action_and_output = + absl::StrSplit(trimmed_arg, absl::MaxSplits('=', 1)); + compile_step_ = + CompileStep{action_and_output.first, action_and_output.second}; + return true; + } + + if (absl::ConsumePrefix(&trimmed_arg, + "-explicit-compile-module-from-interface=")) { + module_or_interface_path_ = std::string(trimmed_arg); + bazel_placeholder_substitutions_.Apply(module_or_interface_path_); + return true; + } + + if (trimmed_arg == "-emit-json-ast") { + emit_json_ast_ = true; + return true; + } + + // TODO(allevato): Report that an unknown wrapper arg was found and give + // the caller a way to exit gracefully. + return true; } - return out_args; -} -std::vector SwiftRunner::ProcessArguments( - const std::vector &args) { - std::vector new_args; - std::vector response_file_args; + // Apply any other text substitutions needed in the argument (i.e., for + // Apple toolchains). + // + // Bazel doesn't quote arguments in multi-line params files, so we need to + // ensure that our defensive quoting kicks in if an argument contains a + // space, even if no other changes would have been made. + std::string new_arg(arg); + bool changed = bazel_placeholder_substitutions_.Apply(new_arg) || + absl::StrContains(new_arg, ' '); + consumer(new_arg); + return changed; +} +void SwiftRunner::ProcessArguments(const std::vector &args) { #if __APPLE__ // On Apple platforms, inject `/usr/bin/xcrun` in front of our command // invocation. - new_args.push_back("/usr/bin/xcrun"); + tool_args_.push_back("/usr/bin/xcrun"); #endif // The tool is assumed to be the first argument. Push it directly. - auto parsed_args = ParseArguments(args); - - auto it = parsed_args.begin(); - new_args.push_back(*it++); - - // If we're forcing response files, push the remaining processed args onto a - // different vector that we write out below. If not, push them directly onto - // the vector being returned. - auto &args_destination = force_response_file_ ? response_file_args : new_args; - while (it != parsed_args.end()) { - ProcessArgument(it, *it, [&](const std::string &arg) { - args_destination.push_back(arg); - }); + auto it = args.begin(); + tool_args_.push_back(*it++); + + auto consumer = [&](absl::string_view arg) { + args_.push_back(std::string(arg)); + }; + while (it != args.end()) { + ProcessArgument(*it, consumer); ++it; } - if (force_response_file_) { - // Write the processed args to the response file, and push the path to that - // file (preceded by '@') onto the arg list being returned. - auto new_file = WriteResponseFile(response_file_args); - new_args.push_back("@" + new_file->GetPath()); - temp_files_.push_back(std::move(new_file)); + // If we're doing an explicit interface build, we need to extract the flags + // from the .swiftinterface file as well. + if (!module_or_interface_path_.empty()) { + ExtractFlagsFromInterfaceFile(module_or_interface_path_, target_triple_, + consumer); + } +} + +int SwiftRunner::PerformGeneratedHeaderRewriting(std::ostream &stdout_stream, + std::ostream &stderr_stream) { +#if __APPLE__ + // Skip the `xcrun` argument that's added when running on Apple platforms, + // since the header rewriter doesn't need it. + int tool_binary_index = StartsWithXcrun(tool_args_) ? 1 : 0; +#else + int tool_binary_index = 0; +#endif + + std::vector rewriter_tool_args; + rewriter_tool_args.push_back(generated_header_rewriter_path_); + const std::vector &passthrough_args = + passthrough_tool_args_["generated_header_rewriter"]; + rewriter_tool_args.insert(rewriter_tool_args.end(), passthrough_args.begin(), + passthrough_args.end()); + rewriter_tool_args.push_back("--"); + rewriter_tool_args.push_back(tool_args_[tool_binary_index]); + + return SpawnJob(rewriter_tool_args, args_, /*env=*/nullptr, stdout_stream, + stderr_stream); +} + +int SwiftRunner::PerformLayeringCheck(std::ostream &stdout_stream, + std::ostream &stderr_stream) { + // Run the compiler again, this time using `-emit-imported-modules` to + // override whatever other behavior was requested and get the list of imported + // modules. + std::string imported_modules_path = + ReplaceExtension(deps_modules_path_, ".imported-modules", + /*all_extensions=*/true); + + std::vector emit_imports_args; + for (auto it = args_.begin(); it != args_.end(); ++it) { + if (!SkipIncompatibleArgs(IncompatibleArgMode::kLayeringCheck, it)) { + emit_imports_args.push_back(*it); + } + } + + emit_imports_args.push_back("-emit-imported-modules"); + emit_imports_args.push_back("-o"); + emit_imports_args.push_back(imported_modules_path); + int exit_code = SpawnJob(tool_args_, emit_imports_args, /*env=*/nullptr, + stdout_stream, stderr_stream); + if (exit_code != 0) { + return exit_code; + } + + absl::btree_set deps_modules = + ReadDepsModules(deps_modules_path_); + + // Use a `btree_set` so that the output is automatically sorted + // lexicographically. + absl::btree_set missing_deps; + std::ifstream imported_modules_stream(imported_modules_path); + std::string module_name; + while (std::getline(imported_modules_stream, module_name)) { + // We have to ignore the name of the module being compiled as well, because + // a Swift module can `@_exported import` itself to import its underlying + // Clang module. + if (module_name != module_name_ && + !IsModuleIgnorableForLayeringCheck(module_name) && + !deps_modules.contains(module_name)) { + // The module names that the build rules write into the direct + // dependencies file are the "physical" names of the module, which will be + // different than what is written in source if the module is aliased. This + // matches what the `-emit-imported-modules` output will show. We look up + // the name among all the known aliases so that we can print the original + // module name (as would be written in source) to the user. + if (auto alias_and_source_name = + alias_to_source_mapping_.find(module_name); + alias_and_source_name != alias_to_source_mapping_.end()) { + missing_deps.insert(alias_and_source_name->second); + } else { + missing_deps.insert(module_name); + } + } + } + + if (!missing_deps.empty()) { + stderr_stream << std::endl; + WithColor(stderr_stream, Color::kBoldRed) << "error: "; + WithColor(stderr_stream, Color::kBold) << "Layering violation in "; + WithColor(stderr_stream, Color::kBoldGreen) << target_label_ << std::endl; + stderr_stream + << "The following modules were imported, but they are not direct " + << "dependencies of the target or they are misspelled:" << std::endl + << std::endl; + + for (const std::string &module_name : missing_deps) { + stderr_stream << " " << module_name << std::endl; + } + stderr_stream << std::endl; + + WithColor(stderr_stream, Color::kBold) + << "Please add the correct 'deps' to " << target_label_ + << " to import those modules." << std::endl; + return 1; } - return new_args; + return 0; } + +int SwiftRunner::PerformJsonAstDump(std::ostream &stdout_stream, + std::ostream &stderr_stream) { + // It is assumed that the Bazel caller spawning this action has already + // populated the output file map with `ast-dump` keys that represent the + // per-source-file outputs. + std::vector ast_dump_args; + bool last_arg_was_output_file_map = false; + std::string output_file_map_path; + for (auto it = args_.begin(); it != args_.end(); ++it) { + if (!SkipIncompatibleArgs(IncompatibleArgMode::kJsonAst, it)) { + ast_dump_args.push_back(*it); + + if (last_arg_was_output_file_map) { + output_file_map_path = *it; + last_arg_was_output_file_map = false; + } else if (*it == "-output-file-map") { + last_arg_was_output_file_map = true; + } + } + } + + ast_dump_args.push_back("-dump-ast"); + ast_dump_args.push_back("-dump-ast-format"); + ast_dump_args.push_back("json"); + // If there were warnings emitted by the actual compilation, we don't need to + // see them again via the AST dump. + ast_dump_args.push_back("-suppress-warnings"); + + int exit_code = SpawnJob(tool_args_, ast_dump_args, /*env=*/nullptr, + stdout_stream, stderr_stream); + if (exit_code == 0) { + return 0; + } + + // If the dump failed, it likely did so because of a crash (there are known + // crashing bugs in the JSON AST dump in Swift 6.1). If this happens, read the + // output file map to determine which files were expected and write dummy + // files so that Bazel is happy. + std::ifstream stream((std::string(output_file_map_path))); + nlohmann::json output_file_map; + stream >> output_file_map; + + for (auto &[src, outputs] : output_file_map.items()) { + if (auto ast_dump_path = outputs.find("ast-dump"); + ast_dump_path != outputs.end()) { + std::ofstream ast_dump_file(ast_dump_path->get()); + ast_dump_file << "{}" << std::endl; + } + } + + // We still return success here so that a bad JSON AST dump doesn't cause the + // whole build to fail. + return 0; +} + +void SwiftRunner::ProcessDiagnostics(absl::string_view stderr_output, + std::ostream &stderr_stream, + int &exit_code) const { + if (stderr_output.empty()) { + // Nothing to do if there was no output. + return; + } + + // Match the "warning: " prefix on a message, also capturing the preceding + // ANSI color sequence if present. + RE2 warning_pattern("((\\x1b\\[(?:\\d+)(?:;\\d+)*m)?warning:\\s)"); + // When `-debug-diagnostic-names` is enabled, names are printed as identifiers + // in square brackets, either at the end of the string (modulo another escape + // sequence like 'reset'), or when followed by a semicolon (for wrapped + // diagnostics). Nothing guarantees this for the wrapped case -- it is just + // observed convention -- but it is sufficient while the compiler doesn't give + // us a more proper way to detect these. + RE2 diagnostic_name_pattern("\\[([_A-Za-z][_A-Za-z0-9]*)\\](;|$|\\x1b)"); + + for (absl::string_view line : absl::StrSplit(stderr_output, '\n')) { + std::unique_ptr modified_line; + + absl::string_view warning_label; + std::optional ansi_sequence; + if (RE2::PartialMatch(line, warning_pattern, &warning_label, + &ansi_sequence)) { + absl::string_view diagnostic_name; + absl::string_view line_cursor = line; + + // Search the diagnostic line for all possible diagnostic names surrounded + // by square brackets. + while (RE2::FindAndConsume(&line_cursor, diagnostic_name_pattern, + &diagnostic_name)) { + if (warnings_as_errors_.contains(diagnostic_name)) { + modified_line = std::make_unique(line); + std::ostringstream error_label; + if (ansi_sequence.has_value()) { + error_label << Color(Color::kBoldRed); + } + error_label << "error (upgraded from warning): "; + modified_line->replace(warning_label.data() - line.data(), + warning_label.length(), error_label.str()); + if (exit_code == 0) { + exit_code = 1; + } + + // In the event that there are multiple diagnostics on the same line + // (this is the case, for example, with "this is an error in Swift 6" + // messages), we can stop after we find the first match; the whole + // line will become an error. + break; + } + } + } + if (modified_line) { + stderr_stream << *modified_line << std::endl; + } else { + stderr_stream << line << std::endl; + } + } +} + +} // namespace bazel_rules_swift diff --git a/tools/worker/swift_runner.h b/tools/worker/swift_runner.h index 74b30b98d..9f3a47c58 100644 --- a/tools/worker/swift_runner.h +++ b/tools/worker/swift_runner.h @@ -17,17 +17,28 @@ #include #include -#include #include +#include #include #include +#include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_set.h" +#include "absl/strings/string_view.h" #include "tools/common/bazel_substitutions.h" #include "tools/common/temp_file.h" -// Returns true if the given command line argument enables whole-module -// optimization in the compiler. -extern bool ArgumentEnablesWMO(const std::string &arg); +namespace bazel_rules_swift { + +// Represents a single step in a parallelized compilation. +struct CompileStep { + // The name of the action that emits this output. + std::string action; + + // The path of the expected primary output file, which identifies the step + // among all of the frontend actions in the driver's job list. + std::string output; +}; // Handles spawning the Swift compiler driver, making any required substitutions // of the command line arguments (for example, Bazel's magic Xcode placeholder @@ -66,12 +77,11 @@ class SwiftRunner { // true, then the remaining arguments will be unconditionally written into a // response file instead of being passed on the command line. SwiftRunner(const std::vector &args, - std::string index_import_path, bool force_response_file = false); - // Run the Swift compiler, redirecting stderr to the specified stream. If - // stdout_to_stderr is true, then stdout is also redirected to that stream. - int Run(std::ostream *stderr_stream, bool stdout_to_stderr = false); + // Run the Swift compiler, redirecting stdout and stderr to the specified + // streams. + int Run(std::ostream &stdout_stream, std::ostream &stderr_stream); private: // Processes an argument that looks like it might be a response file (i.e., it @@ -98,8 +108,7 @@ class SwiftRunner { // written to a new response file, a response file argument pointing to that // file is passed to the consumer, and the method returns true. bool ProcessPossibleResponseFile( - const std::string &arg, - std::function consumer); + absl::string_view arg, std::function consumer); // Applies substitutions for a single argument and passes the new arguments // (or the original, if no substitution was needed) to the consumer. Returns @@ -108,35 +117,55 @@ class SwiftRunner { // // This method has file system side effects, creating temporary files and // directories as needed for a particular substitution. - template - bool ProcessArgument(Iterator &itr, const std::string &arg, - std::function consumer); - - // Parses arguments to ivars and returns a vector of strings from the - // iterator. This method doesn't actually mutate any of the arguments. - template - std::vector ParseArguments(Iterator itr); - - // Applies substitutions to the given command line arguments, returning the - // results in a new vector. - std::vector ProcessArguments( - const std::vector &args); + bool ProcessArgument(absl::string_view arg, + std::function consumer); + + // Applies substitutions to the given command line arguments and populates the + // `tool_args_` and `args_` vectors. + void ProcessArguments(const std::vector &args); + + // Spawns the generated header rewriter to perform any desired transformations + // on the Clang header emitted from a Swift compilation. + int PerformGeneratedHeaderRewriting(std::ostream &stdout_stream, + std::ostream &stderr_stream); + + // Performs a layering check for the compilation, comparing the modules that + // were imported by Swift code being compiled to the list of dependencies + // declared in the build graph. + int PerformLayeringCheck(std::ostream &stdout_stream, + std::ostream &stderr_stream); + + // Performs a safe JSON AST dump of the current compilation, which attempts to + // recover from known crash issues in the Swift 6.1 implementation of the + // feature. + int PerformJsonAstDump(std::ostream &stdout_stream, + std::ostream &stderr_stream); + + // Upgrade any of the requested warnings to errors and then print all of the + // diagnostics to the given stream. Updates the exit code if necessary (to + // turn a previously successful compilation into a failing one). + void ProcessDiagnostics(absl::string_view stderr_output, + std::ostream &stderr_stream, int &exit_code) const; // A mapping of Bazel placeholder strings to the actual paths that should be // substituted for them. Supports Xcode resolution on Apple OSes. bazel_rules_swift::BazelPlaceholderSubstitutions bazel_placeholder_substitutions_; - // The arguments, post-substitution, passed to the spawner. + // The portion of the command line that indicates which tool should be + // spawned; that is, the name/path of the binary, possibly preceded by `xcrun` + // on Apple platforms. This part of the path should never be written into a + // response file. + std::vector tool_args_; + + // The arguments, post-substitution, passed to the spawner. This does not + // include the binary path, and may be written into a response file. std::vector args_; // The environment that should be passed to the original job (but not to other // jobs spawned by the worker, such as the generated header rewriter or the // emit-imports job). - std::map job_env_; - - // The path to the index-import binary. - std::string index_import_path_; + absl::flat_hash_map job_env_; // Temporary files (e.g., rewritten response files) that should be cleaned up // after the driver has terminated. @@ -150,38 +179,71 @@ class SwiftRunner { // to the tool that way. bool force_response_file_; - // Whether the invocation is being used to dump ast files. - // This is used to avoid implicitly adding incompatible flags. - bool is_dump_ast_; - - // Whether `-file-prefix-map PWD=.` is set. - bool file_prefix_pwd_is_dot_; - // The path to the generated header rewriter tool, if one is being used for // this compilation. std::string generated_header_rewriter_path_; + // A map containing arguments that should be passed through to additional + // tools that support them. Each key in the map represents the name of a + // recognized tool. + absl::flat_hash_map> + passthrough_tool_args_; + // The Bazel target label that spawned the worker request, which can be used // in custom diagnostic messages printed by the worker. std::string target_label_; - // The path of the output map file - std::string output_file_map_path_; + // The path to a file generated by the build rules that contains the list of + // module names that are direct dependencies of the code being compiled. This + // is used by layering checks to determine the set of modules that the code is + // actually allowed to import. + std::string deps_modules_path_; + + // Tracks whether the last flag seen was `-module-name`. + bool last_flag_was_module_name_; + + // Tracks whether the last flag seen was `-tools-directory`. + bool last_flag_was_tools_directory_; + + // Tracks whether the last flag seen was `-target`. + bool last_flag_was_target_; + + // Tracks whether the last flag seen was `-module-alias`. + bool last_flag_was_module_alias_; - // The index store path argument passed to the runner - std::string index_store_path_; + // The name of the module currently being compiled. + std::string module_name_; - // The path of the global index store when using - // swift.use_global_index_store. When set, this is passed to `swiftc` as the - // `-index-store-path`. After running `swiftc` `index-import` copies relevant - // index outputs into the `index_store_path` to integrate outputs with Bazel. - std::string global_index_store_import_path_; + // The target triple of the current compilation. + std::string target_triple_; - // The path where the module files will be written - std::string swift_source_info_path_; + // The path to either the `.swiftinterface` file to compile or to a + // `.swiftmodule` directory in which the worker will infer the interface file + // to compile. + std::string module_or_interface_path_; - // Whether `.swiftsourceinfo` files are being generated. - bool emit_swift_source_info_; + // A set containing the diagnostic IDs that should be upgraded from warnings + // to errors by the worker. + absl::flat_hash_set warnings_as_errors_; + + // The step in the compilation plan that is being requested by this specific + // action. If this is present, then the action is being executed as part of a + // parallelized compilation and we should invoke the driver to list all jobs, + // then extract and run the single frontend invocation that generates that + // that output. + std::optional compile_step_; + + // Whether the worker should emit a JSON AST dump of the compilation. + bool emit_json_ast_; + + // The inverse mapping of module aliases passed to the compiler. The + // `-module-alias` flag takes its argument of the form `source=alias`. For + // layering checks, we need to reverse this because `-emit-imported-modules` + // reflects the aliased name and we want to present the original module names + // in the error messages. + absl::flat_hash_map alias_to_source_mapping_; }; +} // namespace bazel_rules_swift + #endif // BUILD_BAZEL_RULES_SWIFT_TOOLS_WORKER_SWIFT_RUNNER_H_ From c970cb8418a1dc6daa4c7a41a84cdad9f67cba4a Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Tue, 23 Sep 2025 23:03:53 -0400 Subject: [PATCH 04/13] Add support for parallel compilation --- swift/internal/action_names.bzl | 15 +- swift/internal/feature_names.bzl | 4 + swift/providers.bzl | 4 + swift/toolchains/config/compile_config.bzl | 255 +++++++++------------ swift/toolchains/xcode_swift_toolchain.bzl | 21 +- 5 files changed, 145 insertions(+), 154 deletions(-) diff --git a/swift/internal/action_names.bzl b/swift/internal/action_names.bzl index c14ad507c..351331c56 100644 --- a/swift/internal/action_names.bzl +++ b/swift/internal/action_names.bzl @@ -20,9 +20,17 @@ SWIFT_ACTION_AUTOLINK_EXTRACT = "SwiftAutolinkExtract" # Compiles one or more `.swift` source files into a `.swiftmodule` and -# object files. +# object files. This is the legacy mode that emits all outputs from a single +# driver invocation. SWIFT_ACTION_COMPILE = "SwiftCompile" +# Emits the object files and other per-source-file outputs for a a batch of +# `.swift` source files in a module. +SWIFT_ACTION_COMPILE_CODEGEN = "SwiftCompileCodegen" + +# Compiles one or more `.swift` source files into a `.swiftmodule`. +SWIFT_ACTION_COMPILE_MODULE = "SwiftCompileModule" + # Compiles a `.swiftinterface` file into a `.swiftmodule` file. SWIFT_ACTION_COMPILE_MODULE_INTERFACE = "SwiftCompileModuleInterface" @@ -54,6 +62,8 @@ def all_action_names(): return ( SWIFT_ACTION_AUTOLINK_EXTRACT, SWIFT_ACTION_COMPILE, + SWIFT_ACTION_COMPILE_CODEGEN, + SWIFT_ACTION_COMPILE_MODULE, SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -67,5 +77,6 @@ def all_compile_action_names(): """Returns all actions that compile source files.""" return [ SWIFT_ACTION_COMPILE, - SWIFT_ACTION_COMPILE_MODULE_INTERFACE, + SWIFT_ACTION_COMPILE_CODEGEN, + SWIFT_ACTION_COMPILE_MODULE, ] diff --git a/swift/internal/feature_names.bzl b/swift/internal/feature_names.bzl index 3242857a4..ec0ac492b 100644 --- a/swift/internal/feature_names.bzl +++ b/swift/internal/feature_names.bzl @@ -369,6 +369,10 @@ SWIFT_FEATURE_INTERNALIZE_AT_LINK = "swift.internalize_at_link" # This disables checking for potentially unavailable APIs. SWIFT_FEATURE_DISABLE_AVAILABILITY_CHECKING = "swift.disable_availability_checking" +# If enabled, parallelize the compilation of Swift modules and their object +# files by registering separate actions for each. +SWIFT_FEATURE_COMPILE_IN_PARALLEL = "swift.compile_in_parallel" + # A private feature that is set by the toolchain if it supports the # `-enable-{experimental,upcoming}-feature` flag (Swift 5.8 and above). Users # should never manually, enable, disable, or query this feature. diff --git a/swift/providers.bzl b/swift/providers.bzl index 15aee1ea8..d0006fb09 100644 --- a/swift/providers.bzl +++ b/swift/providers.bzl @@ -336,6 +336,10 @@ C/Objective-C modules: 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. +""", + "codegen_batch_size": """\ +The number of files to pass to the compiler in a single code generation action +(one that compiles object files from Swift source files). """, "const_protocols_to_gather": """\ `File`. A JSON file specifying a list of protocols for extraction of diff --git a/swift/toolchains/config/compile_config.bzl b/swift/toolchains/config/compile_config.bzl index 588b36ae7..2ecc5afa9 100644 --- a/swift/toolchains/config/compile_config.bzl +++ b/swift/toolchains/config/compile_config.bzl @@ -20,6 +20,8 @@ load("@bazel_skylib//lib:types.bzl", "types") load( "//swift/internal:action_names.bzl", "SWIFT_ACTION_COMPILE", + "SWIFT_ACTION_COMPILE_CODEGEN", + "SWIFT_ACTION_COMPILE_MODULE", "SWIFT_ACTION_COMPILE_MODULE_INTERFACE", "SWIFT_ACTION_DERIVE_FILES", "SWIFT_ACTION_DUMP_AST", @@ -140,23 +142,30 @@ def compile_action_configs( action_configs = [ # Emit object file(s). ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [add_arg("-emit-object")], not_features = [SWIFT_FEATURE_EMIT_BC], ), # Emit llvm bc file(s). ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [add_arg("-emit-bc")], features = [SWIFT_FEATURE_EMIT_BC], ), # Add the single object file or object file map, whichever is needed. ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [_output_object_or_file_map_configurator], ), + ActionConfigInfo( + actions = [ + SWIFT_ACTION_COMPILE_CODEGEN, + SWIFT_ACTION_COMPILE_MODULE, + ], + configurators = [_compile_step_configurator], + ), ActionConfigInfo( actions = [SWIFT_ACTION_DERIVE_FILES], configurators = [_output_swiftmodule_or_file_map_configurator], @@ -177,7 +186,7 @@ def compile_action_configs( # Don't embed Clang module breadcrumbs in debug info. ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [ add_arg("-Xfrontend", "-no-clang-module-breadcrumbs"), ], @@ -203,7 +212,7 @@ def compile_action_configs( # Configure the path to the emitted .swiftmodule file. ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [_emit_module_path_configurator], not_features = [SWIFT_FEATURE_SPLIT_DERIVED_FILES_GENERATION], ), @@ -214,8 +223,7 @@ def compile_action_configs( # Configure library evolution and the path to the .swiftinterface file. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [add_arg("-enable-library-evolution")], @@ -224,6 +232,7 @@ def compile_action_configs( ActionConfigInfo( actions = [ SWIFT_ACTION_COMPILE, + SWIFT_ACTION_COMPILE_MODULE, SWIFT_ACTION_DERIVE_FILES, ], configurators = [_emit_module_interface_path_configurator], @@ -232,6 +241,7 @@ def compile_action_configs( ActionConfigInfo( actions = [ SWIFT_ACTION_COMPILE, + SWIFT_ACTION_COMPILE_MODULE, SWIFT_ACTION_DERIVE_FILES, ], configurators = [_emit_private_module_interface_path_configurator], @@ -240,7 +250,10 @@ def compile_action_configs( # Configure the path to the emitted *-Swift.h file. ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = [ + SWIFT_ACTION_COMPILE, + SWIFT_ACTION_COMPILE_MODULE, + ], configurators = [_emit_objc_header_path_configurator], not_features = [SWIFT_FEATURE_SPLIT_DERIVED_FILES_GENERATION], ), @@ -251,25 +264,25 @@ def compile_action_configs( # Configure enforce exclusivity checks if enabled. ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [add_arg("-enforce-exclusivity=checked")], features = [SWIFT_FEATURE_CHECKED_EXCLUSIVITY], ), # Configure constant value extraction. ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [_constant_value_extraction_configurator], ), # Link Time Optimization (LTO). ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [add_arg("-lto=llvm-thin")], features = [SWIFT_FEATURE_THIN_LTO], ), ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [add_arg("-lto=llvm-full")], features = [SWIFT_FEATURE_FULL_LTO], ), @@ -294,7 +307,10 @@ def compile_action_configs( action_configs.append( ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = [ + SWIFT_ACTION_COMPILE, + SWIFT_ACTION_COMPILE_MODULE, + ], configurators = [generated_header_rewriter_configurator], features = [SWIFT_FEATURE_REWRITE_GENERATED_HEADER], ), @@ -310,8 +326,7 @@ def compile_action_configs( # Define appropriate conditional compilation symbols depending on the # build mode. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, ], @@ -319,8 +334,7 @@ def compile_action_configs( features = [[SWIFT_FEATURE_DBG], [SWIFT_FEATURE_FASTBUILD]], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, ], @@ -332,8 +346,7 @@ def compile_action_configs( # `-O` unless the `swift.opt_uses_osize` feature is enabled, then use # `-Osize`. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, ], @@ -341,8 +354,7 @@ def compile_action_configs( features = [[SWIFT_FEATURE_DBG], [SWIFT_FEATURE_FASTBUILD]], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, ], @@ -351,8 +363,7 @@ def compile_action_configs( not_features = [SWIFT_FEATURE_OPT_USES_OSIZE], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, ], @@ -363,8 +374,7 @@ def compile_action_configs( # If the `swift.opt_uses_wmo` feature is enabled, opt builds should also # automatically imply whole-module optimization. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [add_arg("-whole-module-optimization")], @@ -376,7 +386,7 @@ def compile_action_configs( # Improve dead-code stripping. ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [add_arg("-Xfrontend", "-internalize-at-link")], features = [SWIFT_FEATURE_INTERNALIZE_AT_LINK], ), @@ -384,8 +394,7 @@ def compile_action_configs( # Enable or disable serialization of debugging options into # swiftmodules. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [ @@ -394,8 +403,7 @@ def compile_action_configs( features = [SWIFT_FEATURE_CACHEABLE_SWIFTMODULES], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [ @@ -409,8 +417,7 @@ def compile_action_configs( # Enable testability if requested. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, ], @@ -420,8 +427,7 @@ def compile_action_configs( # Enable suppress-warnings if requested. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, ], @@ -433,8 +439,7 @@ def compile_action_configs( # Enable warnings-as-errors if requested. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, ], @@ -446,8 +451,7 @@ def compile_action_configs( # Disable Swift sandbox. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, ], @@ -459,8 +463,7 @@ def compile_action_configs( # Set Developer Framework search paths ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, SWIFT_ACTION_SYMBOL_GRAPH_EXTRACT, @@ -479,16 +482,14 @@ def compile_action_configs( # `dsymutil` produces spurious warnings about symbols in the debug map # when run on DI emitted by `-gline-tables-only`. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [add_arg("-g")], features = [[SWIFT_FEATURE_DBG], [SWIFT_FEATURE_FULL_DEBUG_INFO]], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [ @@ -510,8 +511,7 @@ def compile_action_configs( features = [[SWIFT_FEATURE_DBG], [SWIFT_FEATURE_FULL_DEBUG_INFO]], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [add_arg("-gline-tables-only")], @@ -524,8 +524,7 @@ def compile_action_configs( # Make paths written into debug info workspace-relative. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_PRECOMPILE_C_MODULE, ], @@ -540,8 +539,7 @@ def compile_action_configs( not_features = [SWIFT_FEATURE_FILE_PREFIX_MAP], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_PRECOMPILE_C_MODULE, ], @@ -553,8 +551,7 @@ def compile_action_configs( # Make paths written into coverage info workspace-relative. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [ @@ -567,9 +564,7 @@ def compile_action_configs( # Ensure that .swiftsourceinfo files are tracked and not deleted by the worker ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, - ], + actions = all_compile_action_names(), configurators = [add_arg("-Xwrapped-swift=-emit-swiftsourceinfo")], features = [SWIFT_FEATURE_DECLARE_SWIFTSOURCEINFO], ), @@ -582,8 +577,7 @@ def compile_action_configs( # supporting them either. action_configs += [ ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [ @@ -593,16 +587,14 @@ def compile_action_configs( features = [SWIFT_FEATURE_COVERAGE], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [add_arg("-sanitize=address")], features = ["asan"], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [ @@ -614,12 +606,12 @@ def compile_action_configs( ], ), ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [add_arg("-sanitize=thread")], features = ["tsan"], ), ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [ add_arg("-sanitize=undefined"), ], @@ -631,7 +623,7 @@ def compile_action_configs( action_configs.append( # Support for order-file instrumentation. ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [ add_arg("-sanitize=undefined"), add_arg("-sanitize-coverage=func"), @@ -652,7 +644,9 @@ def compile_action_configs( # spelling, ensuring that the ClangImporter options section of the # `.swiftmodule` file is hermetic. ActionConfigInfo( - actions = all_compile_action_names(), + actions = all_compile_action_names() + [ + SWIFT_ACTION_COMPILE_MODULE_INTERFACE, + ], configurators = [ add_arg("-file-compilation-dir", "."), ], @@ -661,8 +655,7 @@ def compile_action_configs( # Treat paths in .modulemap files as workspace-relative, not modulemap- # relative. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -680,8 +673,7 @@ def compile_action_configs( # Configure how implicit modules are handled--either using the module # cache, or disabled completely when using explicit modules. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -695,8 +687,7 @@ def compile_action_configs( ], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -710,8 +701,7 @@ def compile_action_configs( not_features = [SWIFT_FEATURE_USE_C_MODULES], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -726,8 +716,7 @@ def compile_action_configs( ], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, ], @@ -739,8 +728,7 @@ def compile_action_configs( # because all of them, including system dependencies, will be provided # explicitly. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -753,8 +741,7 @@ def compile_action_configs( ), # When using C modules, disable the implicit module cache. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_PRECOMPILE_C_MODULE, SWIFT_ACTION_SYMBOL_GRAPH_EXTRACT, @@ -795,8 +782,7 @@ def compile_action_configs( #### Search paths/explicit module map for Swift module dependencies action_configs.extend([ ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, ], @@ -824,8 +810,7 @@ def compile_action_configs( features = [SWIFT_FEATURE_USE_EXPLICIT_SWIFT_MODULE_MAP], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -837,8 +822,7 @@ def compile_action_configs( ], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, ], @@ -859,19 +843,21 @@ def compile_action_configs( features = [SWIFT_FEATURE_VFSOVERLAY], ), ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [_module_aliases_configurator], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [_plugins_configurator], ), ActionConfigInfo( actions = [ + # TODO: b/351801556 - Verify that this is correct; it may need + # to be done by the codegen actions. SWIFT_ACTION_COMPILE, + SWIFT_ACTION_COMPILE_MODULE, SWIFT_ACTION_DERIVE_FILES, ], configurators = [_macro_expansion_configurator], @@ -881,8 +867,7 @@ def compile_action_configs( not_features = [SWIFT_FEATURE_OPT], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [_plugin_search_paths_configurator], @@ -911,8 +896,7 @@ def compile_action_configs( #### Search paths for framework dependencies action_configs.extend([ ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -943,8 +927,7 @@ def compile_action_configs( action_configs.extend([ # Pass flags to Clang for search paths and propagated defines. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -961,8 +944,7 @@ def compile_action_configs( # Pass flags to Clang for dependencies' module maps or explicit modules, # whichever are being used for this build. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -973,8 +955,7 @@ def compile_action_configs( features = [SWIFT_FEATURE_USE_C_MODULES], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -1005,8 +986,7 @@ def compile_action_configs( # Request color diagnostics, since Bazel pipes the output and causes the # driver's TTY check to fail. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_PRECOMPILE_C_MODULE, ], @@ -1028,6 +1008,9 @@ def compile_action_configs( # performance hack. ActionConfigInfo( actions = [ + # Only use for legacy Swift compile planning. Parallelized + # builds cannot use batch mode, because Bazel needs to control + # the batching directly. SWIFT_ACTION_COMPILE, SWIFT_ACTION_DERIVE_FILES, ], @@ -1042,8 +1025,7 @@ def compile_action_configs( # Set the number of threads to use for WMO. (We can skip this if we know # we'll already be applying `-num-threads` via `--swiftcopt` flags.) ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [ @@ -1060,8 +1042,7 @@ def compile_action_configs( not_features = [SWIFT_FEATURE__NUM_THREADS_0_IN_SWIFTCOPTS], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [ @@ -1079,8 +1060,7 @@ def compile_action_configs( # Set the module name. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, SWIFT_ACTION_PRECOMPILE_C_MODULE, @@ -1090,8 +1070,7 @@ def compile_action_configs( configurators = [_module_name_configurator], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_PRECOMPILE_C_MODULE, ], @@ -1103,8 +1082,7 @@ def compile_action_configs( # Set the package name. ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, SWIFT_ACTION_PRECOMPILE_C_MODULE, @@ -1123,12 +1101,12 @@ def compile_action_configs( # Configure index-while-building. ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [_index_while_building_configurator], features = [SWIFT_FEATURE_INDEX_WHILE_BUILDING], ), ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [add_arg("-index-include-locals")], features = [ SWIFT_FEATURE_INDEX_WHILE_BUILDING, @@ -1136,7 +1114,7 @@ def compile_action_configs( ], ), ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [add_arg("-index-ignore-system-modules")], features = [ SWIFT_FEATURE_INDEX_WHILE_BUILDING, @@ -1144,7 +1122,7 @@ def compile_action_configs( ], ), ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [ add_arg("-index-ignore-clang-modules"), ], @@ -1171,7 +1149,7 @@ def compile_action_configs( ], ), ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [_global_index_store_configurator], features = [ SWIFT_FEATURE_INDEX_WHILE_BUILDING, @@ -1181,23 +1159,21 @@ def compile_action_configs( # Disable auto-linking for prebuilt static frameworks. ActionConfigInfo( - actions = [SWIFT_ACTION_COMPILE], + actions = all_compile_action_names(), configurators = [_frameworks_disable_autolink_configurator], ), # User-defined conditional compilation flags (defined for Swift; those # passed directly to ClangImporter are handled above). ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, ], configurators = [_conditional_compilation_flag_configurator], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, ], @@ -1205,36 +1181,28 @@ def compile_action_configs( features = [SWIFT_FEATURE_ENABLE_BARE_SLASH_REGEX], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, - ], + actions = all_compile_action_names(), configurators = [add_arg("-Xfrontend", "-disable-clang-spi")], features = [ SWIFT_FEATURE_DISABLE_CLANG_SPI, ], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, - ], + actions = all_compile_action_names(), configurators = [add_arg("-Xfrontend", "-disable-availability-checking")], features = [ SWIFT_FEATURE_DISABLE_AVAILABILITY_CHECKING, ], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, - ], + actions = all_compile_action_names(), configurators = [_upcoming_and_experimental_features_configurator], features = [ SWIFT_FEATURE__SUPPORTS_UPCOMING_FEATURES, ], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, - ], + actions = all_compile_action_names(), configurators = [add_arg("-swift-version", "6")], features = [ SWIFT_FEATURE_ENABLE_V6, @@ -1251,8 +1219,7 @@ def compile_action_configs( # `copts` attribute. action_configs.append( ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -1263,8 +1230,7 @@ def compile_action_configs( if additional_objc_copts: action_configs.append( ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -1286,8 +1252,7 @@ def compile_action_configs( # TODO(allevato): Determine if there are any uses of # `-Xcc`-prefixed flags that need to be added to explicit module # actions, or if we should advise against/forbid that. - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -1303,8 +1268,7 @@ def compile_action_configs( action_configs.append( ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -1317,8 +1281,7 @@ def compile_action_configs( # Add additional input files to the sandbox (does not modify flags). action_configs.append( ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, ], @@ -1416,6 +1379,14 @@ def _output_ast_path_or_file_map_configurator(prerequisites, args): args = args, ) +def _compile_step_configurator(prerequisites, args): + """Adds the parallelized compilation plan step to the command line.""" + step = prerequisites.compile_step + args.add("-Xwrapped-swift=-compile-step={}={}".format( + step.action, + step.output, + )) + def _output_pcm_file_configurator(prerequisites, args): """Adds the `.pcm` output path to the command line.""" args.add("-o", prerequisites.pcm_file) diff --git a/swift/toolchains/xcode_swift_toolchain.bzl b/swift/toolchains/xcode_swift_toolchain.bzl index 498c6f4d2..3dd5c7165 100644 --- a/swift/toolchains/xcode_swift_toolchain.bzl +++ b/swift/toolchains/xcode_swift_toolchain.bzl @@ -40,12 +40,15 @@ load( load( "//swift/internal:action_names.bzl", "SWIFT_ACTION_COMPILE", + "SWIFT_ACTION_COMPILE_CODEGEN", + "SWIFT_ACTION_COMPILE_MODULE", "SWIFT_ACTION_COMPILE_MODULE_INTERFACE", "SWIFT_ACTION_DERIVE_FILES", "SWIFT_ACTION_DUMP_AST", "SWIFT_ACTION_PRECOMPILE_C_MODULE", "SWIFT_ACTION_SYMBOL_GRAPH_EXTRACT", "SWIFT_ACTION_SYNTHESIZE_INTERFACE", + "all_compile_action_names", ) load("//swift/internal:attrs.bzl", "swift_toolchain_driver_attrs") load("//swift/internal:developer_dirs.bzl", "swift_developer_lib_dir") @@ -327,8 +330,7 @@ def _all_action_configs( # Basic compilation flags (target triple and toolchain search paths). action_configs = [ ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, @@ -346,8 +348,7 @@ def _all_action_configs( action_configs.extend([ # Xcode path remapping ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [ @@ -361,8 +362,7 @@ def _all_action_configs( ], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_COMPILE_MODULE_INTERFACE, SWIFT_ACTION_DERIVE_FILES, ], @@ -381,8 +381,7 @@ def _all_action_configs( ], ), ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, ], configurators = [ @@ -406,8 +405,7 @@ def _all_action_configs( # directory so that modules are found correctly. action_configs.append( ActionConfigInfo( - actions = [ - SWIFT_ACTION_COMPILE, + actions = all_compile_action_names() + [ SWIFT_ACTION_DERIVE_FILES, SWIFT_ACTION_DUMP_AST, SWIFT_ACTION_PRECOMPILE_C_MODULE, @@ -509,6 +507,8 @@ def _all_tool_configs( tool_configs = { SWIFT_ACTION_COMPILE: tool_config, + SWIFT_ACTION_COMPILE_CODEGEN: tool_config, + SWIFT_ACTION_COMPILE_MODULE: tool_config, SWIFT_ACTION_DERIVE_FILES: tool_config, SWIFT_ACTION_DUMP_AST: tool_config, SWIFT_ACTION_PRECOMPILE_C_MODULE: ( @@ -779,6 +779,7 @@ def _xcode_swift_toolchain_impl(ctx): clang_implicit_deps_providers = collect_implicit_deps_providers( ctx.attr.clang_implicit_deps, ), + codegen_batch_size = 8, cross_import_overlays = [ target[SwiftCrossImportOverlayInfo] for target in ctx.attr.cross_import_overlays From 2895800329d8922ea555314998e55ba6a254dcf2 Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Wed, 24 Sep 2025 00:33:21 -0400 Subject: [PATCH 05/13] Add initial parallel compilation support: b9cb4ffb46dc0499b0eab9c780d0ff8eb9d36115 --- swift/internal/compiling.bzl | 732 ++++++++++++++++++++++++----------- 1 file changed, 514 insertions(+), 218 deletions(-) diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index 5c23da2be..fed082693 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -28,6 +28,8 @@ load( load( ":action_names.bzl", "SWIFT_ACTION_COMPILE", + "SWIFT_ACTION_COMPILE_CODEGEN", + "SWIFT_ACTION_COMPILE_MODULE", "SWIFT_ACTION_COMPILE_MODULE_INTERFACE", "SWIFT_ACTION_DERIVE_FILES", "SWIFT_ACTION_DUMP_AST", @@ -38,6 +40,7 @@ load(":explicit_module_map_file.bzl", "write_explicit_swift_module_map_file") load( ":feature_names.bzl", "SWIFT_FEATURE_ADD_TARGET_NAME_TO_OUTPUT", + "SWIFT_FEATURE_COMPILE_IN_PARALLEL", "SWIFT_FEATURE_DECLARE_SWIFTSOURCEINFO", "SWIFT_FEATURE_EMIT_BC", "SWIFT_FEATURE_EMIT_C_MODULE", @@ -455,18 +458,6 @@ def compile( else: original_module_name = None - # 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 = ( - swift_infos + - swift_toolchain.generated_header_module_implicit_deps_providers.swift_infos - ) - # These are the `SwiftInfo` providers that will be merged with the compiled # module context and returned as the `swift_info` field of this function's # result. Note that private deps are explicitly not included here, as they @@ -501,56 +492,17 @@ def compile( const_gather_protocols_file = swift_toolchain.const_protocols_to_gather - compile_outputs = _declare_compile_outputs( + compile_plan = _construct_compile_plan( srcs = srcs, actions = actions, extract_const_values = bool(const_gather_protocols_file), feature_configuration = feature_configuration, generated_header_name = generated_header_name, - generated_module_deps_swift_infos = generated_module_deps_swift_infos, module_name = module_name, target_name = target_name, user_compile_flags = copts, ) - - split_derived_file_generation = is_feature_enabled( - feature_configuration = feature_configuration, - feature_name = SWIFT_FEATURE_SPLIT_DERIVED_FILES_GENERATION, - ) - - if split_derived_file_generation: - all_compile_outputs = compact([ - compile_outputs.swiftinterface_file, - compile_outputs.private_swiftinterface_file, - compile_outputs.indexstore_directory, - compile_outputs.macro_expansion_directory, - ]) + compile_outputs.object_files + compile_outputs.const_values_files - all_derived_outputs = compact([ - # The `.swiftmodule` file is explicitly listed as the first output - # because it will always exist and because Bazel uses it as a key for - # various things (such as the filename prefix for param files generated - # for that action). This guarantees some predictability. - compile_outputs.swiftmodule_file, - compile_outputs.swiftdoc_file, - compile_outputs.swiftsourceinfo_file, - compile_outputs.generated_header_file, - ]) - else: - all_compile_outputs = compact([ - # The `.swiftmodule` file is explicitly listed as the first output - # because it will always exist and because Bazel uses it as a key for - # various things (such as the filename prefix for param files generated - # for that action). This guarantees some predictability. - compile_outputs.swiftmodule_file, - compile_outputs.swiftdoc_file, - compile_outputs.swiftinterface_file, - compile_outputs.private_swiftinterface_file, - compile_outputs.swiftsourceinfo_file, - compile_outputs.generated_header_file, - compile_outputs.indexstore_directory, - compile_outputs.macro_expansion_directory, - ]) + compile_outputs.object_files + compile_outputs.const_values_files - all_derived_outputs = [] + compile_outputs = compile_plan.outputs # In `upstream` they call `merge_compilation_contexts` on passed in # `compilation_contexts` instead of merging `CcInfo`s. This is because @@ -647,64 +599,61 @@ to use swift_common.compile(include_dev_srch_paths = ...) instead.\ upcoming_features, experimental_features = upcoming_and_experimental_features( feature_configuration = feature_configuration, ) - prerequisites = struct( - additional_inputs = additional_inputs, - always_include_headers = is_feature_enabled( + prerequisites = { + "additional_inputs": additional_inputs, + "always_include_headers": is_feature_enabled( feature_configuration = feature_configuration, feature_name = SWIFT_FEATURE_HEADERS_ALWAYS_ACTION_INPUTS, ), - bin_dir = feature_configuration._bin_dir, - cc_compilation_context = merged_cc_info.compilation_context, - const_gather_protocols_file = const_gather_protocols_file, - cc_linking_context = merged_cc_info.linking_context, - defines = sets.to_list(defines_set), - developer_dirs = swift_toolchain.developer_dirs, - experimental_features = experimental_features, - explicit_swift_module_map_file = explicit_swift_module_map_file, - genfiles_dir = feature_configuration._genfiles_dir, - include_dev_srch_paths = include_dev_srch_paths_value, - is_swift = True, - module_name = module_name, - original_module_name = original_module_name, - package_name = package_name, - plugins = collections.uniq(used_plugins), - source_files = srcs, - target_label = feature_configuration._label, - transitive_modules = transitive_modules, - transitive_swiftmodules = transitive_swiftmodules, - upcoming_features = upcoming_features, - user_compile_flags = copts, - vfsoverlay_file = vfsoverlay_file, - vfsoverlay_search_path = _SWIFTMODULES_VFS_ROOT, - workspace_name = workspace_name, - # Merge the compile outputs into the prerequisites. - **struct_fields(compile_outputs) - ) + "bin_dir": feature_configuration._bin_dir, + "cc_compilation_context": merged_cc_info.compilation_context, + "const_gather_protocols_file": const_gather_protocols_file, + "cc_linking_context": merged_cc_info.linking_context, + "defines": sets.to_list(defines_set), + "developer_dirs": swift_toolchain.developer_dirs, + "experimental_features": experimental_features, + "explicit_swift_module_map_file": explicit_swift_module_map_file, + "genfiles_dir": feature_configuration._genfiles_dir, + "include_dev_srch_paths": include_dev_srch_paths_value, + "is_swift": True, + "module_name": module_name, + "original_module_name": original_module_name, + "package_name": package_name, + "plugins": collections.uniq(used_plugins), + "source_files": srcs, + "target_label": feature_configuration._label, + "transitive_modules": transitive_modules, + "transitive_swiftmodules": transitive_swiftmodules, + "upcoming_features": upcoming_features, + "user_compile_flags": copts, + "vfsoverlay_file": vfsoverlay_file, + "vfsoverlay_search_path": _SWIFTMODULES_VFS_ROOT, + "workspace_name": workspace_name, + } | struct_fields(compile_outputs) - if split_derived_file_generation: - run_toolchain_action( + if is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_COMPILE_IN_PARALLEL, + ): + _execute_compile_plan( actions = actions, - action_name = SWIFT_ACTION_DERIVE_FILES, + compile_plan = compile_plan, + exec_group = exec_group, + feature_configuration = feature_configuration, + prerequisites = prerequisites, + swift_toolchain = swift_toolchain, + toolchain_type = toolchain_type, + ) + else: + _plan_legacy_swift_compilation( + actions = actions, + compile_outputs = compile_plan.outputs, exec_group = exec_group, feature_configuration = feature_configuration, - outputs = all_derived_outputs, prerequisites = prerequisites, - progress_message = "Generating derived files for Swift module %{label}", swift_toolchain = swift_toolchain, toolchain_type = toolchain_type, ) - - run_toolchain_action( - actions = actions, - action_name = SWIFT_ACTION_COMPILE, - exec_group = exec_group, - feature_configuration = feature_configuration, - outputs = all_compile_outputs, - prerequisites = prerequisites, - progress_message = "Compiling Swift module %{label}", - swift_toolchain = swift_toolchain, - toolchain_type = toolchain_type, - ) # Dump AST has to run in its own action because `-dump-ast` is incompatible # with emitting dependency files, which compile/derive files use when @@ -718,71 +667,40 @@ to use swift_common.compile(include_dev_srch_paths = ...) instead.\ exec_group = exec_group, feature_configuration = feature_configuration, outputs = compile_outputs.ast_files, - prerequisites = prerequisites, + prerequisites = struct(**prerequisites), progress_message = "Dumping Swift AST for %{label}", swift_toolchain = swift_toolchain, toolchain_type = toolchain_type, ) - # If a header and module map were generated for this Swift module, attempt - # to precompile the explicit module for that header as well. - if generated_header_name and not is_feature_enabled( - feature_configuration = feature_configuration, - feature_name = SWIFT_FEATURE_NO_GENERATED_MODULE_MAP, - ): - compilation_context_to_compile = ( - compilation_context_for_explicit_module_compilation( - compilation_contexts = [ - cc_common.create_compilation_context( - headers = depset([ - compile_outputs.generated_header_file, - ]), - ), - merged_cc_info.compilation_context, - ], - swift_infos = swift_infos, - ) - ) - - pcm_outputs = _precompile_clang_module( - actions = actions, - cc_compilation_context = compilation_context_to_compile, - exec_group = exec_group, - feature_configuration = feature_configuration, - is_swift_generated_header = True, - module_map_file = compile_outputs.generated_module_map_file, - module_name = module_name, - swift_infos = generated_module_deps_swift_infos, - swift_toolchain = swift_toolchain, - target_name = target_name, - toolchain_type = toolchain_type, - ) - if pcm_outputs: - precompiled_module = pcm_outputs.pcm_file - else: - precompiled_module = None - else: - precompiled_module = None - compilation_context = create_compilation_context( defines = defines, srcs = srcs, transitive_modules = transitive_modules, ) - if compile_outputs.generated_header_file: - public_hdrs = [compile_outputs.generated_header_file] - else: - public_hdrs = [] + merged_compilation_context = merge_compilation_contexts( + transitive_compilation_contexts = ( + compilation_contexts + [ + cc_info.compilation_context + for cc_info in swift_toolchain.implicit_deps_providers.cc_infos + ] + ), + ) - if compile_outputs.generated_module_map_file and is_feature_enabled( + generated_header_module = _compile_generated_header_clang_module( + actions = actions, + exec_group = exec_group, feature_configuration = feature_configuration, - feature_name = SWIFT_FEATURE_PROPAGATE_GENERATED_MODULE_MAP, - ): - public_hdrs.append(compile_outputs.generated_module_map_file) - includes = [compile_outputs.generated_module_map_file.dirname] - else: - includes = [] + generated_header_file = compile_outputs.generated_header_file, + generated_header_name = generated_header_name, + merged_compilation_context = merged_compilation_context, + module_name = module_name, + swift_infos = swift_infos, + swift_toolchain = swift_toolchain, + target_name = target_name, + toolchain_type = toolchain_type, + ) module_context = create_swift_module_context( name = module_name, @@ -792,13 +710,13 @@ to use swift_common.compile(include_dev_srch_paths = ...) instead.\ compilation_contexts = compilation_contexts, defines = defines, feature_configuration = feature_configuration, - includes = includes, - public_hdrs = public_hdrs, + includes = generated_header_module.includes, + public_hdrs = generated_header_module.public_hdrs, swift_toolchain = swift_toolchain, target_name = target_name, ), - module_map = compile_outputs.generated_module_map_file, - precompiled_module = precompiled_module, + module_map = generated_header_module.module_map_file, + precompiled_module = generated_header_module.precompiled_module, ), compilation_context = compilation_context, is_system = False, @@ -838,6 +756,382 @@ to use swift_common.compile(include_dev_srch_paths = ...) instead.\ ), ) +def _execute_compile_plan( + actions, + compile_plan, + exec_group, + feature_configuration, + prerequisites, + swift_toolchain, + toolchain_type): + """Executes the planned actions needed to compile a Swift module. + + Args: + actions: The context's `actions` object. + compile_plan: A `struct` containing information about the planned + compilation actions. + exec_group: Runs the Swift compilation action under the given execution + group's context. If `None`, the default execution group is used. + feature_configuration: A feature configuration obtained from + `swift_common.configure_features`. + prerequisites: A `dict` containing the common prerequisites for the + compilation action. + swift_toolchain: The Swift toolchain being used to build. + toolchain_type: The toolchain type of the `swift_toolchain`. + """ + compile_outputs = compile_plan.outputs + module_outputs = compact([ + # We put the module file first so that any generated command line files + # will be named after it. This output will always exist so the names + # will be predictable. + compile_plan.module_outputs.swiftmodule_file, + compile_plan.module_outputs.generated_header_file, + compile_plan.module_outputs.macro_expansion_directory, + compile_plan.module_outputs.swiftdoc_file, + compile_plan.module_outputs.swiftinterface_file, + compile_plan.module_outputs.private_swiftinterface_file, + compile_plan.module_outputs.swiftsourceinfo_file, + ]) + + module_prereqs = dict(prerequisites) + module_prereqs["compile_step"] = struct( + action = SWIFT_ACTION_COMPILE_MODULE, + output = compile_outputs.swiftmodule_file.path, + ) + run_toolchain_action( + actions = actions, + action_name = SWIFT_ACTION_COMPILE_MODULE, + exec_group = exec_group, + feature_configuration = feature_configuration, + outputs = module_outputs, + prerequisites = struct(**module_prereqs), + progress_message = "Compiling Swift module {}".format( + prerequisites["module_name"], + ), + swift_toolchain = swift_toolchain, + toolchain_type = toolchain_type, + ) + + batches = _compute_codegen_batches( + batch_size = swift_toolchain.codegen_batch_size, + compile_plan = compile_plan, + feature_configuration = feature_configuration, + ) + for number, batch in enumerate(batches, 1): + object_prereqs = dict(prerequisites) + object_prereqs["compile_step"] = struct( + action = SWIFT_ACTION_COMPILE_CODEGEN, + output = ",".join([ + object.path + for invocation in batch + for object in invocation.objects + ]), + ) + batch_suffix = "" + if compile_plan.output_nature.emits_multiple_objects: + batch_suffix = " ({} of {})".format(number, len(batches)) + progress_message = "Codegen for Swift module {}{}".format( + prerequisites["module_name"], + batch_suffix, + ) + + batch_outputs = [ + output + for invocation in batch + for output in invocation.objects + invocation.other_outputs + ] + if is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_INDEX_WHILE_BUILDING, + ): + # TODO: b/351801556 - If this is true, then we only have one batch + # (`_compute_codegen_batches` ensures this). Indexing happens when + # object files are emitted, so we need to declare that output here. + # Update the APIs to support multiple indexstore directories per + # target so that we can emit one indexstore per batch instead. + batch_outputs.append(compile_plan.outputs.indexstore_directory) + + run_toolchain_action( + actions = actions, + action_name = SWIFT_ACTION_COMPILE_CODEGEN, + exec_group = exec_group, + feature_configuration = feature_configuration, + outputs = batch_outputs, + prerequisites = struct(**object_prereqs), + progress_message = progress_message, + swift_toolchain = swift_toolchain, + toolchain_type = toolchain_type, + ) + +def _compute_codegen_batches( + batch_size, + compile_plan, + feature_configuration): + """Computes the batches of object files that will be compiled. + + Args: + batch_size: The number of source files to compile in each batch. + compile_plan: A `struct` containing information about the planned + compilation actions. + feature_configuration: The feature configuration for the target being + compiled. + + Returns: + A list of batches. Each batch itself is a list, where each element is a + struct that specifies the outputs for a particular codegen invocation + to be registered. + """ + codegen_outputs = compile_plan.codegen_outputs + codegen_count = len(codegen_outputs) + + # TODO: b/351801556 - Update the APIs to support multiple indexstore + # directories per target so that we can emit one indexstore per batch. For + # now, force one batch if indexing is enabled. + if is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_INDEX_WHILE_BUILDING, + ): + return [codegen_outputs] + + batch_count = codegen_count // batch_size + + # Make sure to round up if we have a partial batch left over. + if codegen_count % batch_size != 0: + batch_count += 1 + + batches = [] + for batch_index in range(batch_count): + batch_start = batch_index * batch_size + batch_end = min(batch_start + batch_size, codegen_count) + batches.append(codegen_outputs[batch_start:batch_end]) + return batches + +def _plan_legacy_swift_compilation( + actions, + compile_outputs, + exec_group, + feature_configuration, + prerequisites, + swift_toolchain, + toolchain_type): + """Plans the single driver invocation needed to compile a Swift module. + + The legacy compilation mode uses a single driver invocation to compile both + the `.swiftmodule` file and the object files. + + Args: + actions: The context's `actions` object. + compile_outputs: A `struct` containing the registered outputs of the + compilation action. + exec_group: Runs the Swift compilation action under the given execution + group's context. If `None`, the default execution group is used. + feature_configuration: A feature configuration obtained from + `swift_common.configure_features`. + prerequisites: A `dict` containing the common prerequisites for the + compilation action. + swift_toolchain: The Swift toolchain being used to build. + toolchain_type: The toolchain type of the `swift_toolchain`. + """ + + split_derived_file_generation = is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_SPLIT_DERIVED_FILES_GENERATION, + ) + + all_derived_outputs = [] + if split_derived_file_generation: + all_compile_outputs = compact([ + compile_outputs.swiftinterface_file, + compile_outputs.private_swiftinterface_file, + compile_outputs.indexstore_directory, + compile_outputs.macro_expansion_directory, + ]) + compile_outputs.object_files + compile_outputs.const_values_files + all_derived_outputs = compact([ + # The `.swiftmodule` file is explicitly listed as the first output + # because it will always exist and because Bazel uses it as a key for + # various things (such as the filename prefix for param files generated + # for that action). This guarantees some predictability. + compile_outputs.swiftmodule_file, + compile_outputs.swiftdoc_file, + compile_outputs.swiftsourceinfo_file, + compile_outputs.generated_header_file, + ]) + else: + all_compile_outputs = compact([ + # The `.swiftmodule` file is explicitly listed as the first output + # because it will always exist and because Bazel uses it as a key for + # various things (such as the filename prefix for param files generated + # for that action). This guarantees some predictability. + compile_outputs.swiftmodule_file, + compile_outputs.swiftdoc_file, + compile_outputs.swiftinterface_file, + compile_outputs.private_swiftinterface_file, + compile_outputs.swiftsourceinfo_file, + compile_outputs.generated_header_file, + compile_outputs.indexstore_directory, + compile_outputs.macro_expansion_directory, + ]) + compile_outputs.object_files + compile_outputs.const_values_files + + if split_derived_file_generation: + run_toolchain_action( + actions = actions, + action_name = SWIFT_ACTION_DERIVE_FILES, + exec_group = exec_group, + feature_configuration = feature_configuration, + outputs = all_derived_outputs, + prerequisites = struct(**prerequisites), + progress_message = "Generating derived files for Swift module %{label}", + swift_toolchain = swift_toolchain, + toolchain_type = toolchain_type, + ) + + run_toolchain_action( + actions = actions, + action_name = SWIFT_ACTION_COMPILE, + exec_group = exec_group, + feature_configuration = feature_configuration, + outputs = all_compile_outputs, + prerequisites = struct(**prerequisites), + progress_message = "Compiling Swift module {}".format( + prerequisites["module_name"], + ), + swift_toolchain = swift_toolchain, + toolchain_type = toolchain_type, + ) + +def _compile_generated_header_clang_module( + actions, + exec_group, + feature_configuration, + generated_header_file, + generated_header_name, + merged_compilation_context, + module_name, + swift_infos, + swift_toolchain, + target_name, + toolchain_type): + """Precompiles the Clang module for a Swift module's generated header. + + Args: + + Returns: + A `struct` (never `None`) containing the following fields: + + * `module_map_file`: The module map file that defines the Clang module + for the Swift generated header. This may be `None` if the + `swift.no_generated_module_map` feature is enabled. + * `precompiled_module`: The precompiled module that contains the + compiled Clang module. This may be `None` if explicit modules are + not enabled. + """ + + # If no generated header and not generating a module map, nothing to do. + if not generated_header_name and is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_NO_GENERATED_MODULE_MAP, + ): + return struct( + module_map_file = None, + precompiled_module = None, + includes = [], + public_hdrs = [], + ) + + # 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. + generated_module_deps_swift_infos = ( + swift_infos + + swift_toolchain.generated_header_module_implicit_deps_providers.swift_infos + ) + dependent_module_names = sets.make() + 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) + + # Create a module map for the generated header file. This ensures that + # inclusions of it are treated modularly, not textually. + # + # Caveat: Generated module maps are incompatible with the hack that some + # folks are using to support mixed Objective-C and Swift modules. This trap + # door lets them escape the module redefinition error, with the caveat that + # that certain import scenarios could lead to incorrect behavior because a + # header can be imported textually instead of modularly. + if generated_header_file and not is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_NO_GENERATED_MODULE_MAP, + ): + generated_module_map = actions.declare_file( + "{}_modulemap/_/module.modulemap".format(target_name), + ) + write_module_map( + actions = actions, + dependent_module_names = sorted(sets.to_list(dependent_module_names)), + module_map_file = generated_module_map, + module_name = module_name, + public_headers = [generated_header_file], + workspace_relative = is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_MODULE_MAP_HOME_IS_CWD, + ), + ) + + compilation_context_to_compile = ( + compilation_context_for_explicit_module_compilation( + compilation_contexts = [ + cc_common.create_compilation_context( + headers = depset([generated_header_file]), + ), + merged_compilation_context, + ], + swift_infos = swift_infos, + ) + ) + pcm_outputs = _precompile_clang_module( + actions = actions, + cc_compilation_context = compilation_context_to_compile, + exec_group = exec_group, + feature_configuration = feature_configuration, + is_swift_generated_header = True, + module_map_file = generated_module_map, + module_name = module_name, + swift_infos = generated_module_deps_swift_infos, + swift_toolchain = swift_toolchain, + target_name = target_name, + toolchain_type = toolchain_type, + ) + if pcm_outputs: + precompiled_module = pcm_outputs.pcm_file + else: + precompiled_module = None + else: + generated_module_map = None + precompiled_module = None + + if generated_header_file: + public_hdrs = [generated_header_file] + else: + public_hdrs = [] + + if generated_module_map and is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_PROPAGATE_GENERATED_MODULE_MAP, + ): + public_hdrs.append(generated_module_map) + includes = [generated_module_map.dirname] + else: + includes = [] + + return struct( + module_map_file = generated_module_map, + precompiled_module = precompiled_module, + includes = includes, + public_hdrs = public_hdrs, + ) + def precompile_clang_module( *, actions, @@ -1156,13 +1450,12 @@ def _cross_imported_swift_infos(*, swift_toolchain, user_swift_infos): return overlay_swift_infos -def _declare_compile_outputs( +def _construct_compile_plan( *, actions, extract_const_values, feature_configuration, generated_header_name, - generated_module_deps_swift_infos, module_name, srcs, target_name, @@ -1177,9 +1470,6 @@ def _declare_compile_outputs( `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_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 @@ -1271,45 +1561,6 @@ def _declare_compile_outputs( else: generated_header = None - # If not disabled, create a module map for the generated header file. This - # ensures that inclusions of it are treated modularly, not textually. - # - # Caveat: Generated module maps are incompatible with the hack that some - # folks are using to support mixed Objective-C and Swift modules. This - # trap door lets them escape the module redefinition error, with the - # caveat that certain import scenarios could lead to incorrect behavior - # because a header can be imported textually instead of modularly. - if generated_header and not is_feature_enabled( - feature_configuration = feature_configuration, - feature_name = SWIFT_FEATURE_NO_GENERATED_MODULE_MAP, - ): - # Collect the names of Clang modules that the module being built - # directly depends on. - dependent_module_names = sets.make() - 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 = actions.declare_file( - "{}_modulemap/_/module.modulemap".format(target_name), - ) - write_module_map( - actions = actions, - dependent_module_names = sorted( - sets.to_list(dependent_module_names), - ), - module_map_file = generated_module_map, - module_name = module_name, - public_headers = [generated_header], - workspace_relative = is_feature_enabled( - feature_configuration = feature_configuration, - feature_name = SWIFT_FEATURE_MODULE_MAP_HOME_IS_CWD, - ), - ) - else: - generated_module_map = None - # Now, declare outputs like object files for which there may be one or many, # depending on the compilation mode. output_nature = _emitted_output_nature( @@ -1357,6 +1608,10 @@ def _declare_compile_outputs( ] output_file_map = None derived_files_output_file_map = None + codegen_outputs = [struct( + objects = object_files, + other_outputs = const_values_files, + )] # TODO(b/147451378): Support indexing even with a single object file. else: @@ -1401,6 +1656,7 @@ def _declare_compile_outputs( const_values_files = output_info.const_values_files output_file_map = output_info.output_file_map derived_files_output_file_map = output_info.derived_files_output_file_map + codegen_outputs = output_info.codegen_outputs if not is_feature_enabled( feature_configuration = feature_configuration, @@ -1416,7 +1672,6 @@ def _declare_compile_outputs( ast_files = ast_files, const_values_files = const_values_files, generated_header_file = generated_header, - generated_module_map_file = generated_module_map, indexstore_directory = indexstore_directory, macro_expansion_directory = macro_expansion_directory, private_swiftinterface_file = private_swiftinterface_file, @@ -1428,7 +1683,26 @@ def _declare_compile_outputs( swiftmodule_file = swiftmodule_file, swiftsourceinfo_file = swiftsourceinfo_file, ) - return compile_outputs + + return struct( + codegen_outputs = codegen_outputs, + module_outputs = struct( + generated_header_file = generated_header, + # TODO: b/351801556 - Verify that this is correct; it may need to be + # done by the codegen actions. + macro_expansion_directory = macro_expansion_directory, + swiftdoc_file = swiftdoc_file, + swiftinterface_file = swiftinterface_file, + private_swiftinterface_file = private_swiftinterface_file, + swiftmodule_file = swiftmodule_file, + swiftsourceinfo_file = swiftsourceinfo_file, + ), + output_file_map = output_file_map, + output_nature = output_nature, + # TODO: b/351801556 - Migrate all the action configuration logic off + # this legacy structure. + outputs = compile_outputs, + ) def _declare_per_source_output_file(actions, extension, target_name, src): """Declares a file for a per-source output file during compilation. @@ -1501,6 +1775,14 @@ def _declare_multiple_outputs_and_write_output_file_map( "{}.output_file_map.json".format(target_name), ) + # Collect the outputs that are expected to be produced by codegen actions. + # In a WMO build, all object files (and related codegen outputs, like + # const-values files) are emitted by a single frontend action, *regardless* + # of whether the compilation is multi-threaded (i.e., produces multiple + # outputs) or not. In a non-WMO build, there will be one frontend action + # per source file. + codegen_outputs = [] + if split_derived_file_generation: derived_files_output_map_file = actions.declare_file( "{}.derived_output_file_map.json".format(target_name), @@ -1518,13 +1800,6 @@ def _declare_multiple_outputs_and_write_output_file_map( output_objs = [] const_values_files = [] - if extract_const_values and is_wmo: - const_values_file = actions.declare_file( - "{}.swiftconstvalues".format(target_name), - ) - const_values_files.append(const_values_file) - whole_module_map["const-values"] = const_values_file.path - for src in srcs: file_outputs = {} @@ -1561,18 +1836,38 @@ def _declare_multiple_outputs_and_write_output_file_map( if include_index_unit_paths: file_outputs["index-unit-output-path"] = obj.path - if extract_const_values and not is_wmo: - const_values_file = _declare_per_source_output_file( - actions = actions, - extension = "swiftconstvalues", - target_name = target_name, - src = src, - ) - const_values_files.append(const_values_file) - file_outputs["const-values"] = const_values_file.path + if not is_wmo: + const_values_file = None + if extract_const_values: + const_values_file = _declare_per_source_output_file( + actions = actions, + extension = "swiftconstvalues", + target_name = target_name, + src = src, + ) + const_values_files.append(const_values_file) + file_outputs["const-values"] = const_values_file.path + + codegen_outputs.append(struct( + objects = [obj], + other_outputs = compact([const_values_file]), + )) output_map[src.path] = file_outputs + if is_wmo: + if extract_const_values: + const_value_file = actions.declare_file( + "{}.swiftconstvalues".format(target_name), + ) + const_values_files.append(const_value_file) + whole_module_map["const-values"] = const_value_file.path + + codegen_outputs.append(struct( + objects = output_objs, + other_outputs = const_values_files, + )) + if whole_module_map: output_map[""] = whole_module_map @@ -1589,6 +1884,7 @@ def _declare_multiple_outputs_and_write_output_file_map( return struct( ast_files = ast_files, + codegen_outputs = codegen_outputs, const_values_files = const_values_files, derived_files_output_file_map = derived_files_output_map_file, object_files = output_objs, From 370db705e9bf0c94c8d5e4245b80742477bea99f Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Wed, 24 Sep 2025 18:37:42 -0400 Subject: [PATCH 06/13] Disable parallel compilation if an optimization flag in the -O group is being used. Unless cross-module-optimization is disabled (which we don't really support at this time), the driver in this case will create a single frontend invocation that emits both the module and performs codegen. Cherry-pick of upstream: 29f0ec28a3363fa99f05d04737a699c38fcce02b --- swift/internal/BUILD | 7 +- swift/internal/compiling.bzl | 28 ++++++- swift/internal/features.bzl | 33 +++++++-- swift/internal/{wmo.bzl => optimization.bzl} | 78 ++++++++++++-------- swift/toolchains/config/compile_config.bzl | 30 ++------ swift/toolchains/swift_toolchain.bzl | 6 +- swift/toolchains/xcode_swift_toolchain.bzl | 22 ++++-- 7 files changed, 125 insertions(+), 79 deletions(-) rename swift/internal/{wmo.bzl => optimization.bzl} (64%) diff --git a/swift/internal/BUILD b/swift/internal/BUILD index 7d47d8ae2..b4247541f 100644 --- a/swift/internal/BUILD +++ b/swift/internal/BUILD @@ -99,9 +99,9 @@ bzl_library( ":feature_names", ":features", ":module_maps", + ":optimization", ":utils", ":vfsoverlay", - ":wmo", "//swift:providers", "@bazel_skylib//lib:paths", "@bazel_skylib//lib:sets", @@ -314,9 +314,8 @@ bzl_library( ) bzl_library( - name = "wmo", - srcs = ["wmo.bzl"], - visibility = ["//swift:__subpackages__"], + name = "optimization", + srcs = ["optimization.bzl"], deps = [ ":feature_names", ], diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index fed082693..148849ff4 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -72,6 +72,12 @@ load( "upcoming_and_experimental_features", ) load(":module_maps.bzl", "write_module_map") +load( + ":optimization.bzl", + "find_num_threads_flag_value", + "is_optimization_manually_requested", + "is_wmo_manually_requested", +) load(":toolchain_utils.bzl", "SWIFT_TOOLCHAIN_TYPE") load( ":utils.bzl", @@ -84,7 +90,6 @@ load( "struct_fields", ) load(":vfsoverlay.bzl", "write_vfsoverlay") -load(":wmo.bzl", "find_num_threads_flag_value", "is_wmo_manually_requested") # VFS root where all .swiftmodule files will be placed when # SWIFT_FEATURE_VFSOVERLAY is enabled. @@ -631,9 +636,9 @@ to use swift_common.compile(include_dev_srch_paths = ...) instead.\ "workspace_name": workspace_name, } | struct_fields(compile_outputs) - if is_feature_enabled( + if _should_plan_parallel_compilation( feature_configuration = feature_configuration, - feature_name = SWIFT_FEATURE_COMPILE_IN_PARALLEL, + user_compile_flags = copts, ): _execute_compile_plan( actions = actions, @@ -756,6 +761,23 @@ to use swift_common.compile(include_dev_srch_paths = ...) instead.\ ), ) +def _should_plan_parallel_compilation( + feature_configuration, + user_compile_flags): + """Returns `True` if the compilation should be done in parallel.""" + parallel_requested = is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_COMPILE_IN_PARALLEL, + ) + + # The Swift driver will not emit separate jobs to compile the module and to + # perform codegen if optimization is requested. See + # https://github.com/swiftlang/swift-driver/blob/c647e91574122f2b104d294ab1ec5baadaa1aa95/Sources/SwiftDriver/Jobs/EmitModuleJob.swift#L156-L181. + opt_requested = is_optimization_manually_requested( + user_compile_flags = user_compile_flags, + ) + return parallel_requested and not opt_requested + def _execute_compile_plan( actions, compile_plan, diff --git a/swift/internal/features.bzl b/swift/internal/features.bzl index f088815e6..ff309eae3 100644 --- a/swift/internal/features.bzl +++ b/swift/internal/features.bzl @@ -20,6 +20,7 @@ load( ":feature_names.bzl", "SWIFT_FEATURE_CACHEABLE_SWIFTMODULES", "SWIFT_FEATURE_CHECKED_EXCLUSIVITY", + "SWIFT_FEATURE_COMPILE_IN_PARALLEL", "SWIFT_FEATURE_COVERAGE", "SWIFT_FEATURE_COVERAGE_PREFIX_MAP", "SWIFT_FEATURE_DEBUG_PREFIX_MAP", @@ -169,7 +170,7 @@ def configure_features( ) def features_for_build_modes(ctx, cpp_fragment = None): - """Returns a list of Swift toolchain features for current build modes. + """Returns features to request and disable for current build modes. This function explicitly breaks the "don't pass `ctx` as an argument" rule-of-thumb because it is internal and only called from the toolchain @@ -177,19 +178,35 @@ def features_for_build_modes(ctx, cpp_fragment = None): Args: ctx: The current rule context. - cpp_fragment: The Cpp configuration fragment, if available. + cpp_fragment: The `cpp` configuration fragment, if available. Returns: - A list of Swift toolchain features to enable. + A tuple containing two lists: + + 1. A list of Swift toolchain features to requested (enable). + 2. A list of Swift toolchain features that are unsupported (disabled). """ + requested_features = [] + unsupported_features = [] + compilation_mode = ctx.var["COMPILATION_MODE"] - features = [] - features.append("swift.{}".format(compilation_mode)) + requested_features.append("swift.{}".format(compilation_mode)) if compilation_mode in ("dbg", "fastbuild"): - features.append(SWIFT_FEATURE_ENABLE_TESTING) + requested_features.append(SWIFT_FEATURE_ENABLE_TESTING) + elif compilation_mode == "opt": + # Disable parallel compilation early if we know we're going to be doing + # an optimized compile, because the driver will not emit separate jobs + # unless we also explicitly disable cross-module optimization. See + # https://github.com/swiftlang/swift-driver/blob/c647e91574122f2b104d294ab1ec5baadaa1aa95/Sources/SwiftDriver/Jobs/EmitModuleJob.swift#L156-L181. + unsupported_features.append(SWIFT_FEATURE_COMPILE_IN_PARALLEL) + + if ctx.configuration.coverage_enabled: + requested_features.append(SWIFT_FEATURE_COVERAGE) + if cpp_fragment and cpp_fragment.apple_generate_dsym: - features.append(SWIFT_FEATURE_FULL_DEBUG_INFO) - return features + requested_features.append(SWIFT_FEATURE_FULL_DEBUG_INFO) + + return requested_features, unsupported_features def get_cc_feature_configuration(feature_configuration): """Returns the C++ feature configuration in a Swift feature configuration. diff --git a/swift/internal/wmo.bzl b/swift/internal/optimization.bzl similarity index 64% rename from swift/internal/wmo.bzl rename to swift/internal/optimization.bzl index 50fc8b272..f7807f878 100644 --- a/swift/internal/wmo.bzl +++ b/swift/internal/optimization.bzl @@ -12,10 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Functionality releated to detecting whole-module optimization.""" +"""Detection of various optimization settings.""" load( ":feature_names.bzl", + "SWIFT_FEATURE_COMPILE_IN_PARALLEL", "SWIFT_FEATURE__NUM_THREADS_0_IN_SWIFTCOPTS", "SWIFT_FEATURE__WMO_IN_SWIFTCOPTS", ) @@ -28,29 +29,14 @@ _WMO_FLAGS = { "-force-single-frontend-invocation": True, } -def features_from_swiftcopts(swiftcopts): - """Returns a list of features to enable based on `--swiftcopt` flags. - - Since `--swiftcopt` flags are hooked into the action configuration when the - toolchain is configured, it's not possible for individual actions to query - them easily if those flags may determine the nature of outputs (for example, - single- vs. multi-threaded WMO). The toolchain can call this function to map - those flags to private features that can be queried instead. - - Args: - swiftcopts: The list of command line flags that were passed using - `--swiftcopt`. - - Returns: - A list (possibly empty) of strings denoting feature names that should be - enabled on the toolchain. - """ - features = [] - if is_wmo_manually_requested(user_compile_flags = swiftcopts): - features.append(SWIFT_FEATURE__WMO_IN_SWIFTCOPTS) - if find_num_threads_flag_value(user_compile_flags = swiftcopts) == 0: - features.append(SWIFT_FEATURE__NUM_THREADS_0_IN_SWIFTCOPTS) - return features +# Swift command line flags in the `O` group that enable some kind of +# optimization. This explicitly excludes `-Onone`. +_OPT_FLAGS = { + "-O": True, + "-Oplayground": True, + "-Osize": True, + "-Ounchecked": True, +} def find_num_threads_flag_value(user_compile_flags): """Finds the value of the `-num-threads` flag. @@ -75,6 +61,20 @@ def find_num_threads_flag_value(user_compile_flags): saw_num_threads = True return num_threads +def is_optimization_manually_requested(user_compile_flags): + """Returns `True` if some optimization flag is in the given list of flags. + + Args: + user_compile_flags: A list of compiler flags to scan for optimization. + + Returns: + True if some optimization is enabled in the given list of flags. + """ + for copt in user_compile_flags: + if copt in _OPT_FLAGS: + return True + return False + def is_wmo_manually_requested(user_compile_flags): """Returns `True` if a WMO flag is in the given list of compiler flags. @@ -89,8 +89,8 @@ def is_wmo_manually_requested(user_compile_flags): return True return False -def wmo_features_from_swiftcopts(swiftcopts): - """Returns a list of features to enable based on `--swiftcopt` flags. +def optimization_features_from_swiftcopts(swiftcopts): + """Returns features to enable or disable based on `--swiftcopt` flags. Since `--swiftcopt` flags are hooked into the action configuration when the toolchain is configured, it's not possible for individual actions to query @@ -103,15 +103,29 @@ def wmo_features_from_swiftcopts(swiftcopts): `--swiftcopt`. Returns: - A list (possibly empty) of strings denoting feature names that should be - enabled on the toolchain. + A tuple containing two lists: + + 1. A list (possibly empty) of strings denoting feature names that + should be enabled on the toolchain. + 2. A list (possibly empty) of strings denoting feature names that + should be disabled on the toolchain. """ - features = [] + requested_features = [] + unsupported_features = [] + + if is_optimization_manually_requested(user_compile_flags = swiftcopts): + # Disable parallel compilation early if we know we're going to be doing + # an optimized compile, because the driver will not emit separate jobs + # unless we also explicitly disable cross-module optimization. See + # https://github.com/swiftlang/swift-driver/blob/c647e91574122f2b104d294ab1ec5baadaa1aa95/Sources/SwiftDriver/Jobs/EmitModuleJob.swift#L156-L181. + unsupported_features.append(SWIFT_FEATURE_COMPILE_IN_PARALLEL) + if is_wmo_manually_requested(user_compile_flags = swiftcopts): - features.append(SWIFT_FEATURE__WMO_IN_SWIFTCOPTS) + requested_features.append(SWIFT_FEATURE__WMO_IN_SWIFTCOPTS) if find_num_threads_flag_value(user_compile_flags = swiftcopts) == 1: - features.append(SWIFT_FEATURE__NUM_THREADS_0_IN_SWIFTCOPTS) - return features + requested_features.append(SWIFT_FEATURE__NUM_THREADS_0_IN_SWIFTCOPTS) + + return requested_features, unsupported_features def _safe_int(s): """Returns the base-10 integer value of `s` or `None` if it is invalid. diff --git a/swift/toolchains/config/compile_config.bzl b/swift/toolchains/config/compile_config.bzl index 2ecc5afa9..47b22d933 100644 --- a/swift/toolchains/config/compile_config.bzl +++ b/swift/toolchains/config/compile_config.bzl @@ -91,6 +91,10 @@ load( "SWIFT_FEATURE__SUPPORTS_V6", "SWIFT_FEATURE__WMO_IN_SWIFTCOPTS", ) +load( + "//swift/internal:optimization.bzl", + "is_wmo_manually_requested", +) load(":action_config.bzl", "ActionConfigInfo", "ConfigResultInfo", "add_arg") # The number of threads to use for WMO builds, using the same number of cores @@ -99,14 +103,6 @@ load(":action_config.bzl", "ActionConfigInfo", "ConfigResultInfo", "add_arg") # when an API to obtain this is available. _DEFAULT_WMO_THREAD_COUNT = 12 -# Swift command line flags that enable whole module optimization. (This -# dictionary is used as a set for quick lookup; the values are irrelevant.) -_WMO_FLAGS = { - "-wmo": True, - "-whole-module-optimization": True, - "-force-single-frontend-invocation": True, -} - def compile_action_configs( *, additional_objc_copts = [], @@ -1494,7 +1490,7 @@ def _pcm_developer_framework_paths_configurator(prerequisites, args): def _batch_mode_configurator(prerequisites, args): """Adds flags to enable batch compilation mode.""" - if not _is_wmo_manually_requested(prerequisites.user_compile_flags): + if not is_wmo_manually_requested(prerequisites.user_compile_flags): args.add("-enable-batch-mode") def _c_layering_check_configurator(prerequisites, args): @@ -2082,25 +2078,11 @@ def _make_wmo_thread_count_configurator(should_check_flags): return lambda _prerequisites, args: _add_num_threads(args) def _flag_checking_wmo_thread_count_configurator(prerequisites, args): - if _is_wmo_manually_requested(prerequisites.user_compile_flags): + if is_wmo_manually_requested(prerequisites.user_compile_flags): _add_num_threads(args) return _flag_checking_wmo_thread_count_configurator -def _is_wmo_manually_requested(user_compile_flags): - """Returns `True` if a WMO flag is in the given list of compiler flags. - - Args: - user_compile_flags: A list of compiler flags to scan for WMO usage. - - Returns: - True if WMO is enabled in the given list of flags. - """ - for copt in user_compile_flags: - if copt in _WMO_FLAGS: - return True - return False - def _exclude_swift_incompatible_define(define): """A `map_each` helper that excludes a define if it is not Swift-compatible. diff --git a/swift/toolchains/swift_toolchain.bzl b/swift/toolchains/swift_toolchain.bzl index 0f3b21f29..85cf1c740 100644 --- a/swift/toolchains/swift_toolchain.bzl +++ b/swift/toolchains/swift_toolchain.bzl @@ -60,6 +60,7 @@ load( "default_features_for_toolchain", "features_for_build_modes", ) +load("//swift/internal:optimization.bzl", "optimization_features_from_swiftcopts") load( "//swift/internal:providers.bzl", "SwiftCrossImportOverlayInfo", @@ -71,7 +72,6 @@ load( "collect_implicit_deps_providers", "get_swift_executable_for_toolchain", ) -load("//swift/internal:wmo.bzl", "features_from_swiftcopts") load( "//swift/toolchains/config:action_config.bzl", "ActionConfigInfo", @@ -453,9 +453,9 @@ def _swift_toolchain_impl(ctx): # Combine build mode features, autoconfigured features, and required # features. + features_from_swiftcopts, _ = optimization_features_from_swiftcopts(swiftcopts = swiftcopts) requested_features = ( - features_for_build_modes(ctx) + - features_from_swiftcopts(swiftcopts = swiftcopts) + features_for_build_modes(ctx) + features_from_swiftcopts ) requested_features.extend(default_features_for_toolchain( target_triple = target_triple, diff --git a/swift/toolchains/xcode_swift_toolchain.bzl b/swift/toolchains/xcode_swift_toolchain.bzl index 3dd5c7165..f9d7a9734 100644 --- a/swift/toolchains/xcode_swift_toolchain.bzl +++ b/swift/toolchains/xcode_swift_toolchain.bzl @@ -69,6 +69,10 @@ load( "default_features_for_toolchain", "features_for_build_modes", ) +load( + "//swift/internal:optimization.bzl", + "optimization_features_from_swiftcopts", +) load( "//swift/internal:providers.bzl", "SwiftCrossImportOverlayInfo", @@ -81,7 +85,6 @@ load( "compact", "get_swift_executable_for_toolchain", ) -load("//swift/internal:wmo.bzl", "wmo_features_from_swiftcopts") load( "//swift/toolchains/config:action_config.bzl", "ActionConfigInfo", @@ -683,10 +686,16 @@ def _xcode_swift_toolchain_impl(ctx): # Compute the default requested features and conditional ones based on Xcode # version. - requested_features = features_for_build_modes( - ctx, - cpp_fragment = cpp_fragment, - ) + wmo_features_from_swiftcopts(swiftcopts = swiftcopts) + requested_features, unsupported_features = ( + optimization_features_from_swiftcopts(swiftcopts = swiftcopts) + ) + build_mode_requested_features, build_mode_unsupported_features = ( + features_for_build_modes( + ctx, + cpp_fragment = cpp_fragment, + ) + ) + requested_features.extend(build_mode_requested_features) requested_features.extend(ctx.features) requested_features.extend(ctx.attr.default_enabled_features) requested_features.extend(default_features_for_toolchain( @@ -711,9 +720,12 @@ def _xcode_swift_toolchain_impl(ctx): requested_features.append(SWIFT_FEATURE__SUPPORTS_V6) unsupported_features = ctx.disabled_features + [ + # `-fmodule-map-file-home-is-cwd` is incompatible with Apple's module maps, + # which we must use for system frameworks. SWIFT_FEATURE_MODULE_MAP_HOME_IS_CWD, ] unsupported_features.extend(ctx.attr.default_unsupported_features) + unsupported_features.extend(build_mode_unsupported_features) env = _xcode_env(target_triple = target_triple, xcode_config = xcode_config) From 1fc74f6cd146e5e9e4c6049f76439542a93125fd Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Wed, 24 Sep 2025 18:46:10 -0400 Subject: [PATCH 07/13] Simplify how we represent single-batch codegen jobs by passing an empty string instead of the full list of object files. This reduces unnecessary processing for huge modules with hundreds of source files. Cherry-pick: c413bd3f7010a1f61a9b7b86851231b23a57d486 --- swift/internal/compiling.bzl | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index 148849ff4..9665a5336 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -841,14 +841,24 @@ def _execute_compile_plan( ) for number, batch in enumerate(batches, 1): object_prereqs = dict(prerequisites) - object_prereqs["compile_step"] = struct( - action = SWIFT_ACTION_COMPILE_CODEGEN, - output = ",".join([ + + # If there is only one batch (for small libraries, or libraries of any + # size compiled with whole-module optimization), we omit the requested + # file paths to eliminate some unneeded work in the worker. It will + # treat a blank value as "emit all outputs". + if len(batches) == 1: + step_detail = "" + else: + step_detail = ",".join([ object.path for invocation in batch for object in invocation.objects - ]), + ]) + object_prereqs["compile_step"] = struct( + action = SWIFT_ACTION_COMPILE_CODEGEN, + output = step_detail, ) + batch_suffix = "" if compile_plan.output_nature.emits_multiple_objects: batch_suffix = " ({} of {})".format(number, len(batches)) From c317debd01fb5a025f7fc5ba55f3304ec155b144 Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Wed, 24 Sep 2025 19:04:43 -0400 Subject: [PATCH 08/13] Force swift.compile_in_parallel in the tests that depend on looking up specific mnemonics. --- swift/internal/compiling.bzl | 9 ++++++++- test/coverage_settings_tests.bzl | 6 +++--- test/debug_settings_tests.bzl | 16 ++++++++++------ test/features_tests.bzl | 14 +++++++++----- test/fixtures/debug_settings/BUILD | 13 +++++++++++++ test/module_cache_settings_tests.bzl | 6 +++--- test/pch_output_dir_tests.bzl | 2 +- test/split_derived_files_tests.bzl | 6 +++--- 8 files changed, 50 insertions(+), 22 deletions(-) diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index 9665a5336..8a75c3ace 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -770,13 +770,20 @@ def _should_plan_parallel_compilation( feature_name = SWIFT_FEATURE_COMPILE_IN_PARALLEL, ) + # TODO: are we able to support split derived file generation in parallel, this feature is not in upstream. + # For now, force non-parallel compilation when split derived file generation is enabled. + split_derived_file_generation = is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_SPLIT_DERIVED_FILES_GENERATION, + ) + # The Swift driver will not emit separate jobs to compile the module and to # perform codegen if optimization is requested. See # https://github.com/swiftlang/swift-driver/blob/c647e91574122f2b104d294ab1ec5baadaa1aa95/Sources/SwiftDriver/Jobs/EmitModuleJob.swift#L156-L181. opt_requested = is_optimization_manually_requested( user_compile_flags = user_compile_flags, ) - return parallel_requested and not opt_requested + return parallel_requested and not opt_requested and not split_derived_file_generation def _execute_compile_plan( actions, diff --git a/test/coverage_settings_tests.bzl b/test/coverage_settings_tests.bzl index 4788109b6..06b755ac7 100644 --- a/test/coverage_settings_tests.bzl +++ b/test/coverage_settings_tests.bzl @@ -46,7 +46,7 @@ def coverage_settings_test_suite(name, tags = []): "-profile-coverage-mapping", "-Xwrapped-swift=-coverage-prefix-pwd-is-dot", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -60,7 +60,7 @@ def coverage_settings_test_suite(name, tags = []): not_expected_argv = [ "-Xwrapped-swift=-coverage-prefix-pwd-is-dot", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -72,6 +72,6 @@ def coverage_settings_test_suite(name, tags = []): "__BAZEL_XCODE_DEVELOPER_DIR__=/PLACEHOLDER_DEVELOPER_DIR", ], target_compatible_with = ["@platforms//os:macos"], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", target_under_test = "//test/fixtures/debug_settings:simple", ) diff --git a/test/debug_settings_tests.bzl b/test/debug_settings_tests.bzl index 15db65d1e..a8c59fcef 100644 --- a/test/debug_settings_tests.bzl +++ b/test/debug_settings_tests.bzl @@ -144,7 +144,7 @@ def debug_settings_test_suite(name, tags = []): "-Xfrontend -no-serialize-debugging-options", "-gline-tables-only", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -158,7 +158,7 @@ def debug_settings_test_suite(name, tags = []): not_expected_argv = [ "-Xwrapped-swift=-debug-prefix-pwd-is-dot", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -179,7 +179,7 @@ def debug_settings_test_suite(name, tags = []): "-Xfrontend -serialize-debugging-options", "-gline-tables-only", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -199,7 +199,7 @@ def debug_settings_test_suite(name, tags = []): "-Xfrontend -no-serialize-debugging-options", "-g", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -219,7 +219,7 @@ def debug_settings_test_suite(name, tags = []): "-Xfrontend -no-serialize-debugging-options", "-gline-tables-only", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -238,6 +238,8 @@ def debug_settings_test_suite(name, tags = []): "-g", "-gline-tables-only", ], + # In optimized mode, the driver still uses a single invocation for both + # the module and for codegen. mnemonic = "SwiftCompile", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", @@ -254,6 +256,8 @@ def debug_settings_test_suite(name, tags = []): not_expected_argv = [ "-Xfrontend -serialize-debugging-options", ], + # In optimized mode, the driver still uses a single invocation for both + # the module and for codegen. mnemonic = "SwiftCompile", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", @@ -266,7 +270,7 @@ def debug_settings_test_suite(name, tags = []): "__BAZEL_XCODE_DEVELOPER_DIR__=/PLACEHOLDER_DEVELOPER_DIR", ], target_compatible_with = ["@platforms//os:macos"], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", ) diff --git a/test/features_tests.bzl b/test/features_tests.bzl index d31d97b93..ddfee00e5 100644 --- a/test/features_tests.bzl +++ b/test/features_tests.bzl @@ -149,7 +149,7 @@ def features_test_suite(name, tags = []): not_expected_argv = [ "-Xwrapped-swift=-file-prefix-pwd-is-dot", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -162,7 +162,7 @@ def features_test_suite(name, tags = []): "__BAZEL_XCODE_DEVELOPER_DIR__=/PLACEHOLDER_DEVELOPER_DIR", ], target_compatible_with = ["@platforms//os:macos"], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -172,7 +172,7 @@ def features_test_suite(name, tags = []): not_expected_argv = [ "-Xwrapped-swift=-global-index-store-import-path=", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -182,7 +182,7 @@ def features_test_suite(name, tags = []): expected_argv = [ "-Xwrapped-swift=-global-index-store-import-path=bazel-out/_global_index_store", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", target_compatible_with = ["@platforms//os:macos"], target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -193,7 +193,7 @@ def features_test_suite(name, tags = []): expected_argv = [ "-disable-sandbox", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", target_compatible_with = ["@platforms//os:macos"], target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -202,6 +202,8 @@ def features_test_suite(name, tags = []): name = "{}_default_opt_test".format(name), tags = all_tags, expected_argv = ["-emit-object", "-O", "-whole-module-optimization"], + # In optimized mode, the driver still uses a single invocation for both + # the module and for codegen. mnemonic = "SwiftCompile", target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -211,6 +213,8 @@ def features_test_suite(name, tags = []): tags = all_tags, expected_argv = ["-emit-object", "-O"], not_expected_argv = ["-whole-module-optimization"], + # In optimized mode, the driver still uses a single invocation for both + # the module and for codegen. mnemonic = "SwiftCompile", target_under_test = "//test/fixtures/debug_settings:simple", ) diff --git a/test/fixtures/debug_settings/BUILD b/test/fixtures/debug_settings/BUILD index d42aee8e8..35a9707b2 100644 --- a/test/fixtures/debug_settings/BUILD +++ b/test/fixtures/debug_settings/BUILD @@ -16,5 +16,18 @@ licenses(["notice"]) swift_library( name = "simple", srcs = ["Empty.swift"], + features = [ + # TODO: b/351801556 - Remove this when it is the default on all + # toolchains. + "swift.compile_in_parallel", + ], + tags = FIXTURE_TAGS, +) + +# TODO: The `default_no_split_args` split_derived_files_generation is not supported in parallel. +# We need to handle falling back to non-parallel compilation when this feature is enabled, but dont currently. +swift_library( + name = "simple_batch_mode", + srcs = ["Empty.swift"], tags = FIXTURE_TAGS, ) diff --git a/test/module_cache_settings_tests.bzl b/test/module_cache_settings_tests.bzl index 8b41e7ce4..6b66050c4 100644 --- a/test/module_cache_settings_tests.bzl +++ b/test/module_cache_settings_tests.bzl @@ -67,7 +67,7 @@ def module_cache_settings_test_suite(name, tags = []): "-Xwrapped-swift=-ephemeral-module-cache", "/tmp/__build_bazel_rules_swift/swift_module_cache/$(WORKSPACE_NAME)", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -83,7 +83,7 @@ def module_cache_settings_test_suite(name, tags = []): not_expected_argv = [ "-Xwrapped-swift=-ephemeral-module-cache", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", ) @@ -95,7 +95,7 @@ def module_cache_settings_test_suite(name, tags = []): not_expected_argv = [ "/tmp/__build_bazel_rules_swift/swift_module_cache/$(WORKSPACE_NAME)", ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", ) diff --git a/test/pch_output_dir_tests.bzl b/test/pch_output_dir_tests.bzl index 3b2db318c..294ec5175 100644 --- a/test/pch_output_dir_tests.bzl +++ b/test/pch_output_dir_tests.bzl @@ -45,7 +45,7 @@ def pch_output_dir_test_suite(name, tags = []): # can't verify the whole argument here. It has the configuration # fragment baked in. ], - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", ) diff --git a/test/split_derived_files_tests.bzl b/test/split_derived_files_tests.bzl index b53c7cbd6..14e771569 100644 --- a/test/split_derived_files_tests.bzl +++ b/test/split_derived_files_tests.bzl @@ -138,14 +138,14 @@ def split_derived_files_test_suite(name, tags = []): "-emit-module-path", "-emit-object", "-enable-batch-mode", - "simple.output_file_map.json", + "simple_batch_mode.output_file_map.json", ], not_expected_argv = [ - "simple.derived_output_file_map.json", + "simple_batch_mode.derived_output_file_map.json", ], mnemonic = "SwiftCompile", tags = all_tags, - target_under_test = "//test/fixtures/debug_settings:simple", + target_under_test = "//test/fixtures/debug_settings:simple_batch_mode", ) default_no_split_provider_test( From e88c8c84e09eaeaf5ec3ab88d6c95339e96e2eac Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Tue, 24 Sep 2024 05:11:45 -0700 Subject: [PATCH 09/13] Add tests for parallel compilation to make sure that the various optimization modes interact as we expect. The tests highlight a couple cases that are currently known to be wrong so we can fix and test them without just manually building all possible combinations. Chery-pick: f6302a27808363222f4b18f8a807187271ef664a --- test/BUILD | 5 + test/fixtures/parallel_compilation/BUILD | 54 +++++++++++ .../fixtures/parallel_compilation/Empty.swift | 1 + test/parallel_compilation_tests.bzl | 97 +++++++++++++++++++ test/rules/actions_created_test.bzl | 93 ++++++++++++++++++ 5 files changed, 250 insertions(+) create mode 100644 test/fixtures/parallel_compilation/BUILD create mode 100644 test/fixtures/parallel_compilation/Empty.swift create mode 100644 test/parallel_compilation_tests.bzl create mode 100644 test/rules/actions_created_test.bzl diff --git a/test/BUILD b/test/BUILD index 35deeeb31..6155ff6a0 100644 --- a/test/BUILD +++ b/test/BUILD @@ -16,6 +16,7 @@ load(":module_cache_settings_tests.bzl", "module_cache_settings_test_suite") load(":module_interface_tests.bzl", "module_interface_test_suite") load(":module_mapping_tests.bzl", "module_mapping_test_suite") load(":output_file_map_tests.bzl", "output_file_map_test_suite") +load(":parallel_compilation_tests.bzl", "parallel_compilation_test_suite") load(":pch_output_dir_tests.bzl", "pch_output_dir_test_suite") load(":private_deps_tests.bzl", "private_deps_test_suite") load(":private_swiftinterface_tests.bzl", "private_swiftinterface_test_suite") @@ -80,6 +81,10 @@ synthesize_interface_test_suite( ], ) +parallel_compilation_test_suite( + name = "parallel_compilation", +) + pch_output_dir_test_suite(name = "pch_output_dir_settings") private_swiftinterface_test_suite(name = "private_swiftinterface") diff --git a/test/fixtures/parallel_compilation/BUILD b/test/fixtures/parallel_compilation/BUILD new file mode 100644 index 000000000..8bb1b17d7 --- /dev/null +++ b/test/fixtures/parallel_compilation/BUILD @@ -0,0 +1,54 @@ +load( + "//swift:swift_library.bzl", + "swift_library", +) +load("//test/fixtures:common.bzl", "FIXTURE_TAGS") + +package( + default_visibility = ["//test:__subpackages__"], + features = ["swift.compile_in_parallel"], +) + +licenses(["notice"]) + +############################################################################### + +swift_library( + name = "no_opt_no_wmo", + srcs = ["Empty.swift"], + tags = FIXTURE_TAGS, +) + +swift_library( + name = "no_opt_with_wmo", + srcs = ["Empty.swift"], + copts = ["-wmo"], + tags = FIXTURE_TAGS, +) + +swift_library( + name = "with_opt_no_wmo", + srcs = ["Empty.swift"], + copts = ["-O"], + tags = FIXTURE_TAGS, +) + +swift_library( + name = "with_opt_with_wmo", + srcs = ["Empty.swift"], + copts = [ + "-O", + "-wmo", + ], + tags = FIXTURE_TAGS, +) + +swift_library( + name = "onone_with_wmo", + srcs = ["Empty.swift"], + copts = [ + "-Onone", + "-wmo", + ], + tags = FIXTURE_TAGS, +) diff --git a/test/fixtures/parallel_compilation/Empty.swift b/test/fixtures/parallel_compilation/Empty.swift new file mode 100644 index 000000000..bd9ec079d --- /dev/null +++ b/test/fixtures/parallel_compilation/Empty.swift @@ -0,0 +1 @@ +// Intentionally empty. diff --git a/test/parallel_compilation_tests.bzl b/test/parallel_compilation_tests.bzl new file mode 100644 index 000000000..aff5bf390 --- /dev/null +++ b/test/parallel_compilation_tests.bzl @@ -0,0 +1,97 @@ +# Copyright 2024 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for parallel compilation.""" + +load("@bazel_skylib//rules:build_test.bzl", "build_test") +load( + "//test/rules:actions_created_test.bzl", + "actions_created_test", +) + +visibility("private") + +def parallel_compilation_test_suite(name, tags = []): + """Test suite for parallel compilation. + + Args: + name: The base name to be used in targets created by this macro. + tags: Additional tags to apply to each test. + """ + all_tags = [name] + tags + + # Non-optimized, non-WMO can be compiled in parallel. + actions_created_test( + name = "{}_no_opt_no_wmo".format(name), + mnemonics = ["SwiftCompileModule", "SwiftCompileCodegen", "-SwiftCompile"], + tags = all_tags, + target_under_test = "//test/fixtures/parallel_compilation:no_opt_no_wmo", + ) + + # Non-optimized, with-WMO can be compiled in parallel. + actions_created_test( + name = "{}_no_opt_with_wmo".format(name), + mnemonics = ["SwiftCompileModule", "SwiftCompileCodegen", "-SwiftCompile"], + tags = all_tags, + target_under_test = "//test/fixtures/parallel_compilation:no_opt_with_wmo", + ) + + # Optimized, non-WMO cannot be compiled in parallel. + # TODO: b/351801556 - This is actually incorrect based on further driver + # testing; update the rules to allow compiling these in parallel. + actions_created_test( + name = "{}_with_opt_no_wmo".format(name), + mnemonics = ["-SwiftCompileModule", "-SwiftCompileCodegen", "SwiftCompile"], + tags = all_tags, + target_under_test = "//test/fixtures/parallel_compilation:with_opt_no_wmo", + ) + + # Optimized, with-WMO cannot be compiled in parallel. + # TODO: b/351801556 - This should be allowed if cross-module-optimization is + # disabled. Update the rules to allow this and add a new version of this + # target that disables CMO so we can test both situtations. + actions_created_test( + name = "{}_with_opt_with_wmo".format(name), + mnemonics = ["-SwiftCompileModule", "-SwiftCompileCodegen", "SwiftCompile"], + tags = all_tags, + target_under_test = "//test/fixtures/parallel_compilation:with_opt_with_wmo", + ) + + # Make sure that when we look for optimizer flags, we don't treat `-Onone` + # as being optimized. + actions_created_test( + name = "{}_onone_with_wmo".format(name), + mnemonics = ["SwiftCompileModule", "SwiftCompileCodegen", "-SwiftCompile"], + tags = all_tags, + target_under_test = "//test/fixtures/parallel_compilation:onone_with_wmo", + ) + + # The analysis tests verify that we register the actions we expect. Use a + # `build_test` to make sure the actions execute successfully. + build_test( + name = "{}_build_test".format(name), + targets = [ + "//test/fixtures/parallel_compilation:no_opt_no_wmo", + "//test/fixtures/parallel_compilation:no_opt_with_wmo", + "//test/fixtures/parallel_compilation:with_opt_no_wmo", + "//test/fixtures/parallel_compilation:with_opt_with_wmo", + "//test/fixtures/parallel_compilation:onone_with_wmo", + ], + tags = all_tags, + ) + + native.test_suite( + name = name, + tags = all_tags, + ) diff --git a/test/rules/actions_created_test.bzl b/test/rules/actions_created_test.bzl new file mode 100644 index 000000000..3fa5768ba --- /dev/null +++ b/test/rules/actions_created_test.bzl @@ -0,0 +1,93 @@ +# Copyright 2020 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Rules for testing whether or not actions are simply created by a rule.""" + +load("@bazel_skylib//lib:collections.bzl", "collections") +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "unittest") + +visibility([ + "//test/...", +]) + +def _actions_created_test_impl(ctx): + env = analysistest.begin(ctx) + target_under_test = analysistest.target_under_test(env) + + actions = analysistest.target_actions(env) + for mnemonic in ctx.attr.mnemonics: + is_negative_test = mnemonic.startswith("-") + if is_negative_test: + mnemonic = mnemonic[1:] + + matching_actions = [ + action + for action in actions + if action.mnemonic == mnemonic + ] + actual_mnemonics = collections.uniq( + [action.mnemonic for action in actions], + ) + + if is_negative_test and matching_actions: + unittest.fail( + env, + ("Target '{}' registered actions with the mnemonic '{}', " + + "but it was not expected to (it had {}).").format( + str(target_under_test.label), + mnemonic, + actual_mnemonics, + ), + ) + elif not is_negative_test and not matching_actions: + unittest.fail( + env, + ("Target '{}' registered no actions with the expected " + + "mnemonic '{}' (it had {}).").format( + str(target_under_test.label), + mnemonic, + actual_mnemonics, + ), + ) + + return analysistest.end(env) + +def make_actions_created_test_rule(config_settings = {}): + """Returns a new `actions_created_test`-like rule with custom configs. + + Args: + config_settings: A dictionary of configuration settings and their values + that should be applied during tests. + + Returns: + A rule returned by `analysistest.make` that has the + `actions_created_test` interface and the given config settings. + """ + return analysistest.make( + _actions_created_test_impl, + attrs = { + "mnemonics": attr.string_list( + mandatory = True, + doc = """\ +A list of mnemonics that are expected to be created by the target under test. +A mnemonic may also be preceded by a `-` to indicate that it is not expected +to be created and the test should fail if it finds one. +""", + ), + }, + config_settings = config_settings, + ) + +# A default instantiation of the rule when no custom config settings are needed. +actions_created_test = make_actions_created_test_rule() From 6c411918e9e48e37a71ae4e1c5f871f78d4b25ab Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Thu, 26 Sep 2024 08:34:47 -0700 Subject: [PATCH 10/13] Update parallel compilation to handle optimization correctly. Explicitly disable cross-module optimization in `exec` configs so that Swift tools can be built in parallel to speed up critical build paths. Cherry-pic: d1ecc8a4145d20840bba5cb1a194e436f59d5293 --- swift/internal/compiling.bzl | 61 ++++++++++++++++++---- swift/internal/feature_names.bzl | 9 ++++ swift/internal/features.bzl | 22 +++++--- swift/internal/optimization.bzl | 8 --- swift/toolchains/config/compile_config.bzl | 6 +++ test/debug_settings_tests.bzl | 2 - test/features_tests.bzl | 4 +- test/fixtures/parallel_compilation/BUILD | 24 ++++++++- test/parallel_compilation_tests.bzl | 52 ++++++++++++++---- 9 files changed, 146 insertions(+), 42 deletions(-) diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index 8a75c3ace..b65434755 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -47,6 +47,7 @@ load( "SWIFT_FEATURE_EMIT_PRIVATE_SWIFTINTERFACE", "SWIFT_FEATURE_EMIT_SWIFTDOC", "SWIFT_FEATURE_EMIT_SWIFTINTERFACE", + "SWIFT_FEATURE_ENABLE_LIBRARY_EVOLUTION", "SWIFT_FEATURE_FULL_LTO", "SWIFT_FEATURE_HEADERS_ALWAYS_ACTION_INPUTS", "SWIFT_FEATURE_INDEX_WHILE_BUILDING", @@ -54,6 +55,7 @@ load( "SWIFT_FEATURE_MODULE_MAP_HOME_IS_CWD", "SWIFT_FEATURE_NO_GENERATED_MODULE_MAP", "SWIFT_FEATURE_OPT", + "SWIFT_FEATURE_OPT_USES_CMO", "SWIFT_FEATURE_OPT_USES_WMO", "SWIFT_FEATURE_PROPAGATE_GENERATED_MODULE_MAP", "SWIFT_FEATURE_SPLIT_DERIVED_FILES_GENERATION", @@ -765,25 +767,64 @@ def _should_plan_parallel_compilation( feature_configuration, user_compile_flags): """Returns `True` if the compilation should be done in parallel.""" - parallel_requested = is_feature_enabled( + if not is_feature_enabled( feature_configuration = feature_configuration, feature_name = SWIFT_FEATURE_COMPILE_IN_PARALLEL, - ) + ): + return False # TODO: are we able to support split derived file generation in parallel, this feature is not in upstream. # For now, force non-parallel compilation when split derived file generation is enabled. - split_derived_file_generation = is_feature_enabled( + if is_feature_enabled( feature_configuration = feature_configuration, feature_name = SWIFT_FEATURE_SPLIT_DERIVED_FILES_GENERATION, - ) - - # The Swift driver will not emit separate jobs to compile the module and to - # perform codegen if optimization is requested. See + ): + return False + + # When the Swift driver plans a compilation, the default behavior is to emit + # separate frontend jobs to emit the module and to perform codegen. However, + # this will *not* happen if cross-module optimization is possible; in that + # case, the driver emits a single frontend job to compile everything. If any + # of the following conditions is true, then cross-module optimization is not + # possible and we can plan parallel compilation: + # + # - Whole-module optimization is not enabled. + # - Library evolution is enabled. + # - Cross-module optimization has been explicitly disabled. + # - Optimization (via the `-O` flag group) has not been requested. + # + # This logic mirrors that defined in # https://github.com/swiftlang/swift-driver/blob/c647e91574122f2b104d294ab1ec5baadaa1aa95/Sources/SwiftDriver/Jobs/EmitModuleJob.swift#L156-L181. - opt_requested = is_optimization_manually_requested( - user_compile_flags = user_compile_flags, + if not ( + is_wmo_manually_requested( + user_compile_flags = user_compile_flags, + ) or are_all_features_enabled( + feature_configuration = feature_configuration, + feature_names = [SWIFT_FEATURE_OPT, SWIFT_FEATURE_OPT_USES_WMO], + ) + ): + return True + + if is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_ENABLE_LIBRARY_EVOLUTION, + ): + return True + + if not is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_OPT_USES_CMO, + ): + return True + + return ( + not is_optimization_manually_requested( + user_compile_flags = user_compile_flags, + ) and not is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_OPT, + ) ) - return parallel_requested and not opt_requested and not split_derived_file_generation def _execute_compile_plan( actions, diff --git a/swift/internal/feature_names.bzl b/swift/internal/feature_names.bzl index ec0ac492b..4ea0a7750 100644 --- a/swift/internal/feature_names.bzl +++ b/swift/internal/feature_names.bzl @@ -173,6 +173,15 @@ SWIFT_FEATURE_OPT_USES_WMO = "swift.opt_uses_wmo" # the `-Osize` flag instead of `-O`. SWIFT_FEATURE_OPT_USES_OSIZE = "swift.opt_uses_osize" +# If enabled, compilations that are using whole-module optimization and also +# request optimizations via the `-O` flag group will have cross-module +# optimization performed. This is the default behavior for the Swift compiler +# and for these build rules, with one exception: because CMO prevents +# parallelized compilation, we unconditionally disable CMO for targets built in +# an `exec` configuration under the presumption that build speed is more +# important than runtime performance for build tools. +SWIFT_FEATURE_OPT_USES_CMO = "swift.opt_uses_cmo" + # If enabled, and if the toolchain specifies a generated header rewriting tool, # that tool will be invoked after compilation to rewrite the generated header in # place. diff --git a/swift/internal/features.bzl b/swift/internal/features.bzl index ff309eae3..14182f0ae 100644 --- a/swift/internal/features.bzl +++ b/swift/internal/features.bzl @@ -20,7 +20,6 @@ load( ":feature_names.bzl", "SWIFT_FEATURE_CACHEABLE_SWIFTMODULES", "SWIFT_FEATURE_CHECKED_EXCLUSIVITY", - "SWIFT_FEATURE_COMPILE_IN_PARALLEL", "SWIFT_FEATURE_COVERAGE", "SWIFT_FEATURE_COVERAGE_PREFIX_MAP", "SWIFT_FEATURE_DEBUG_PREFIX_MAP", @@ -37,6 +36,7 @@ load( "SWIFT_FEATURE_INTERNALIZE_AT_LINK", "SWIFT_FEATURE_NO_GENERATED_MODULE_MAP", "SWIFT_FEATURE_OBJC_LINK_FLAGS", + "SWIFT_FEATURE_OPT_USES_CMO", "SWIFT_FEATURE_OPT_USES_WMO", "SWIFT_FEATURE_REMAP_XCODE_PATH", "SWIFT_FEATURE_USE_GLOBAL_MODULE_CACHE", @@ -137,6 +137,19 @@ def configure_features( unsupported_features = unsupported_features, ) + # Disable CMO for builds in `exec` configurations so that we can use + # parallelized compilation for build tools written in Swift. The speedup + # from parallelized compilation is significantly higher than the performance + # loss from disabling CMO. + # + # HACK: There is no supported API yet to detect whether a build is in an + # `exec` configuration. https://github.com/bazelbuild/bazel/issues/14444. + if "-exec" in ctx.bin_dir.path or "/host/" in ctx.bin_dir.path: + unsupported_features.append(SWIFT_FEATURE_OPT_USES_CMO) + else: + requested_features = list(requested_features) + requested_features.append(SWIFT_FEATURE_OPT_USES_CMO) + all_requestable_features, all_unsupported_features = _compute_features( label = ctx.label, requested_features = requested_features, @@ -150,6 +163,7 @@ def configure_features( requested_features = all_requestable_features, unsupported_features = all_unsupported_features, ) + return struct( _cc_feature_configuration = cc_feature_configuration, _enabled_features = all_requestable_features, @@ -193,12 +207,6 @@ def features_for_build_modes(ctx, cpp_fragment = None): requested_features.append("swift.{}".format(compilation_mode)) if compilation_mode in ("dbg", "fastbuild"): requested_features.append(SWIFT_FEATURE_ENABLE_TESTING) - elif compilation_mode == "opt": - # Disable parallel compilation early if we know we're going to be doing - # an optimized compile, because the driver will not emit separate jobs - # unless we also explicitly disable cross-module optimization. See - # https://github.com/swiftlang/swift-driver/blob/c647e91574122f2b104d294ab1ec5baadaa1aa95/Sources/SwiftDriver/Jobs/EmitModuleJob.swift#L156-L181. - unsupported_features.append(SWIFT_FEATURE_COMPILE_IN_PARALLEL) if ctx.configuration.coverage_enabled: requested_features.append(SWIFT_FEATURE_COVERAGE) diff --git a/swift/internal/optimization.bzl b/swift/internal/optimization.bzl index f7807f878..ca00b134e 100644 --- a/swift/internal/optimization.bzl +++ b/swift/internal/optimization.bzl @@ -16,7 +16,6 @@ load( ":feature_names.bzl", - "SWIFT_FEATURE_COMPILE_IN_PARALLEL", "SWIFT_FEATURE__NUM_THREADS_0_IN_SWIFTCOPTS", "SWIFT_FEATURE__WMO_IN_SWIFTCOPTS", ) @@ -113,13 +112,6 @@ def optimization_features_from_swiftcopts(swiftcopts): requested_features = [] unsupported_features = [] - if is_optimization_manually_requested(user_compile_flags = swiftcopts): - # Disable parallel compilation early if we know we're going to be doing - # an optimized compile, because the driver will not emit separate jobs - # unless we also explicitly disable cross-module optimization. See - # https://github.com/swiftlang/swift-driver/blob/c647e91574122f2b104d294ab1ec5baadaa1aa95/Sources/SwiftDriver/Jobs/EmitModuleJob.swift#L156-L181. - unsupported_features.append(SWIFT_FEATURE_COMPILE_IN_PARALLEL) - if is_wmo_manually_requested(user_compile_flags = swiftcopts): requested_features.append(SWIFT_FEATURE__WMO_IN_SWIFTCOPTS) if find_num_threads_flag_value(user_compile_flags = swiftcopts) == 1: diff --git a/swift/toolchains/config/compile_config.bzl b/swift/toolchains/config/compile_config.bzl index 47b22d933..8d53ac964 100644 --- a/swift/toolchains/config/compile_config.bzl +++ b/swift/toolchains/config/compile_config.bzl @@ -72,6 +72,7 @@ load( "SWIFT_FEATURE_MODULE_MAP_HOME_IS_CWD", "SWIFT_FEATURE_NO_ASAN_VERSION_CHECK", "SWIFT_FEATURE_OPT", + "SWIFT_FEATURE_OPT_USES_CMO", "SWIFT_FEATURE_OPT_USES_OSIZE", "SWIFT_FEATURE_OPT_USES_WMO", "SWIFT_FEATURE_REWRITE_GENERATED_HEADER", @@ -366,6 +367,11 @@ def compile_action_configs( configurators = [add_arg("-Osize")], features = [SWIFT_FEATURE_OPT, SWIFT_FEATURE_OPT_USES_OSIZE], ), + ActionConfigInfo( + actions = all_compile_action_names(), + configurators = [add_arg("-disable-cmo")], + not_features = [SWIFT_FEATURE_OPT_USES_CMO], + ), # If the `swift.opt_uses_wmo` feature is enabled, opt builds should also # automatically imply whole-module optimization. diff --git a/test/debug_settings_tests.bzl b/test/debug_settings_tests.bzl index a8c59fcef..35566bc22 100644 --- a/test/debug_settings_tests.bzl +++ b/test/debug_settings_tests.bzl @@ -238,8 +238,6 @@ def debug_settings_test_suite(name, tags = []): "-g", "-gline-tables-only", ], - # In optimized mode, the driver still uses a single invocation for both - # the module and for codegen. mnemonic = "SwiftCompile", tags = all_tags, target_under_test = "//test/fixtures/debug_settings:simple", diff --git a/test/features_tests.bzl b/test/features_tests.bzl index ddfee00e5..648516f75 100644 --- a/test/features_tests.bzl +++ b/test/features_tests.bzl @@ -213,9 +213,7 @@ def features_test_suite(name, tags = []): tags = all_tags, expected_argv = ["-emit-object", "-O"], not_expected_argv = ["-whole-module-optimization"], - # In optimized mode, the driver still uses a single invocation for both - # the module and for codegen. - mnemonic = "SwiftCompile", + mnemonic = "SwiftCompileModule", target_under_test = "//test/fixtures/debug_settings:simple", ) diff --git a/test/fixtures/parallel_compilation/BUILD b/test/fixtures/parallel_compilation/BUILD index 8bb1b17d7..153015c33 100644 --- a/test/fixtures/parallel_compilation/BUILD +++ b/test/fixtures/parallel_compilation/BUILD @@ -34,12 +34,34 @@ swift_library( ) swift_library( - name = "with_opt_with_wmo", + name = "with_opt_with_wmo_no_cmo", srcs = ["Empty.swift"], copts = [ "-O", "-wmo", ], + features = ["-swift.opt_uses_cmo"], + tags = FIXTURE_TAGS, +) + +swift_library( + name = "with_opt_with_wmo_with_cmo", + srcs = ["Empty.swift"], + copts = [ + "-O", + "-wmo", + ], + tags = FIXTURE_TAGS, +) + +swift_library( + name = "with_opt_with_wmo_with_library_evolution", + srcs = ["Empty.swift"], + copts = [ + "-O", + "-wmo", + ], + library_evolution = True, tags = FIXTURE_TAGS, ) diff --git a/test/parallel_compilation_tests.bzl b/test/parallel_compilation_tests.bzl index aff5bf390..96f416659 100644 --- a/test/parallel_compilation_tests.bzl +++ b/test/parallel_compilation_tests.bzl @@ -18,10 +18,17 @@ load("@bazel_skylib//rules:build_test.bzl", "build_test") load( "//test/rules:actions_created_test.bzl", "actions_created_test", + "make_actions_created_test_rule", ) visibility("private") +opt_actions_create_test = make_actions_created_test_rule( + config_settings = { + "//command_line_option:compilation_mode": "opt", + }, +) + def parallel_compilation_test_suite(name, tags = []): """Test suite for parallel compilation. @@ -47,25 +54,46 @@ def parallel_compilation_test_suite(name, tags = []): target_under_test = "//test/fixtures/parallel_compilation:no_opt_with_wmo", ) - # Optimized, non-WMO cannot be compiled in parallel. - # TODO: b/351801556 - This is actually incorrect based on further driver - # testing; update the rules to allow compiling these in parallel. + # Optimized, non-WMO can be compiled in parallel. actions_created_test( name = "{}_with_opt_no_wmo".format(name), - mnemonics = ["-SwiftCompileModule", "-SwiftCompileCodegen", "SwiftCompile"], + mnemonics = ["SwiftCompileModule", "SwiftCompileCodegen", "-SwiftCompile"], tags = all_tags, target_under_test = "//test/fixtures/parallel_compilation:with_opt_no_wmo", ) - # Optimized, with-WMO cannot be compiled in parallel. - # TODO: b/351801556 - This should be allowed if cross-module-optimization is - # disabled. Update the rules to allow this and add a new version of this - # target that disables CMO so we can test both situtations. + # Optimized, with-WMO can be compiled in parallel if CMO is also disabled. + actions_created_test( + name = "{}_with_opt_with_wmo_no_cmo".format(name), + mnemonics = ["SwiftCompileModule", "SwiftCompileCodegen", "-SwiftCompile"], + tags = all_tags, + target_under_test = "//test/fixtures/parallel_compilation:with_opt_with_wmo_no_cmo", + ) + + # Optimized, with-WMO cannot be compiled in parallel if CMO is enabled. actions_created_test( - name = "{}_with_opt_with_wmo".format(name), + name = "{}_with_opt_with_wmo_with_cmo".format(name), mnemonics = ["-SwiftCompileModule", "-SwiftCompileCodegen", "SwiftCompile"], tags = all_tags, - target_under_test = "//test/fixtures/parallel_compilation:with_opt_with_wmo", + target_under_test = "//test/fixtures/parallel_compilation:with_opt_with_wmo_with_cmo", + ) + + # Force `-c opt` on a non-optimized, with-WMO target and make sure we don't + # plan parallel compilation there. + opt_actions_create_test( + name = "{}_no_opt_with_wmo_but_compilation_mode_opt".format(name), + mnemonics = ["-SwiftCompileModule", "-SwiftCompileCodegen", "SwiftCompile"], + tags = all_tags, + target_under_test = "//test/fixtures/parallel_compilation:no_opt_with_wmo", + ) + + # Optimized, with-WMO can be compiled in parallel if library evolution is + # enabled (which implicitly disables CMO). + actions_created_test( + name = "{}_with_opt_with_wmo_with_library_evolution".format(name), + mnemonics = ["SwiftCompileModule", "SwiftCompileCodegen", "-SwiftCompile"], + tags = all_tags, + target_under_test = "//test/fixtures/parallel_compilation:with_opt_with_wmo_with_library_evolution", ) # Make sure that when we look for optimizer flags, we don't treat `-Onone` @@ -85,7 +113,9 @@ def parallel_compilation_test_suite(name, tags = []): "//test/fixtures/parallel_compilation:no_opt_no_wmo", "//test/fixtures/parallel_compilation:no_opt_with_wmo", "//test/fixtures/parallel_compilation:with_opt_no_wmo", - "//test/fixtures/parallel_compilation:with_opt_with_wmo", + "//test/fixtures/parallel_compilation:with_opt_with_wmo_no_cmo", + "//test/fixtures/parallel_compilation:with_opt_with_wmo_with_cmo", + "//test/fixtures/parallel_compilation:with_opt_with_wmo_with_library_evolution", "//test/fixtures/parallel_compilation:onone_with_wmo", ], tags = all_tags, From 8c164a9bae85c4aef82b905c10e0e7005035491c Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Thu, 25 Sep 2025 08:45:52 -0500 Subject: [PATCH 11/13] Fix progress messages Signed-off-by: Brentley Jones --- swift/internal/compiling.bzl | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index b65434755..b01293be1 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -875,9 +875,7 @@ def _execute_compile_plan( feature_configuration = feature_configuration, outputs = module_outputs, prerequisites = struct(**module_prereqs), - progress_message = "Compiling Swift module {}".format( - prerequisites["module_name"], - ), + progress_message = "Compiling Swift module %{label}", swift_toolchain = swift_toolchain, toolchain_type = toolchain_type, ) @@ -910,8 +908,7 @@ def _execute_compile_plan( batch_suffix = "" if compile_plan.output_nature.emits_multiple_objects: batch_suffix = " ({} of {})".format(number, len(batches)) - progress_message = "Codegen for Swift module {}{}".format( - prerequisites["module_name"], + progress_message = "Codegen for Swift module %{{label}}{}".format( batch_suffix, ) @@ -1072,9 +1069,7 @@ def _plan_legacy_swift_compilation( feature_configuration = feature_configuration, outputs = all_compile_outputs, prerequisites = struct(**prerequisites), - progress_message = "Compiling Swift module {}".format( - prerequisites["module_name"], - ), + progress_message = "Compiling Swift module %{label}", swift_toolchain = swift_toolchain, toolchain_type = toolchain_type, ) From 399a52f655cd9dd78b68468c49e2045bdb295a7a Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Thu, 10 Oct 2024 07:39:52 -0700 Subject: [PATCH 12/13] Also detect `-O` flags being set via `--swiftcopt` when planning parallel compilation. In the future, we should consider whether we should more strictly control where these flags can be applied, since they don't just control optimizations but also how the driver plans its work. Cherry-pick: ab55a129042f4a5417479780930b7057cbf2a6d0 --- swift/internal/compiling.bzl | 4 ++++ swift/internal/feature_names.bzl | 5 +++++ swift/internal/optimization.bzl | 3 +++ test/parallel_compilation_tests.bzl | 15 +++++++++++++++ 4 files changed, 27 insertions(+) diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index b01293be1..85d917ef2 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -64,6 +64,7 @@ load( "SWIFT_FEATURE_USE_EXPLICIT_SWIFT_MODULE_MAP", "SWIFT_FEATURE_VFSOVERLAY", "SWIFT_FEATURE__NUM_THREADS_0_IN_SWIFTCOPTS", + "SWIFT_FEATURE__OPT_IN_SWIFTCOPTS", "SWIFT_FEATURE__WMO_IN_SWIFTCOPTS", ) load( @@ -823,6 +824,9 @@ def _should_plan_parallel_compilation( ) and not is_feature_enabled( feature_configuration = feature_configuration, feature_name = SWIFT_FEATURE_OPT, + ) and not is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE__OPT_IN_SWIFTCOPTS, ) ) diff --git a/swift/internal/feature_names.bzl b/swift/internal/feature_names.bzl index 4ea0a7750..dd68d4d70 100644 --- a/swift/internal/feature_names.bzl +++ b/swift/internal/feature_names.bzl @@ -356,6 +356,11 @@ SWIFT_FEATURE_LLD_GC_WORKAROUND = "swift.lld_gc_workaround" # objects if you know that isn't required. SWIFT_FEATURE_OBJC_LINK_FLAGS = "swift.objc_link_flag" +# A private feature that is set by the toolchain if an optimization flag in the +# `-O` flag group was passed on the command line using `--swiftcopt`. Users +# should never manually enable, disable, or query this feature. +SWIFT_FEATURE__OPT_IN_SWIFTCOPTS = "swift._opt_in_swiftcopts" + # If enabled, requests the `-enforce-exclusivity=checked` swiftc flag which # enables runtime checking of exclusive memory access on mutation. SWIFT_FEATURE_CHECKED_EXCLUSIVITY = "swift.checked_exclusivity" diff --git a/swift/internal/optimization.bzl b/swift/internal/optimization.bzl index ca00b134e..fbca8001f 100644 --- a/swift/internal/optimization.bzl +++ b/swift/internal/optimization.bzl @@ -17,6 +17,7 @@ load( ":feature_names.bzl", "SWIFT_FEATURE__NUM_THREADS_0_IN_SWIFTCOPTS", + "SWIFT_FEATURE__OPT_IN_SWIFTCOPTS", "SWIFT_FEATURE__WMO_IN_SWIFTCOPTS", ) @@ -112,6 +113,8 @@ def optimization_features_from_swiftcopts(swiftcopts): requested_features = [] unsupported_features = [] + if is_optimization_manually_requested(user_compile_flags = swiftcopts): + requested_features.append(SWIFT_FEATURE__OPT_IN_SWIFTCOPTS) if is_wmo_manually_requested(user_compile_flags = swiftcopts): requested_features.append(SWIFT_FEATURE__WMO_IN_SWIFTCOPTS) if find_num_threads_flag_value(user_compile_flags = swiftcopts) == 1: diff --git a/test/parallel_compilation_tests.bzl b/test/parallel_compilation_tests.bzl index 96f416659..bb90868cd 100644 --- a/test/parallel_compilation_tests.bzl +++ b/test/parallel_compilation_tests.bzl @@ -29,6 +29,12 @@ opt_actions_create_test = make_actions_created_test_rule( }, ) +opt_via_swiftcopt_actions_create_test = make_actions_created_test_rule( + config_settings = { + str(Label("//swift:copt")): ["-O"], + }, +) + def parallel_compilation_test_suite(name, tags = []): """Test suite for parallel compilation. @@ -87,6 +93,15 @@ def parallel_compilation_test_suite(name, tags = []): target_under_test = "//test/fixtures/parallel_compilation:no_opt_with_wmo", ) + # Force `-O` using the `copt` flag on a non-optimized, with-WMO target and + # make sure we don't plan parallel compilation there. + opt_via_swiftcopt_actions_create_test( + name = "{}_no_opt_with_wmo_but_swiftcopt_dash_O".format(name), + mnemonics = ["-SwiftCompileModule", "-SwiftCompileCodegen", "SwiftCompile"], + tags = all_tags, + target_under_test = "//test/fixtures/parallel_compilation:no_opt_with_wmo", + ) + # Optimized, with-WMO can be compiled in parallel if library evolution is # enabled (which implicitly disables CMO). actions_created_test( From 6ab4bec259e0dc2705f8a77f881776b3dfb66c1b Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Fri, 11 Oct 2024 06:54:39 -0700 Subject: [PATCH 13/13] Fix another edge case in compile planning; this time if `-c opt` is used and `-wmo` is passed on the command line. Cherry-pick: 66a6a6b7e21e245e1440b4a89302cc37d2d58e11 --- swift/internal/compiling.bzl | 3 +++ test/parallel_compilation_tests.bzl | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index 85d917ef2..fd7b80af3 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -802,6 +802,9 @@ def _should_plan_parallel_compilation( ) or are_all_features_enabled( feature_configuration = feature_configuration, feature_names = [SWIFT_FEATURE_OPT, SWIFT_FEATURE_OPT_USES_WMO], + ) or is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE__WMO_IN_SWIFTCOPTS, ) ): return True diff --git a/test/parallel_compilation_tests.bzl b/test/parallel_compilation_tests.bzl index bb90868cd..afe6ebfa2 100644 --- a/test/parallel_compilation_tests.bzl +++ b/test/parallel_compilation_tests.bzl @@ -35,6 +35,15 @@ opt_via_swiftcopt_actions_create_test = make_actions_created_test_rule( }, ) +opt_with_wmo_via_swiftcopt_actions_create_test = make_actions_created_test_rule( + config_settings = { + "//command_line_option:compilation_mode": "opt", + str(Label("//swift:copt")): [ + "-whole-module-optimization", + ], + }, +) + def parallel_compilation_test_suite(name, tags = []): """Test suite for parallel compilation. @@ -120,6 +129,15 @@ def parallel_compilation_test_suite(name, tags = []): target_under_test = "//test/fixtures/parallel_compilation:onone_with_wmo", ) + # Optimized (via `-c opt`) with-WMO cannot be compiled in parallel if CMO is + # enabled (the default). + opt_with_wmo_via_swiftcopt_actions_create_test( + name = "{}_with_opt_via_compilation_mode_opt_with_wmo".format(name), + mnemonics = ["-SwiftCompileModule", "-SwiftCompileCodegen", "SwiftCompile"], + tags = all_tags, + target_under_test = "//test/fixtures/parallel_compilation:no_opt_no_wmo", + ) + # The analysis tests verify that we register the actions we expect. Use a # `build_test` to make sure the actions execute successfully. build_test(