Add 'gen_deps' variable This will allow targets to ensure that other targets in non-default toolchains will get generated even when they're not transitively included in the build graph. Change-Id: I5369f2440c02d3b3b8ef9bee598a37c95b8bfbeb Reviewed-on: https://gn-review.googlesource.com/c/gn/+/12280 Reviewed-by: Brett Wilson <brettw@chromium.org> Commit-Queue: Brett Wilson <brettw@chromium.org>
diff --git a/misc/vim/syntax/gn.vim b/misc/vim/syntax/gn.vim index 79faa70..851cec6 100644 --- a/misc/vim/syntax/gn.vim +++ b/misc/vim/syntax/gn.vim
@@ -51,7 +51,7 @@ syn keyword gnVariable lib_dirs libs output_extension output_name outputs syn keyword gnVariable public public_configs public_deps scripte sources syn keyword gnVariable testonly visibility contents output_conversion rebase -syn keyword gnVariable walk_keys +syn keyword gnVariable walk_keys gen_deps hi def link gnVariable Keyword " Strings
diff --git a/src/gn/builder.cc b/src/gn/builder.cc index d31eaa4..d1ba662 100644 --- a/src/gn/builder.cc +++ b/src/gn/builder.cc
@@ -238,6 +238,7 @@ !AddDeps(record, target->configs().vector(), err) || !AddDeps(record, target->all_dependent_configs(), err) || !AddDeps(record, target->public_configs(), err) || + !AddGenDeps(record, target->gen_deps(), err) || !AddActionValuesDep(record, target->action_values(), err) || !AddToolchainDep(record, target, err)) return false; @@ -404,6 +405,19 @@ return true; } +bool Builder::AddGenDeps(BuilderRecord* record, + const LabelTargetVector& targets, + Err* err) { + for (const auto& target : targets) { + BuilderRecord* dep_record = GetOrCreateRecordOfType( + target.label, target.origin, BuilderRecord::ITEM_TARGET, err); + if (!dep_record) + return false; + record->AddGenDep(dep_record); + } + return true; +} + bool Builder::AddActionValuesDep(BuilderRecord* record, const ActionValues& action_values, Err* err) { @@ -435,6 +449,9 @@ void Builder::RecursiveSetShouldGenerate(BuilderRecord* record, bool force) { if (!record->should_generate()) { + // This function can encounter cycles because gen_deps aren't a DAG. Setting + // the should_generate flag before iterating avoids infinite recursion in + // that case. record->set_should_generate(true); // This may have caused the item to go into "resolved and generated" state.
diff --git a/src/gn/builder.h b/src/gn/builder.h index 8f328fa..bd62c18 100644 --- a/src/gn/builder.h +++ b/src/gn/builder.h
@@ -94,6 +94,9 @@ bool AddDeps(BuilderRecord* record, const LabelTargetVector& targets, Err* err); + bool AddGenDeps(BuilderRecord* record, + const LabelTargetVector& targets, + Err* err); bool AddActionValuesDep(BuilderRecord* record, const ActionValues& action_values, Err* err);
diff --git a/src/gn/builder_record.cc b/src/gn/builder_record.cc index cbe8dae..6325cdc 100644 --- a/src/gn/builder_record.cc +++ b/src/gn/builder_record.cc
@@ -58,6 +58,12 @@ return ITEM_UNKNOWN; } +void BuilderRecord::AddGenDep(BuilderRecord* record) { + // Records don't have to wait on resolution of their gen deps, since all they + // need to do is propagate should_generate to them. + all_deps_.insert(record); +} + void BuilderRecord::AddDep(BuilderRecord* record) { all_deps_.insert(record); if (!record->resolved()) {
diff --git a/src/gn/builder_record.h b/src/gn/builder_record.h index 989c239..439893b 100644 --- a/src/gn/builder_record.h +++ b/src/gn/builder_record.h
@@ -74,7 +74,8 @@ bool can_resolve() const { return item_ && unresolved_deps_.empty(); } - // All records this one is depending on. + // All records this one is depending on. Note that this includes gen_deps for + // targets, which can have cycles. BuilderRecordSet& all_deps() { return all_deps_; } const BuilderRecordSet& all_deps() const { return all_deps_; } @@ -89,6 +90,7 @@ return waiting_on_resolution_; } + void AddGenDep(BuilderRecord* record); void AddDep(BuilderRecord* record); private:
diff --git a/src/gn/builder_unittest.cc b/src/gn/builder_unittest.cc index b0f48f0..81b9f89 100644 --- a/src/gn/builder_unittest.cc +++ b/src/gn/builder_unittest.cc
@@ -2,10 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include <algorithm> + #include "gn/builder.h" #include "gn/config.h" #include "gn/loader.h" #include "gn/target.h" +#include "gn/test_with_scheduler.h" #include "gn/test_with_scope.h" #include "gn/toolchain.h" #include "util/test/test.h" @@ -55,6 +58,9 @@ files_.clear(); return match; } + bool HasLoadedOnce(const SourceFile& f) { + return count(files_.begin(), files_.end(), f) == 1; + } private: ~MockLoader() override = default; @@ -62,7 +68,7 @@ std::vector<SourceFile> files_; }; -class BuilderTest : public testing::Test { +class BuilderTest : public TestWithScheduler { public: BuilderTest() : loader_(new MockLoader), @@ -223,6 +229,92 @@ EXPECT_TRUE(b_record->should_generate()); } +// Test that "gen_deps" forces targets to be generated. +TEST_F(BuilderTest, GenDeps) { + DefineToolchain(); + + // Define another toolchain + Settings settings2(&build_settings_, "alternate/"); + Label alt_tc(SourceDir("//tc/"), "alternate"); + settings2.set_toolchain_label(alt_tc); + Toolchain* tc2 = new Toolchain(&settings2, alt_tc); + TestWithScope::SetupToolchain(tc2); + builder_.ItemDefined(std::unique_ptr<Item>(tc2)); + + // Construct the dependency chain A -> B -gen-> C -gen-> D where A is the only + // target in the default toolchain. This should cause all 4 targets to be + // generated. + Label a_label(SourceDir("//a/"), "a", settings_.toolchain_label().dir(), + settings_.toolchain_label().name()); + Label b_label(SourceDir("//b/"), "b", alt_tc.dir(), alt_tc.name()); + Label c_label(SourceDir("//c/"), "c", alt_tc.dir(), alt_tc.name()); + Label d_label(SourceDir("//d/"), "d", alt_tc.dir(), alt_tc.name()); + + Target* c = new Target(&settings2, c_label); + c->set_output_type(Target::EXECUTABLE); + c->gen_deps().push_back(LabelTargetPair(d_label)); + builder_.ItemDefined(std::unique_ptr<Item>(c)); + + Target* b = new Target(&settings2, b_label); + b->set_output_type(Target::EXECUTABLE); + b->gen_deps().push_back(LabelTargetPair(c_label)); + builder_.ItemDefined(std::unique_ptr<Item>(b)); + + Target* a = new Target(&settings_, a_label); + a->set_output_type(Target::EXECUTABLE); + a->private_deps().push_back(LabelTargetPair(b_label)); + builder_.ItemDefined(std::unique_ptr<Item>(a)); + + // At this point, "should generate" should have propogated to C which should + // request for D to be loaded + EXPECT_TRUE(loader_->HasLoadedOnce(SourceFile("//d/BUILD.gn"))); + + Target* d = new Target(&settings2, d_label); + d->set_output_type(Target::EXECUTABLE); + builder_.ItemDefined(std::unique_ptr<Item>(d)); + + BuilderRecord* a_record = builder_.GetRecord(a_label); + BuilderRecord* b_record = builder_.GetRecord(b_label); + BuilderRecord* c_record = builder_.GetRecord(c_label); + BuilderRecord* d_record = builder_.GetRecord(d_label); + EXPECT_TRUE(a_record->should_generate()); + EXPECT_TRUE(b_record->should_generate()); + EXPECT_TRUE(c_record->should_generate()); + EXPECT_TRUE(d_record->should_generate()); +} + +// Test that circular dependencies between gen_deps and deps are allowed +TEST_F(BuilderTest, GenDepsCircle) { + DefineToolchain(); + Settings settings2(&build_settings_, "alternate/"); + Label alt_tc(SourceDir("//tc/"), "alternate"); + settings2.set_toolchain_label(alt_tc); + Toolchain* tc2 = new Toolchain(&settings2, alt_tc); + TestWithScope::SetupToolchain(tc2); + builder_.ItemDefined(std::unique_ptr<Item>(tc2)); + + // A is in the default toolchain and lists B as a gen_dep + // B is in an alternate toolchain and lists A as a normal dep + Label a_label(SourceDir("//a/"), "a", settings_.toolchain_label().dir(), + settings_.toolchain_label().name()); + Label b_label(SourceDir("//b/"), "b", alt_tc.dir(), alt_tc.name()); + + Target* a = new Target(&settings_, a_label); + a->gen_deps().push_back(LabelTargetPair(b_label)); + a->set_output_type(Target::EXECUTABLE); + builder_.ItemDefined(std::unique_ptr<Item>(a)); + + Target* b = new Target(&settings2, b_label); + b->private_deps().push_back(LabelTargetPair(a_label)); + b->set_output_type(Target::EXECUTABLE); + builder_.ItemDefined(std::unique_ptr<Item>(b)); + + Err err; + EXPECT_TRUE(builder_.CheckForBadItems(&err)); + BuilderRecord* b_record = builder_.GetRecord(b_label); + EXPECT_TRUE(b_record->should_generate()); +} + // Tests that configs applied to a config get loaded (bug 536844). TEST_F(BuilderTest, ConfigLoad) { SourceDir toolchain_dir = settings_.toolchain_label().dir();
diff --git a/src/gn/command_desc.cc b/src/gn/command_desc.cc index 82a3822..fcb3434 100644 --- a/src/gn/command_desc.cc +++ b/src/gn/command_desc.cc
@@ -296,6 +296,7 @@ {variables::kPrecompiledHeader, DefaultHandler}, {variables::kPrecompiledSource, DefaultHandler}, {variables::kDeps, DepsHandler}, + {variables::kGenDeps, DefaultHandler}, {variables::kLibs, DefaultHandler}, {variables::kLibDirs, DefaultHandler}, {variables::kDataKeys, DefaultHandler},
diff --git a/src/gn/desc_builder.cc b/src/gn/desc_builder.cc index 3b6ab23..5a8fc4c 100644 --- a/src/gn/desc_builder.cc +++ b/src/gn/desc_builder.cc
@@ -52,6 +52,7 @@ // "precompiled_header" : "name of precompiled header file", // "precompiled_source" : "path to precompiled source", // "deps : [ list of target dependencies ], +// "gen_deps : [ list of generate dependencies ], // "libs" : [ list of libraries ], // "lib_dirs" : [ list of library directories ] // "metadata" : [ dictionary of target metadata values ] @@ -569,6 +570,9 @@ if (what(variables::kDeps)) res->SetWithoutPathExpansion(variables::kDeps, RenderDeps()); + if (what(variables::kGenDeps) && !target_->gen_deps().empty()) + res->SetWithoutPathExpansion(variables::kGenDeps, RenderGenDeps()); + // Runtime deps are special, print only when explicitly asked for and not in // overview mode. if (what_.find("runtime_deps") != what_.end()) @@ -711,6 +715,18 @@ return std::move(res); } + ValuePtr RenderGenDeps() { + auto res = std::make_unique<base::ListValue>(); + Label default_tc = target_->settings()->default_toolchain_label(); + std::vector<std::string> gen_deps; + for (const auto& pair : target_->gen_deps()) + gen_deps.push_back(pair.label.GetUserVisibleName(default_tc)); + std::sort(gen_deps.begin(), gen_deps.end()); + for (const auto& dep : gen_deps) + res->AppendString(dep); + return std::move(res); + } + ValuePtr RenderRuntimeDeps() { auto res = std::make_unique<base::ListValue>();
diff --git a/src/gn/target.cc b/src/gn/target.cc index 1dcd88d..da6b7e4 100644 --- a/src/gn/target.cc +++ b/src/gn/target.cc
@@ -260,6 +260,13 @@ required (directly or transitively) to build a target in the default toolchain. + Some targets might be associated but without a formal build dependency (for + example, related tools or optional variants). A target that is marked as + "generated" can propagate its generated state to an associated target using + "gen_deps". This will make the referenced dependency have Ninja rules + generated in the same cases the source target has but without a build-time + dependency and even in non-default toolchains. + See also "gn help ninja_rules". Dependencies
diff --git a/src/gn/target.h b/src/gn/target.h index 590d78e..59fe806 100644 --- a/src/gn/target.h +++ b/src/gn/target.h
@@ -262,6 +262,11 @@ const LabelTargetVector& data_deps() const { return data_deps_; } LabelTargetVector& data_deps() { return data_deps_; } + // gen_deps only propagate the "should_generate" flag. These dependencies can + // have cycles so care should be taken if iterating over them recursively. + const LabelTargetVector& gen_deps() const { return gen_deps_; } + LabelTargetVector& gen_deps() { return gen_deps_; } + // List of configs that this class inherits settings from. Once a target is // resolved, this will also list all-dependent and public configs. const UniqueVector<LabelConfigPair>& configs() const { return configs_; } @@ -477,6 +482,7 @@ LabelTargetVector private_deps_; LabelTargetVector public_deps_; LabelTargetVector data_deps_; + LabelTargetVector gen_deps_; // See getters for more info. UniqueVector<LabelConfigPair> configs_;
diff --git a/src/gn/target_generator.cc b/src/gn/target_generator.cc index 0ae9daf..4138889 100644 --- a/src/gn/target_generator.cc +++ b/src/gn/target_generator.cc
@@ -260,6 +260,8 @@ return false; if (!FillGenericDeps(variables::kDataDeps, &target_->data_deps())) return false; + if (!FillGenericDeps(variables::kGenDeps, &target_->gen_deps())) + return false; // "data_deps" was previously named "datadeps". For backwards-compat, read // the old one if no "data_deps" were specified.
diff --git a/src/gn/variables.cc b/src/gn/variables.cc index 285f3b9..433d280 100644 --- a/src/gn/variables.cc +++ b/src/gn/variables.cc
@@ -12,10 +12,9 @@ // Built-in variables ---------------------------------------------------------- const char kGnVersion[] = "gn_version"; -const char kGnVersion_HelpShort[] = - "gn_version: [number] The version of gn."; +const char kGnVersion_HelpShort[] = "gn_version: [number] The version of gn."; const char kGnVersion_Help[] = - R"(gn_version: [number] The version of gn. + R"(gn_version: [number] The version of gn. Corresponds to the number printed by `gn --version`. @@ -411,7 +410,7 @@ " those configs appear in the list.\n" \ " 5. all_dependent_configs pulled from dependencies, in the order of\n" \ " the \"deps\" list. This is done recursively. If a config appears\n" \ - " more than once, only the first occurrence will be used.\n" \ + " more than once, only the first occurrence will be used.\n" \ " 6. public_configs pulled from dependencies, in the order of the\n" \ " \"deps\" list. If a dependency is public, they will be applied\n" \ " recursively.\n" @@ -509,6 +508,23 @@ } )"; +const char kGenDeps[] = "gen_deps"; +const char kGenDeps_HelpShort[] = + "gen_deps: [label list] " + "Declares targets that should generate when this one does."; +const char kGenDeps_Help[] = + R"(gen_deps: Declares targets that should generate when this one does. + + A list of target labels. + + Not all GN targets that get evaluated are actually turned into ninja targets + (see "gn help execution"). If this target is generated, then any targets in + the "gen_deps" list will also be generated, regardless of the usual critera. + + Since "gen_deps" are not build time dependencies, there can be cycles between + "deps" and "gen_deps" or within "gen_deps" itself. +)"; + const char kArflags[] = "arflags"; const char kArflags_HelpShort[] = "arflags: [string list] Arguments passed to static_library archiver."; @@ -2265,6 +2281,7 @@ if (info_map.empty()) { INSERT_VARIABLE(AllDependentConfigs) INSERT_VARIABLE(AllowCircularIncludesFrom) + INSERT_VARIABLE(GenDeps) INSERT_VARIABLE(Arflags) INSERT_VARIABLE(Args) INSERT_VARIABLE(Asmflags)
diff --git a/src/gn/variables.h b/src/gn/variables.h index f5794d1..169a5af 100644 --- a/src/gn/variables.h +++ b/src/gn/variables.h
@@ -354,6 +354,10 @@ extern const char kXcodeExtraAttributes_HelpShort[]; extern const char kXcodeExtraAttributes_Help[]; +extern const char kGenDeps[]; +extern const char kGenDeps_HelpShort[]; +extern const char kGenDeps_Help[]; + // ----------------------------------------------------------------------------- struct VariableInfo {