GN style guide on static_libs and source_sets

tools/gn/docs/style_guide.md

 # GN Style Guide 
 
 [TOC]
 ## Naming and ordering within the file
 
 ### Location of build files
 
 It usually makes sense to have more build files closer to the code than
 fewer ones at the toplevel (this is in contrast with what we did with
 GYP). This makes things easier to find and owners reviews easier since
 changes are more focused.
 
 ### Targets
 
   * Most BUILD files should have a target with the same name of the
     directory. This target should be the first target.
   * Other targets should be in "some order that makes sense." Usually
     more important targets will be first, and unit tests will follow the
     corresponding target. If there's no clear ordering, consider
     alphabetical order.
-  * Test support libraries should be source sets named "test\_support".
+  * Test support libraries should be static libraries named "test\_support".
     So "//ui/compositor:test\_support". Test support libraries should
     include as public deps the non-test-support version of the library
     so tests need only depend on the test\_support target (rather than
     both).
 
 Naming advice
 
   * Targets and configs should be named using lowercase with underscores
     separating words, unless there is a strong reason to do otherwise.
   * Source sets, groups, and static libraries do not need globally unique names.
@@ skipping 14 matching lines @@
   * A config associated with a single target should be named the same as
     the target with `_config` following it.
   * A config should appear immediately before the corresponding target
     that uses it.
 
 ### Example
 
 Example for the `src/foo/BUILD.gn` file:
 
 ```
-# Copyright 2013 The Chromium Authors. All rights reserved.
+# Copyright 2016 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.
 
 # Config for foo is named foo_config and immediately precedes it in the file.
 config("foo_config") {
 }
 
 # Target matching path name is the first target.
 executable("foo") {
 }
 
 # Test for foo follows it.
 test("foo_unittests") {
 }
 
 config("bar_config") {
 }
 
 source_set("bar") {
 }
 ```
 
 ## Ordering within a target
 
   1. `output_name` / `visibility` / `testonly`
   2. `sources`
   3. `cflags`, `include_dirs`, `defines`, `configs` etc. in whatever
-     order makes sense to you.
+      order makes sense to you.

[Dirk Pranke, 2016/06/16 20:01:27]

Nit: I would've thought the prior indentation was correct.

[brettw, 2016/06/16 20:27:36]

Whoops, it sure looks wrong though.

   4. `public_deps`
   5. `deps`
 
 ### Conditions
 
 Simple conditions affecting just one variable (e.g. adding a single
 source or adding a flag for one particular OS) can go beneath the
 variable they affect. More complicated conditions affecting more than
 one thing should go at the bottom.
 
@@ skipping 48 matching lines @@
 ### Import
 
 Use fully-qualified paths for imports:
 
 ```
 import("//foo/bar/baz.gni")  # Even if this file is in the foo/bar directory
 ```
 
 ## Usage
 
-Use `source_set` rather than `static_library` unless you have a reason
-to do otherwise. A static library is a standalone library which can be
-slow to generate. A source set just links all the object files from that
-target into the targets depending on it, which saves the "lib" step.
+### Source sets versus static libraries
+
+Source sets and static libraries can be used interchangeably in most cases. If
+you're unsure what to use, a source set is almost never wrong and is less likely
+to cause problems.
+
+Static libraries follow different linking rules. When a static library is
+included in a link, only the object files that contain unresolved symbols will
+be brought into the build. Source sets result in every object file being added
+to the link line of the final binary.
+
+  * If you're eventually linking code into a component, shared library, or
+    loadable module, you normally need to use source sets. This is because
+    object files with no symbols referenced from within the shared library will
+    not be linked into the final library at all. This omission will happen even
+    if that object file has a symbol marked for export that targets dependant
+    on that shared library need. This will result in undefined symbols when
+    linking later targets.

[Dirk Pranke, 2016/06/16 20:01:27]

This really applies to code that is exported from a component, but not code that
is simply needed internally by the component. It might help to be slightly
clearer about that.

[brettw, 2016/06/16 20:27:36]

That's true, but I think a lot of people don't see the difference. I bet it's
very uncommon for a part of a component to have no exported files, and people
will always come later and add stuff. I think it's better for always be
consistent for stuff that goes into components.

+
+  * Unit tests (and anything else with static initializers with side effects)
+    must use source sets. The gtest TEST macros create static initializers
+    that register the test. But since no code references symbols in the object
+    file, linking a test into a static library and then into a test executable
+    means the tests will get stripped.
+
+  * Static libraries involve duplicating all of the data in the object files
+    that comprise it. This takes more disk space and for certain very large
+    libraries in configurations with very large object files can cause
+    internal limits on the size of static libraries to be exceeded. Source
+    sets do not have this limitation. Some targets switch between source sets
+    and static libraries depending on the build configuration to avoid this
+    problem.
+
+  * In cases where a lot of the symbols are not needed for a particular link
+    (this especially happens when linking test binaries), putting that code in
+    a static library can dramatically increase linking performance. This is
+    because the object files not needed for the link are never considered in
+    the first place, rather than forcing the linker to strip the unused code
+    in a later pass when nothing references it.
+
+### Loadable modules versus shared libraries versus components
+
+A component is a Chrome primitive (rather than a built-in GN concept) that
+expands either to a shared library or a static library / source set depending
+on the value of the `is_component_build` variable. This allows release builds
+to be linked statically in a large binary, but for developers to use shared
+libraries for most operations.
+
+A shared library will be listed on the link line of dependant targets and will
+be loaded automatically by the operating system when the application starts
+and symbols automatically resolved. A loadable module will not be linked
+directly and the application must manually load it.
+
+On Windows and Linux shared libraries and loadable modules result in the same
+type of file (`.dll` and `.so`, respectively). The only difference is in how
+they are linked to dependant targets. On these platforms, having a `deps`
+dependency on a loadable module is the same as having a `data_deps`
+(non-linked) dependency on a shared library.
+
+On Mac, these targets have different formats: a shared library will generate a
+`.dylib` file and a loadable module will generate a `.so` file.
+
+Use loadable modules for things like plugins. Shared libraries should be
+seldom-used outside of components because most Chrome code is shipped to the
+end-user as a small number of large binaries. In the case of plugin-like
+libraries, it's good practice to use both a loadable module for the target type
+(even for platforms where it doesn't matter) and data deps for targets that
+depend on it so it's clear from both places that how the library will be linked
+and loaded.
 
 ## Build arguments
 
 ### Scope
 
 Build arguments should be scoped to a unit of behavior, e.g. enabling a feature.
 Typically an argument would be declared in an imported file to share it with
 the subset of the build that could make use of it.
 
 Chrome has many legacy flags in `//build/config/features.gni`,
@@ skipping 38 matching lines @@
 `enable_google_now`, `enable_nacl`, `enable_remoting`, `enable_pdf`)
 
 `disable_foo` - _NOT_ recommended, use `enable_foo` instead with swapped default
 value
 
 `is_foo` - usually a global state descriptor (e.g. `is_chrome_branded`,
 `is_desktop_linux`); poor choice for non-globals
 
 `foo_use_bar` - prefixes can be used to indicate a limited scope for an argument
 (e.g. `rtc_use_h264`, `v8_use_snapshot`)