Commit a9adafea authored by brettw's avatar brettw Committed by Commit bot

Make GN header checker more lenient about toolchains.

The header checker will now automatically accept any headers not known in the
current toolchain. The previous code would do this but required it to not be
present in any toolchain in the entire build.

See the long comment added in header_checker.cc for details.

Adds a dependency fix for the Android build in base.

BUG=

Review URL: https://codereview.chromium.org/1142423004

Cr-Commit-Position: refs/heads/master@{#330839}
parent f3670439
......@@ -52,6 +52,12 @@ source_set("memory") {
sources -= [ "shared_memory_nacl.cc" ]
}
if (is_android) {
deps = [
"//third_party/ashmem",
]
}
configs += [ "//base:base_implementation" ]
visibility = [ "//base/*" ]
......
......@@ -320,6 +320,33 @@ bool HeaderChecker::CheckInclude(const Target* from_target,
const TargetVector& targets = found->second;
Chain chain; // Prevent reallocating in the loop.
// If the file is unknown in the current toolchain (rather than being private
// or in a target not visible to the current target), ignore it. This is a
// bit of a hack to account for the fact that the include finder doesn't
// understand the preprocessor.
//
// When not cross-compiling, if a platform specific header is conditionally
// included in the build, and preprocessor conditions around #includes of
// that match the build conditions, everything will be OK because the file
// won't be known to GN even though the #include finder identified the file.
//
// Cross-compiling breaks this. When compiling Android on Linux, for example,
// we might see both Linux and Android definitions of a target and know
// about the union of all headers in the build. Since the #include finder
// ignores preprocessor, we will find the Linux headers in the Android
// build and note that a dependency from the Android target to the Linux
// one is missing (these might even be the same target in different
// toolchains!).
bool present_in_current_toolchain = false;
for (const auto& target : targets) {
if (from_target->label().ToolchainsEqual(target.target->label())) {
present_in_current_toolchain = true;
break;
}
}
if (!present_in_current_toolchain)
return true;
// For all targets containing this file, we require that at least one be
// a direct or public dependency of the current target, and that the header
// is public within the target.
......@@ -337,18 +364,6 @@ bool HeaderChecker::CheckInclude(const Target* from_target,
if (to_target == from_target)
return true;
// Additionally, allow a target to include files from that same target
// in other toolchains. This is a bit of a hack to account for the fact that
// the include finder doesn't understand the preprocessor.
//
// If a source file conditionally depends on a platform-specific include in
// the same target, and there is a cross-compile such that GN sees
// definitions of the target both with and without that include, it would
// give an error that the target needs to depend on itself in the other
// toolchain (where the platform-specific header is defined as a source).
if (TargetLabelsMatchExceptToolchain(to_target, from_target))
return true;
bool is_permitted_chain = false;
if (IsDependencyOf(to_target, from_target, &chain, &is_permitted_chain)) {
DCHECK(chain.size() >= 2);
......
......@@ -162,12 +162,33 @@ TEST_F(HeaderCheckerTest, CheckInclude) {
c_.public_headers().push_back(c_public);
c_.set_all_headers_public(false);
// Create another toolchain.
Settings other_settings(setup_.build_settings(), "other/");
Toolchain other_toolchain(&other_settings,
Label(SourceDir("//toolchain/"), "other"));
TestWithScope::SetupToolchain(&other_toolchain);
other_settings.set_toolchain_label(other_toolchain.label());
other_settings.set_default_toolchain_label(setup_.toolchain()->label());
// Add a target in the other toolchain with a header in it that is not
// connected to any targets in the main toolchain.
Target otc(&other_settings, Label(SourceDir("//p/"), "otc",
other_toolchain.label().dir(), other_toolchain.label().name()));
otc.set_output_type(Target::SOURCE_SET);
Err err;
EXPECT_TRUE(otc.SetToolchain(&other_toolchain, &err));
otc.visibility().SetPublic();
targets_.push_back(&otc);
SourceFile otc_header("//otc_header.h");
otc.sources().push_back(otc_header);
EXPECT_TRUE(otc.OnResolved(&err));
scoped_refptr<HeaderChecker> 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;
EXPECT_FALSE(checker->CheckInclude(&a_, input_file, d_header, range, &err));
EXPECT_TRUE(err.has_error());
......@@ -188,6 +209,12 @@ TEST_F(HeaderCheckerTest, CheckInclude) {
EXPECT_TRUE(checker->CheckInclude(&a_, input_file, SourceFile("//random.h"),
range, &err));
EXPECT_FALSE(err.has_error());
// A can depend on a file present only in another toolchain even with no
// dependency path.
err = Err();
EXPECT_TRUE(checker->CheckInclude(&a_, input_file, otc_header, range, &err));
EXPECT_FALSE(err.has_error());
}
// A public chain of dependencies should always be identified first, even if
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment