GN: Refine 'complete' static library handling

tools/gn/target.cc

 // Copyright (c) 2013 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/target.h"
 
 #include "base/bind.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "tools/gn/config_values_extractors.h"
@@ skipping 226 matching lines @@
     if (dep->output_type() == STATIC_LIBRARY ||
         dep->output_type() == SHARED_LIBRARY ||
         dep->output_type() == SOURCE_SET)
       inherited_libraries_.push_back(dep);
 
     // Inherited libraries and flags are inherited across static library
     // boundaries.
     if (!dep->IsFinal()) {
       inherited_libraries_.Append(dep->inherited_libraries().begin(),
                                   dep->inherited_libraries().end());
+    } else if (dep->complete_static_lib()) {

[brettw, 2014/09/16 16:46:02]

I don't think there should have to be a separate step to pull libs across
complete static libraries beyond additional conditions. It seems like everything
should be to do something when a target isn't final or it's an incomplete static
lib (as you do below). If the inherited libs aren't getting propagated, then I
think we're missing some other place.

[brettw, 2014/09/16 23:56:12]

I get what's happening here. Can you add a comment about inheriting across the
boundary only things that weren't linked into the complete static library?

Also, I think complete static libraries should be excluded, since they'll
themselves be linked into the current complete static library.

We should get tests for this.

[kal, 2014/09/17 05:03:54]

OK I will extend the comment here.

The way its implemented now, complete static libraries don't include other
complete static libraries. I think this is the best behavior. You may have a
suite of libraries you are distributing that have dependencies on each other so
if you include complete static libraries in each other you end up with
unnecessary code duplication. Take chromium for example, if you made the
components into static libraries for embedding, basically every static lib would
include most of "base" but "base" is also distributed, so just let the user link
in base themselves. Furthermore, not linking them in is more flexible because if
you really do want to link the dependent complete static library's code you can
just create a source set for each static library and link in the appropriate
source sets you want to have linked in.

+      // Inherit final libraries through _complete_ static libraries.
+      for (size_t i = 0; i < dep->inherited_libraries().size(); i++) {
+        const Target* inherited = dep->inherited_libraries()[i];
+        if (inherited->IsFinal())
+          inherited_libraries_.push_back(inherited);
+      }
+    }
 
-      // Inherited library settings.
+    // Library settings are always inherited across static library boundaries.
+    if (!dep->IsFinal() || dep->output_type() == STATIC_LIBRARY) {
       all_lib_dirs_.append(dep->all_lib_dirs());
       all_libs_.append(dep->all_libs());
     }
   }
 }
 
 void Target::PullForwardedDependentConfigs() {
   // Groups implicitly forward all if its dependency's configs.
   if (output_type() == GROUP) {
     for (size_t i = 0; i < deps_.size(); i++)
@@ skipping 127 matching lines @@
 
   for (size_t i = 0; i < datadeps_.size(); i++) {
     if (datadeps_[i].ptr->testonly()) {
       *err = MakeTestOnlyError(this, datadeps_[i].ptr);
       return false;
     }
   }
 
   return true;
 }