From 126d8b5e3a7efa3e2584de4c1bc31c6460233ca6 Mon Sep 17 00:00:00 2001 From: "brettw@chromium.org" Date: Mon, 7 Apr 2014 22:17:35 +0000 Subject: [PATCH] Add optional public header checking to GN build You can invoke this either using the "--check" argument to the gen command (for use on buildbots) or by running "gn check" (for developer on-demand use). This adds support for "public" headers for a target, and an optional "--check" flag to the gen command that implements checkdeps-like include checking. Basically if you include a file that's declared in the build, it has to be in the public section of one of your dependents (or that dependent doesn't have a public section, which implies everything is public). This roughly maps to Blaze's behavior around the public headers. Moves modp_b64 build file to main tree. Remove modp_b64 hack for older versions of VC missing stdint (this included basictypes which caused a header check failure). I also added some base dependencies and some other minor build changes to solve some other issues identified by the check. The remaining one is that a file in base/metrics depends on ipc (!) BUG= R=scottmg@chromium.org Review URL: https://codereview.chromium.org/216903004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@262216 0039d316-1c4b-4281-b951-d872f2087c98 --- base/BUILD.gn | 8 +- base/test/BUILD.gn | 1 + device/usb/BUILD.gn | 5 +- .../modp_b64/BUILD.gn | 0 third_party/modp_b64/README.chromium | 9 +- third_party/modp_b64/modp_b64_data.h | 9 - tools/gn/BUILD.gn | 7 + tools/gn/binary_target_generator.cc | 4 + tools/gn/c_include_iterator.cc | 143 +++++++++++ tools/gn/c_include_iterator.h | 44 ++++ tools/gn/c_include_iterator_unittest.cc | 92 +++++++ tools/gn/command_check.cc | 40 +++ tools/gn/command_gen.cc | 5 + tools/gn/commands.cc | 1 + tools/gn/commands.h | 5 + tools/gn/filesystem_utils.cc | 34 +-- tools/gn/filesystem_utils.h | 3 +- tools/gn/gn.gyp | 7 + tools/gn/header_checker.cc | 242 ++++++++++++++++++ tools/gn/header_checker.h | 116 +++++++++ tools/gn/header_checker_unittest.cc | 102 ++++++++ tools/gn/ninja_binary_target_writer.cc | 6 +- tools/gn/secondary/testing/gmock/BUILD.gn | 4 +- tools/gn/setup.cc | 23 +- tools/gn/setup.h | 7 + tools/gn/target.cc | 1 + tools/gn/target.h | 12 + tools/gn/target_generator.cc | 15 ++ tools/gn/target_generator.h | 1 + tools/gn/trace.cc | 28 ++ tools/gn/trace.h | 4 +- tools/gn/variables.cc | 40 +++ tools/gn/variables.h | 4 + 33 files changed, 965 insertions(+), 57 deletions(-) rename {tools/gn/secondary/third_party => third_party}/modp_b64/BUILD.gn (100%) create mode 100644 tools/gn/c_include_iterator.cc create mode 100644 tools/gn/c_include_iterator.h create mode 100644 tools/gn/c_include_iterator_unittest.cc create mode 100644 tools/gn/command_check.cc create mode 100644 tools/gn/header_checker.cc create mode 100644 tools/gn/header_checker.h create mode 100644 tools/gn/header_checker_unittest.cc diff --git a/base/BUILD.gn b/base/BUILD.gn index 7aa8659b50c078..468d76c39026ea 100644 --- a/base/BUILD.gn +++ b/base/BUILD.gn @@ -6,7 +6,6 @@ import("//build/config/ui.gni") component("base") { sources = [ - "../build/build_config.h", "third_party/dmg_fp/dmg_fp.h", "third_party/dmg_fp/g_fmt.cc", "third_party/dmg_fp/dtoa_wrapper.cc", @@ -368,6 +367,10 @@ component("base") { "native_library_mac.mm", "native_library_posix.cc", "native_library_win.cc", + "numerics/safe_conversions.h", + "numerics/safe_conversions_impl.h", + "numerics/safe_math.h", + "numerics/safe_math_impl.h", "nix/mime_util_xdg.cc", "nix/mime_util_xdg.h", "nix/xdg_util.cc", @@ -391,7 +394,6 @@ component("base") { "port.h", "posix/eintr_wrapper.h", "posix/file_descriptor_shuffle.cc", - "posix/file_descriptor_shuffle.y", "posix/global_descriptors.cc", "posix/global_descriptors.h", "posix/unix_domain_socket_linux.cc", @@ -453,7 +455,6 @@ component("base") { "process/process_metrics_posix.cc", "process/process_metrics_win.cc", "process/process_posix.cc", - "process/process_util.h", "process/process_win.cc", "profiler/scoped_profile.cc", "profiler/scoped_profile.h", @@ -468,7 +469,6 @@ component("base") { "rand_util_win.cc", "run_loop.cc", "run_loop.h", - "safe_numerics.h", "safe_strerror_posix.cc", "safe_strerror_posix.h", "scoped_generic.h", diff --git a/base/test/BUILD.gn b/base/test/BUILD.gn index ed100eb26ee6b5..24e4e104d7da0c 100644 --- a/base/test/BUILD.gn +++ b/base/test/BUILD.gn @@ -128,6 +128,7 @@ static_library("test_support_perf") { "run_all_perftests.cc", ] deps = [ + ":test_support_base", "//base", "//testing/gtest", ] diff --git a/device/usb/BUILD.gn b/device/usb/BUILD.gn index d7fa6d05fca941..428a7172d6ab93 100644 --- a/device/usb/BUILD.gn +++ b/device/usb/BUILD.gn @@ -12,7 +12,10 @@ static_library("usb") { "usb_ids.h", generated_ids, ] - deps = [ ":usb_device_ids" ] + deps = [ + ":usb_device_ids", + "//base", + ] } action("usb_device_ids") { diff --git a/tools/gn/secondary/third_party/modp_b64/BUILD.gn b/third_party/modp_b64/BUILD.gn similarity index 100% rename from tools/gn/secondary/third_party/modp_b64/BUILD.gn rename to third_party/modp_b64/BUILD.gn diff --git a/third_party/modp_b64/README.chromium b/third_party/modp_b64/README.chromium index 26eed1e80c8bef..35b9e548408276 100644 --- a/third_party/modp_b64/README.chromium +++ b/third_party/modp_b64/README.chromium @@ -11,12 +11,5 @@ and to fix compilation errors that occur under VC8. The file was renamed modp_b64.cc to force it to be compiled as C++ so that the inclusion of basictypes.h could be possible. -The file modp_b64_data.h was generated by modp_b64_gen.c (under Linux), -which is not included in this directory. The resulting header was -modified to not include when COMPILER_MSVC is defined (since -that header file does not exist under VC8), but instead in that case to -include "base/basictypes.h" and provide the required typedefs for -uint8_t and uint32_t using uint8 and uint32. - The modp_b64.cc and modp_b64.h files were modified to make them safe on -64-bit systems. \ No newline at end of file +64-bit systems. diff --git a/third_party/modp_b64/modp_b64_data.h b/third_party/modp_b64/modp_b64_data.h index aca6f0fb0ef664..fb85890d9daa7d 100644 --- a/third_party/modp_b64/modp_b64_data.h +++ b/third_party/modp_b64/modp_b64_data.h @@ -1,14 +1,5 @@ #include "build/build_config.h" -#if !defined(COMPILER_MSVC) #include -#else -// VC8 doesn't have stdint.h. On the other hand, some compilers don't like -// the below code, because basictypes.h itself includes stdint.h and the -// typedefs below can cause conflicts. -#include "base/basictypes.h" -typedef uint8 uint8_t; -typedef uint32 uint32_t; -#endif #define CHAR62 '+' #define CHAR63 '/' diff --git a/tools/gn/BUILD.gn b/tools/gn/BUILD.gn index e176b92127e5f0..44305f7a5ca29a 100644 --- a/tools/gn/BUILD.gn +++ b/tools/gn/BUILD.gn @@ -20,7 +20,10 @@ static_library("gn_lib") { "builder.h", "builder_record.cc", "builder_record.h", + "c_include_iterator.cc", + "c_include_iterator.h", "command_args.cc", + "command_check.cc", "command_desc.cc", "command_gen.cc", "command_help.cc", @@ -59,6 +62,8 @@ static_library("gn_lib") { "function_write_file.cc", "group_target_generator.cc", "group_target_generator.h", + "header_checker.cc", + "header_checker.h", "import_manager.cc", "import_manager.h", "input_conversion.cc", @@ -167,11 +172,13 @@ executable("gn") { test("gn_unittests") { sources = [ "builder_unittest.cc", + "c_include_iterator_unittest.cc", "escape_unittest.cc", "filesystem_utils_unittest.cc", "file_template_unittest.cc", "function_rebase_path_unittest.cc", "functions_unittest.cc", + "header_checker_unittest.cc", "input_conversion_unittest.cc", "label_unittest.cc", "loader_unittest.cc", diff --git a/tools/gn/binary_target_generator.cc b/tools/gn/binary_target_generator.cc index 3e36297ab2ef92..0eba531568231e 100644 --- a/tools/gn/binary_target_generator.cc +++ b/tools/gn/binary_target_generator.cc @@ -37,6 +37,10 @@ void BinaryTargetGenerator::DoRun() { if (err_->has_error()) return; + FillPublic(); + if (err_->has_error()) + return; + FillSourcePrereqs(); if (err_->has_error()) return; diff --git a/tools/gn/c_include_iterator.cc b/tools/gn/c_include_iterator.cc new file mode 100644 index 00000000000000..3690dedd3d2f5e --- /dev/null +++ b/tools/gn/c_include_iterator.cc @@ -0,0 +1,143 @@ +// Copyright 2014 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/c_include_iterator.h" + +#include "base/logging.h" + +namespace { + +enum IncludeType { + INCLUDE_NONE, + INCLUDE_SYSTEM, // #include <...> + INCLUDE_USER // #include "..." +}; + +// Returns true if str starts with the prefix. +bool StartsWith(const base::StringPiece& str, const base::StringPiece& prefix) { + base::StringPiece extracted = str.substr(0, prefix.size()); + return extracted == prefix; +} + +// Returns a new string piece referencing the same buffer as the argument, but +// with leading space trimmed. This only checks for space and tab characters +// since we're dealing with lines in C source files. +base::StringPiece TrimLeadingWhitespace(const base::StringPiece& str) { + size_t new_begin = 0; + while (new_begin < str.size() && + (str[new_begin] == ' ' || str[new_begin] == '\t')) + new_begin++; + return str.substr(new_begin); +} + +// We don't want to count comment lines and preprocessor lines toward our +// "max lines to look at before giving up" since the beginnings of some files +// may have a lot of comments. +// +// We only handle C-style "//" comments since this is the normal commenting +// style used in Chrome, and do so pretty stupidly. We don't want to write a +// full C++ parser here, we're just trying to get a good heuristic for checking +// the file. +// +// We assume the line has leading whitespace trimmed. We also assume that empty +// lines have already been filtered out. +bool ShouldCountTowardNonIncludeLines(const base::StringPiece& line) { + if (StartsWith(line, "//")) + return false; // Don't count comments. + if (StartsWith(line, "#")) + return false; // Don't count preprocessor. + return true; // Count everything else. +} + +// Given a line, checks to see if it looks like an include or import and +// extract the path. The type of include is returned. Returns INCLUDE_NONE on +// error or if this is not an include line. +IncludeType ExtractInclude(const base::StringPiece& line, + base::StringPiece* path) { + static const char kInclude[] = "#include"; + static const size_t kIncludeLen = arraysize(kInclude) - 1; // No null. + static const char kImport[] = "#import"; + static const size_t kImportLen = arraysize(kImport) - 1; // No null. + + base::StringPiece contents; + if (StartsWith(line, base::StringPiece(kInclude, kIncludeLen))) + contents = TrimLeadingWhitespace(line.substr(kIncludeLen)); + else if (StartsWith(line, base::StringPiece(kImport, kImportLen))) + contents = TrimLeadingWhitespace(line.substr(kImportLen)); + + if (contents.empty()) + return INCLUDE_NONE; + + IncludeType type = INCLUDE_NONE; + char terminating_char = 0; + if (contents[0] == '"') { + type = INCLUDE_USER; + terminating_char = '"'; + } else if (contents[0] == '<') { + type = INCLUDE_SYSTEM; + terminating_char = '>'; + } else { + return INCLUDE_NONE; + } + + // Count everything to next "/> as the contents. + size_t terminator_index = contents.find(terminating_char, 1); + if (terminator_index == base::StringPiece::npos) + return INCLUDE_NONE; + + *path = contents.substr(1, terminator_index - 1); + return type; +} + +} // namespace + +const int CIncludeIterator::kMaxNonIncludeLines = 10; + +CIncludeIterator::CIncludeIterator(const base::StringPiece& file) + : file_(file), + offset_(0), + lines_since_last_include_(0) { +} + +CIncludeIterator::~CIncludeIterator() { +} + +bool CIncludeIterator::GetNextIncludeString(base::StringPiece* out) { + base::StringPiece line; + while (lines_since_last_include_ <= kMaxNonIncludeLines && + GetNextLine(&line)) { + base::StringPiece trimmed = TrimLeadingWhitespace(line); + if (trimmed.empty()) + continue; // Just ignore all empty lines. + + base::StringPiece include_contents; + IncludeType type = ExtractInclude(trimmed, &include_contents); + if (type == INCLUDE_USER) { + // Only count user includes for now. + *out = include_contents; + lines_since_last_include_ = 0; + return true; + } + + if (ShouldCountTowardNonIncludeLines(trimmed)) + lines_since_last_include_++; + } + return false; +} + +bool CIncludeIterator::GetNextLine(base::StringPiece* line) { + if (offset_ == file_.size()) + return false; + + size_t begin = offset_; + while (offset_ < file_.size() && file_[offset_] != '\n') + offset_++; + + *line = file_.substr(begin, offset_ - begin); + + // If we didn't hit EOF, skip past the newline for the next one. + if (offset_ < file_.size()) + offset_++; + return true; +} diff --git a/tools/gn/c_include_iterator.h b/tools/gn/c_include_iterator.h new file mode 100644 index 00000000000000..2a25ef6f95fa9e --- /dev/null +++ b/tools/gn/c_include_iterator.h @@ -0,0 +1,44 @@ +// Copyright 2014 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. + +#ifndef TOOLS_GN_C_INCLUDE_ITERATOR_H_ +#define TOOLS_GN_C_INCLUDE_ITERATOR_H_ + +#include "base/basictypes.h" +#include "base/strings/string_piece.h" + +// Iterates through #includes in C source and header files. +// +// This only returns includes we want to check, which is user includes with +// double-quotes: #include "..." +class CIncludeIterator { + public: + // The buffer pointed to must outlive this class. + CIncludeIterator(const base::StringPiece& file); + ~CIncludeIterator(); + + // Fills in the string with the contents of the next include and returns + // true, or returns false if there are no more includes. + bool GetNextIncludeString(base::StringPiece* out); + + // Maximum numbef of non-includes we'll tolerate before giving up. This does + // not count comments or preprocessor. + static const int kMaxNonIncludeLines; + + private: + // Returns false on EOF, otherwise fills in the given line. + bool GetNextLine(base::StringPiece* line); + + base::StringPiece file_; + + size_t offset_; + + // Number of lines we've processed since seeing the last include (or the + // beginning of the file) with some exceptions. + int lines_since_last_include_; + + DISALLOW_COPY_AND_ASSIGN(CIncludeIterator); +}; + +#endif // TOOLS_GN_INCLUDE_ITERATOR_H_ diff --git a/tools/gn/c_include_iterator_unittest.cc b/tools/gn/c_include_iterator_unittest.cc new file mode 100644 index 00000000000000..53d41edd65822f --- /dev/null +++ b/tools/gn/c_include_iterator_unittest.cc @@ -0,0 +1,92 @@ +// Copyright 2014 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 "testing/gtest/include/gtest/gtest.h" +#include "tools/gn/c_include_iterator.h" + +TEST(CIncludeIterator, Basic) { + std::string buffer; + buffer.append("// Some comment\n"); + buffer.append("\n"); + buffer.append("#include \"foo/bar.h\"\n"); + buffer.append("\n"); + buffer.append("#include \n"); + buffer.append("\n"); + buffer.append(" #include \"foo/baz.h\"\n"); // Leading whitespace + buffer.append("#include \"la/deda.h\"\n"); + buffer.append("#import \"weird_mac_import.h\"\n"); + buffer.append("\n"); + buffer.append("void SomeCode() {\n"); + + CIncludeIterator iter(buffer); + + base::StringPiece contents; + EXPECT_TRUE(iter.GetNextIncludeString(&contents)); + EXPECT_EQ("foo/bar.h", contents); + EXPECT_TRUE(iter.GetNextIncludeString(&contents)); + EXPECT_EQ("foo/baz.h", contents); + EXPECT_TRUE(iter.GetNextIncludeString(&contents)); + EXPECT_EQ("la/deda.h", contents); + EXPECT_TRUE(iter.GetNextIncludeString(&contents)); + EXPECT_EQ("weird_mac_import.h", contents); + EXPECT_FALSE(iter.GetNextIncludeString(&contents)); +} + +// Tests that we don't search for includes indefinitely. +TEST(CIncludeIterator, GiveUp) { + std::string buffer; + for (size_t i = 0; i < 1000; i++) + buffer.append("x\n"); + buffer.append("#include \"foo/bar.h\"\n"); + + base::StringPiece contents; + + CIncludeIterator iter(buffer); + EXPECT_FALSE(iter.GetNextIncludeString(&contents)); + EXPECT_TRUE(contents.empty()); +} + +// Don't count blank lines, comments, and preprocessor when giving up. +TEST(CIncludeIterator, DontGiveUp) { + std::string buffer; + for (size_t i = 0; i < 1000; i++) + buffer.push_back('\n'); + for (size_t i = 0; i < 1000; i++) + buffer.append("// comment\n"); + for (size_t i = 0; i < 1000; i++) + buffer.append("#preproc\n"); + buffer.append("#include \"foo/bar.h\"\n"); + + base::StringPiece contents; + + CIncludeIterator iter(buffer); + EXPECT_TRUE(iter.GetNextIncludeString(&contents)); + EXPECT_EQ("foo/bar.h", contents); +} + +// Tests that we'll tolerate some small numbers of non-includes interspersed +// with real includes. +TEST(CIncludeIterator, TolerateNonIncludes) { + const size_t kSkip = CIncludeIterator::kMaxNonIncludeLines - 2; + const size_t kGroupCount = 100; + + std::string include("foo/bar.h"); + + // Allow a series of includes with blanks in between. + std::string buffer; + for (size_t group = 0; group < kGroupCount; group++) { + for (size_t i = 0; i < kSkip; i++) + buffer.append("foo\n"); + buffer.append("#include \"" + include + "\"\n"); + } + + base::StringPiece contents; + + CIncludeIterator iter(buffer); + for (size_t group = 0; group < kGroupCount; group++) { + EXPECT_TRUE(iter.GetNextIncludeString(&contents)); + EXPECT_EQ(include, contents.as_string()); + } + EXPECT_FALSE(iter.GetNextIncludeString(&contents)); +} diff --git a/tools/gn/command_check.cc b/tools/gn/command_check.cc new file mode 100644 index 00000000000000..a84bdf3096e2f6 --- /dev/null +++ b/tools/gn/command_check.cc @@ -0,0 +1,40 @@ +// Copyright 2014 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/commands.h" +#include "tools/gn/setup.h" +#include "tools/gn/standard_out.h" + +namespace commands { + +const char kCheck[] = "check"; +const char kCheck_HelpShort[] = + "check: Check header dependencies."; +const char kCheck_Help[] = + "gn check: Check header dependencies.\n" + "\n" + " \"gn check\" is the same thing as \"gn gen\" with the \"--check\" flag\n" + " except that this command does not write out any build files. It's\n" + " intended to be an easy way to manually trigger include file checking.\n" + "\n" + " See \"gn help\" for the common command-line switches.\n"; + +// Note: partially duplicated in command_gyp.cc. +int RunCheck(const std::vector& args) { + // Deliberately leaked to avoid expensive process teardown. + Setup* setup = new Setup(); + // TODO(brettw) bug 343726: Use a temporary directory instead of this + // default one to avoid messing up any build that's in there. + if (!setup->DoSetup("//out/Default")) + return 1; + setup->set_check_public_headers(true); + + if (!setup->Run()) + return 1; + + OutputString("Header dependency check OK\n", DECORATION_GREEN); + return 0; +} + +} // namespace commands diff --git a/tools/gn/command_gen.cc b/tools/gn/command_gen.cc index 139e0dca0f8d0f..b8e5e0fbb81d73 100644 --- a/tools/gn/command_gen.cc +++ b/tools/gn/command_gen.cc @@ -22,6 +22,8 @@ namespace { // Suppress output on success. const char kSwitchQuiet[] = "q"; +const char kSwitchCheck[] = "check"; + void BackgroundDoWrite(const Target* target, const Toolchain* toolchain, const std::vector& deps_for_visibility) { @@ -103,6 +105,9 @@ int RunGen(const std::vector& args) { if (!setup->DoSetup(args[0])) return 1; + if (CommandLine::ForCurrentProcess()->HasSwitch(kSwitchCheck)) + setup->set_check_public_headers(true); + // Cause the load to also generate the ninja files for each target. We wrap // the writing to maintain a counter. base::subtle::Atomic32 write_counter = 0; diff --git a/tools/gn/commands.cc b/tools/gn/commands.cc index 963b4e895289f2..09ff012b896a12 100644 --- a/tools/gn/commands.cc +++ b/tools/gn/commands.cc @@ -35,6 +35,7 @@ const CommandInfoMap& GetCommands() { &Run##cmd); INSERT_COMMAND(Args) + INSERT_COMMAND(Check) INSERT_COMMAND(Desc) INSERT_COMMAND(Gen) INSERT_COMMAND(Help) diff --git a/tools/gn/commands.h b/tools/gn/commands.h index 1900820fec32d5..9b8db88a3a9b55 100644 --- a/tools/gn/commands.h +++ b/tools/gn/commands.h @@ -24,6 +24,11 @@ extern const char kArgs_HelpShort[]; extern const char kArgs_Help[]; int RunArgs(const std::vector& args); +extern const char kCheck[]; +extern const char kCheck_HelpShort[]; +extern const char kCheck_Help[]; +int RunCheck(const std::vector& args); + extern const char kDesc[]; extern const char kDesc_HelpShort[]; extern const char kDesc_Help[]; diff --git a/tools/gn/filesystem_utils.cc b/tools/gn/filesystem_utils.cc index 10837621c3e4bb..4e7e604a6fbb0a 100644 --- a/tools/gn/filesystem_utils.cc +++ b/tools/gn/filesystem_utils.cc @@ -165,8 +165,7 @@ bool FilesystemStringsEqual(const base::FilePath::StringType& a, } // namespace -SourceFileType GetSourceFileType(const SourceFile& file, - Settings::TargetOS os) { +SourceFileType GetSourceFileType(const SourceFile& file) { base::StringPiece extension = FindExtension(&file.value()); if (extension == "cc" || extension == "cpp" || extension == "cxx") return SOURCE_CC; @@ -174,29 +173,14 @@ SourceFileType GetSourceFileType(const SourceFile& file, return SOURCE_H; if (extension == "c") return SOURCE_C; - - switch (os) { - case Settings::MAC: - if (extension == "m") - return SOURCE_M; - if (extension == "mm") - return SOURCE_MM; - break; - - case Settings::WIN: - if (extension == "rc") - return SOURCE_RC; - // TODO(brettw) asm files. - break; - - default: - break; - } - - if (os != Settings::WIN) { - if (extension == "S") - return SOURCE_S; - } + if (extension == "m") + return SOURCE_M; + if (extension == "mm") + return SOURCE_MM; + if (extension == "rc") + return SOURCE_RC; + if (extension == "S") + return SOURCE_S; return SOURCE_UNKNOWN; } diff --git a/tools/gn/filesystem_utils.h b/tools/gn/filesystem_utils.h index d2658c2416b7e8..a6bb0fe4725400 100644 --- a/tools/gn/filesystem_utils.h +++ b/tools/gn/filesystem_utils.h @@ -28,8 +28,7 @@ enum SourceFileType { SOURCE_RC, }; -SourceFileType GetSourceFileType(const SourceFile& file, - Settings::TargetOS os); +SourceFileType GetSourceFileType(const SourceFile& file); // Returns the extension, not including the dot, for the given file type on the // given system. diff --git a/tools/gn/gn.gyp b/tools/gn/gn.gyp index e1a0dfac575e80..0ae37ca0697594 100644 --- a/tools/gn/gn.gyp +++ b/tools/gn/gn.gyp @@ -24,7 +24,10 @@ 'builder.h', 'builder_record.cc', 'builder_record.h', + 'c_include_iterator.cc', + 'c_include_iterator.h', 'command_args.cc', + 'command_check.cc', 'command_desc.cc', 'command_gen.cc', 'command_help.cc', @@ -63,6 +66,8 @@ 'function_write_file.cc', 'group_target_generator.cc', 'group_target_generator.h', + 'header_checker.cc', + 'header_checker.h', 'import_manager.cc', 'import_manager.h', 'input_conversion.cc', @@ -168,11 +173,13 @@ 'type': '<(gtest_target_type)', 'sources': [ 'builder_unittest.cc', + 'c_include_iterator_unittest.cc', 'escape_unittest.cc', 'filesystem_utils_unittest.cc', 'file_template_unittest.cc', 'function_rebase_path_unittest.cc', 'functions_unittest.cc', + 'header_checker_unittest.cc', 'input_conversion_unittest.cc', 'label_unittest.cc', 'loader_unittest.cc', diff --git a/tools/gn/header_checker.cc b/tools/gn/header_checker.cc new file mode 100644 index 00000000000000..54dfb50c8a7e3f --- /dev/null +++ b/tools/gn/header_checker.cc @@ -0,0 +1,242 @@ +// Copyright 2014 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/header_checker.h" + +#include + +#include "base/bind.h" +#include "base/file_util.h" +#include "base/message_loop/message_loop.h" +#include "base/threading/sequenced_worker_pool.h" +#include "tools/gn/build_settings.h" +#include "tools/gn/builder.h" +#include "tools/gn/c_include_iterator.h" +#include "tools/gn/err.h" +#include "tools/gn/filesystem_utils.h" +#include "tools/gn/scheduler.h" +#include "tools/gn/target.h" +#include "tools/gn/trace.h" + +HeaderChecker::HeaderChecker(const BuildSettings* build_settings, + const std::vector& targets) + : main_loop_(base::MessageLoop::current()), + build_settings_(build_settings) { + for (size_t i = 0; i < targets.size(); i++) + AddTargetToFileMap(targets[i]); +} + +HeaderChecker::~HeaderChecker() { +} + +bool HeaderChecker::Run(std::vector* errors) { + ScopedTrace trace(TraceItem::TRACE_CHECK_HEADERS, "Check headers"); + + if (file_map_.empty()) + return true; + + scoped_refptr pool( + new base::SequencedWorkerPool(16, "HeaderChecker")); + for (FileMap::const_iterator file_i = file_map_.begin(); + file_i != file_map_.end(); ++file_i) { + const TargetVector& vect = file_i->second; + + // Only check C-like source files (RC files also have includes). + SourceFileType type = GetSourceFileType(file_i->first); + if (type != SOURCE_CC && type != SOURCE_H && type != SOURCE_C && + type != SOURCE_M && type != SOURCE_MM && type != SOURCE_RC) + continue; + + for (size_t vect_i = 0; vect_i < vect.size(); ++vect_i) { + pool->PostWorkerTaskWithShutdownBehavior( + FROM_HERE, + base::Bind(&HeaderChecker::DoWork, this, + vect[vect_i].target, file_i->first), + base::SequencedWorkerPool::BLOCK_SHUTDOWN); + } + } + + // After this call we're single-threaded again. + pool->Shutdown(); + + if (errors_.empty()) + return true; + *errors = errors_; + return false; +} + +void HeaderChecker::DoWork(const Target* target, const SourceFile& file) { + Err err; + if (!CheckFile(target, file, &err)) { + base::AutoLock lock(lock_); + errors_.push_back(err); + } +} + +void HeaderChecker::AddTargetToFileMap(const Target* target) { + // Files in the sources have this public bit by default. + bool default_public = target->all_headers_public(); + + // First collect the normal files, they get the default visibility. + std::map files_to_public; + const Target::FileList& sources = target->sources(); + for (size_t i = 0; i < sources.size(); i++) + files_to_public[sources[i]] = default_public; + + // Add in the public files, forcing them to public. This may overwrite some + // entries, and it may add new ones. + const Target::FileList& public_list = target->public_headers(); + if (default_public) + DCHECK(public_list.empty()); // List only used when default is not public. + for (size_t i = 0; i < public_list.size(); i++) + files_to_public[public_list[i]] = true; + + // Add the merged list to the master list of all files. + for (std::map::const_iterator i = files_to_public.begin(); + i != files_to_public.end(); ++i) + file_map_[i->first].push_back(TargetInfo(target, i->second)); +} + +bool HeaderChecker::IsFileInOuputDir(const SourceFile& file) const { + const std::string& build_dir = build_settings_->build_dir().value(); + return file.value().compare(0, build_dir.size(), build_dir) == 0; +} + +// This current assumes all include paths are relative to the source root +// which is generally the case for Chromium. +// +// A future enhancement would be to search the include path for the target +// containing the source file containing this include and find the file to +// handle the cases where people do weird things with the paths. +SourceFile HeaderChecker::SourceFileForInclude( + const base::StringPiece& input) const { + std::string str("//"); + input.AppendToString(&str); + return SourceFile(str); +} + +bool HeaderChecker::CheckFile(const Target* from_target, + const SourceFile& file, + Err* err) const { + ScopedTrace trace(TraceItem::TRACE_CHECK_HEADER, file.value()); + + // Sometimes you have generated source files included as sources in another + // target. These won't exist at checking time. Since we require all generated + // files to be somewhere in the output tree, we can just check the name to + // see if they should be skipped. + if (IsFileInOuputDir(file)) + return true; + + base::FilePath path = build_settings_->GetFullPath(file); + std::string contents; + if (!base::ReadFileToString(path, &contents)) { + *err = Err(from_target->defined_from(), "Source file not found.", + "This target includes as a source:\n " + file.value() + + "\nwhich was not found."); + return false; + } + + CIncludeIterator iter(contents); + base::StringPiece current_include; + while (iter.GetNextIncludeString(¤t_include)) { + SourceFile include = SourceFileForInclude(current_include); + if (!CheckInclude(from_target, file, include, err)) + return false; + } + + return true; +} + +// If the file exists, it must be in a dependency of the given target, and it +// must be public in that dependency. +bool HeaderChecker::CheckInclude(const Target* from_target, + const SourceFile& source_file, + const SourceFile& include_file, + Err* err) const { + // Assume if the file isn't declared in our sources that we don't need to + // check it. It would be nice if we could give an error if this happens, but + // our include finder is too primitive and returns all includes, even if + // they're in a #if not executed in the current build. In that case, it's + // not unusual for the buildfiles to not specify that header at all. + FileMap::const_iterator found = file_map_.find(include_file); + if (found == file_map_.end()) + return true; + + const TargetVector& targets = found->second; + + // For all targets containing this file, we require that at least one be + // a dependency of the current target, and all targets that are dependencies + // must have the file listed in the public section. + bool found_dependency = false; + for (size_t i = 0; i < targets.size(); i++) { + // We always allow source files in a target to include headers also in that + // target. + if (targets[i].target == from_target) + return true; + + if (IsDependencyOf(targets[i].target, from_target)) { + // The include is in a target that's a proper dependency. Now verify + // that the include is a public file. + if (!targets[i].is_public) { + // Depending on a private header. + std::string msg = "The file " + source_file.value() + + "\nincludes " + include_file.value() + + "\nwhich is private to the target " + + targets[i].target->label().GetUserVisibleName(false); + + // TODO(brettw) blame the including file. + *err = Err(NULL, "Including a private header.", msg); + return false; + } + found_dependency = true; + } + } + + if (!found_dependency) { + std::string msg = + source_file.value() + " includes the header\n" + + include_file.value() + " which is not in any dependency of\n" + + from_target->label().GetUserVisibleName(false); + msg += "\n\nThe include file is in the target(s):\n"; + for (size_t i = 0; i < targets.size(); i++) + msg += " " + targets[i].target->label().GetUserVisibleName(false) + "\n"; + + msg += "\nMake sure one of these is a direct or indirect dependency\n" + "of " + from_target->label().GetUserVisibleName(false); + + // TODO(brettw) blame the including file. + // Probably this means making and leaking an input file for it, and also + // tracking the locations for each include. + *err = Err(NULL, "Include not allowed.", msg); + return false; + } + + return true; +} + +bool HeaderChecker::IsDependencyOf(const Target* search_for, + const Target* search_from) const { + std::set checked; + return IsDependencyOf(search_for, search_from, &checked); +} + +bool HeaderChecker::IsDependencyOf(const Target* search_for, + const Target* search_from, + std::set* checked) const { + if (checked->find(search_for) != checked->end()) + return false; // Already checked this subtree. + + const LabelTargetVector& deps = search_from->deps(); + for (size_t i = 0; i < deps.size(); i++) { + if (deps[i].ptr == search_for) + return true; // Found it. + + // Recursive search. + checked->insert(deps[i].ptr); + if (IsDependencyOf(search_for, deps[i].ptr, checked)) + return true; + } + + return false; +} diff --git a/tools/gn/header_checker.h b/tools/gn/header_checker.h new file mode 100644 index 00000000000000..f1c7aa321a6daf --- /dev/null +++ b/tools/gn/header_checker.h @@ -0,0 +1,116 @@ +// Copyright 2014 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. + +#ifndef TOOLS_GN_HEADER_CHECKER_H_ +#define TOOLS_GN_HEADER_CHECKER_H_ + +#include +#include +#include + +#include "base/basictypes.h" +#include "base/gtest_prod_util.h" +#include "base/memory/ref_counted.h" +#include "base/run_loop.h" +#include "base/strings/string_piece.h" +#include "base/synchronization/lock.h" +#include "tools/gn/err.h" + +class BuildSettings; +class Label; +class SourceFile; +class Target; + +namespace base { +class MessageLoop; +} + +class HeaderChecker : public base::RefCountedThreadSafe { + public: + HeaderChecker(const BuildSettings* build_settings, + const std::vector& targets); + + // This assumes that the current thread already has a message loop. On + // error, fills the given vector with the errors and returns false. Returns + // true on success. + bool Run(std::vector* errors); + + private: + friend class base::RefCountedThreadSafe; + FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, IsDependencyOf); + FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, CheckInclude); + ~HeaderChecker(); + + struct TargetInfo { + TargetInfo() : target(NULL), is_public(false) {} + TargetInfo(const Target* t, bool p) : target(t), is_public(p) {} + + const Target* target; + bool is_public; + }; + + typedef std::vector TargetVector; + + void DoWork(const Target* target, const SourceFile& file); + + // Adds the sources and public files from the given target to the file_map_. + // Not threadsafe! Called only during init. + void AddTargetToFileMap(const Target* target); + + // Returns true if the given file is in the output directory. + bool IsFileInOuputDir(const SourceFile& file) const; + + // Resolves the contents of an include to a SourceFile. + SourceFile SourceFileForInclude(const base::StringPiece& input) const; + + // from_target is the target the file was defined from. It will be used in + // error messages. + bool CheckFile(const Target* from_target, + const SourceFile& file, + Err* err) const; + + // Checks that the given file in the given target can include the given + // include file. If disallowed, returns false and sets the error. + bool CheckInclude(const Target* from_target, + const SourceFile& source_file, + const SourceFile& include_file, + Err* err) const; + + // Returns true if the given search_for target is a dependency of + // search_from. Many subtrees are duplicated so this function avoids + // duplicate checking across recursive calls by keeping track of checked + // targets in the given set. It should point to an empty set for the first + // call. A target is not considered to be a dependency of itself. + bool IsDependencyOf(const Target* search_for, + const Target* search_from) const; + bool IsDependencyOf(const Target* search_for, + const Target* search_from, + std::set* checked) const; + + // Non-locked variables ------------------------------------------------------ + // + // These are initialized during construction (which happens on one thread) + // and are not modified after, so any thread can read these without locking. + + base::MessageLoop* main_loop_; + base::RunLoop main_thread_runner_; + + const BuildSettings* build_settings_; + + // Maps source files to targets it appears in (usually just one target). + typedef std::map FileMap; + FileMap file_map_; + + // Locked variables ---------------------------------------------------------- + // + // These are mutable during runtime and require locking. + + base::Lock lock_; + + std::vector errors_; + + DISALLOW_COPY_AND_ASSIGN(HeaderChecker); +}; + +#endif // TOOLS_GN_HEADER_CHECKER_H_ diff --git a/tools/gn/header_checker_unittest.cc b/tools/gn/header_checker_unittest.cc new file mode 100644 index 00000000000000..90e4165529c8db --- /dev/null +++ b/tools/gn/header_checker_unittest.cc @@ -0,0 +1,102 @@ +// Copyright 2014 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 + +#include "testing/gtest/include/gtest/gtest.h" +#include "tools/gn/header_checker.h" +#include "tools/gn/scheduler.h" +#include "tools/gn/target.h" +#include "tools/gn/test_with_scope.h" + +namespace { + +class HeaderCheckerTest : public testing::Test { + public: + HeaderCheckerTest() + : a_(setup_.settings(), Label(SourceDir("//"), "a")), + b_(setup_.settings(), Label(SourceDir("//"), "a")), + c_(setup_.settings(), Label(SourceDir("//"), "c")) { + a_.deps().push_back(LabelTargetPair(&b_)); + b_.deps().push_back(LabelTargetPair(&c_)); + + targets_.push_back(&a_); + targets_.push_back(&b_); + targets_.push_back(&c_); + } + + protected: + Scheduler scheduler_; + + TestWithScope setup_; + + // Some headers that are automatically set up with a dependency chain. + // a -> b -> c + Target a_; + Target b_; + Target c_; + + std::vector targets_; +}; + +} // namespace + +TEST_F(HeaderCheckerTest, IsDependencyOf) { + scoped_refptr checker( + new HeaderChecker(setup_.build_settings(), targets_)); + + EXPECT_FALSE(checker->IsDependencyOf(&a_, &a_)); + EXPECT_TRUE(checker->IsDependencyOf(&b_, &a_)); + EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_)); + EXPECT_FALSE(checker->IsDependencyOf(&a_, &c_)); +} + +TEST_F(HeaderCheckerTest, CheckInclude) { + // Add a disconnected target d with a header to check that you have to have + // to depend on a target listing a header. + Target d(setup_.settings(), Label(SourceDir("//"), "d")); + SourceFile d_header("//d_header.h"); + d.sources().push_back(SourceFile(d_header)); + + // Add a header on B and say everything in B is public. + SourceFile b_public("//b_public.h"); + b_.sources().push_back(b_public); + c_.set_all_headers_public(true); + + // Add a public and private header on C. + SourceFile c_public("//c_public.h"); + SourceFile c_private("//c_private.h"); + c_.sources().push_back(c_private); + c_.public_headers().push_back(c_public); + c_.set_all_headers_public(false); + + targets_.push_back(&d); + scoped_refptr checker( + new HeaderChecker(setup_.build_settings(), targets_)); + + // A file in target A can't include a header from D because A has no + // dependency on D. + Err err; + SourceFile source_file("//some_file.cc"); + EXPECT_FALSE(checker->CheckInclude(&a_, source_file, d_header, &err)); + EXPECT_TRUE(err.has_error()); + + // A can include the public header in B. + err = Err(); + EXPECT_TRUE(checker->CheckInclude(&a_, source_file, b_public, &err)); + EXPECT_FALSE(err.has_error()); + + // Check A depending on the public and private headers in C. + err = Err(); + EXPECT_TRUE(checker->CheckInclude(&a_, source_file, c_public, &err)); + EXPECT_FALSE(err.has_error()); + EXPECT_FALSE(checker->CheckInclude(&a_, source_file, c_private, &err)); + EXPECT_TRUE(err.has_error()); + + // A can depend on a random file unknown to the build. + err = Err(); + EXPECT_TRUE(checker->CheckInclude(&a_, source_file, SourceFile("//random.h"), + &err)); + EXPECT_FALSE(err.has_error()); +} diff --git a/tools/gn/ninja_binary_target_writer.cc b/tools/gn/ninja_binary_target_writer.cc index ae29b4642daea1..392bc54b84fd64 100644 --- a/tools/gn/ninja_binary_target_writer.cc +++ b/tools/gn/ninja_binary_target_writer.cc @@ -151,8 +151,7 @@ void NinjaBinaryTargetWriter::WriteSources( for (size_t i = 0; i < sources.size(); i++) { const SourceFile& input_file = sources[i]; - SourceFileType input_file_type = GetSourceFileType(input_file, - settings_->target_os()); + SourceFileType input_file_type = GetSourceFileType(input_file); if (input_file_type == SOURCE_UNKNOWN) continue; // Skip unknown file types. std::string command = @@ -429,8 +428,7 @@ void NinjaBinaryTargetWriter::ClassifyDependency( } else { // Linking in a source set, copy its object files. for (size_t i = 0; i < dep->sources().size(); i++) { - SourceFileType input_file_type = GetSourceFileType( - dep->sources()[i], dep->settings()->target_os()); + SourceFileType input_file_type = GetSourceFileType(dep->sources()[i]); if (input_file_type != SOURCE_UNKNOWN && input_file_type != SOURCE_H) { // Note we need to specify the target as the source_set target diff --git a/tools/gn/secondary/testing/gmock/BUILD.gn b/tools/gn/secondary/testing/gmock/BUILD.gn index 907d117ac09710..8b5f8dbeb9ba68 100644 --- a/tools/gn/secondary/testing/gmock/BUILD.gn +++ b/tools/gn/secondary/testing/gmock/BUILD.gn @@ -28,7 +28,7 @@ static_library("gmock") { "src/gmock-matchers.cc", "src/gmock-spec-builders.cc", "src/gmock.cc", - "gmock_mutant.h", # gMock helpers + "../gmock_mutant.h", # gMock helpers ] # This project includes some stuff form gtest's guts. @@ -38,6 +38,8 @@ static_library("gmock") { ":gmock_config", "//testing/gtest:gtest_config", ] + + deps = [ "//base" ] } static_library("gmock_main") { diff --git a/tools/gn/setup.cc b/tools/gn/setup.cc index e2e17faa244c70..da6f88c8173244 100644 --- a/tools/gn/setup.cc +++ b/tools/gn/setup.cc @@ -18,6 +18,7 @@ #include "base/strings/utf_string_conversions.h" #include "build/build_config.h" #include "tools/gn/filesystem_utils.h" +#include "tools/gn/header_checker.h" #include "tools/gn/input_file.h" #include "tools/gn/parse_tree.h" #include "tools/gn/parser.h" @@ -138,7 +139,8 @@ CommonSetup::CommonSetup() builder_(new Builder(loader_.get())), root_build_file_("//BUILD.gn"), check_for_bad_items_(true), - check_for_unused_overrides_(true) { + check_for_unused_overrides_(true), + check_public_headers_(false) { loader_->set_complete_callback(base::Bind(&DecrementWorkCount)); } @@ -148,7 +150,8 @@ CommonSetup::CommonSetup(const CommonSetup& other) builder_(new Builder(loader_.get())), root_build_file_(other.root_build_file_), check_for_bad_items_(other.check_for_bad_items_), - check_for_unused_overrides_(other.check_for_unused_overrides_) { + check_for_unused_overrides_(other.check_for_unused_overrides_), + check_public_headers_(other.check_public_headers_) { loader_->set_complete_callback(base::Bind(&DecrementWorkCount)); } @@ -181,6 +184,22 @@ bool CommonSetup::RunPostMessageLoop() { } } + if (check_public_headers_) { + std::vector targets = builder_->GetAllResolvedTargets(); + scoped_refptr header_checker( + new HeaderChecker(&build_settings_, targets)); + + std::vector header_errors; + header_checker->Run(&header_errors); + for (size_t i = 0; i < header_errors.size(); i++) { + if (i > 0) + OutputString("___________________\n", DECORATION_YELLOW); + header_errors[i].PrintToStdout(); + } + if (!header_errors.empty()) + return false; + } + // Write out tracing and timing if requested. const CommandLine* cmdline = CommandLine::ForCurrentProcess(); if (cmdline->HasSwitch(kTimeSwitch)) diff --git a/tools/gn/setup.h b/tools/gn/setup.h index 8fbe3ff21be3c6..eb9d37b2b44528 100644 --- a/tools/gn/setup.h +++ b/tools/gn/setup.h @@ -48,6 +48,12 @@ class CommonSetup { check_for_unused_overrides_ = s; } + // After a successful run, setting this will additionally cause the public + // headers to be checked. Defaults to false. + void set_check_public_headers(bool s) { + check_public_headers_ = s; + } + BuildSettings& build_settings() { return build_settings_; } Builder* builder() { return builder_.get(); } LoaderImpl* loader() { return loader_.get(); } @@ -69,6 +75,7 @@ class CommonSetup { bool check_for_bad_items_; bool check_for_unused_overrides_; + bool check_public_headers_; private: CommonSetup& operator=(const CommonSetup& other); // Disallow. diff --git a/tools/gn/target.cc b/tools/gn/target.cc index 7a2b625b219073..0763a8c7d9098c 100644 --- a/tools/gn/target.cc +++ b/tools/gn/target.cc @@ -54,6 +54,7 @@ void MergeAllDependentConfigsFrom(const Target* from_target, Target::Target(const Settings* settings, const Label& label) : Item(settings, label), output_type_(UNKNOWN), + all_headers_public_(true), hard_dep_(false) { } diff --git a/tools/gn/target.h b/tools/gn/target.h index 5e257ec3e53c6c..6b8eb99e186bbe 100644 --- a/tools/gn/target.h +++ b/tools/gn/target.h @@ -69,6 +69,16 @@ class Target : public Item { const FileList& sources() const { return sources_; } FileList& sources() { return sources_; } + // Set to true when all sources are public. This is the default. In this case + // the public headers list should be empty. + bool all_headers_public() const { return all_headers_public_; } + void set_all_headers_public(bool p) { all_headers_public_ = p; } + + // When all_headers_public is false, this is the list of public headers. It + // could be empty which would mean no headers are public. + const FileList& public_headers() const { return public_headers_; } + FileList& public_headers() { return public_headers_; } + // Compile-time extra dependencies. const FileList& source_prereqs() const { return source_prereqs_; } FileList& source_prereqs() { return source_prereqs_; } @@ -145,6 +155,8 @@ class Target : public Item { std::string output_extension_; FileList sources_; + bool all_headers_public_; + FileList public_headers_; FileList source_prereqs_; FileList data_; diff --git a/tools/gn/target_generator.cc b/tools/gn/target_generator.cc index 5a7078cfb13cad..570e1f89441b9f 100644 --- a/tools/gn/target_generator.cc +++ b/tools/gn/target_generator.cc @@ -137,6 +137,21 @@ void TargetGenerator::FillSources() { target_->sources().swap(dest_sources); } +void TargetGenerator::FillPublic() { + const Value* value = scope_->GetValue(variables::kPublic, true); + if (!value) + return; + + // If the public headers are defined, don't default to public. + target_->set_all_headers_public(false); + + Target::FileList dest_public; + if (!ExtractListOfRelativeFiles(scope_->settings()->build_settings(), *value, + scope_->GetSourceDir(), &dest_public, err_)) + return; + target_->public_headers().swap(dest_public); +} + void TargetGenerator::FillSourcePrereqs() { const Value* value = scope_->GetValue(variables::kSourcePrereqs, true); if (!value) diff --git a/tools/gn/target_generator.h b/tools/gn/target_generator.h index bcfc7283092b12..42ad6e7da2d34e 100644 --- a/tools/gn/target_generator.h +++ b/tools/gn/target_generator.h @@ -49,6 +49,7 @@ class TargetGenerator { const BuildSettings* GetBuildSettings() const; void FillSources(); + void FillPublic(); void FillSourcePrereqs(); void FillConfigs(); void FillOutputs(); diff --git a/tools/gn/trace.cc b/tools/gn/trace.cc index a2f34a32d6624d..acd735df365cef 100644 --- a/tools/gn/trace.cc +++ b/tools/gn/trace.cc @@ -185,6 +185,8 @@ std::string SummarizeTraces() { std::vector parses; std::vector file_execs; std::vector script_execs; + std::vector check_headers; + int headers_checked = 0; for (size_t i = 0; i < events.size(); i++) { switch (events[i]->type()) { case TraceItem::TRACE_FILE_PARSE: @@ -196,6 +198,12 @@ std::string SummarizeTraces() { case TraceItem::TRACE_SCRIPT_EXECUTE: script_execs.push_back(events[i]); break; + case TraceItem::TRACE_CHECK_HEADERS: + check_headers.push_back(events[i]); + break; + case TraceItem::TRACE_CHECK_HEADER: + headers_checked++; + break; case TraceItem::TRACE_FILE_LOAD: case TraceItem::TRACE_FILE_WRITE: case TraceItem::TRACE_DEFINE_TARGET: @@ -211,6 +219,19 @@ std::string SummarizeTraces() { SummarizeScriptExecs(script_execs, out); out << std::endl; + // Generally there will only be one header check, but it's theoretically + // possible for more than one to run if more than one build is going in + // parallel. Just report the total of all of them. + if (!check_headers.empty()) { + float check_headers_time = 0; + for (size_t i = 0; i < check_headers.size(); i++) + check_headers_time += check_headers[i]->delta().InMillisecondsF(); + + out << "Header check time: (total time in ms, files checked)\n"; + out << base::StringPrintf(" %8.2f %d\n", + check_headers_time, headers_checked); + } + return out.str(); } @@ -261,6 +282,13 @@ void SaveTraces(const base::FilePath& file_name) { break; case TraceItem::TRACE_DEFINE_TARGET: out << "\"define\""; + break; + case TraceItem::TRACE_CHECK_HEADER: + out << "\"hdr\""; + break; + case TraceItem::TRACE_CHECK_HEADERS: + out << "\"header_check\""; + break; } if (!item.toolchain().empty() || !item.cmdline().empty()) { diff --git a/tools/gn/trace.h b/tools/gn/trace.h index 61811a796f6f52..2063be836a986e 100644 --- a/tools/gn/trace.h +++ b/tools/gn/trace.h @@ -24,7 +24,9 @@ class TraceItem { TRACE_FILE_EXECUTE, TRACE_FILE_WRITE, TRACE_SCRIPT_EXECUTE, - TRACE_DEFINE_TARGET + TRACE_DEFINE_TARGET, + TRACE_CHECK_HEADER, // One file. + TRACE_CHECK_HEADERS, // All files. }; TraceItem(Type type, diff --git a/tools/gn/variables.cc b/tools/gn/variables.cc index 40f60aa525f58e..8e41e62cbe5ff4 100644 --- a/tools/gn/variables.cc +++ b/tools/gn/variables.cc @@ -678,6 +678,45 @@ const char kOutputs_Help[] = " file(s). For actions, the outputs should be the list of files\n" " generated by the script.\n"; +const char kPublic[] = "public"; +const char kPublic_HelpShort[] = + "public: [file list] Declare public header files for a target."; +const char kPublic_Help[] = + "public: Declare public header files for a target.\n" + "\n" + " A list of files and patterns that other targets can include. These\n" + " permissions are checked via the \"check\" command\n" + " (see \"gn help check\").\n" + "\n" + " If no public files are declared, other targets (assuming they have\n" + " visibility to depend on this target) can include any file. If this\n" + " variable is defined on a target, dependent targets may only include\n" + " files on this whitelist.\n" + "\n" + " The entries in this list are patterns (see \"gn help patterns\") so\n" + " you can use simple wildcard matching if you have a directory of public\n" + " files.\n" + "\n" + " Header file permissions are also subject to visibility. A target\n" + " must be visible to another target to include any files from it at all\n" + " and the public headers indicate which subset of those files are\n" + " permitted.\n" + "\n" + " Public files are inherited through the dependency tree. So if there is\n" + " a dependency A -> B -> C, then A can include C's public headers.\n" + " However, the same is NOT true of visibility, so unless A is in C's\n" + " visibility list, the include will be rejected.\n" + "\n" + "Examples:\n" + " These exact files are public:\n" + " public = [ \"foo.h\", \"bar.h\" ]\n" + "\n" + " All files in the \"public\" directory are public:\n" + " public = [ \"public/*\" ]\n" + "\n" + " No files are public (no targets may include headers from this one):\n" + " public = []\n"; + const char kScript[] = "script"; const char kScript_HelpShort[] = "script: [file name] Script file for actions."; @@ -870,6 +909,7 @@ const VariableInfoMap& GetTargetVariables() { INSERT_VARIABLE(OutputExtension) INSERT_VARIABLE(OutputName) INSERT_VARIABLE(Outputs) + INSERT_VARIABLE(Public) INSERT_VARIABLE(Script) INSERT_VARIABLE(SourcePrereqs) INSERT_VARIABLE(Sources) diff --git a/tools/gn/variables.h b/tools/gn/variables.h index 27b0c151dd91e4..80404ae93c5134 100644 --- a/tools/gn/variables.h +++ b/tools/gn/variables.h @@ -159,6 +159,10 @@ extern const char kOutputs[]; extern const char kOutputs_HelpShort[]; extern const char kOutputs_Help[]; +extern const char kPublic[]; +extern const char kPublic_HelpShort[]; +extern const char kPublic_Help[]; + extern const char kScript[]; extern const char kScript_HelpShort[]; extern const char kScript_Help[];