Skip to content

Commit

Permalink
Add optional public header checking to GN build
Browse files Browse the repository at this point in the history
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
  • Loading branch information
brettw@chromium.org committed Apr 7, 2014
1 parent 44b4f0e commit 126d8b5
Show file tree
Hide file tree
Showing 33 changed files with 965 additions and 57 deletions.
8 changes: 4 additions & 4 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down
1 change: 1 addition & 0 deletions base/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ static_library("test_support_perf") {
"run_all_perftests.cc",
]
deps = [
":test_support_base",
"//base",
"//testing/gtest",
]
Expand Down
5 changes: 4 additions & 1 deletion device/usb/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
File renamed without changes.
9 changes: 1 addition & 8 deletions third_party/modp_b64/README.chromium
Original file line number Diff line number Diff line change
Expand Up @@ -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 <stdint.h> 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.
64-bit systems.
9 changes: 0 additions & 9 deletions third_party/modp_b64/modp_b64_data.h
Original file line number Diff line number Diff line change
@@ -1,14 +1,5 @@
#include "build/build_config.h"
#if !defined(COMPILER_MSVC)
#include <stdint.h>
#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 '/'
Expand Down
7 changes: 7 additions & 0 deletions tools/gn/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
4 changes: 4 additions & 0 deletions tools/gn/binary_target_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ void BinaryTargetGenerator::DoRun() {
if (err_->has_error())
return;

FillPublic();
if (err_->has_error())
return;

FillSourcePrereqs();
if (err_->has_error())
return;
Expand Down
143 changes: 143 additions & 0 deletions tools/gn/c_include_iterator.cc
Original file line number Diff line number Diff line change
@@ -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;
}
44 changes: 44 additions & 0 deletions tools/gn/c_include_iterator.h
Original file line number Diff line number Diff line change
@@ -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_
Loading

0 comments on commit 126d8b5

Please sign in to comment.