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}

4 files changed, 95 insertions, 3 deletions

diff --git a/tools/gn/target.cc b/tools/gn/target.cc
index 9e693b1..d512894 100644
--- a/tools/gn/target.cc
+++ b/tools/gn/target.cc
@@ -49,6 +49,19 @@ Err MakeTestOnlyError(const Target* from, const Target* to) {
       "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
 
 Target::Target(const Settings* settings, const Label& label)
@@ -126,6 +139,8 @@ bool Target::OnResolved(Err* err) {
     return false;
   if (!CheckTestonly(err))
     return false;
+  if (!CheckNoNestedStaticLibs(err))
+    return false;
 
   return true;
 }
@@ -326,7 +341,7 @@ bool Target::CheckVisibility(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.
   if (testonly())
     return true;
@@ -341,3 +356,27 @@ bool Target::CheckTestonly(Err* err) const {
 
   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;
+}
diff --git a/tools/gn/target.h b/tools/gn/target.h
index 9dd3064..9d4b4be 100644
--- a/tools/gn/target.h
+++ b/tools/gn/target.h
@@ -244,6 +244,7 @@ class Target : public Item {
   // Validates the given thing when a target is resolved.
   bool CheckVisibility(Err* err) const;
   bool CheckTestonly(Err* err) const;
+  bool CheckNoNestedStaticLibs(Err* err) const;
 
   OutputType output_type_;
   std::string output_name_;
diff --git a/tools/gn/target_unittest.cc b/tools/gn/target_unittest.cc
index 151cb31..6f1a5cf 100644
--- a/tools/gn/target_unittest.cc
+++ b/tools/gn/target_unittest.cc
@@ -225,6 +225,55 @@ TEST(Target, InheritCompleteStaticLib) {
   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) {
   TestWithScope setup;
   Err err;
diff --git a/tools/gn/variables.cc b/tools/gn/variables.cc
index 75172ec..de214ef 100644
--- a/tools/gn/variables.cc
+++ b/tools/gn/variables.cc
@@ -375,7 +375,7 @@ 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 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"
     "  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"
@@ -384,7 +384,10 @@ const char kCompleteStaticLib_Help[] =
     "  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.\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"
     "Example\n"
     "\n"