Add more documentation for GN header checking

tools/gn/variables.cc

 // Copyright (c) 2013 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 "tools/gn/variables.h"
 
 namespace variables {
 
 // Built-in variables ----------------------------------------------------------
 
@@ skipping 336 matching lines @@
 const char kAllowCircularIncludesFrom[] = "allow_circular_includes_from";
 const char kAllowCircularIncludesFrom_HelpShort[] =
     "allow_circular_includes_from: [label list] Permit includes from deps.";
 const char kAllowCircularIncludesFrom_Help[] =
     "allow_circular_includes_from: Permit includes from deps.\n"
     "\n"
     "  A list of target labels. Must be a subset of the target's \"deps\".\n"
     "  These targets will be permitted to include headers from the current\n"
     "  target despite the dependency going in the opposite direction.\n"
     "\n"
-    "Tedious exposition\n"
+    "  When you use this, both targets must be included in a final binary\n"
+    "  for it to link. To keep linker errors from happening, it is good\n"
+    "  practice to have all external dependencies depend only on one of\n"
+    "  the two targets, and to set the visibility on the other to enforce\n"
+    "  this. Thus the targets will always be linked together in any output.\n"
+    "\n"
+    "Details\n"
     "\n"
     "  Normally, for a file in target A to include a file from target B,\n"
     "  A must list B as a dependency. This invariant is enforced by the\n"
-    "  \"gn check\" command (and the --check flag to \"gn gen\").\n"
+    "  \"gn check\" command (and the --check flag to \"gn gen\" -- see\n"
+    "  \"gn help check\").\n"
     "\n"
     "  Sometimes, two targets might be the same unit for linking purposes\n"
     "  (two source sets or static libraries that would always be linked\n"
-    "  together in a final executable or shared library). In this case,\n"
-    "  you want A to be able to include B's headers, and B to include A's\n"
-    "  headers.\n"
+    "  together in a final executable or shared library) and they each\n"
+    "  include headers from the other: you want A to be able to include B's\n"
+    "  headers, and B to include A's headers. This is not an ideal situation\n"
+    "  but is sometimes unavoidable.\n"
     "\n"
     "  This list, if specified, lists which of the dependencies of the\n"
     "  current target can include header files from the current target.\n"
     "  That is, if A depends on B, B can only include headers from A if it is\n"
-    "  in A's allow_circular_includes_from list.\n"
+    "  in A's allow_circular_includes_from list. Normally includes must\n"
+    "  follow the direction of dependencies, this flag allows them to go\n"
+    "  in the opposite direction.\n"
+    "\n"
+    "Danger\n"
+    "\n"
+    "  In the above example, A's headers are likely to include headers from\n"
+    "  A's dependencies. Those dependencies may have public_configs that\n"
+    "  apply flags, defines, and include paths that make those headers work\n"
+    "  properly.\n"
+    "\n"
+    "  With allow_circular_includes_from, B can include A's headers, and\n"
+    "  transitively from A's dependencies, without having the dependencies\n"
+    "  that would bring in the public_configs those headers need. The result\n"
+    "  may be errors or inconsistent builds.\n"
+    "\n"
+    "  So when you use allow_circular_includes_from, make sure that any\n"
+    "  compiler settings, flags, and include directories are the same between\n"
+    "  both targets (consider putting such things in a shared config they can\n"
+    "  both reference). Make sure the dependencies are also the same (you\n"
+    "  might consider a group to collect such dependencies they both\n"
+    "  depend on).\n"
     "\n"
     "Example\n"
     "\n"
     "  source_set(\"a\") {\n"
-    "    deps = [ \":b\", \":c\" ]\n"
+    "    deps = [ \":b\", \":a_b_shared_deps\" ]\n"
     "    allow_circular_includes_from = [ \":b\" ]\n"
     "    ...\n"
+    "  }\n"
+    "\n"
+    "  source_set(\"b\") {\n"
+    "    deps = [ \":a_b_shared_deps\" ]\n"
+    "    # Sources here can include headers from a despite lack of deps.\n"
+    "    ...\n"
+    "  }\n"
+    "\n"
+    "  group(\"a_b_shared_deps\") {\n"
+    "    public_deps = [ \":c\" ]\n"
     "  }\n";
 
 const char kArgs[] = "args";
 const char kArgs_HelpShort[] =
     "args: [string list] Arguments passed to an action.";
 const char kArgs_Help[] =
     "args: Arguments passed to an action.\n"
     "\n"
     "  For action and action_foreach targets, args is the list of arguments\n"
     "  to pass to the script. Typically you would use source expansion (see\n"
@@ skipping 83 matching lines @@
     "\n"
     "  This string is used by the \"create_bundle\" target to expand the\n"
     "  {{bundle_resources_dir}} of the \"bundle_data\" target it depends on.\n"
     "  This must correspond to a path under \"bundle_root_dir\".\n"
     "\n"
     "  See \"gn help bundle_root_dir\" for examples.\n";
 
 const char kBundleExecutableDir[] = "bundle_executable_dir";
 const char kBundleExecutableDir_HelpShort[] =
     "bundle_executable_dir: "
-        "Expansion of {{bundle_executable_dir}} in create_bundle.";

[brettw, 2016/03/18 20:15:05]

This was on purpose, this line was 81 cols and wrapped funny.

+        "Expansion of {{bundle_executable_dir}} in create_bundle";
 const char kBundleExecutableDir_Help[] =
     "bundle_executable_dir: "
         "Expansion of {{bundle_executable_dir}} in create_bundle.\n"
     "\n"
     "  A string corresponding to a path in $root_build_dir.\n"
     "\n"
     "  This string is used by the \"create_bundle\" target to expand the\n"
     "  {{bundle_executable_dir}} of the \"bundle_data\" target it depends on.\n"
     "  This must correspond to a path under \"bundle_root_dir\".\n"
     "\n"
@@ skipping 74 matching lines @@
     "check_includes: [boolean] Controls whether a target's files are checked.\n"
     "\n"
     "  When true (the default), the \"gn check\" command (as well as\n"
     "  \"gn gen\" with the --check flag) will check this target's sources\n"
     "  and headers for proper dependencies.\n"
     "\n"
     "  When false, the files in this target will be skipped by default.\n"
     "  This does not affect other targets that depend on the current target,\n"
     "  it just skips checking the includes of the current target's files.\n"
     "\n"
-    "Controlling includes individually\n"
+    "  If there are a few conditionally included headers that trip up\n"
+    "  checking, you can exclude headers individually by annotating them with\n"
+    "  \"nogncheck\" (see \"gn help nogncheck\").\n"
     "\n"
-    "  If only certain includes are problematic, you can annotate them\n"
-    "  individually rather than disabling header checking on an entire\n"
-    "  target. Add the string \"nogncheck\" to the include line:\n"
-    "\n"
-    "    #include \"foo/something_weird.h\"  // nogncheck (bug 12345)\n"
-    "\n"
-    "  It is good form to include a reference to a bug (if the include is\n"
-    "  improper, or some other comment expressing why the header checker\n"
-    "  doesn't work for this particular case.\n"
-    "\n"
-    "  The most common reason to need \"nogncheck\" is conditional includes.\n"
-    "  The header checker does not understand the preprocessor, so may flag\n"
-    "  some includes as improper even if the dependencies and #defines are\n"
-    "  always matched correctly:\n"
-    "\n"
-    "    #if defined(ENABLE_DOOM_MELON)\n"
-    "    #include \"doom_melon/beam_controller.h\"  // nogncheck\n"
-    "    #endif\n"
+    "  The topic \"gn help check\" has general information on how checking\n"
+    "  works and advice on how to pass a check in problematic cases.\n"
     "\n"
     "Example\n"
     "\n"
     "  source_set(\"busted_includes\") {\n"
     "    # This target's includes are messed up, exclude it from checking.\n"
     "    check_includes = false\n"
     "    ...\n"
     "  }\n";
 
 const char kCompleteStaticLib[] = "complete_static_lib";
 const char kCompleteStaticLib_HelpShort[] =
     "complete_static_lib: [boolean] Links all deps into a static library.";
 const char kCompleteStaticLib_Help[] =
     "complete_static_lib: [boolean] Links all deps into a static library.\n"
     "\n"
     "  A static library normally doesn't include code from dependencies, but\n"
     "  instead forwards the static libraries and source sets in its deps up\n"
     "  the dependency chain until a linkable target (an executable or shared\n"
     "  library) is reached. The final linkable target only links each static\n"
     "  library once, even if it appears more than once in its dependency\n"
     "  graph.\n"
     "\n"
     "  In some cases the static library might be the final desired output.\n"
     "  For example, you may be producing a static library for distribution to\n"
     "  third parties. In this case, the static library should include code\n"
     "  for all dependencies in one complete package. Since GN does not unpack\n"
     "  static libraries to forward their contents up the dependency chain,\n"
     "  it is an error for complete static libraries to depend on other static\n"
+    "\n"
+    "  In rare cases it makes sense to list a header in more than one\n"
+    "  target if it could be considered conceptually a member of both.\n"
     "  libraries.\n"
     "\n"
     "Example\n"
     "\n"
     "  static_library(\"foo\") {\n"
     "    complete_static_lib = true\n"
     "    deps = [ \"bar\" ]\n"
     "  }\n";
 
 const char kConfigs[] = "configs";
@@ skipping 932 matching lines @@
     INSERT_VARIABLE(Sources)
     INSERT_VARIABLE(Testonly)
     INSERT_VARIABLE(Visibility)
   }
   return info_map;
 }
 
 #undef INSERT_VARIABLE
 
 }  // namespace variables