Add no_check_generated_targets .gn variable A "gn check --check-generated" failure in a generated source can only be suppressed with no_check_targets or check_includes = false, both of which also skip the target's handwritten files. no_check_generated_targets is a list of label patterns in .gn, in the same format as no_check_targets. With --check-generated, gn check skips the generated files (files in the output directory) of matching targets and still checks their other files. Like check_targets and no_check_targets, the list is not used when a label pattern is given on the command line. gn check results are unchanged if the list is unset. Bug: 559888973 Change-Id: I500cb396ca271daa69a407ea333e56438a28d6f4 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/27220 Reviewed-by: Takuto Ikuta <tikuta@google.com> Reviewed-by: Matt Stark <msta@google.com> Commit-Queue: Ryosuke Miyake <rysk@google.com>
diff --git a/docs/reference.md b/docs/reference.md index 2796a0b..3201930 100644 --- a/docs/reference.md +++ b/docs/reference.md
@@ -388,7 +388,9 @@ --check-generated Generated files are normally not checked since they do not exist until after a build. With this flag, those generated files that - can be found on disk are also checked. + can be found on disk are also checked. The .gn file can exclude the + generated files of some targets with no_check_generated_targets (see + "gn help dotfile"). --check-system Check system style includes (using <angle brackets>) in addition to @@ -415,9 +417,10 @@ ``` The .gn file may specify a list of targets to be checked in the list check_targets (see "gn help dotfile"). Alternatively, the .gn file may - specify a list of targets not to be checked in no_check_targets. If a label - pattern is specified on the command line, neither check_targets or - no_check_targets is used. + specify a list of targets not to be checked in no_check_targets. It may also + list targets whose generated files are not checked with --check-generated in + no_check_generated_targets. If a label pattern is specified on the command + line, none of these lists is used. Targets can opt-out from checking with "check_includes = false" (see "gn help check_includes"). @@ -7708,6 +7711,18 @@ The format of this list is identical to that of "visibility" so see "gn help visibility" for examples. + no_check_generated_targets [optional] + A list of labels and label patterns whose generated files should *not* + be checked when running "gn check --check-generated". Unlike + no_check_targets, the other files of these targets are still checked. + Generated files are the target's files in the output directory, such as + action outputs listed in its sources. This list can be combined with + check_targets or no_check_targets. To bypass it, request an explicit + check of targets, like "//*". + + The format of this list is identical to that of "visibility" so see "gn + help visibility" for examples. + check_system_includes [optional] Boolean to control whether system style includes are checked by default when running "gn check" or "gn gen --check". System style includes are
diff --git a/src/gn/command_check.cc b/src/gn/command_check.cc index c02c106..8c032d2 100644 --- a/src/gn/command_check.cc +++ b/src/gn/command_check.cc
@@ -75,7 +75,9 @@ --check-generated Generated files are normally not checked since they do not exist until after a build. With this flag, those generated files that - can be found on disk are also checked. + can be found on disk are also checked. The .gn file can exclude the + generated files of some targets with no_check_generated_targets (see + "gn help dotfile"). --check-system Check system style includes (using <angle brackets>) in addition to @@ -94,9 +96,10 @@ The .gn file may specify a list of targets to be checked in the list check_targets (see "gn help dotfile"). Alternatively, the .gn file may - specify a list of targets not to be checked in no_check_targets. If a label - pattern is specified on the command line, neither check_targets or - no_check_targets is used. + specify a list of targets not to be checked in no_check_targets. It may also + list targets whose generated files are not checked with --check-generated in + no_check_generated_targets. If a label pattern is specified on the command + line, none of these lists is used. Targets can opt-out from checking with "check_includes = false" (see "gn help check_includes"). @@ -248,9 +251,19 @@ setup->check_system_includes() || cmdline->HasSwitch("check-system"); bool fix = cmdline->HasSwitch("fix"); + // Like check_targets and no_check_targets, no_check_generated_targets is not + // used when a label pattern is given. It only matters when generated files + // are checked. + std::vector<const Target*> no_check_generated; + if (args.size() == 1 && check_generated) { + FilterTargetsByPatterns(targets_to_check, + setup->no_check_generated_patterns(), + &no_check_generated); + } + if (!CheckPublicHeaders(&setup->build_settings(), all_targets, - targets_to_check, force, check_generated, - check_system, fix, setup)) + targets_to_check, no_check_generated, force, + check_generated, check_system, fix, setup)) return 1; if (!base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kQuiet)) { @@ -262,6 +275,12 @@ static_cast<int>(targets_to_check.size()), static_cast<int>(all_targets.size()))); } + if (!no_check_generated.empty()) { + OutputString(base::StringPrintf( + "Generated files of %d targets not checked based on the " + "no_check_generated_targets defined in \".gn\".\n", + static_cast<int>(no_check_generated.size()))); + } OutputString("Header dependency check OK\n", DECORATION_GREEN); } return 0; @@ -270,6 +289,7 @@ bool CheckPublicHeaders(const BuildSettings* build_settings, const std::vector<const Target*>& all_targets, const std::vector<const Target*>& to_check, + const std::vector<const Target*>& no_check_generated, bool force_check, bool check_generated, bool check_system, @@ -292,7 +312,7 @@ build_settings, all_targets, check_generated, check_system)); std::vector<HeaderChecker::Violation> violations; - header_checker->Run(to_check, force_check, &violations); + header_checker->Run(to_check, no_check_generated, force_check, &violations); bool remaining_violations = false; bool needs_separator = false;
diff --git a/src/gn/command_suggest_unittest.cc b/src/gn/command_suggest_unittest.cc index 4ddc70c..23ba34f 100644 --- a/src/gn/command_suggest_unittest.cc +++ b/src/gn/command_suggest_unittest.cc
@@ -732,7 +732,7 @@ }; EXPECT_TRUE(commands::CheckPublicHeaders( - &project.setup.build_settings(), project.targets(), project.targets(), + &project.setup.build_settings(), project.targets(), project.targets(), {}, false, false, false, true, &project.setup, collect)); EXPECT_EQ( "ERROR at //includer/includer.cc:1:11: Include not allowed.\n" @@ -781,7 +781,7 @@ }; EXPECT_TRUE(commands::CheckPublicHeaders( - &project.setup.build_settings(), project.targets(), project.targets(), + &project.setup.build_settings(), project.targets(), project.targets(), {}, false, false, false, true, &project.setup, collect)); EXPECT_EQ( "ERROR at //includer/includer.h:1:11: Public headers cannot include "
diff --git a/src/gn/commands.h b/src/gn/commands.h index 08917b9..efa9775 100644 --- a/src/gn/commands.h +++ b/src/gn/commands.h
@@ -424,13 +424,15 @@ // // Generated files are normally not checked since they do not exist // unless a build has been run, but passing true for |check_generated| -// will attempt to check them anyway, assuming they exist. +// will attempt to check them anyway, assuming they exist. The generated +// files of the targets in |no_check_generated| are not checked even then. // // On success, returns true. If the check fails, the error(s) will be printed // to stdout and false will be returned. bool CheckPublicHeaders(const BuildSettings* build_settings, const std::vector<const Target*>& all_targets, const std::vector<const Target*>& to_check, + const std::vector<const Target*>& no_check_generated, bool force_check, bool check_generated, bool check_system,
diff --git a/src/gn/header_checker.cc b/src/gn/header_checker.cc index 992b564..344c85a 100644 --- a/src/gn/header_checker.cc +++ b/src/gn/header_checker.cc
@@ -221,11 +221,15 @@ HeaderChecker::~HeaderChecker() = default; bool HeaderChecker::Run(const std::vector<const Target*>& to_check, + const std::vector<const Target*>& no_check_generated, bool force_check, std::vector<Violation>* violations) { std::unordered_set<const Target*> to_check_set(to_check.begin(), to_check.end()); - std::vector<FileInformation> files = FilesToCheck(to_check_set); + std::unordered_set<const Target*> no_check_generated_set( + no_check_generated.begin(), no_check_generated.end()); + std::vector<FileInformation> files = + FilesToCheck(to_check_set, no_check_generated_set); WorkerPool pool; { @@ -260,7 +264,8 @@ } std::vector<HeaderChecker::FileInformation> HeaderChecker::FilesToCheck( - const std::unordered_set<const Target*>& to_check) const { + const std::unordered_set<const Target*>& to_check, + const std::unordered_set<const Target*>& no_check_generated) const { std::vector<FileInformation> files; files.reserve(file_map_.size()); @@ -282,10 +287,14 @@ continue; } + // All generated files, including action outputs, are in the output dir. + const bool in_output_dir = IsFileInOuputDir(info.file); + TargetVector targets_to_check; for (const auto& vect_i : info.targets) { if (vect_i.target->IsBinary() && to_check.contains(vect_i.target) && - vect_i.target->check_includes()) { + vect_i.target->check_includes() && + !(in_output_dir && no_check_generated.contains(vect_i.target))) { targets_to_check.push_back(vect_i); } }
diff --git a/src/gn/header_checker.h b/src/gn/header_checker.h index e014d4a..51b12f4 100644 --- a/src/gn/header_checker.h +++ b/src/gn/header_checker.h
@@ -218,9 +218,13 @@ // error, fills the given vector with the violations and returns false. // Returns true on success. // + // The generated files of the targets in no_check_generated are not checked + // even if check_generated is set. Their other files are still checked. + // // force_check, if true, will override targets opting out of header checking // with "check_includes = false" and will check them anyway. bool Run(const std::vector<const Target*>& to_check, + const std::vector<const Target*>& no_check_generated, bool force_check, std::vector<Violation>* violations); @@ -242,6 +246,7 @@ CheckIncludesStrictPrivateInPublicHeader); FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, CheckIncludesStrictSameTargetPrivateHeader); + FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, NoCheckGenerated); ~HeaderChecker(); @@ -280,9 +285,11 @@ using PathExistsCallback = std::function<bool(const base::FilePath& path)>; // Collects the files of the given targets that need checking, each with the - // targets it is checked against. + // targets it is checked against. Generated files are not checked against + // the targets in no_check_generated. std::vector<FileInformation> FilesToCheck( - const std::unordered_set<const Target*>& to_check) const; + const std::unordered_set<const Target*>& to_check, + const std::unordered_set<const Target*>& no_check_generated) const; // Backend for Run() that checks the given files. The errors_ list will be // populated on failure.
diff --git a/src/gn/header_checker_unittest.cc b/src/gn/header_checker_unittest.cc index 21e8f8b..e575ec3 100644 --- a/src/gn/header_checker_unittest.cc +++ b/src/gn/header_checker_unittest.cc
@@ -5,9 +5,12 @@ #include "gn/header_checker.h" #include <ostream> +#include <string> +#include <unordered_set> #include <vector> #include "gn/scheduler.h" +#include "gn/substitution_list.h" #include "gn/target.h" #include "gn/test_with_scheduler.h" #include "gn/test_with_scope.h" @@ -54,8 +57,7 @@ } protected: - scoped_refptr<HeaderChecker> CreateChecker() { - bool check_generated = false; + scoped_refptr<HeaderChecker> CreateChecker(bool check_generated = false) { bool check_system = true; return base::MakeRefCounted<HeaderChecker>( setup_.build_settings(), targets_, check_generated, check_system); @@ -361,7 +363,7 @@ auto checker = CreateChecker(); std::vector<HeaderChecker::Violation> violations; - EXPECT_TRUE(checker->Run(targets_, false, &violations)); + EXPECT_TRUE(checker->Run(targets_, {}, false, &violations)); auto has_cache = [&checker](const Target* target) { for (const auto& shard : checker->dependency_cache_) { @@ -601,3 +603,51 @@ checker->CheckInclude(a_cache, false, input_file, a_private, range, &errors); EXPECT_TRUE(errors.empty()); } + +TEST_F(HeaderCheckerTest, NoCheckGenerated) { + // An action G that generates a source, which A lists in its sources next to + // a handwritten one. This mirrors generated bindings (e.g. mojom) compiled + // by a source_set. + Err err; + Target g(setup_.settings(), Label(SourceDir("//g/"), "g")); + g.set_output_type(Target::ACTION); + g.SetToolchain(setup_.toolchain(), &err); + ASSERT_SUCCESS(err); + g.action_values().outputs() = + SubstitutionList::MakeForTest("//out/Debug/gen/a/generated.cc"); + g.visibility().SetPublic(); + ASSERT_TRUE(g.OnResolvedWithoutChecks(&err)); + targets_.push_back(&g); + + const SourceFile handwritten("//a/handwritten.cc"); + const SourceFile generated("//out/Debug/gen/a/generated.cc"); + a_.sources().push_back(handwritten); + a_.sources().push_back(generated); + a_.private_deps().push_back(LabelTargetPair(&g)); + + using Files = std::unordered_set<SourceFile>; + std::unordered_set<const Target*> to_check(targets_.begin(), targets_.end()); + auto files_to_check = + [&](bool check_generated, + const std::unordered_set<const Target*>& no_check_generated) { + Files result; + for (const auto& info : + CreateChecker(check_generated) + ->FilesToCheck(to_check, no_check_generated)) + result.insert(info.file); + return result; + }; + + // By default, --check-generated checks both files. + EXPECT_EQ(files_to_check(true, {}), (Files{handwritten, generated})); + + // Without --check-generated, only the handwritten file is checked. + EXPECT_EQ(files_to_check(false, {}), Files{handwritten}); + + // Listing A drops only its generated file from --check-generated. The + // handwritten file is still checked. + EXPECT_EQ(files_to_check(true, {&a_}), Files{handwritten}); + + // Listing another target does not affect A. + EXPECT_EQ(files_to_check(true, {&b_}), (Files{handwritten, generated})); +}
diff --git a/src/gn/setup.cc b/src/gn/setup.cc index c4c2b8b..c91fbf1 100644 --- a/src/gn/setup.cc +++ b/src/gn/setup.cc
@@ -117,6 +117,18 @@ The format of this list is identical to that of "visibility" so see "gn help visibility" for examples. + no_check_generated_targets [optional] + A list of labels and label patterns whose generated files should *not* + be checked when running "gn check --check-generated". Unlike + no_check_targets, the other files of these targets are still checked. + Generated files are the target's files in the output directory, such as + action outputs listed in its sources. This list can be combined with + check_targets or no_check_targets. To bypass it, request an explicit + check of targets, like "//*". + + The format of this list is identical to that of "visibility" so see "gn + help visibility" for examples. + check_system_includes [optional] Boolean to control whether system style includes are checked by default when running "gn check" or "gn gen --check". System style includes are @@ -641,8 +653,8 @@ bool fix = cmdline.HasSwitch("fix"); if (!commands::CheckPublicHeaders(&build_settings_, all_targets, to_check, - false, false, check_system_includes_, fix, - this)) { + {}, false, false, check_system_includes_, + fix, this)) { return false; } } @@ -1207,6 +1219,18 @@ } } + // Targets whose generated files are not checked. + const Value* no_check_generated_targets_value = + dotfile_scope_.GetValue("no_check_generated_targets", true); + if (no_check_generated_targets_value) { + ExtractListOfLabelPatterns(&build_settings_, + *no_check_generated_targets_value, current_dir, + &no_check_generated_patterns_, err); + if (err->has_error()) { + return false; + } + } + const Value* check_system_includes_value = dotfile_scope_.GetValue("check_system_includes", true); if (check_system_includes_value) {
diff --git a/src/gn/setup.h b/src/gn/setup.h index c6a190b..06f7890 100644 --- a/src/gn/setup.h +++ b/src/gn/setup.h
@@ -129,6 +129,13 @@ return no_check_patterns_.get(); } + // Read from the .gn file, these are the targets whose generated files are + // *not* checked by "gn check --check-generated". Empty if the .gn file does + // not specify anything. + const std::vector<LabelPattern>& no_check_generated_patterns() const { + return no_check_generated_patterns_; + } + // This is a combination of the export_compile_commands list in the dotfile, // and any additions specified on the command-line. const std::vector<LabelPattern>& export_compile_commands() const { @@ -197,6 +204,7 @@ // See getter for info. std::unique_ptr<std::vector<LabelPattern>> check_patterns_; std::unique_ptr<std::vector<LabelPattern>> no_check_patterns_; + std::vector<LabelPattern> no_check_generated_patterns_; Scheduler scheduler_;
diff --git a/src/gn/setup_unittest.cc b/src/gn/setup_unittest.cc index 0b323c3..648eb13 100644 --- a/src/gn/setup_unittest.cc +++ b/src/gn/setup_unittest.cc
@@ -206,6 +206,45 @@ EXPECT_EQ("//src/gn:*", export_cc[2].Describe()); } +TEST_F(SetupTest, NoCheckGeneratedTargets) { + base::CommandLine cmdline(base::CommandLine::NO_PROGRAM); + + // no_check_generated_targets can be combined with no_check_targets. + const char kDotfileContents[] = R"( +buildconfig = "//BUILDCONFIG.gn" +no_check_targets = [ "//third_party/*" ] +no_check_generated_targets = [ + "//base:base", + "//foo/*", +] +)"; + + // Create a temp directory containing the build. + base::ScopedTempDir in_temp_dir; + ASSERT_TRUE(in_temp_dir.CreateUniqueTempDir()); + base::FilePath in_path = base::MakeAbsoluteFilePath(in_temp_dir.GetPath()); + WriteFile(in_path.Append(FILE_PATH_LITERAL(".gn")), kDotfileContents); + WriteFile(in_path.Append(FILE_PATH_LITERAL("BUILDCONFIG.gn")), ""); + cmdline.AppendSwitch(switches::kRoot, FilePathToUTF8(in_path)); + + // Create another temp dir for writing the generated files to. + base::ScopedTempDir build_temp_dir; + ASSERT_TRUE(build_temp_dir.CreateUniqueTempDir()); + + Setup setup; + Err err; + EXPECT_TRUE(setup.DoSetupWithErr(FilePathToUTF8(build_temp_dir.GetPath()), + true, cmdline, &err)); + + ASSERT_TRUE(setup.no_check_patterns()); + EXPECT_EQ(1u, setup.no_check_patterns()->size()); + const std::vector<LabelPattern>& patterns = + setup.no_check_generated_patterns(); + ASSERT_EQ(2u, patterns.size()); + EXPECT_EQ("//base:base", patterns[0].Describe()); + EXPECT_EQ("//foo/*", patterns[1].Describe()); +} + TEST_F(SetupTest, RootPatternsInGnConfig) { base::CommandLine cmdline(base::CommandLine::NO_PROGRAM);