[swift] Remove problematic use of "stamp" tool
Recent versions of ninja supports listing multiple outputs for the
same build command, so remove a problematic "stamp" tool (caused
incremental build issues).
Previously, the ninja file for a swift source_set would look like:
build output0: swift input0 input1 ...
build output1 output2 ...: stamp output0
With this change, it becomes the following:
build output0 output1 output2 ...: swift input0 input1 ...
This removes unnecessary target, and ensure that building any of
output$n will result in launching the swift command instead of
creating empty files if the output0 already exists.
Bug: none
Change-Id: Ibbe2433a87de0ca86a628d6120c9e004780818c3
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/16800
Reviewed-by: David Turner <digit@google.com>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc
index eada97d..aa5345c 100644
--- a/src/gn/ninja_c_binary_target_writer.cc
+++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -510,76 +510,44 @@
// If the target contains .swift source files, they needs to be compiled as
// a single unit but still can produce more than one object file (if the
// whole module optimization is disabled).
- if (target_->source_types_used().SwiftSourceUsed()) {
- const Tool* tool =
- target_->toolchain()->GetToolForSourceType(SourceFile::SOURCE_SWIFT);
+ const Tool* tool =
+ target_->toolchain()->GetToolForSourceType(SourceFile::SOURCE_SWIFT);
+ DCHECK(!tool->outputs().list().empty());
- const OutputFile swiftmodule_output_file =
- target_->swift_values().module_output_file();
+ std::vector<OutputFile> outputs;
+ SubstitutionWriter::ApplyListToLinkerAsOutputFile(target_, tool,
+ tool->outputs(), &outputs);
- std::vector<OutputFile> additional_outputs;
- SubstitutionWriter::ApplyListToLinkerAsOutputFile(
- target_, tool, tool->outputs(), &additional_outputs);
+ // Expand partial outputs too (if any).
+ for (const SourceFile& source : target_->sources()) {
+ if (!source.IsSwiftType())
+ continue;
- additional_outputs.erase(
- std::remove(additional_outputs.begin(), additional_outputs.end(),
- swiftmodule_output_file),
- additional_outputs.end());
+ SubstitutionWriter::ApplyListToCompilerAsOutputFile(
+ target_, source, tool->partial_outputs(), &outputs);
+ }
- for (const OutputFile& output : additional_outputs) {
- const SourceFile output_as_source =
- output.AsSourceFile(target_->settings()->build_settings());
+ for (const OutputFile& output : outputs) {
+ const SourceFile output_as_source =
+ output.AsSourceFile(target_->settings()->build_settings());
- if (output_as_source.IsObjectType()) {
- object_files->push_back(output);
- }
- }
-
- const SubstitutionList& partial_outputs_subst = tool->partial_outputs();
- if (!partial_outputs_subst.list().empty()) {
- // Avoid re-allocation during loop.
- std::vector<OutputFile> partial_outputs;
- for (const auto& source : target_->sources()) {
- if (!source.IsSwiftType())
- continue;
-
- partial_outputs.resize(0);
- SubstitutionWriter::ApplyListToCompilerAsOutputFile(
- target_, source, partial_outputs_subst, &partial_outputs);
-
- for (const OutputFile& output : partial_outputs) {
- additional_outputs.push_back(output);
- SourceFile output_as_source =
- output.AsSourceFile(target_->settings()->build_settings());
- if (output_as_source.IsObjectType()) {
- object_files->push_back(output);
- }
- }
- }
- }
-
- UniqueVector<OutputFile> swift_order_only_deps;
- swift_order_only_deps.reserve(order_only_deps.size());
- swift_order_only_deps.Append(order_only_deps.begin(),
- order_only_deps.end());
-
- for (const Target* swiftmodule :
- resolved().GetSwiftModuleDependencies(target_))
- swift_order_only_deps.push_back(swiftmodule->dependency_output_file());
-
- WriteCompilerBuildLine(target_->sources(), input_deps,
- swift_order_only_deps.vector(), tool->name(),
- {swiftmodule_output_file}, false);
-
- if (!additional_outputs.empty()) {
- out_ << std::endl;
- WriteCompilerBuildLine(
- {swiftmodule_output_file.AsSourceFile(settings_->build_settings())},
- input_deps, swift_order_only_deps.vector(),
- GeneralTool::kGeneralToolStamp, additional_outputs, false);
+ if (output_as_source.IsObjectType()) {
+ object_files->push_back(output);
}
}
+ UniqueVector<OutputFile> swift_order_only_deps;
+ swift_order_only_deps.reserve(order_only_deps.size());
+ swift_order_only_deps.Append(order_only_deps.begin(), order_only_deps.end());
+
+ for (const Target* swiftmodule :
+ resolved().GetSwiftModuleDependencies(target_))
+ swift_order_only_deps.push_back(swiftmodule->dependency_output_file());
+
+ WriteCompilerBuildLine(target_->sources(), input_deps,
+ swift_order_only_deps.vector(), tool->name(), outputs,
+ false);
+
out_ << std::endl;
}
diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc
index b4d49d9..085914a 100644
--- a/src/gn/ninja_c_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -2298,11 +2298,9 @@
"target_out_dir = obj/foo\n"
"target_output_name = foo\n"
"\n"
- "build obj/foo/Foo.swiftmodule: swift"
+ "build obj/foo/Foo.swiftmodule obj/foo/file1.o obj/foo/file2.o: swift"
" ../../foo/file1.swift ../../foo/file2.swift\n"
"\n"
- "build obj/foo/file1.o obj/foo/file2.o: stamp obj/foo/Foo.swiftmodule\n"
- "\n"
"build obj/foo/foo.stamp: stamp"
" obj/foo/file1.o obj/foo/file2.o\n";
@@ -2335,10 +2333,7 @@
"target_out_dir = obj/bar\n"
"target_output_name = bar\n"
"\n"
- "build obj/bar/Bar.swiftmodule: swift ../../bar/bar.swift"
- " || obj/foo/foo.stamp\n"
- "\n"
- "build obj/bar/bar.o: stamp obj/bar/Bar.swiftmodule"
+ "build obj/bar/Bar.swiftmodule obj/bar/bar.o: swift ../../bar/bar.swift"
" || obj/foo/foo.stamp\n"
"\n"
"build obj/bar/bar.stamp: stamp obj/bar/bar.o "
@@ -2381,10 +2376,7 @@
"target_out_dir = obj/bar\n"
"target_output_name = bar\n"
"\n"
- "build obj/bar/Bar.swiftmodule: swift ../../bar/bar.swift"
- " || obj/bar/group.stamp obj/foo/foo.stamp\n"
- "\n"
- "build obj/bar/bar.o: stamp obj/bar/Bar.swiftmodule"
+ "build obj/bar/Bar.swiftmodule obj/bar/bar.o: swift ../../bar/bar.swift"
" || obj/bar/group.stamp obj/foo/foo.stamp\n"
"\n"
"build obj/bar/bar.stamp: stamp obj/bar/bar.o "
diff --git a/src/gn/swift_values.cc b/src/gn/swift_values.cc
index 6065478..6faa9af 100644
--- a/src/gn/swift_values.cc
+++ b/src/gn/swift_values.cc
@@ -26,24 +26,39 @@
const Tool* tool =
target->toolchain()->GetToolForSourceType(SourceFile::SOURCE_SWIFT);
- CHECK(tool->outputs().list().size() >= 1);
- OutputFile module_output_file =
- SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
- target, tool, tool->outputs().list()[0]);
+ std::vector<OutputFile> outputs;
+ SubstitutionWriter::ApplyListToLinkerAsOutputFile(target, tool,
+ tool->outputs(), &outputs);
- const SourceFile module_output_file_as_source =
- module_output_file.AsSourceFile(target->settings()->build_settings());
- if (!module_output_file_as_source.IsSwiftModuleType()) {
- *err = Err(tool->defined_from(), "Incorrect outputs for tool",
- "The first output of tool " + std::string(tool->name()) +
- " must be a .swiftmodule file.");
- return false;
+ bool swiftmodule_output_found = false;
+ SwiftValues& swift_values = target->swift_values();
+ for (const OutputFile& output : outputs) {
+ const SourceFile output_as_source =
+ output.AsSourceFile(target->settings()->build_settings());
+ if (!output_as_source.IsSwiftModuleType()) {
+ continue;
+ }
+
+ if (swiftmodule_output_found) {
+ *err = Err(tool->defined_from(), "Incorrect outputs for tool",
+ "The outputs of tool " + std::string(tool->name()) +
+ " must list exactly one .swiftmodule file");
+ return false;
+ }
+
+ swift_values.module_output_file_ = output;
+ swift_values.module_output_dir_ = output_as_source.GetDir();
+
+ swiftmodule_output_found = true;
}
- SwiftValues& swift_values = target->swift_values();
- swift_values.module_output_file_ = std::move(module_output_file);
- swift_values.module_output_dir_ = module_output_file_as_source.GetDir();
+ if (!swiftmodule_output_found) {
+ *err = Err(tool->defined_from(), "Incorrect outputs for tool",
+ "The outputs of tool " + std::string(tool->name()) +
+ " must list exactly one .swiftmodule file");
+ return false;
+ }
return true;
}
