Add an allowlist for allow_circular_includes_from. Bug: 556482973 Change-Id: I6dffad2dfe0844be442cd554dbe12e2c6a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/21942 Reviewed-by: Takuto Ikuta <tikuta@google.com> Commit-Queue: Matt Stark <msta@google.com>
diff --git a/docs/reference.md b/docs/reference.md index e77824a..d8d2a6a 100644 --- a/docs/reference.md +++ b/docs/reference.md
@@ -5304,6 +5304,14 @@ public_deps = [ ":c" ] } ``` + +#### **Allowlist** + +``` + The use of allow_circular_includes_from can be restricted to a specific list + of target labels by setting allow_circular_includes_from_allowlist in the .gn + file. See "gn help dotfile". +``` ### <a name="var_arflags"></a>**arflags**: Arguments passed to static_library archiver. [Back to Top](#gn-reference) ``` @@ -7627,6 +7635,28 @@ #### **Variables** ``` + allow_circular_includes_from_allowlist [optional] + A list of target label patterns that have permission to use the + allow_circular_includes_from variable. If this list is defined, usages of + allow_circular_includes_from will be checked against this list and GN + will fail if the target label isn't in the list. + + This is to allow the use of allow_circular_includes_from to be restricted + since circular dependencies between targets are discouraged and should + generally be avoided. + + The format of this list is identical to that of "visibility" so see "gn + help visibility" for examples. + + If unspecified, the ability to use allow_circular_includes_from is + unrestricted. + + Example: + allow_circular_includes_from_allowlist = [ + "//foo:*", + "//foo:bar", + ] + arg_file_template [optional] Path to a file containing the text that should be used as the default args.gn content when you run `gn args`.
diff --git a/src/gn/binary_target_generator.cc b/src/gn/binary_target_generator.cc index 3fa160d..7d6d416 100644 --- a/src/gn/binary_target_generator.cc +++ b/src/gn/binary_target_generator.cc
@@ -9,6 +9,7 @@ #include "gn/err.h" #include "gn/filesystem_utils.h" #include "gn/functions.h" +#include "gn/label_pattern.h" #include "gn/parse_tree.h" #include "gn/rust_values_generator.h" #include "gn/rust_variables.h" @@ -209,6 +210,30 @@ if (!value) return true; + const BuildSettings* build_settings = scope_->settings()->build_settings(); + const auto* allowlist = + build_settings->allow_circular_includes_from_allowlist(); + if (allowlist && !LabelPattern::VectorMatches(*allowlist, target_->label())) { + const ParseNode* blame_node = + value->origin() ? value->origin() : function_call_; + *err_ = Err( + blame_node, + "Usage of allow_circular_includes_from is not allowed here.", + "Circular dependencies between targets should be avoided:\n" + "* If circular dependencies were added accidentally, try removing\n" + " the circular deps and running `gn check <out_dir> --fix` to add\n" + " back only the strictly required header dependencies.\n" + "* If one direction is needed for compiling headers and the other for\n" + " linking symbols, extract the sources into a source_set (so other\n" + " targets can depend on headers without linking), and make the\n" + " linking library (e.g. shared_library) an empty wrapper that links\n" + " them together.\n" + "\n" + "If an exception is strictly required, add an entry to\n" + "allow_circular_includes_from_allowlist in your \"//.gn\" file.\n"); + return false; + } + UniqueVector<Label> circular; ExtractListOfUniqueLabels(scope_->settings()->build_settings(), *value, scope_->GetSourceDir(),
diff --git a/src/gn/binary_target_generator_unittest.cc b/src/gn/binary_target_generator_unittest.cc index 75660eb..2a1de74 100644 --- a/src/gn/binary_target_generator_unittest.cc +++ b/src/gn/binary_target_generator_unittest.cc
@@ -4,6 +4,7 @@ #include "gn/binary_target_generator.h" #include "gn/err.h" +#include "gn/label_pattern.h" #include "gn/scheduler.h" #include "gn/target.h" #include "gn/test_with_scheduler.h" @@ -88,7 +89,6 @@ EXPECT_TRUE(target->module_type().test(Target::MODULEMAP_IS_GENERATED)); EXPECT_TRUE(target->module_type().test(Target::MODULEMAP_IS_TEXTUAL)); } - TEST_F(BinaryTargetGeneratorTest, CheckIncludesStrict) { TestWithScope setup; Scope::ItemVector items_; @@ -134,3 +134,74 @@ items_.clear(); } } + +TEST_F(BinaryTargetGeneratorTest, AllowCircularIncludesAllowlist) { + TestWithScope setup; + Scope::ItemVector items_; + setup.scope()->set_item_collector(&items_); + setup.scope()->set_source_dir(SourceDir("//test/")); + + // Create a dep for foo to reference. + Target* dep = + new Target(setup.settings(), Label(SourceDir("//test/"), "dep")); + dep->set_output_type(Target::SOURCE_SET); + dep->visibility().SetPublic(); + Err err_setup; + dep->SetToolchain(setup.toolchain(), &err_setup); + dep->OnResolved(&err_setup); + items_.push_back(std::unique_ptr<Item>(dep)); + + TestParseInput input( + R"(static_library("foo") { + sources = [ "//foo.cc" ] + deps = [ ":dep" ] + allow_circular_includes_from = [ ":dep" ] + })"); + ASSERT_FALSE(input.has_error()); + + // No allowlist -> allowed + { + Err err; + input.parsed()->Execute(setup.scope(), &err); + EXPECT_FALSE(err.has_error()) << err.message(); + } + + // In allowlist -> Allowed + { + auto allowlist = std::make_unique<std::vector<LabelPattern>>(); + allowlist->push_back(LabelPattern(LabelPattern::MATCH, SourceDir("//test/"), + "foo", Label())); + setup.build_settings()->set_allow_circular_includes_from_allowlist( + std::move(allowlist)); + + Err err; + input.parsed()->Execute(setup.scope(), &err); + EXPECT_FALSE(err.has_error()) << err.message(); + } + + // Pattern in allowlist -> Allowed + { + auto allowlist = std::make_unique<std::vector<LabelPattern>>(); + allowlist->push_back(LabelPattern(LabelPattern::DIRECTORY, + SourceDir("//test/"), "", Label())); + setup.build_settings()->set_allow_circular_includes_from_allowlist( + std::move(allowlist)); + + Err err; + input.parsed()->Execute(setup.scope(), &err); + EXPECT_FALSE(err.has_error()) << err.message(); + } + + // Not in allowlist -> Denied + { + auto allowlist = std::make_unique<std::vector<LabelPattern>>(); + allowlist->push_back(LabelPattern(LabelPattern::MATCH, + SourceDir("//other/"), "other", Label())); + setup.build_settings()->set_allow_circular_includes_from_allowlist( + std::move(allowlist)); + + Err err; + input.parsed()->Execute(setup.scope(), &err); + EXPECT_TRUE(err.has_error()); + } +}
diff --git a/src/gn/build_settings.h b/src/gn/build_settings.h index 0d4decb..1c24852 100644 --- a/src/gn/build_settings.h +++ b/src/gn/build_settings.h
@@ -153,6 +153,18 @@ expand_directory_allowlist_ = std::move(list); } + // A list of target label patterns that can use allow_circular_includes_from. + // If the returned pointer is null, allow_circular_includes_from may be used + // anywhere. + const std::vector<LabelPattern>* allow_circular_includes_from_allowlist() + const { + return allow_circular_includes_from_allowlist_.get(); + } + void set_allow_circular_includes_from_allowlist( + std::unique_ptr<std::vector<LabelPattern>> list) { + allow_circular_includes_from_allowlist_ = std::move(list); + } + private: Label root_target_label_; std::vector<LabelPattern> root_patterns_; @@ -178,6 +190,8 @@ std::unique_ptr<SourceFileSet> exec_script_allowlist_; std::unique_ptr<SourceFileSet> expand_directory_allowlist_ = std::make_unique<SourceFileSet>(); + std::unique_ptr<std::vector<LabelPattern>> + allow_circular_includes_from_allowlist_; BuildSettings& operator=(const BuildSettings&) = delete; };
diff --git a/src/gn/setup.cc b/src/gn/setup.cc index eb946eb..abb28c5 100644 --- a/src/gn/setup.cc +++ b/src/gn/setup.cc
@@ -66,6 +66,28 @@ Variables + allow_circular_includes_from_allowlist [optional] + A list of target label patterns that have permission to use the + allow_circular_includes_from variable. If this list is defined, usages of + allow_circular_includes_from will be checked against this list and GN + will fail if the target label isn't in the list. + + This is to allow the use of allow_circular_includes_from to be restricted + since circular dependencies between targets are discouraged and should + generally be avoided. + + The format of this list is identical to that of "visibility" so see "gn + help visibility" for examples. + + If unspecified, the ability to use allow_circular_includes_from is + unrestricted. + + Example: + allow_circular_includes_from_allowlist = [ + "//foo:*", + "//foo:bar", + ] + arg_file_template [optional] Path to a file containing the text that should be used as the default args.gn content when you run `gn args`. @@ -1208,6 +1230,21 @@ std::make_unique<SourceFileSet>()); } + // Fill allow_circular_includes_from_allowlist. + const Value* allow_circular_includes_from_allowlist_value = + dotfile_scope_.GetValue("allow_circular_includes_from_allowlist", true); + if (allow_circular_includes_from_allowlist_value) { + auto allowlist = std::make_unique<std::vector<LabelPattern>>(); + ExtractListOfLabelPatterns(&build_settings_, + *allow_circular_includes_from_allowlist_value, + current_dir, allowlist.get(), err); + if (err->has_error()) { + return false; + } + build_settings_.set_allow_circular_includes_from_allowlist( + std::move(allowlist)); + } + // Fill optional default_args. const Value* default_args_value = dotfile_scope_.GetValue("default_args", true);
diff --git a/src/gn/variables.cc b/src/gn/variables.cc index f90a758..f3cf257 100644 --- a/src/gn/variables.cc +++ b/src/gn/variables.cc
@@ -563,6 +563,12 @@ group("a_b_shared_deps") { public_deps = [ ":c" ] } + +Allowlist + + The use of allow_circular_includes_from can be restricted to a specific list + of target labels by setting allow_circular_includes_from_allowlist in the .gn + file. See "gn help dotfile". )"; const char kGenDeps[] = "gen_deps";