Commit a6fa7142 authored by cmasone's avatar cmasone Committed by Commit bot

GN: Complete static libs can't depend on static libs

Trying to use ar to put a static lib inside a complete static lib doesn't
work. GN should enforce and document this restriction.

BUG=None
TEST=gn_unittests, including new test for this case.
R=brettw

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

Cr-Commit-Position: refs/heads/master@{#295609}
parent a1d6cd31
...@@ -49,6 +49,19 @@ Err MakeTestOnlyError(const Target* from, const Target* to) { ...@@ -49,6 +49,19 @@ Err MakeTestOnlyError(const Target* from, const Target* to) {
"Either mark it test-only or don't do this dependency."); "Either mark it test-only or don't do this dependency.");
} }
Err MakeStaticLibDepsError(const Target* from, const Target* to) {
return Err(from->defined_from(),
"Complete static libraries can't depend on static libraries.",
from->label().GetUserVisibleName(false) +
"\n"
"which is a complete static library can't depend on\n" +
to->label().GetUserVisibleName(false) +
"\n"
"which is a static library.\n"
"\n"
"Use source sets for intermediate targets instead.");
}
} // namespace } // namespace
Target::Target(const Settings* settings, const Label& label) Target::Target(const Settings* settings, const Label& label)
...@@ -126,6 +139,8 @@ bool Target::OnResolved(Err* err) { ...@@ -126,6 +139,8 @@ bool Target::OnResolved(Err* err) {
return false; return false;
if (!CheckTestonly(err)) if (!CheckTestonly(err))
return false; return false;
if (!CheckNoNestedStaticLibs(err))
return false;
return true; return true;
} }
...@@ -326,7 +341,7 @@ bool Target::CheckVisibility(Err* err) const { ...@@ -326,7 +341,7 @@ bool Target::CheckVisibility(Err* err) const {
} }
bool Target::CheckTestonly(Err* err) const { bool Target::CheckTestonly(Err* err) const {
// If there current target is marked testonly, it can include both testonly // If the current target is marked testonly, it can include both testonly
// and non-testonly targets, so there's nothing to check. // and non-testonly targets, so there's nothing to check.
if (testonly()) if (testonly())
return true; return true;
...@@ -341,3 +356,27 @@ bool Target::CheckTestonly(Err* err) const { ...@@ -341,3 +356,27 @@ bool Target::CheckTestonly(Err* err) const {
return true; return true;
} }
bool Target::CheckNoNestedStaticLibs(Err* err) const {
// If the current target is not a complete static library, it can depend on
// static library targets with no problem.
if (!(output_type() == Target::STATIC_LIBRARY && complete_static_lib()))
return true;
// Verify no deps are static libraries.
for (DepsIterator iter(this); !iter.done(); iter.Advance()) {
if (iter.target()->output_type() == Target::STATIC_LIBRARY) {
*err = MakeStaticLibDepsError(this, iter.target());
return false;
}
}
// Verify no inherited libraries are static libraries.
for (size_t i = 0; i < inherited_libraries().size(); ++i) {
if (inherited_libraries()[i]->output_type() == Target::STATIC_LIBRARY) {
*err = MakeStaticLibDepsError(this, inherited_libraries()[i]);
return false;
}
}
return true;
}
...@@ -244,6 +244,7 @@ class Target : public Item { ...@@ -244,6 +244,7 @@ class Target : public Item {
// Validates the given thing when a target is resolved. // Validates the given thing when a target is resolved.
bool CheckVisibility(Err* err) const; bool CheckVisibility(Err* err) const;
bool CheckTestonly(Err* err) const; bool CheckTestonly(Err* err) const;
bool CheckNoNestedStaticLibs(Err* err) const;
OutputType output_type_; OutputType output_type_;
std::string output_name_; std::string output_name_;
......
...@@ -225,6 +225,55 @@ TEST(Target, InheritCompleteStaticLib) { ...@@ -225,6 +225,55 @@ TEST(Target, InheritCompleteStaticLib) {
EXPECT_TRUE(a_inherited.IndexOf(&b) != static_cast<size_t>(-1)); EXPECT_TRUE(a_inherited.IndexOf(&b) != static_cast<size_t>(-1));
} }
TEST(Target, InheritCompleteStaticLibNoDirectStaticLibDeps) {
TestWithScope setup;
Err err;
// Create a dependency chain:
// A (complete static lib) -> B (static lib)
Target a(setup.settings(), Label(SourceDir("//foo/"), "a"));
a.set_output_type(Target::STATIC_LIBRARY);
a.visibility().SetPublic();
a.set_complete_static_lib(true);
a.SetToolchain(setup.toolchain());
Target b(setup.settings(), Label(SourceDir("//foo/"), "b"));
b.set_output_type(Target::STATIC_LIBRARY);
b.visibility().SetPublic();
b.SetToolchain(setup.toolchain());
a.public_deps().push_back(LabelTargetPair(&b));
ASSERT_TRUE(b.OnResolved(&err));
ASSERT_FALSE(a.OnResolved(&err));
}
TEST(Target, InheritCompleteStaticLibNoIheritedStaticLibDeps) {
TestWithScope setup;
Err err;
// Create a dependency chain:
// A (complete static lib) -> B (source set) -> C (static lib)
Target a(setup.settings(), Label(SourceDir("//foo/"), "a"));
a.set_output_type(Target::STATIC_LIBRARY);
a.visibility().SetPublic();
a.set_complete_static_lib(true);
a.SetToolchain(setup.toolchain());
Target b(setup.settings(), Label(SourceDir("//foo/"), "b"));
b.set_output_type(Target::SOURCE_SET);
b.visibility().SetPublic();
b.SetToolchain(setup.toolchain());
Target c(setup.settings(), Label(SourceDir("//foo/"), "c"));
c.set_output_type(Target::STATIC_LIBRARY);
c.visibility().SetPublic();
c.SetToolchain(setup.toolchain());
a.public_deps().push_back(LabelTargetPair(&b));
b.public_deps().push_back(LabelTargetPair(&c));
ASSERT_TRUE(c.OnResolved(&err));
ASSERT_TRUE(b.OnResolved(&err));
ASSERT_FALSE(a.OnResolved(&err));
}
TEST(Target, GetComputedOutputName) { TEST(Target, GetComputedOutputName) {
TestWithScope setup; TestWithScope setup;
Err err; Err err;
......
...@@ -375,7 +375,7 @@ const char kCompleteStaticLib_Help[] = ...@@ -375,7 +375,7 @@ const char kCompleteStaticLib_Help[] =
"complete_static_lib: [boolean] Links all deps into a static library.\n" "complete_static_lib: [boolean] Links all deps into a static library.\n"
"\n" "\n"
" A static library normally doesn't include code from dependencies, but\n" " A static library normally doesn't include code from dependencies, but\n"
" instead forward the static libraries and source sets in its deps up\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" " 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) is reached. The final linkable target only links each static\n"
" library once, even if it appears more than once in its dependency\n" " library once, even if it appears more than once in its dependency\n"
...@@ -384,7 +384,10 @@ const char kCompleteStaticLib_Help[] = ...@@ -384,7 +384,10 @@ const char kCompleteStaticLib_Help[] =
" In some cases the static library might be the final desired output.\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" " 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" " third parties. In this case, the static library should include code\n"
" for all dependencies in one complete package.\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"
" libraries.\n"
"\n" "\n"
"Example\n" "Example\n"
"\n" "\n"
......
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