diff --git a/test/worker/BUILD b/test/worker/BUILD new file mode 100644 index 000000000..9ac485f4f --- /dev/null +++ b/test/worker/BUILD @@ -0,0 +1,26 @@ +load(":worker_test.bzl", "worker_test") + +[ + worker_test( + name = name + "_test", + size = "medium", + scenario = name, + target_compatible_with = select({ + "@platforms//os:windows": ["@platforms//:incompatible"], + "//conditions:default": [], + }), + ) + for name in [ + "module_outputs_survive_failure", + "changed_dependency_digest", + "changed_dependency_digest_without_hashing", + "changed_arguments", + "changed_universal_arguments", + "missing_digest", + "removed_dependency_input", + "missing_input_record", + "corrupt_input_record", + "atomic_input_record", + "failed_dependency_change", + ] +] diff --git a/test/worker/observe_swiftc.sh b/test/worker/observe_swiftc.sh new file mode 100755 index 000000000..b69bb22ca --- /dev/null +++ b/test/worker/observe_swiftc.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash + +set -euo pipefail + +# Observe the worker's invalidation decisions before Swift updates its records. +mkdir -p observed_dependencies +for dependency in "$INCREMENTAL_DIR"/*.swiftdeps "$INCREMENTAL_DIR"/*.priors; do + if [[ -f "$dependency" ]]; then + cp "$dependency" observed_dependencies/ + fi +done + +# Forward Swift's response file unchanged, including its argument quoting. +if [[ "$OSTYPE" == darwin* ]]; then + exec /usr/bin/xcrun "$WORKER_TEST_COMPILER" "$@" +fi +exec "$WORKER_TEST_COMPILER" "$@" diff --git a/test/worker/worker_test.bzl b/test/worker/worker_test.bzl new file mode 100644 index 000000000..5342bd3d3 --- /dev/null +++ b/test/worker/worker_test.bzl @@ -0,0 +1,71 @@ +"""Runs worker regression tests with the configured Swift compiler.""" + +load("@bazel_skylib//lib:shell.bzl", "shell") + +# buildifier: disable=bzl-visibility +load( + "//swift/internal:action_names.bzl", + "SWIFT_ACTION_AUTOLINK_EXTRACT", + "SWIFT_ACTION_COMPILE", +) + +# buildifier: disable=bzl-visibility +load("//swift/internal:feature_names.bzl", "SWIFT_FEATURE_INCREMENTAL_FILE_HASHING") + +def _worker_test_impl(ctx): + toolchain = ctx.toolchains["//toolchains:toolchain_type"].swift_toolchain + compiler = toolchain.tool_configs[SWIFT_ACTION_COMPILE] + files = [ctx.file._script, ctx.file._observer, ctx.executable._worker] + files.extend(compiler.additional_tools) + + # The test also links a client, which invokes the autolink extractor on Linux. + autolink_extractor = toolchain.tool_configs.get(SWIFT_ACTION_AUTOLINK_EXTRACT) + if autolink_extractor: + files.extend(autolink_extractor.additional_tools) + if type(autolink_extractor.executable) == "File": + files.append(autolink_extractor.executable) + + executable = compiler.executable + if type(executable) == "File": + files.append(executable) + executable = executable.short_path + elif executable.startswith("external/"): + executable = "../" + executable.removeprefix("external/") + + runner = ctx.actions.declare_file(ctx.label.name + ".sh") + ctx.actions.write( + output = runner, + content = "#!/usr/bin/env bash\nexec bash {} \"$@\"\n".format(" ".join([ + shell.quote(arg) + for arg in [ + ctx.file._script.short_path, + ctx.executable._worker.short_path, + ctx.file._observer.short_path, + ctx.attr.scenario, + executable, + ] + compiler.args + ])), + is_executable = True, + ) + return [ + DefaultInfo( + executable = runner, + runfiles = ctx.runfiles(files = files).merge(ctx.attr._worker[DefaultInfo].default_runfiles), + ), + testing.ExecutionInfo(compiler.execution_requirements), + testing.TestEnvironment(compiler.env | { + "WORKER_TEST_FILE_HASHING": "1" if SWIFT_FEATURE_INCREMENTAL_FILE_HASHING in toolchain.requested_features else "0", + }), + ] + +worker_test = rule( + implementation = _worker_test_impl, + attrs = { + "scenario": attr.string(mandatory = True), + "_observer": attr.label(default = "observe_swiftc.sh", allow_single_file = True), + "_script": attr.label(default = "worker_test.sh", allow_single_file = True), + "_worker": attr.label(default = "//tools/worker:worker", executable = True, cfg = "exec"), + }, + test = True, + toolchains = ["//toolchains:toolchain_type"], +) diff --git a/test/worker/worker_test.sh b/test/worker/worker_test.sh new file mode 100755 index 000000000..475e15db3 --- /dev/null +++ b/test/worker/worker_test.sh @@ -0,0 +1,329 @@ +#!/usr/bin/env bash + +set -euo pipefail + +export WORKER_TEST_WORKER="$PWD/$1" +readonly observer="$PWD/$2" +readonly test_name="$3" +compiler="$4" +if [[ "$compiler" == */* && "$compiler" != /* ]]; then + compiler="$PWD/$compiler" +fi +shift 4 +readonly compiler +export WORKER_TEST_COMPILER="$compiler" +compiler_arguments=(--driver-mode=swiftc "$@") +cd "$TEST_TMPDIR" + +readonly output_dir="bazel-out/config/bin" +export INCREMENTAL_DIR="$output_dir/_swift_incremental" + +compile() { + "$WORKER_TEST_WORKER" "$compiler" "${compiler_arguments[@]}" \ + -module-cache-path "$TEST_TMPDIR/module-cache" "$@" +} + +# Worker digests are opaque strings. Compute them from the actual fixture bytes. +digest() { + cksum <"$1" | awk '{print $1 ":" $2}' +} + +build_dependency() { + echo "public typealias Value = $1" >Dependency.swift + compile -emit-module -module-name Dependency Dependency.swift \ + -emit-module-path Dependency.swiftmodule + # Reproduce cache restoration: changed bytes with an old timestamp. + touch -t 200001010000 Dependency.swiftmodule +} + +write_source() { + api="$1" + cat >source.swift < Int { +#if NEW_DEFINE + return 100 + MemoryLayout.size +#else + return MemoryLayout.size +#endif +} +EOF_SOURCE + # Use distinct timestamps without sleeps, even on coarse filesystems. + touch -t "$2" source.swift +} + +mkdir -p "$output_dir" +cat >"$output_dir/module.json" <request.json <response.json 2>worker.log || worker_exit_code=$? + if [[ "$worker_exit_code" != 254 ]] || + ! grep -Fq "\"exitCode\":$expected_exit_code," response.json; then + cat response.json worker.log >&2 + echo "Expected request exit code $expected_exit_code" >&2 + exit 1 + fi + if "$fail_output_copy"; then + rmdir "$output_dir/source.o" + grep -Fq 'Could not copy' response.json + fi +} + +# These assertions inspect record presence before Swift runs. +assert_dependencies_kept() { + for dependency in "${dependencies[@]}"; do + if [[ ! -f "observed_dependencies/$dependency" ]]; then + echo "Expected the worker to retain $dependency before compilation" >&2 + exit 1 + fi + done +} + +assert_dependencies_removed() { + for dependency in "${dependencies[@]}"; do + if [[ -f "observed_dependencies/$dependency" ]]; then + echo "Expected the worker to invalidate $dependency" >&2 + exit 1 + fi + done +} + +assert_module_outputs() { + local expected="$1" + local extensions=(swiftmodule swiftdoc h) + if [[ "$test_name" != module_outputs_survive_failure ]]; then + extensions+=(swiftsourceinfo) + fi + for extension in "${extensions[@]}"; do + if [[ ! -s "$output_dir/module.$extension" ]]; then + echo "Missing module.$extension" >&2 + exit 1 + fi + done + # A client must see the current module API and execute the current object. + # Use the filename expected by Swift's module lookup. + cp "$output_dir/module.swiftmodule" "$output_dir/WorkerModule.swiftmodule" + cat >main.swift <&2 + exit 1 + fi +} + +# Every test starts with real outputs and real incremental dependency records. +run_worker +assert_module_outputs 4 +# Drivers differ in whether the module record is .swiftdeps or .priors. +dependencies=(source.swiftdeps) +for record in module.swiftdeps module.priors; do + if [[ -f "$INCREMENTAL_DIR/$record" ]]; then + dependencies+=("$record") + fi +done +[[ "${#dependencies[@]}" -gt 1 ]] + +case "$test_name" in +module_outputs_survive_failure) + write_source newAPI 202001010001 + fail_output_copy=true + run_worker 1 + assert_dependencies_kept + # Swift succeeded and updated its records, but publishing the object failed. + # The retry must preserve the newer module even if Swift skips compilation. + cp "$INCREMENTAL_DIR/source.o" expected.o + touch -r "$INCREMENTAL_DIR/module.swiftmodule" module_timestamp + fail_output_copy=false + run_worker + assert_dependencies_kept + cmp expected.o "$output_dir/source.o" + assert_module_outputs 4 + if [[ "$INCREMENTAL_DIR/module.swiftmodule" -nt module_timestamp || + "$INCREMENTAL_DIR/module.swiftmodule" -ot module_timestamp ]]; then + echo 'Expected Swift to reuse the module on the retry' >&2 + exit 1 + fi + ;; +changed_dependency_digest | changed_dependency_digest_without_hashing) + build_dependency Int64 + run_worker + assert_module_outputs 8 + assert_dependencies_removed + run_worker + assert_dependencies_kept + assert_module_outputs 8 + ;; +changed_arguments) + extra_arguments=', "-DNEW_DEFINE"' + run_worker + assert_dependencies_removed + assert_module_outputs 104 + run_worker + assert_dependencies_kept + ;; +changed_universal_arguments) + universal_argument="-DNEW_DEFINE" + run_worker + assert_dependencies_removed + assert_module_outputs 104 + run_worker + assert_dependencies_kept + ;; +missing_digest) + missing_digest=true + run_worker + assert_dependencies_removed + # Equal but empty digests must not make subsequent builds reusable. + run_worker + assert_dependencies_removed + assert_module_outputs 4 + ;; +removed_dependency_input) + # Remove the dependency from both the compilation and the request. Source + # digests are excluded from the comparison, so the missing input entry must + # invalidate the records even though the remaining non-source digests match. + include_dependency=false + rm Dependency.swiftmodule + echo 'public func oldAPI() -> Int { return 8 }' >source.swift + touch -t 202001010001 source.swift + run_worker + assert_dependencies_removed + assert_module_outputs 8 + # With the import removed and non-source inputs unchanged, the worker should + # retain the dependency records on the next request. + run_worker + assert_dependencies_kept + assert_module_outputs 8 + # Reintroducing an input must also invalidate the records. + include_dependency=true + build_dependency Int32 + write_source oldAPI 202001010002 + run_worker + assert_dependencies_removed + assert_module_outputs 4 + ;; +missing_input_record) + rm -f "$INCREMENTAL_DIR/module.inputs.json" + run_worker + assert_dependencies_removed + assert_module_outputs 4 + ;; +corrupt_input_record) + echo invalid-json >"$INCREMENTAL_DIR/module.inputs.json" + run_worker + assert_dependencies_removed + assert_module_outputs 4 + ;; +atomic_input_record) + # A failed metadata write must leave a reusable record intact. + cp "$INCREMENTAL_DIR/module.inputs.json" expected.inputs.json + ln "$INCREMENTAL_DIR/module.inputs.json" original.inputs.json + mkdir "$INCREMENTAL_DIR/module.inputs.json.tmp" + run_worker 1 + grep -Fq 'Could not write' response.json + cmp expected.inputs.json "$INCREMENTAL_DIR/module.inputs.json" + assert_dependencies_kept + [[ ! -e "$INCREMENTAL_DIR/module.inputs.json.tmp" ]] + + # Replacement must publish a new file, leaving readers of the old one intact. + run_worker + assert_dependencies_kept + assert_module_outputs 4 + cmp expected.inputs.json original.inputs.json + cmp expected.inputs.json "$INCREMENTAL_DIR/module.inputs.json" + [[ ! "$INCREMENTAL_DIR/module.inputs.json" -ef original.inputs.json ]] + [[ ! -e "$INCREMENTAL_DIR/module.inputs.json.tmp" ]] + ;; +failed_dependency_change) + cp Dependency.swiftmodule original.swiftmodule + build_dependency Int64 + fail_output_copy=true + run_worker 1 + assert_dependencies_removed + + # Returning to the old inputs must not reuse the failed build's state. + cp original.swiftmodule Dependency.swiftmodule + touch -t 200001010000 Dependency.swiftmodule + fail_output_copy=false + run_worker + assert_dependencies_removed + assert_module_outputs 4 + ;; +*) + echo "Unknown test: $test_name" >&2 + exit 1 + ;; +esac diff --git a/tools/worker/BUILD b/tools/worker/BUILD index d846aa093..06f5cd330 100644 --- a/tools/worker/BUILD +++ b/tools/worker/BUILD @@ -29,6 +29,7 @@ cc_library( ":worker_protocol", "//tools/common:file_system", "//tools/common:temp_file", + "@nlohmann_json//:json", ], ) diff --git a/tools/worker/compile_with_worker.cc b/tools/worker/compile_with_worker.cc index 3b24ffa8c..9b37ca53d 100644 --- a/tools/worker/compile_with_worker.cc +++ b/tools/worker/compile_with_worker.cc @@ -87,6 +87,16 @@ // `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. +// +// We also redirect module-level outputs, such as the final .swiftmodule and +// generated header, to this location. The compiler updates these files along +// with its dependency records, so we need to keep them together even if a build +// fails. Otherwise, the next invocation could treat an older module as up to +// date and copy it into Bazel's output location without recompiling. +// +// Cached inputs can change without newer timestamps. We track arguments and +// non-source input digests to invalidate stale incremental state, leaving +// source changes to Swift's dependency tracking. 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 8b46e0759..87b4337ea 100644 --- a/tools/worker/output_file_map.cc +++ b/tools/worker/output_file_map.cc @@ -52,12 +52,16 @@ static std::string MakeIncrementalOutputPath(std::string 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); + UpdateForIncremental(path); +} + +std::string OutputFileMap::AddOutput(const std::string& path) { + auto incremental_path = MakeIncrementalOutputPath(path, is_derived_); + incremental_outputs_[path] = incremental_path; + return incremental_path; } void OutputFileMap::WriteToPath(const std::string& path) { @@ -65,16 +69,12 @@ void OutputFileMap::WriteToPath(const std::string& 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; +void OutputFileMap::UpdateForIncremental(const std::string& path) { + is_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; + std::vector incremental_dependencies; // The empty string key is used to represent outputs that are for the whole // module, rather than for a particular source file. @@ -82,8 +82,9 @@ void OutputFileMap::UpdateForIncremental( // 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); + auto swiftdeps_path = MakeIncrementalOutputPath(new_path, is_derived_); module_map["swift-dependencies"] = swiftdeps_path; + incremental_dependencies.push_back(swiftdeps_path); new_output_file_map[""] = module_map; for (auto& element : json_.items()) { @@ -102,7 +103,7 @@ void OutputFileMap::UpdateForIncremental( // 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); + auto new_path = MakeIncrementalOutputPath(path, is_derived_); src_map[kind] = new_path; incremental_outputs[path] = new_path; @@ -112,12 +113,12 @@ void OutputFileMap::UpdateForIncremental( .string(); } - incremental_cleanup_outputs.push_back(swiftdeps_path); + incremental_dependencies.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); + auto new_path = MakeIncrementalOutputPath(path, is_derived_); src_map[kind] = new_path; incremental_outputs[path] = new_path; @@ -127,14 +128,14 @@ void OutputFileMap::UpdateForIncremental( .string(); } - incremental_cleanup_outputs.push_back(swiftdeps_path); + incremental_dependencies.push_back(swiftdeps_path); } else if (kind == "swift-dependencies") { // Only derived-file maps need explicit per-source dependency paths. // So we ignore other entries, including those added by a previous // rewrite. - if (derived && !src.empty()) { - swiftdeps_path = MakeIncrementalOutputPath(path, derived); - incremental_cleanup_outputs.push_back(swiftdeps_path); + if (is_derived_ && !src.empty()) { + swiftdeps_path = MakeIncrementalOutputPath(path, is_derived_); + incremental_dependencies.push_back(swiftdeps_path); // Module-only compilations also produce per-source partial modules. // The driver checks that all outputs exist before skipping a source; @@ -161,29 +162,7 @@ void OutputFileMap::UpdateForIncremental( 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; - } - - 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; + incremental_dependencies_ = incremental_dependencies; } diff --git a/tools/worker/output_file_map.h b/tools/worker/output_file_map.h index 467a247f5..cac221621 100644 --- a/tools/worker/output_file_map.h +++ b/tools/worker/output_file_map.h @@ -18,6 +18,7 @@ #include #include #include +#include // Supports loading and rewriting a `swiftc` output file map to support // incremental compilation. @@ -39,25 +40,17 @@ class OutputFileMap { return incremental_outputs_; } - // 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_; + // Dependency files written directly by Swift. Their parent directories must + // exist even for compilations that do not produce object files. + const std::vector& incremental_dependencies() const { + return incremental_dependencies_; } // 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); + void ReadFromPath(const std::string& path); + + std::string AddOutput(const std::string& path); // Writes the output file map as JSON to the file at the given path. void WriteToPath(const std::string& path); @@ -65,14 +58,12 @@ class OutputFileMap { 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); + void UpdateForIncremental(const std::string& path); nlohmann::json json_; std::map incremental_outputs_; - std::map incremental_inputs_; - std::vector incremental_cleanup_outputs_; + std::vector incremental_dependencies_; + bool is_derived_ = false; }; #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 939b14736..bef69601a 100644 --- a/tools/worker/swift_runner.cc +++ b/tools/worker/swift_runner.cc @@ -326,10 +326,10 @@ bool CreateVerifyOutputs(const std::string& output_file_map_path, std::ostream* stderr_stream) { if (!output_file_map_path.empty()) { OutputFileMap output_file_map; - output_file_map.ReadFromPath(output_file_map_path, "", ""); - for (const auto& expected_output_pair : + output_file_map.ReadFromPath(output_file_map_path); + for (const auto& [declared_path, incremental_path] : output_file_map.incremental_outputs()) { - if (!TouchFile(expected_output_pair.first, stderr_stream)) { + if (!TouchFile(declared_path, stderr_stream)) { return false; } } @@ -551,10 +551,7 @@ int SwiftRunner::Run(std::ostream* stderr_stream, bool stdout_to_stderr) { } OutputFileMap output_file_map; - output_file_map.ReadFromPath(output_file_map_path_, "", ""); - - auto outputs = output_file_map.incremental_outputs(); - std::map::iterator it; + output_file_map.ReadFromPath(output_file_map_path_); std::vector ii_args; ii_args.push_back(index_import_path_); @@ -564,9 +561,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++) { + for (const auto& [output_path, incremental_path] : + output_file_map.incremental_outputs()) { // 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"); diff --git a/tools/worker/work_processor.cc b/tools/worker/work_processor.cc index fa10212a4..e1b73002f 100644 --- a/tools/worker/work_processor.cc +++ b/tools/worker/work_processor.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -47,10 +48,24 @@ bool copy_file(const std::filesystem::path& from, ec = std::error_code(); return true; #else - return std::filesystem::copy_file(LongPath(from), LongPath(to), ec); + return std::filesystem::copy_file( + LongPath(from), LongPath(to), + std::filesystem::copy_options::overwrite_existing, ec); #endif } +bool RemoveFile(const std::filesystem::path& path, + std::ostream& stderr_stream) { + std::error_code ec; + std::filesystem::remove(LongPath(path), ec); + if (ec) { + stderr_stream << "swift_worker: Could not remove " << path << " (" + << ec.message() << ")\n"; + return false; + } + return true; +} + static void FinalizeWorkRequest( const bazel_rules_swift::worker_protocol::WorkRequest& request, bazel_rules_swift::worker_protocol::WorkResponse& response, int exit_code, @@ -85,76 +100,88 @@ void WorkProcessor::ProcessWorkRequest( OutputFileMap output_file_map; std::string output_file_map_path; - std::string emit_module_path; - std::string emit_objc_header_path; + std::map module_outputs; + const std::set module_output_flags = { + "-emit-module-path", + "-emit-module-source-info-path", + "-emit-module-interface-path", + "-emit-private-module-interface-path", + "-emit-package-module-interface-path", + "-emit-objc-header-path", + }; bool is_wmo = false; bool is_dump_ast = false; bool enable_incremental_file_hashing = false; + bool avoid_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(); + for (const auto& arg : request.arguments) { + if (prev_arg == "-output-file-map") { + output_file_map_path = arg; + } else if (module_output_flags.count(prev_arg)) { + module_outputs[prev_arg] = arg; } 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 (arg == "-avoid-emit-module-source-info") { + avoid_source_info = true; } else if (ArgumentEnablesWMO(arg)) { is_wmo = true; } else if (arg == "-Xwrapped-swift=-enable-incremental-file-hashing") { enable_incremental_file_hashing = true; - arg.clear(); } + prev_arg = arg; + } - if (!arg.empty()) { - params_file_stream << arg << '\n'; + bool is_incremental = + !is_wmo && !is_dump_ast && !output_file_map_path.empty(); + std::string incremental_file_map_path; + std::set optional_outputs; + if (is_incremental) { + output_file_map.ReadFromPath(output_file_map_path); + incremental_file_map_path = std::filesystem::path(output_file_map_path) + .replace_extension(".incremental.json") + .string(); + output_file_map.WriteToPath(incremental_file_map_path); + + auto module = module_outputs.find("-emit-module-path"); + if (module != module_outputs.end()) { + auto documentation = std::filesystem::path(module->second) + .replace_extension(".swiftdoc") + .string(); + output_file_map.AddOutput(documentation); + optional_outputs.insert(documentation); + if (!avoid_source_info && + !module_outputs.count("-emit-module-source-info-path")) { + auto source_info = std::filesystem::path(module->second) + .replace_extension(".swiftsourceinfo") + .string(); + output_file_map.AddOutput(source_info); + optional_outputs.insert(source_info); + } } - - 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"; - if (enable_incremental_file_hashing) { - params_file_stream << "-enable-incremental-file-hashing\n"; - } + // We rewrite the output paths so swiftc writes module files directly into the + // incremental storage area. This keeps them consistent with the dependency + // records even if the request fails before we copy the outputs back. + prev_arg.clear(); + for (const auto& arg : request.arguments) { + if (arg == "-Xwrapped-swift=-enable-incremental-file-hashing") { + continue; + } + if (is_incremental && prev_arg == "-output-file-map") { + params_file_stream << incremental_file_map_path << '\n'; + } else if (is_incremental && module_output_flags.count(prev_arg)) { + params_file_stream << output_file_map.AddOutput(arg) << '\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'; + params_file_stream << arg << '\n'; + } + prev_arg = arg; + } + if (is_incremental) { + params_file_stream << "-incremental\n"; + if (enable_incremental_file_hashing) { + params_file_stream << "-enable-incremental-file-hashing\n"; } } @@ -162,35 +189,25 @@ void WorkProcessor::ProcessWorkRequest( params_file_stream.close(); std::ostringstream stderr_stream; + std::filesystem::path incremental_inputs_path; + nlohmann::json incremental_inputs; 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); - - // 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 : + for (const auto& [declared_path, incremental_path] : 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(); + std::filesystem::path(incremental_path).parent_path().string(); dir_paths.insert(dir_path); + dir_paths.insert( + std::filesystem::path(declared_path).parent_path().string()); } - for (const auto& output : output_file_map.incremental_cleanup_outputs()) { + for (const auto& output : output_file_map.incremental_dependencies()) { dir_paths.insert(std::filesystem::path(output).parent_path().string()); } @@ -205,45 +222,61 @@ void WorkProcessor::ProcessWorkRequest( } } - // 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(LongPath(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; - } + // Swift can skip checking an imported module when its timestamp predates + // the build record, even if Bazel restored different contents from cache. + // Compare Bazel's digests instead. Leave source edits to Swift's per-file + // dependency tracking, but rebuild the target when other inputs change. + nlohmann::json dependency_digests = nlohmann::json::object(); + bool have_digests = !request.inputs.empty(); + for (const auto& input : request.inputs) { + if (!output_file_map.json().contains(input.path)) { + dependency_digests[input.path] = input.digest; + have_digests = have_digests && !input.digest.empty(); } - } else { - auto cleanup_outputs = output_file_map.incremental_cleanup_outputs(); - for (const auto& cleanup_output : cleanup_outputs) { - if (!std::filesystem::exists(LongPath(cleanup_output))) { - continue; - } - - std::error_code ec; - std::filesystem::remove(LongPath(cleanup_output), ec); - if (ec) { - stderr_stream << "swift_worker: Could not remove " << cleanup_output - << " (" << ec.message() << ")\n"; + } + incremental_inputs = { + {"inputs", dependency_digests}, + {"arguments", request.arguments}, + {"universal_arguments", universal_args_}, + }; + std::filesystem::path build_record = output_file_map.json() + .at("") + .at("swift-dependencies") + .get(); + incremental_inputs_path = build_record; + incremental_inputs_path.replace_extension(".inputs.json"); + bool can_reuse = false; + { + std::ifstream previous_inputs(LongPath(incremental_inputs_path)); + if (previous_inputs) { + auto previous = nlohmann::json::parse(previous_inputs, nullptr, false); + can_reuse = have_digests && previous == incremental_inputs; + } + } + if (!can_reuse) { + // Once the inputs change, a failed compilation can leave state from + // different input sets. Remove the old record before changing any state, + // and only record the new inputs after compilation and output copying + // succeed. With matching inputs, keep the record even on failure: Swift + // tracks unfinished source jobs and can recover incrementally. + if (!RemoveFile(incremental_inputs_path, stderr_stream)) { + FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); + return; + } + for (const auto& dependency : + output_file_map.incremental_dependencies()) { + if (!RemoveFile(dependency, stderr_stream)) { FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); return; } } + // Older drivers use the module swiftdeps file as the build record; + // newer drivers replace its extension with .priors. + build_record.replace_extension(".priors"); + if (!RemoveFile(build_record, stderr_stream)) { + FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); + return; + } } } @@ -258,45 +291,40 @@ void WorkProcessor::ProcessWorkRequest( 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 : + for (const auto& [declared_path, incremental_path] : output_file_map.incremental_outputs()) { + if (optional_outputs.count(declared_path) && + !std::filesystem::exists(LongPath(incremental_path))) { + continue; + } std::error_code ec; - copy_file(expected_object_pair.second, expected_object_pair.first, ec); + copy_file(incremental_path, declared_path, ec); if (ec) { - stderr_stream << "swift_worker: Could not copy " - << expected_object_pair.second << " to " - << expected_object_pair.first << " (" << ec.message() + stderr_stream << "swift_worker: Could not copy " << incremental_path + << " to " << declared_path << " (" << 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(LongPath(expected_object_pair.first))) { - if (std::filesystem::exists(LongPath(expected_object_pair.second))) { - // CopyFile fails if the file already exists - std::filesystem::remove(LongPath(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; - } + auto temporary_inputs_path = incremental_inputs_path; + temporary_inputs_path += ".tmp"; + std::ofstream inputs_stream(LongPath(temporary_inputs_path)); + inputs_stream << incremental_inputs; + inputs_stream.close(); + std::error_code ec; + if (inputs_stream) { + std::filesystem::rename(LongPath(temporary_inputs_path), + LongPath(incremental_inputs_path), ec); + } + if (!inputs_stream || ec) { + stderr_stream << "swift_worker: Could not write " + << incremental_inputs_path << " (" + << (ec ? ec.message() : "stream failure") << ")\n"; + RemoveFile(temporary_inputs_path, stderr_stream); + FinalizeWorkRequest(request, response, EXIT_FAILURE, stderr_stream); + return; } }