Switch source_set stamp targets to phony targets GN uses stamp files for a variety of output types as a mechanism for completing a group of dependencies. Ninja's phony targets can be used in the same manner. Semantically, stamp and phony only differ in one way: if a phony target has no dependencies, then it is treated as always dirty and is run every time. The source_set output type always generates a stamp file in the target ninja file. This CL replaces the source_set's stamp target with a phony target. In the case where the source_set's new phony target does not have any inputs, no targets will depend on it. An example of where that might happen is a header only source_set. There are no object files to generate, but targets that depend on the header only source_set will generate dependencies on those headers through depfiles. Bug: 187 Change-Id: I159f2f39a988eea189e4df9f5d9207fda6db5c58 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/9820 Reviewed-by: Brett Wilson <brettw@chromium.org> Commit-Queue: Brett Wilson <brettw@chromium.org>
diff --git a/build/gen.py b/build/gen.py index 486354b..a16f16f 100755 --- a/build/gen.py +++ b/build/gen.py
@@ -504,6 +504,7 @@ 'src/gn/bundle_data.cc', 'src/gn/bundle_data_target_generator.cc', 'src/gn/bundle_file_rule.cc', + 'src/gn/builtin_tool.cc', 'src/gn/c_include_iterator.cc', 'src/gn/c_substitution_type.cc', 'src/gn/c_tool.cc',
diff --git a/src/gn/builtin_tool.cc b/src/gn/builtin_tool.cc new file mode 100644 index 0000000..8bc1e52 --- /dev/null +++ b/src/gn/builtin_tool.cc
@@ -0,0 +1,42 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/logging.h" +#include "gn/builtin_tool.h" +#include "gn/target.h" + +const char* BuiltinTool::kBuiltinToolPhony = "phony"; + +BuiltinTool::BuiltinTool(const char* n) : Tool(n) { + CHECK(ValidateName(n)); +} + +BuiltinTool::~BuiltinTool() = default; + +BuiltinTool* BuiltinTool::AsBuiltin() { + return this; +} +const BuiltinTool* BuiltinTool::AsBuiltin() const { + return this; +} + +bool BuiltinTool::ValidateName(const char* name) const { + return name == kBuiltinToolPhony; +} + +void BuiltinTool::SetComplete() { + SetToolComplete(); +} + +bool BuiltinTool::InitTool(Scope* scope, Toolchain* toolchain, Err* err) { + // Initialize default vars. + return Tool::InitTool(scope, toolchain, err); +} + +bool BuiltinTool::ValidateSubstitution(const Substitution* sub_type) const { + if (name_ == kBuiltinToolPhony) + return IsValidToolSubstitution(sub_type); + NOTREACHED(); + return false; +}
diff --git a/src/gn/builtin_tool.h b/src/gn/builtin_tool.h new file mode 100644 index 0000000..00cc809 --- /dev/null +++ b/src/gn/builtin_tool.h
@@ -0,0 +1,39 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef TOOLS_GN_BUILTIN_TOOL_H_ +#define TOOLS_GN_BUILTIN_TOOL_H_ + +#include <string> + +#include "base/macros.h" +#include "gn/substitution_list.h" +#include "gn/substitution_pattern.h" +#include "gn/tool.h" + +// A built-in tool that is always available regardless of toolchain. So far, the +// only example of this is the phony rule that ninja provides. +class BuiltinTool : public Tool { + public: + // Builtin tools + static const char* kBuiltinToolPhony; + + explicit BuiltinTool(const char* n); + ~BuiltinTool(); + + // Manual RTTI and required functions --------------------------------------- + + bool InitTool(Scope* block_scope, Toolchain* toolchain, Err* err); + bool ValidateName(const char* name) const override; + void SetComplete() override; + bool ValidateSubstitution(const Substitution* sub_type) const override; + + BuiltinTool* AsBuiltin() override; + const BuiltinTool* AsBuiltin() const override; + + private: + DISALLOW_COPY_AND_ASSIGN(BuiltinTool); +}; + +#endif // TOOLS_GN_BUILTIN_TOOL_H_
diff --git a/src/gn/commands.cc b/src/gn/commands.cc index 97d2629..1d2b899 100644 --- a/src/gn/commands.cc +++ b/src/gn/commands.cc
@@ -328,8 +328,13 @@ // Use the link output file if there is one, otherwise fall back to the // dependency output file (for actions, for example). OutputFile output_file = target->link_output_file(); + if (output_file.value().empty() && target->dependency_output_file_or_phony()) + output_file = *target->dependency_output_file_or_phony(); + + // This output might be an omitted phony target, but that would mean we + // don't have an output file to list. if (output_file.value().empty()) - output_file = target->dependency_output_file(); + continue; SourceFile output_as_source = output_file.AsSourceFile(build_settings); std::string result =
diff --git a/src/gn/filesystem_utils.cc b/src/gn/filesystem_utils.cc index ae6d4d9..ab06545 100644 --- a/src/gn/filesystem_utils.cc +++ b/src/gn/filesystem_utils.cc
@@ -1028,6 +1028,8 @@ result.value().append("gen/"); else if (type == BuildDirType::OBJ) result.value().append("obj/"); + else if (type == BuildDirType::PHONY) + result.value().append("phony/"); return result; }
diff --git a/src/gn/filesystem_utils.h b/src/gn/filesystem_utils.h index 830478a..9ba3b4e 100644 --- a/src/gn/filesystem_utils.h +++ b/src/gn/filesystem_utils.h
@@ -232,6 +232,12 @@ // Output file directory. OBJ, + + // Phony file directory. As the name implies, this is not a real file + // directory, but a path that is used for the declaration of phony targets. + // This is done to avoid duplicate target names between real files and phony + // aliases that point to them. + PHONY, }; // In different contexts, different information is known about the toolchain in
diff --git a/src/gn/function_get_target_outputs.cc b/src/gn/function_get_target_outputs.cc index fa851f3..ca37d9a 100644 --- a/src/gn/function_get_target_outputs.cc +++ b/src/gn/function_get_target_outputs.cc
@@ -46,8 +46,8 @@ process_file_template"). source sets and groups: this will return a list containing the path of the - "stamp" file that Ninja will produce once all outputs are generated. This - probably isn't very useful. + phony target or the "stamp" file that Ninja will produce once all outputs are + generated. This probably isn't very useful. Example
diff --git a/src/gn/ninja_action_target_writer.cc b/src/gn/ninja_action_target_writer.cc index 28f28c7..0b49066 100644 --- a/src/gn/ninja_action_target_writer.cc +++ b/src/gn/ninja_action_target_writer.cc
@@ -89,8 +89,10 @@ // TODO(thakis): If the action has just a single output, make things depend // on that output directly without writing a stamp file. std::vector<OutputFile> data_outs; - for (const auto& dep : target_->data_deps()) - data_outs.push_back(dep.ptr->dependency_output_file()); + for (const auto& dep : target_->data_deps()) { + if (dep.ptr->dependency_output_file_or_phony()) + data_outs.push_back(*dep.ptr->dependency_output_file_or_phony()); + } WriteStampForTarget(output_files, data_outs); }
diff --git a/src/gn/ninja_binary_target_writer.cc b/src/gn/ninja_binary_target_writer.cc index 08e8c93..e06700a 100644 --- a/src/gn/ninja_binary_target_writer.cc +++ b/src/gn/ninja_binary_target_writer.cc
@@ -102,9 +102,9 @@ return {stamp_file}; } -void NinjaBinaryTargetWriter::WriteSourceSetStamp( +void NinjaBinaryTargetWriter::WriteSourceSetPhony( const std::vector<OutputFile>& object_files) { - // The stamp rule for source sets is generally not used, since targets that + // The phony rule for source sets is generally not used, since targets that // depend on this will reference the object files directly. However, writing // this rule allows the user to type the name of the target and get a build // which can be convenient for development. @@ -116,10 +116,12 @@ DCHECK(classified_deps.extra_object_files.empty()); std::vector<OutputFile> order_only_deps; - for (auto* dep : classified_deps.non_linkable_deps) - order_only_deps.push_back(dep->dependency_output_file()); + for (auto* dep : classified_deps.non_linkable_deps) { + if (dep->dependency_output_file_or_phony()) + order_only_deps.push_back(*dep->dependency_output_file_or_phony()); + } - WriteStampForTarget(object_files, order_only_deps); + WritePhonyForTarget(object_files, order_only_deps); } NinjaBinaryTargetWriter::ClassifiedDeps @@ -177,7 +179,7 @@ AddSourceSetFiles(dep, &classified_deps->extra_object_files); // Add the source set itself as a non-linkable dependency on the current - // target. This will make sure that anything the source set's stamp file + // target. This will make sure that anything the source set's phony target // depends on (like data deps) are also built before the current target // can be complete. Otherwise, these will be skipped since this target // will depend only on the source set's object files.
diff --git a/src/gn/ninja_binary_target_writer.h b/src/gn/ninja_binary_target_writer.h index 76a8a4e..eb75c89 100644 --- a/src/gn/ninja_binary_target_writer.h +++ b/src/gn/ninja_binary_target_writer.h
@@ -43,8 +43,8 @@ std::vector<OutputFile> WriteInputsStampAndGetDep( size_t num_stamp_uses) const; - // Writes the stamp line for a source set. These are not linked. - void WriteSourceSetStamp(const std::vector<OutputFile>& object_files); + // Writes the phony line for a source set. These are not linked. + void WriteSourceSetPhony(const std::vector<OutputFile>& object_files); // Gets all target dependencies and classifies them, as well as accumulates // object files from source sets we need to link.
diff --git a/src/gn/ninja_binary_target_writer_unittest.cc b/src/gn/ninja_binary_target_writer_unittest.cc index 970aa82..9e4037c 100644 --- a/src/gn/ninja_binary_target_writer_unittest.cc +++ b/src/gn/ninja_binary_target_writer_unittest.cc
@@ -44,7 +44,7 @@ "build obj/foo/bar.input1.o: cxx ../../foo/input1.cc\n" "build obj/foo/bar.input2.o: cxx ../../foo/input2.cc\n" "\n" - "build obj/foo/bar.stamp: stamp obj/foo/bar.input1.o " + "build phony/foo/bar: phony obj/foo/bar.input1.o " "obj/foo/bar.input2.o ../../foo/input3.o ../../foo/input4.obj\n"; std::string out_str = out.str(); EXPECT_EQ(expected, out_str); @@ -71,8 +71,7 @@ "target_out_dir = obj/foo\n" "target_output_name = bar\n" "\n" - "\n" - "build obj/foo/bar.stamp: stamp\n"; + "\n"; std::string out_str = out.str(); EXPECT_EQ(expected, out_str); } @@ -138,7 +137,7 @@ "build obj/foo/bar.source1.o: cxx ../../foo/source1.cc | " "../../foo/input1 ../../foo/input2\n" "\n" - "build obj/foo/bar.stamp: stamp obj/foo/bar.source1.o\n"; + "build phony/foo/bar: phony obj/foo/bar.source1.o\n"; std::string out_str = out.str(); EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; } @@ -175,7 +174,7 @@ "build obj/foo/bar.source2.o: cxx ../../foo/source2.cc | " "obj/foo/bar.inputs.stamp\n" "\n" - "build obj/foo/bar.stamp: stamp obj/foo/bar.source1.o " + "build phony/foo/bar: phony obj/foo/bar.source1.o " "obj/foo/bar.source2.o\n"; std::string out_str = out.str(); EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
diff --git a/src/gn/ninja_build_writer.cc b/src/gn/ninja_build_writer.cc index f801fb6..50f1155 100644 --- a/src/gn/ninja_build_writer.cc +++ b/src/gn/ninja_build_writer.cc
@@ -595,8 +595,11 @@ EscapeOptions ninja_escape; ninja_escape.mode = ESCAPE_NINJA; for (const Target* target : default_toolchain_targets_) { - out_ << " $\n "; - path_output_.WriteFile(out_, target->dependency_output_file()); + if (target->dependency_output_file_or_phony()) { + out_ << " $\n "; + path_output_.WriteFile(out_, + *target->dependency_output_file_or_phony()); + } } } out_ << std::endl; @@ -605,9 +608,13 @@ // Use the short name when available if (written_rules.find("default") != written_rules.end()) { out_ << "\ndefault default" << std::endl; - } else { + } else if (default_target->dependency_output_file_or_phony()) { + // If the default target does not have a dependency output file or phony, + // then the target specified as default is a no-op. We omit the default + // statement entirely to avoid ninja runtime failure. out_ << "\ndefault "; - path_output_.WriteFile(out_, default_target->dependency_output_file()); + path_output_.WriteFile( + out_, *default_target->dependency_output_file_or_phony()); out_ << std::endl; } } else if (!default_toolchain_targets_.empty()) { @@ -625,7 +632,12 @@ // Escape for special chars Ninja will handle. std::string escaped = EscapeString(phony_name, ninja_escape, nullptr); + // If the target doesn't have a dependency_output_file_or_phony, we should + // still emit the phony rule, but with no dependencies. This allows users to + // continue to use the phony rule, but it will effectively be a no-op. out_ << "build " << escaped << ": phony "; - path_output_.WriteFile(out_, target->dependency_output_file()); + if (target->dependency_output_file_or_phony()) { + path_output_.WriteFile(out_, *target->dependency_output_file_or_phony()); + } out_ << std::endl; }
diff --git a/src/gn/ninja_bundle_data_target_writer.cc b/src/gn/ninja_bundle_data_target_writer.cc index 0e3bcb0..239c0c2 100644 --- a/src/gn/ninja_bundle_data_target_writer.cc +++ b/src/gn/ninja_bundle_data_target_writer.cc
@@ -26,8 +26,10 @@ output_files.insert(output_files.end(), input_deps.begin(), input_deps.end()); std::vector<OutputFile> order_only_deps; - for (const auto& pair : target_->data_deps()) - order_only_deps.push_back(pair.ptr->dependency_output_file()); + for (const auto& pair : target_->data_deps()) { + if (pair.ptr->dependency_output_file_or_phony()) + order_only_deps.push_back(*pair.ptr->dependency_output_file_or_phony()); + } WriteStampForTarget(output_files, order_only_deps); }
diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc index 17f9c08..2e7e613 100644 --- a/src/gn/ninja_c_binary_target_writer.cc +++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -207,7 +207,7 @@ return; if (target_->output_type() == Target::SOURCE_SET) { - WriteSourceSetStamp(obj_files); + WriteSourceSetPhony(obj_files); #ifndef NDEBUG // Verify that the function that separately computes a source set's object // files match the object files just computed. @@ -667,8 +667,11 @@ swift_order_only_deps.Append(order_only_deps.begin(), order_only_deps.end()); - for (const Target* swiftmodule : target_->swift_values().modules()) - swift_order_only_deps.push_back(swiftmodule->dependency_output_file()); + for (const Target* swiftmodule : target_->swift_values().modules()) { + CHECK(swiftmodule->dependency_output_file_or_phony()); + swift_order_only_deps.push_back( + *swiftmodule->dependency_output_file_or_phony()); + } WriteCompilerBuildLine(target_->sources(), input_deps, swift_order_only_deps.vector(), tool->name(), @@ -719,11 +722,12 @@ cur->output_type() == Target::RUST_PROC_MACRO) continue; - if (cur->dependency_output_file().value() != - cur->link_output_file().value()) { + if (cur->dependency_output_file_or_phony() && + (cur->dependency_output_file_or_phony()->value() != + cur->link_output_file().value())) { // This is a shared library with separate link and deps files. Save for // later. - implicit_deps.push_back(cur->dependency_output_file()); + implicit_deps.push_back(*cur->dependency_output_file_or_phony()); solibs.push_back(cur->link_output_file()); } else { // Normal case, just link to this target. @@ -754,12 +758,13 @@ } // If any target creates a framework bundle, then treat it as an implicit - // dependency via the .stamp file. This is a pessimisation as it is not + // dependency via the phony target. This is a pessimisation as it is not // always necessary to relink the current target if one of the framework // is regenerated, but it ensure that if one of the framework API changes, // any dependent target will relink it (see crbug.com/1037607). for (const Target* dep : classified_deps.framework_deps) { - implicit_deps.push_back(dep->dependency_output_file()); + if (dep->dependency_output_file_or_phony()) + implicit_deps.push_back(*dep->dependency_output_file_or_phony()); } // The input dependency is only needed if there are no object files, as the @@ -774,8 +779,9 @@ for (const auto* dep : target_->rust_values().transitive_libs().GetOrdered()) { if (dep->output_type() == Target::RUST_LIBRARY) { - transitive_rustlibs.push_back(dep->dependency_output_file()); - implicit_deps.push_back(dep->dependency_output_file()); + CHECK(dep->dependency_output_file()); + transitive_rustlibs.push_back(*dep->dependency_output_file()); + implicit_deps.push_back(*dep->dependency_output_file()); } } } @@ -873,8 +879,11 @@ // Non-linkable targets. for (auto* non_linkable_dep : non_linkable_deps) { - out_ << " "; - path_output_.WriteFile(out_, non_linkable_dep->dependency_output_file()); + if (non_linkable_dep->dependency_output_file_or_phony()) { + out_ << " "; + path_output_.WriteFile( + out_, *non_linkable_dep->dependency_output_file_or_phony()); + } } } }
diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc index 6290301..df6e8b5 100644 --- a/src/gn/ninja_c_binary_target_writer_unittest.cc +++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -55,7 +55,7 @@ "build obj/foo/bar.input1.o: cxx ../../foo/input1.cc\n" "build obj/foo/bar.input2.o: cxx ../../foo/input2.cc\n" "\n" - "build obj/foo/bar.stamp: stamp obj/foo/bar.input1.o " + "build phony/foo/bar: phony obj/foo/bar.input1.o " "obj/foo/bar.input2.o ../../foo/input3.o ../../foo/input4.obj\n"; std::string out_str = out.str(); EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; @@ -86,7 +86,7 @@ // order. "build ./libshlib.so: solink obj/foo/bar.input1.o " "obj/foo/bar.input2.o ../../foo/input3.o ../../foo/input4.obj " - "|| obj/foo/bar.stamp\n" + "|| phony/foo/bar\n" " ldflags =\n" " libs =\n" " frameworks =\n" @@ -119,7 +119,7 @@ "\n" // There are no sources so there are no params to alink. (In practice // this will probably fail in the archive tool.) - "build obj/foo/libstlib.a: alink || obj/foo/bar.stamp\n" + "build obj/foo/libstlib.a: alink || phony/foo/bar\n" " arflags =\n" " output_extension = \n" " output_dir = \n"; @@ -147,7 +147,7 @@ // order. "build obj/foo/libstlib.a: alink obj/foo/bar.input1.o " "obj/foo/bar.input2.o ../../foo/input3.o ../../foo/input4.obj " - "|| obj/foo/bar.stamp\n" + "|| phony/foo/bar\n" " arflags =\n" " output_extension = \n" " output_dir = \n"; @@ -394,7 +394,7 @@ "build obj/out/Debug/gen_obj.generated.o: cxx generated.cc" " || obj/foo/generate.stamp\n" "\n" - "build obj/foo/gen_obj.stamp: stamp obj/out/Debug/gen_obj.generated.o" + "build phony/foo/gen_obj: phony obj/out/Debug/gen_obj.generated.o" // The order-only dependency here is strictly unnecessary since the // sources list this as an order-only dep. " || obj/foo/generate.stamp\n"; @@ -429,8 +429,8 @@ "build ./libgen_lib.so: solink obj/out/Debug/gen_obj.generated.o" // The order-only dependency here is strictly unnecessary since // obj/out/Debug/gen_obj.generated.o has dependency to - // obj/foo/gen_obj.stamp - " || obj/foo/gen_obj.stamp\n" + // phony/foo/gen_obj + " || phony/foo/gen_obj\n" " ldflags =\n" " libs =\n" " frameworks =\n" @@ -649,7 +649,7 @@ NinjaCBinaryTargetWriter inter_writer(&inter, inter_out); inter_writer.Run(); - // The intermediate source set will be a stamp file that depends on the + // The intermediate source set will be a phony target that depends on the // object files, and will have an order-only dependency on its data dep and // data file. const char inter_expected[] = @@ -663,7 +663,7 @@ "\n" "build obj/foo/inter.inter.o: cxx ../../foo/inter.cc\n" "\n" - "build obj/foo/inter.stamp: stamp obj/foo/inter.inter.o || " + "build phony/foo/inter: phony obj/foo/inter.inter.o || " "./data_target\n"; EXPECT_EQ(inter_expected, inter_out.str()); @@ -682,7 +682,7 @@ // The final output depends on both object files (one from the final target, // one from the source set) and has an order-only dependency on the source - // set's stamp file and the final target's data file. The source set stamp + // set's phony target and the final target's data file. The source set phony // dependency will create an implicit order-only dependency on the data // target. const char final_expected[] = @@ -697,7 +697,7 @@ "build obj/foo/exe.final.o: cxx ../../foo/final.cc\n" "\n" "build ./exe: link obj/foo/exe.final.o obj/foo/inter.inter.o || " - "obj/foo/inter.stamp\n" + "phony/foo/inter\n" " ldflags =\n" " libs =\n" " frameworks =\n" @@ -886,8 +886,8 @@ "build withpch/obj/foo/no_pch_target.input2.o: " "withpch_cc ../../foo/input2.c\n" "\n" - "build withpch/obj/foo/no_pch_target.stamp: " - "withpch_stamp withpch/obj/foo/no_pch_target.input1.o " + "build withpch/phony/foo/no_pch_target: " + "phony withpch/obj/foo/no_pch_target.input1.o " "withpch/obj/foo/no_pch_target.input2.o\n"; EXPECT_EQ(no_pch_expected, out.str()); } @@ -940,7 +940,7 @@ // Explicit dependency on the PCH build step. "withpch/obj/build/pch_target.precompile.c.o\n" "\n" - "build withpch/obj/foo/pch_target.stamp: withpch_stamp " + "build withpch/phony/foo/pch_target: phony " "withpch/obj/foo/pch_target.input1.o " "withpch/obj/foo/pch_target.input2.o " // The precompiled object files were added to the outputs. @@ -1020,8 +1020,8 @@ "build withpch/obj/foo/no_pch_target.input2.o: " "withpch_cc ../../foo/input2.c\n" "\n" - "build withpch/obj/foo/no_pch_target.stamp: " - "withpch_stamp withpch/obj/foo/no_pch_target.input1.o " + "build withpch/phony/foo/no_pch_target: " + "phony withpch/obj/foo/no_pch_target.input1.o " "withpch/obj/foo/no_pch_target.input2.o\n"; EXPECT_EQ(no_pch_expected, out.str()); } @@ -1072,8 +1072,8 @@ // Explicit dependency on the PCH build step. "withpch/obj/build/pch_target.precompile.h-c.gch\n" "\n" - "build withpch/obj/foo/pch_target.stamp: " - "withpch_stamp withpch/obj/foo/pch_target.input1.o " + "build withpch/phony/foo/pch_target: " + "phony withpch/obj/foo/pch_target.input1.o " "withpch/obj/foo/pch_target.input2.o\n"; EXPECT_EQ(pch_gcc_expected, out.str()); } @@ -1138,7 +1138,7 @@ "build obj/foo/bar.input2.o: cxx ../../foo/input2.cc" " | ../../foo/input.data\n" "\n" - "build obj/foo/bar.stamp: stamp obj/foo/bar.input1.o " + "build phony/foo/bar: phony obj/foo/bar.input1.o " "obj/foo/bar.input2.o\n"; EXPECT_EQ(expected, out.str()); @@ -1209,7 +1209,7 @@ "build obj/foo/bar.input2.o: cxx ../../foo/input2.cc" " | obj/foo/bar.inputs.stamp\n" "\n" - "build obj/foo/bar.stamp: stamp obj/foo/bar.input1.o " + "build phony/foo/bar: phony obj/foo/bar.input1.o " "obj/foo/bar.input2.o\n"; EXPECT_EQ(expected, out.str()); @@ -1259,7 +1259,7 @@ "build obj/foo/bar.input2.o: cxx ../../foo/input2.cc" " | obj/foo/bar.inputs.stamp\n" "\n" - "build obj/foo/bar.stamp: stamp obj/foo/bar.input1.o " + "build phony/foo/bar: phony obj/foo/bar.input1.o " "obj/foo/bar.input2.o\n"; EXPECT_EQ(expected, out.str()); @@ -1571,7 +1571,7 @@ "\n" "build obj/foo/file1.o obj/foo/file2.o: stamp obj/foo/Foo.swiftmodule\n" "\n" - "build obj/foo/foo.stamp: stamp" + "build phony/foo/foo: phony" " obj/foo/file1.o obj/foo/file2.o\n"; const std::string out_str = out.str(); @@ -1604,13 +1604,13 @@ "target_output_name = bar\n" "\n" "build obj/bar/Bar.swiftmodule: swift ../../bar/bar.swift" - " || obj/foo/foo.stamp\n" + " || phony/foo/foo\n" "\n" "build obj/bar/bar.o: stamp obj/bar/Bar.swiftmodule" - " || obj/foo/foo.stamp\n" + " || phony/foo/foo\n" "\n" - "build obj/bar/bar.stamp: stamp obj/bar/bar.o " - "|| obj/foo/foo.stamp\n"; + "build phony/bar/bar: phony obj/bar/bar.o " + "|| phony/foo/foo\n"; const std::string out_str = out.str(); EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; @@ -1650,13 +1650,13 @@ "target_output_name = bar\n" "\n" "build obj/bar/Bar.swiftmodule: swift ../../bar/bar.swift" - " || obj/foo/foo.stamp\n" + " || phony/foo/foo\n" "\n" "build obj/bar/bar.o: stamp obj/bar/Bar.swiftmodule" - " || obj/foo/foo.stamp\n" + " || phony/foo/foo\n" "\n" - "build obj/bar/bar.stamp: stamp obj/bar/bar.o " - "|| obj/bar/group.stamp obj/foo/foo.stamp\n"; + "build phony/bar/bar: phony obj/bar/bar.o " + "|| obj/bar/group.stamp phony/foo/foo\n"; const std::string out_str = out.str(); EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; @@ -1685,7 +1685,7 @@ "\n" "build ./bar: link obj/foo/file1.o obj/foo/file2.o " "| obj/foo/Foo.swiftmodule " - "|| obj/foo/foo.stamp\n" + "|| phony/foo/foo\n" " ldflags =\n" " libs =\n" " frameworks =\n"
diff --git a/src/gn/ninja_copy_target_writer.cc b/src/gn/ninja_copy_target_writer.cc index 9299223..c1a94ee 100644 --- a/src/gn/ninja_copy_target_writer.cc +++ b/src/gn/ninja_copy_target_writer.cc
@@ -74,8 +74,10 @@ std::vector<const Target*>(), num_stamp_uses); std::vector<OutputFile> data_outs; - for (const auto& dep : target_->data_deps()) - data_outs.push_back(dep.ptr->dependency_output_file()); + for (const auto& dep : target_->data_deps()) { + if (dep.ptr->dependency_output_file_or_phony()) + data_outs.push_back(*dep.ptr->dependency_output_file_or_phony()); + } // Note that we don't write implicit deps for copy steps. "copy" only // depends on the output files themselves, rather than having includes
diff --git a/src/gn/ninja_create_bundle_target_writer.cc b/src/gn/ninja_create_bundle_target_writer.cc index 9f000f3..9623e32 100644 --- a/src/gn/ninja_create_bundle_target_writer.cc +++ b/src/gn/ninja_create_bundle_target_writer.cc
@@ -90,8 +90,10 @@ WriteCompileAssetsCatalogStep(order_only_deps, &output_files); WriteCodeSigningStep(code_signing_rule_name, order_only_deps, &output_files); - for (const auto& pair : target_->data_deps()) - order_only_deps.push_back(pair.ptr->dependency_output_file()); + for (const auto& pair : target_->data_deps()) { + if (pair.ptr->dependency_output_file_or_phony()) + order_only_deps.push_back(*pair.ptr->dependency_output_file_or_phony()); + } WriteStampForTarget(output_files, order_only_deps); // Write a phony target for the outer bundle directory. This allows other @@ -102,7 +104,9 @@ out_, OutputFile(settings_->build_settings(), target_->bundle_data().GetBundleRootDirOutput(settings_))); - out_ << ": phony " << target_->dependency_output_file().value(); + CHECK(target_->dependency_output_file_or_phony()); + out_ << ": " << BuiltinTool::kBuiltinToolPhony << " "; + out_ << target_->dependency_output_file_or_phony()->value(); out_ << std::endl; } @@ -281,8 +285,11 @@ NinjaCreateBundleTargetWriter::WriteCompileAssetsCatalogInputDepsStamp( const std::vector<const Target*>& dependencies) { DCHECK(!dependencies.empty()); - if (dependencies.size() == 1) - return dependencies[0]->dependency_output_file(); + if (dependencies.size() == 1) { + return dependencies[0]->dependency_output_file_or_phony() + ? *dependencies[0]->dependency_output_file_or_phony() + : OutputFile{}; + } OutputFile xcassets_input_stamp_file = GetBuildDirForTargetAsOutputFile(target_, BuildDirType::OBJ); @@ -295,8 +302,10 @@ << GeneralTool::kGeneralToolStamp; for (const Target* target : dependencies) { - out_ << " "; - path_output_.WriteFile(out_, target->dependency_output_file()); + if (target->dependency_output_file_or_phony()) { + out_ << " "; + path_output_.WriteFile(out_, *target->dependency_output_file_or_phony()); + } } out_ << std::endl; return xcassets_input_stamp_file;
diff --git a/src/gn/ninja_generated_file_target_writer.cc b/src/gn/ninja_generated_file_target_writer.cc index 6b0db1b..16fcf96 100644 --- a/src/gn/ninja_generated_file_target_writer.cc +++ b/src/gn/ninja_generated_file_target_writer.cc
@@ -30,13 +30,17 @@ // on each of the deps and data_deps in the target. The actual collection is // done at gen time, and so ninja doesn't need to know about it. std::vector<OutputFile> output_files; - for (const auto& pair : target_->GetDeps(Target::DEPS_LINKED)) - output_files.push_back(pair.ptr->dependency_output_file()); + for (const auto& pair : target_->GetDeps(Target::DEPS_LINKED)) { + if (pair.ptr->dependency_output_file_or_phony()) + output_files.push_back(*pair.ptr->dependency_output_file_or_phony()); + } std::vector<OutputFile> data_output_files; const LabelTargetVector& data_deps = target_->data_deps(); - for (const auto& pair : data_deps) - data_output_files.push_back(pair.ptr->dependency_output_file()); + for (const auto& pair : data_deps) { + if (pair.ptr->dependency_output_file_or_phony()) + data_output_files.push_back(*pair.ptr->dependency_output_file_or_phony()); + } WriteStampForTarget(output_files, data_output_files); }
diff --git a/src/gn/ninja_group_target_writer.cc b/src/gn/ninja_group_target_writer.cc index b518977..1ba6d1c 100644 --- a/src/gn/ninja_group_target_writer.cc +++ b/src/gn/ninja_group_target_writer.cc
@@ -20,13 +20,17 @@ // A group rule just generates a stamp file with dependencies on each of // the deps and data_deps in the group. std::vector<OutputFile> output_files; - for (const auto& pair : target_->GetDeps(Target::DEPS_LINKED)) - output_files.push_back(pair.ptr->dependency_output_file()); + for (const auto& pair : target_->GetDeps(Target::DEPS_LINKED)) { + if (pair.ptr->dependency_output_file_or_phony()) + output_files.push_back(*pair.ptr->dependency_output_file_or_phony()); + } std::vector<OutputFile> data_output_files; const LabelTargetVector& data_deps = target_->data_deps(); - for (const auto& pair : data_deps) - data_output_files.push_back(pair.ptr->dependency_output_file()); + for (const auto& pair : data_deps) { + if (pair.ptr->dependency_output_file_or_phony()) + data_output_files.push_back(*pair.ptr->dependency_output_file_or_phony()); + } WriteStampForTarget(output_files, data_output_files); }
diff --git a/src/gn/ninja_rust_binary_target_writer.cc b/src/gn/ninja_rust_binary_target_writer.cc index 59960a1..a8aecaf 100644 --- a/src/gn/ninja_rust_binary_target_writer.cc +++ b/src/gn/ninja_rust_binary_target_writer.cc
@@ -144,14 +144,19 @@ classified_deps.extra_object_files.begin(), classified_deps.extra_object_files.end()); for (const auto* framework_dep : classified_deps.framework_deps) { - order_only_deps.push_back(framework_dep->dependency_output_file()); + CHECK(framework_dep->dependency_output_file()); + order_only_deps.push_back(*framework_dep->dependency_output_file()); } for (const auto* non_linkable_dep : classified_deps.non_linkable_deps) { - if (non_linkable_dep->source_types_used().RustSourceUsed() && - non_linkable_dep->output_type() != Target::SOURCE_SET) { - rustdeps.push_back(non_linkable_dep->dependency_output_file()); + if (non_linkable_dep->dependency_output_file_or_phony()) { + if (non_linkable_dep->source_types_used().RustSourceUsed() && + non_linkable_dep->output_type() != Target::SOURCE_SET) { + rustdeps.push_back( + *non_linkable_dep->dependency_output_file_or_phony()); + } + order_only_deps.push_back( + *non_linkable_dep->dependency_output_file_or_phony()); } - order_only_deps.push_back(non_linkable_dep->dependency_output_file()); } for (const auto* linkable_dep : classified_deps.linkable_deps) { if (linkable_dep->source_types_used().RustSourceUsed()) { @@ -159,7 +164,8 @@ } else { nonrustdeps.push_back(linkable_dep->link_output_file()); } - implicit_deps.push_back(linkable_dep->dependency_output_file()); + CHECK(linkable_dep->dependency_output_file()); + implicit_deps.push_back(*linkable_dep->dependency_output_file()); } // Rust libraries specified by paths. @@ -178,7 +184,8 @@ for (const auto* dep : target_->rust_values().transitive_libs().GetOrdered()) { if (dep->source_types_used().RustSourceUsed()) { - transitive_rustlibs.push_back(dep->dependency_output_file()); + CHECK(dep->dependency_output_file()); + transitive_rustlibs.push_back(*dep->dependency_output_file()); } } @@ -249,6 +256,7 @@ for (const Target* target : deps) { if (target->output_type() == Target::RUST_LIBRARY || target->output_type() == Target::RUST_PROC_MACRO) { + CHECK(target->dependency_output_file()); out_ << " --extern "; const auto& renamed_dep = target_->rust_values().aliased_deps().find(target->label()); @@ -257,7 +265,7 @@ } else { out_ << std::string(target->rust_values().crate_name()) << "="; } - path_output_.WriteFile(out_, target->dependency_output_file()); + path_output_.WriteFile(out_, *target->dependency_output_file()); } }
diff --git a/src/gn/ninja_rust_binary_target_writer_unittest.cc b/src/gn/ninja_rust_binary_target_writer_unittest.cc index 34d87f0..a440a6f 100644 --- a/src/gn/ninja_rust_binary_target_writer_unittest.cc +++ b/src/gn/ninja_rust_binary_target_writer_unittest.cc
@@ -436,7 +436,7 @@ "../../foo/main.rs obj/baz/sourceset.csourceset.o " "obj/bar/libmylib.rlib " "obj/foo/libstatic.a ./libshared.so ./libshared_with_toc.so.TOC " - "|| obj/baz/sourceset.stamp\n" + "|| phony/baz/sourceset\n" " externs = --extern mylib=obj/bar/libmylib.rlib\n" " rustdeps = -Ldependency=obj/bar -Lnative=obj/baz -Lnative=obj/foo " "-Lnative=. -Clink-arg=obj/baz/sourceset.csourceset.o -lstatic "
diff --git a/src/gn/ninja_target_writer.cc b/src/gn/ninja_target_writer.cc index b6bbb9f..5b443f1 100644 --- a/src/gn/ninja_target_writer.cc +++ b/src/gn/ninja_target_writer.cc
@@ -8,6 +8,7 @@ #include "base/files/file_util.h" #include "base/strings/string_util.h" +#include "gn/builtin_tool.h" #include "gn/config_values_extractors.h" #include "gn/err.h" #include "gn/escape.h" @@ -267,9 +268,11 @@ return std::vector<OutputFile>{ OutputFile(settings_->build_settings(), *input_deps_sources[0])}; if (input_deps_sources.size() == 0 && input_deps_targets.size() == 1) { - const OutputFile& dep = input_deps_targets[0]->dependency_output_file(); - DCHECK(!dep.value().empty()); - return std::vector<OutputFile>{dep}; + const std::optional<OutputFile>& dep = + input_deps_targets[0]->dependency_output_file_or_phony(); + if (!dep) + return std::vector<OutputFile>(); + return std::vector<OutputFile>{*dep}; } std::vector<OutputFile> outs; @@ -283,8 +286,8 @@ input_deps_targets.begin(), input_deps_targets.end(), [](const Target* a, const Target* b) { return a->label() < b->label(); }); for (auto* dep : input_deps_targets) { - DCHECK(!dep->dependency_output_file().value().empty()); - outs.push_back(dep->dependency_output_file()); + if (dep->dependency_output_file_or_phony()) + outs.push_back(*dep->dependency_output_file_or_phony()); } // If there are multiple inputs, but the stamp file would be referenced only @@ -311,7 +314,8 @@ void NinjaTargetWriter::WriteStampForTarget( const std::vector<OutputFile>& files, const std::vector<OutputFile>& order_only_deps) { - const OutputFile& stamp_file = target_->dependency_output_file(); + CHECK(target_->dependency_output_file()); + const OutputFile& stamp_file = *target_->dependency_output_file(); // First validate that the target's dependency is a stamp file. Otherwise, // we shouldn't have gotten here! @@ -333,3 +337,29 @@ } out_ << std::endl; } + +void NinjaTargetWriter::WritePhonyForTarget( + const std::vector<OutputFile>& files, + const std::vector<OutputFile>& order_only_deps) { + // If there's no phony, then we should not have any inputs and it is okay to + // omit the build rule. + if (!target_->dependency_output_phony()) { + CHECK(files.empty()); + CHECK(order_only_deps.empty()); + return; + } + const OutputFile& phony_target = *target_->dependency_output_phony(); + CHECK(!phony_target.value().empty()); + + out_ << "build "; + path_output_.WriteFile(out_, phony_target); + + out_ << ": " << BuiltinTool::kBuiltinToolPhony; + path_output_.WriteFiles(out_, files); + + if (!order_only_deps.empty()) { + out_ << " ||"; + path_output_.WriteFiles(out_, order_only_deps); + } + out_ << std::endl; +}
diff --git a/src/gn/ninja_target_writer.h b/src/gn/ninja_target_writer.h index f4c9eae..f6c4e04 100644 --- a/src/gn/ninja_target_writer.h +++ b/src/gn/ninja_target_writer.h
@@ -57,6 +57,11 @@ void WriteStampForTarget(const std::vector<OutputFile>& deps, const std::vector<OutputFile>& order_only_deps); + // Writes to the output file a final phony rule for the target that aliases + // the given list of files. + void WritePhonyForTarget(const std::vector<OutputFile>& deps, + const std::vector<OutputFile>& order_only_deps); + const Settings* settings_; // Non-owning. const Target* target_; // Non-owning. std::ostream& out_;
diff --git a/src/gn/runtime_deps.cc b/src/gn/runtime_deps.cc index 3b6d683..d002458 100644 --- a/src/gn/runtime_deps.cc +++ b/src/gn/runtime_deps.cc
@@ -176,7 +176,7 @@ return false; } - OutputFile output_file; + std::optional<OutputFile> output_file; const char extension[] = ".runtime_deps"; if (target->output_type() == Target::SHARED_LIBRARY || target->output_type() == Target::LOADABLE_MODULE) { @@ -185,11 +185,12 @@ CHECK(!target->computed_outputs().empty()); output_file = OutputFile(target->computed_outputs()[0].value() + extension); - } else { - output_file = - OutputFile(target->dependency_output_file().value() + extension); + } else if (target->dependency_output_file_or_phony()) { + output_file = OutputFile( + target->dependency_output_file_or_phony()->value() + extension); } - files_to_write->push_back(std::make_pair(output_file, target)); + if (output_file) + files_to_write->push_back(std::make_pair(*output_file, target)); } return true; }
diff --git a/src/gn/switches.cc b/src/gn/switches.cc index e7b3a81..cbeaa26 100644 --- a/src/gn/switches.cc +++ b/src/gn/switches.cc
@@ -199,8 +199,9 @@ build directory. If a source set, action, copy, or group is listed, the runtime deps file will - correspond to the .stamp file corresponding to that target. This is probably - not useful; the use-case for this feature is generally executable targets. + correspond to the .stamp file or phony rule corresponding to that target. This + is probably not useful; the use-case for this feature is generally executable + targets. The runtime dependency file will list one file per line, with no escaping. The files will be relative to the root_build_dir. The first line of the file
diff --git a/src/gn/target.cc b/src/gn/target.cc index b8bf2f9..a5493e8 100644 --- a/src/gn/target.cc +++ b/src/gn/target.cc
@@ -5,6 +5,7 @@ #include "gn/target.h" #include <stddef.h> +#include <algorithm> #include "base/stl_util.h" #include "base/strings/string_util.h" @@ -522,8 +523,7 @@ if (!bundle_data().GetOutputsAsSourceFiles(settings(), this, outputs, err)) return false; } else if (IsBinary() && output_type() != Target::SOURCE_SET) { - // Binary target with normal outputs (source sets have stamp outputs like - // groups). + // Binary target with normal outputs (source sets have phony targets). DCHECK(IsBinary()) << static_cast<int>(output_type()); if (!build_complete) { // Can't access the toolchain for a target before the build is complete. @@ -543,15 +543,20 @@ output_file.AsSourceFile(settings()->build_settings())); } } else { - // Everything else (like a group or something) has a stamp output. The - // dependency output file should have computed what this is. This won't be - // valid unless the build is complete. + // Everything else (like a group or something) has a stamp or phony output. + // The dependency output file should have computed what this is. This won't + // be valid unless the build is complete. if (!build_complete) { *err = Err(loc_for_error, kBuildIncompleteMsg); return false; } - outputs->push_back( - dependency_output_file().AsSourceFile(settings()->build_settings())); + + // The dependency output might be empty if there is no output file or a + // phony alias for a set of inputs. + if (dependency_output_file_or_phony()) { + outputs->push_back(dependency_output_file_or_phony()->AsSourceFile( + settings()->build_settings())); + } } return true; } @@ -782,14 +787,48 @@ bundle_data_.OnTargetResolved(this); } +bool Target::HasRealInputs() const { + // This check is only necessary if this target will result in a phony target. + // Phony targets with no real inputs are treated as always dirty. + + // TODO(bug 194): This method is currently just checking the relevant inputs + // for the current list of output types that result in phony targets. As the + // list of phony targets expands, this method should be updated to properly + // account for which inputs matter for the given output type. + + // If any of this target's dependencies is non-phony target or a phony target + // with real inputs, then this target should be considered to have inputs. + for (const auto& pair : GetDeps(DEPS_ALL)) { + if (pair.ptr->dependency_output_file_or_phony()) { + return true; + } + } + + // If any of this target's sources will result in output files, then this + // target should be considered to have real inputs. + std::vector<OutputFile> tool_outputs; + return std::any_of( + sources().begin(), sources().end(), [&, this](const auto& source) { + const char* tool_name = Tool::kToolNone; + return GetOutputFilesForSource(source, &tool_name, &tool_outputs); + }); +} + bool Target::FillOutputFiles(Err* err) { const Tool* tool = toolchain_->GetToolForTargetFinalOutput(this); bool check_tool_outputs = false; switch (output_type_) { + case SOURCE_SET: { + if (HasRealInputs()) { + dependency_output_phony_ = + GetBuildDirForTargetAsOutputFile(this, BuildDirType::PHONY); + dependency_output_phony_->value().append(GetComputedOutputName()); + } + break; + } case GROUP: case BUNDLE_DATA: case CREATE_BUNDLE: - case SOURCE_SET: case COPY_FILES: case ACTION: case ACTION_FOREACH: @@ -799,8 +838,8 @@ // "<target_out_dir>/<targetname>.stamp". dependency_output_file_ = GetBuildDirForTargetAsOutputFile(this, BuildDirType::OBJ); - dependency_output_file_.value().append(GetComputedOutputName()); - dependency_output_file_.value().append(".stamp"); + dependency_output_file_->value().append(GetComputedOutputName()); + dependency_output_file_->value().append(".stamp"); break; } case EXECUTABLE: @@ -815,7 +854,7 @@ if (tool->runtime_outputs().list().empty()) { // Default to the first output for the runtime output. - runtime_outputs_.push_back(dependency_output_file_); + runtime_outputs_.push_back(*dependency_output_file_); } else { SubstitutionWriter::ApplyListToLinkerAsOutputFile( this, tool, tool->runtime_outputs(), &runtime_outputs_); @@ -827,9 +866,10 @@ // first output. CHECK(tool->outputs().list().size() >= 1); check_tool_outputs = true; - link_output_file_ = dependency_output_file_ = + dependency_output_file_ = SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( this, tool, tool->outputs().list()[0]); + link_output_file_ = *dependency_output_file_; break; case RUST_PROC_MACRO: case SHARED_LIBRARY: @@ -838,9 +878,10 @@ if (const CTool* ctool = tool->AsC()) { if (ctool->link_output().empty() && ctool->depend_output().empty()) { // Default behavior, use the first output file for both. - link_output_file_ = dependency_output_file_ = + dependency_output_file_ = SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( this, tool, tool->outputs().list()[0]); + link_output_file_ = *dependency_output_file_; } else { // Use the tool-specified ones. if (!ctool->link_output().empty()) { @@ -863,9 +904,10 @@ } } else if (const RustTool* rstool = tool->AsRust()) { // Default behavior, use the first output file for both. - link_output_file_ = dependency_output_file_ = + dependency_output_file_ = SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( this, tool, tool->outputs().list()[0]); + link_output_file_ = *dependency_output_file_; } break; case UNKNOWN:
diff --git a/src/gn/target.h b/src/gn/target.h index dbcfa69..dcbaf2c 100644 --- a/src/gn/target.h +++ b/src/gn/target.h
@@ -320,7 +320,7 @@ // action or a copy step, and the output library or executable file(s) from // binary targets. // - // It will NOT include stamp files and object files. + // It will NOT include stamp files, phony targets or object files. const std::vector<OutputFile>& computed_outputs() const { return computed_outputs_; } @@ -335,13 +335,30 @@ // could be the same or different than the link output file, depending on the // system. For actions this will be the stamp file. // + // The dependency output phony is only set when the target does not have an + // output file and is using a phony alias to represent it. The exception to + // this is for phony targets without any real inputs. Ninja treats empty phony + // targets as always dirty, so no other targets should depend on that target. + // In that scenario, both dependency_output_phony or dependency_output_file + // will be std::nullopt. + // + // Callers that do not care whether the dependency is represented by a file or + // a phony should use dependency_output_file_or_phony(). + // // These are only known once the target is resolved and will be empty before // that. This is a cache of the files to prevent every target that depends on // a given library from recomputing the same pattern. const OutputFile& link_output_file() const { return link_output_file_; } - const OutputFile& dependency_output_file() const { + const std::optional<OutputFile>& dependency_output_file() const { return dependency_output_file_; } + const std::optional<OutputFile>& dependency_output_phony() const { + return dependency_output_phony_; + } + const std::optional<OutputFile>& dependency_output_file_or_phony() const { + return dependency_output_file_ ? dependency_output_file_ + : dependency_output_phony_; + } // The subset of computed_outputs that are considered runtime outputs. const std::vector<OutputFile>& runtime_outputs() const { @@ -366,6 +383,11 @@ // The |loc_for_error| is used to blame a location for any errors produced. It // can be empty if there is no range (like this is being called based on the // command-line. + // + // It is possible for |outputs| to be returned empty without an error being + // reported. This can occur when the output type will result in a phony alias + // target (like a source_set) that is omitted from build files when they have + // no real inputs. bool GetOutputsAsSourceFiles(const LocationRange& loc_for_error, bool build_complete, std::vector<SourceFile>* outputs, @@ -402,6 +424,11 @@ void PullRecursiveHardDeps(); void PullRecursiveBundleData(); + // Checks to see whether this target or any of its dependencies have real + // inputs. If not, this target should be omitted as a dependency. This check + // only applies to targets that will result in a phony rule. + bool HasRealInputs() const; + // Fills the link and dependency output files when a target is resolved. bool FillOutputFiles(Err* err); @@ -489,7 +516,8 @@ // Output files. Empty until the target is resolved. std::vector<OutputFile> computed_outputs_; OutputFile link_output_file_; - OutputFile dependency_output_file_; + std::optional<OutputFile> dependency_output_file_; + std::optional<OutputFile> dependency_output_phony_; std::vector<OutputFile> runtime_outputs_; Metadata metadata_;
diff --git a/src/gn/target_unittest.cc b/src/gn/target_unittest.cc index 792c530..2496696 100644 --- a/src/gn/target_unittest.cc +++ b/src/gn/target_unittest.cc
@@ -726,7 +726,8 @@ ASSERT_TRUE(target.OnResolved(&err)); EXPECT_EQ("./liba.so", target.link_output_file().value()); - EXPECT_EQ("./liba.so.TOC", target.dependency_output_file().value()); + ASSERT_TRUE(target.dependency_output_file()); + EXPECT_EQ("./liba.so.TOC", target.dependency_output_file()->value()); ASSERT_EQ(1u, target.runtime_outputs().size()); EXPECT_EQ("./liba.so", target.runtime_outputs()[0].value()); @@ -772,7 +773,8 @@ ASSERT_TRUE(target.OnResolved(&err)); EXPECT_EQ("./a.dll.lib", target.link_output_file().value()); - EXPECT_EQ("./a.dll.lib", target.dependency_output_file().value()); + ASSERT_TRUE(target.dependency_output_file()); + EXPECT_EQ("./a.dll.lib", target.dependency_output_file()->value()); ASSERT_EQ(2u, target.runtime_outputs().size()); EXPECT_EQ("./a.dll", target.runtime_outputs()[0].value()); @@ -802,6 +804,7 @@ Target target(setup.settings(), Label(SourceDir("//a/"), "a")); target.set_output_type(Target::SOURCE_SET); + target.sources().push_back(SourceFile("//a/source_file1.cc")); target.SetToolchain(&toolchain); Err err; ASSERT_TRUE(target.OnResolved(&err)); @@ -818,12 +821,12 @@ EXPECT_EQ("input.cc.o", output[0].value()) << output[0].value(); // Test GetOutputsAsSourceFiles(). Since this is a source set it should give a - // stamp file. + // phony target. std::vector<SourceFile> computed_outputs; EXPECT_TRUE(target.GetOutputsAsSourceFiles(LocationRange(), true, &computed_outputs, &err)); ASSERT_EQ(1u, computed_outputs.size()); - EXPECT_EQ("//out/Debug/obj/a/a.stamp", computed_outputs[0].value()); + EXPECT_EQ("//out/Debug/phony/a/a", computed_outputs[0].value()); } // Tests Target::GetOutputFilesForSource for action_foreach targets (these, like
diff --git a/src/gn/tool.cc b/src/gn/tool.cc index 5e4186d..9708e44 100644 --- a/src/gn/tool.cc +++ b/src/gn/tool.cc
@@ -4,6 +4,7 @@ #include "gn/tool.h" +#include "gn/builtin_tool.h" #include "gn/c_tool.h" #include "gn/general_tool.h" #include "gn/rust_tool.h" @@ -52,6 +53,13 @@ return nullptr; } +BuiltinTool* Tool::AsBuiltin() { + return nullptr; +} +const BuiltinTool* Tool::AsBuiltin() const { + return nullptr; +} + bool Tool::IsPatternInOutputList(const SubstitutionList& output_list, const SubstitutionPattern& pattern) const { for (const auto& cur : output_list.list()) { @@ -392,7 +400,7 @@ case Target::STATIC_LIBRARY: return CTool::kCToolAlink; case Target::SOURCE_SET: - return GeneralTool::kGeneralToolStamp; + return BuiltinTool::kBuiltinToolPhony; case Target::ACTION: case Target::ACTION_FOREACH: case Target::BUNDLE_DATA:
diff --git a/src/gn/tool.h b/src/gn/tool.h index e64935f..5e824c9 100644 --- a/src/gn/tool.h +++ b/src/gn/tool.h
@@ -24,6 +24,7 @@ class CTool; class GeneralTool; class RustTool; +class BuiltinTool; // To add a new Tool category, create a subclass implementing SetComplete() // Add a new category to ToolCategories @@ -63,6 +64,8 @@ virtual const GeneralTool* AsGeneral() const; virtual RustTool* AsRust(); virtual const RustTool* AsRust() const; + virtual BuiltinTool* AsBuiltin(); + virtual const BuiltinTool* AsBuiltin() const; // Basic information ---------------------------------------------------------
diff --git a/src/gn/toolchain.cc b/src/gn/toolchain.cc index bfad81d..a00aec4 100644 --- a/src/gn/toolchain.cc +++ b/src/gn/toolchain.cc
@@ -9,13 +9,15 @@ #include <utility> #include "base/logging.h" +#include "gn/builtin_tool.h" #include "gn/target.h" #include "gn/value.h" Toolchain::Toolchain(const Settings* settings, const Label& label, const SourceFileSet& build_dependency_files) - : Item(settings, label, build_dependency_files) {} + : Item(settings, label, build_dependency_files), + phony_tool_(BuiltinTool::kBuiltinToolPhony) {} Toolchain::~Toolchain() = default; @@ -29,6 +31,9 @@ Tool* Toolchain::GetTool(const char* name) { DCHECK(name != Tool::kToolNone); + if (name == BuiltinTool::kBuiltinToolPhony) { + return &phony_tool_; + } auto pair = tools_.find(name); if (pair != tools_.end()) { return pair->second.get(); @@ -38,6 +43,9 @@ const Tool* Toolchain::GetTool(const char* name) const { DCHECK(name != Tool::kToolNone); + if (name == BuiltinTool::kBuiltinToolPhony) { + return &phony_tool_; + } auto pair = tools_.find(name); if (pair != tools_.end()) { return pair->second.get(); @@ -87,6 +95,20 @@ return nullptr; } +BuiltinTool* Toolchain::GetToolAsBuiltin(const char* name) { + if (Tool* tool = GetTool(name)) { + return tool->AsBuiltin(); + } + return nullptr; +} + +const BuiltinTool* Toolchain::GetToolAsBuiltin(const char* name) const { + if (const Tool* tool = GetTool(name)) { + return tool->AsBuiltin(); + } + return nullptr; +} + void Toolchain::SetTool(std::unique_ptr<Tool> t) { DCHECK(t->name() != Tool::kToolNone); DCHECK(tools_.find(t->name()) == tools_.end()); @@ -120,6 +142,11 @@ return GetToolAsRust(Tool::GetToolTypeForSourceType(type)); } +const BuiltinTool* Toolchain::GetToolForSourceTypeAsBuiltin( + SourceFile::Type type) const { + return GetToolAsBuiltin(Tool::GetToolTypeForSourceType(type)); +} + const Tool* Toolchain::GetToolForTargetFinalOutput(const Target* target) const { return GetTool(Tool::GetToolTypeForTargetFinalOutput(target)); } @@ -138,3 +165,8 @@ const Target* target) const { return GetToolAsRust(Tool::GetToolTypeForTargetFinalOutput(target)); } + +const BuiltinTool* Toolchain::GetToolForTargetFinalOutputAsBuiltin( + const Target* target) const { + return GetToolAsBuiltin(Tool::GetToolTypeForTargetFinalOutput(target)); +}
diff --git a/src/gn/toolchain.h b/src/gn/toolchain.h index eb5a60c..3f2b83a 100644 --- a/src/gn/toolchain.h +++ b/src/gn/toolchain.h
@@ -9,6 +9,7 @@ #include <string_view> #include "base/logging.h" +#include "gn/builtin_tool.h" #include "gn/item.h" #include "gn/label_ptr.h" #include "gn/scope.h" @@ -62,6 +63,8 @@ const CTool* GetToolAsC(const char* name) const; RustTool* GetToolAsRust(const char* name); const RustTool* GetToolAsRust(const char* name) const; + BuiltinTool* GetToolAsBuiltin(const char* name); + const BuiltinTool* GetToolAsBuiltin(const char* name) const; // Set a tool. When all tools are configured, you should call // ToolchainSetupComplete(). @@ -93,6 +96,7 @@ const CTool* GetToolForSourceTypeAsC(SourceFile::Type type) const; const GeneralTool* GetToolForSourceTypeAsGeneral(SourceFile::Type type) const; const RustTool* GetToolForSourceTypeAsRust(SourceFile::Type type) const; + const BuiltinTool* GetToolForSourceTypeAsBuiltin(SourceFile::Type type) const; // Returns the tool that produces the final output for the given target type. // This isn't necessarily the tool you would expect. For copy target, this @@ -103,6 +107,8 @@ const GeneralTool* GetToolForTargetFinalOutputAsGeneral( const Target* target) const; const RustTool* GetToolForTargetFinalOutputAsRust(const Target* target) const; + const BuiltinTool* GetToolForTargetFinalOutputAsBuiltin( + const Target* target) const; const SubstitutionBits& substitution_bits() const { DCHECK(setup_complete_); @@ -114,6 +120,7 @@ } private: + BuiltinTool phony_tool_; std::map<const char*, std::unique_ptr<Tool>> tools_; bool setup_complete_ = false;
diff --git a/src/gn/visual_studio_writer.cc b/src/gn/visual_studio_writer.cc index fd16536..3717345 100644 --- a/src/gn/visual_studio_writer.cc +++ b/src/gn/visual_studio_writer.cc
@@ -521,14 +521,14 @@ project.SubElement("PropertyGroup", XmlAttributes("Label", "UserMacros")); - std::string ninja_target = GetNinjaTarget(target); + auto [ninja_target, ninja_target_is_phony] = GetNinjaTarget(target); { std::unique_ptr<XmlElementWriter> properties = project.SubElement("PropertyGroup"); properties->SubElement("OutDir")->Text("$(SolutionDir)"); properties->SubElement("TargetName")->Text("$(ProjectName)"); - if (target->output_type() != Target::GROUP) { + if (target->output_type() != Target::GROUP && !ninja_target_is_phony) { properties->SubElement("TargetPath")->Text("$(OutDir)\\" + ninja_target); } } @@ -904,13 +904,20 @@ } } -std::string VisualStudioWriter::GetNinjaTarget(const Target* target) { +std::pair<std::string, bool> VisualStudioWriter::GetNinjaTarget(const Target* target) { std::ostringstream ninja_target_out; - DCHECK(!target->dependency_output_file().value().empty()); - ninja_path_output_.WriteFile(ninja_target_out, - target->dependency_output_file()); + bool is_phony = false; + OutputFile output_file; + if (target->dependency_output_file()) { + output_file = *target->dependency_output_file(); + } else if (target->dependency_output_phony()) { + output_file = *target->dependency_output_phony(); + is_phony = true; + } + + ninja_path_output_.WriteFile(ninja_target_out, output_file); std::string s = ninja_target_out.str(); if (s.compare(0, 2, "./") == 0) s = s.substr(2); - return s; + return std::make_pair(s, is_phony); }
diff --git a/src/gn/visual_studio_writer.h b/src/gn/visual_studio_writer.h index e4957a1..55ae705 100644 --- a/src/gn/visual_studio_writer.h +++ b/src/gn/visual_studio_writer.h
@@ -127,7 +127,8 @@ // and updates |root_folder_dir_|. Also sets |parent_folder| for |projects_|. void ResolveSolutionFolders(); - std::string GetNinjaTarget(const Target* target); + // Returns the ninja target string and whether the target is phony. + std::pair<std::string, bool> GetNinjaTarget(const Target* target); const BuildSettings* build_settings_;