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>

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

src/gn/command_check.cc

@@ -75,7 +75,9 @@ Command-specific switches
   --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 @@ What gets checked
 
   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 @@ int RunCheck(const std::vector<std::string>& args) {
       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 @@ int RunCheck(const std::vector<std::string>& args) {
           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 @@ int RunCheck(const std::vector<std::string>& args) {
 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 @@ bool CheckPublicHeaders(const BuildSettings* build_settings,
       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;

src/gn/command_suggest_unittest.cc

@@ -732,7 +732,7 @@ group("all") {
   };
 
   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 @@ group("all") {
   };
 
   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 "

src/gn/commands.h

@@ -424,13 +424,15 @@ bool ResolveFromCommandLineInput(
 //
 // 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,

src/gn/header_checker.cc

@@ -221,11 +221,15 @@ HeaderChecker::HeaderChecker(const BuildSettings* build_settings,
 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 @@ bool HeaderChecker::Run(const std::vector<const Target*>& to_check,
 }
 
 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 @@ std::vector<HeaderChecker::FileInformation> HeaderChecker::FilesToCheck(
         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);
       }
     }

src/gn/header_checker.h

@@ -218,9 +218,13 @@ class HeaderChecker : public base::RefCountedThreadSafe<HeaderChecker> {
   // 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 @@ class HeaderChecker : public base::RefCountedThreadSafe<HeaderChecker> {
                            CheckIncludesStrictPrivateInPublicHeader);
   FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest,
                            CheckIncludesStrictSameTargetPrivateHeader);
+  FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, NoCheckGenerated);
 
   ~HeaderChecker();
 
@@ -280,9 +285,11 @@ class HeaderChecker : public base::RefCountedThreadSafe<HeaderChecker> {
   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.

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 @@ class HeaderCheckerTest : public TestWithScheduler {
   }
 
  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 @@ TEST_F(HeaderCheckerTest, RunPrecomputesReachabilityOnlyForCheckedFiles) {
 
   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 @@ TEST_F(HeaderCheckerTest, CheckIncludesStrictSameTargetPrivateHeader) {
   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}));
+}

src/gn/setup.cc

@@ -117,6 +117,18 @@ Variables
       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 Setup::RunPostMessageLoop(const base::CommandLine& cmdline) {
     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 @@ bool Setup::FillOtherConfig(const base::CommandLine& cmdline, Err* err) {
     }
   }
 
+  // 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) {

src/gn/setup.h

@@ -129,6 +129,13 @@ class Setup {
     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 @@ class Setup {
   // 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_;
 

src/gn/setup_unittest.cc

@@ -206,6 +206,45 @@ export_compile_commands = [ "//base/*" ]
   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);