Fix GN NaCl Debug build of base.

Fix GN NaCl Debug build of base.

BUG=401761
TBR=brettw
CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:android_chromium_gn_compile_dbg,android_chromium_gn_compile_rel;tryserver.chromium.win:win8_chromium_gn_rel,win8_chromium_gn_dbg;tryserver.chromium.mac:mac_chromium_gn_rel,mac_chromium_gn_dbg

Committed: https://crrev.com/9099f49130311a760ce3fe88b7782a5574e6ba9d
Cr-Commit-Position: refs/heads/master@{#319987}

[Nick Bray (chromium), 2015-03-03 18:36:20]

ncbray@chromium.org changed reviewers:
+ brettw@chromium.org, dpranke@chromium.org

[Nick Bray (chromium), 2015-03-03 18:36:20]

Converting "component"s from source_sets to static_libraries may be
controversial.  The main issue is that the NaCl build of "base" is hacked up and
there are files in in that reference non-existant symbols (such as
File::Exists).  Treating base as a source set causes the compiler to try and
resolve all the symbols in all the .o files - which fails.  The release build
dodges this bullet because --gc-sections lets the compiler ignore dead .o files.
 How much of a build time improvement to source_sets actually provide?  Without
--gc-sections, I could see source sets actually increasing link time (more
symbols to resolve).

Alternatives, in order of desirability:
1) copy the implementation of the "component" template a third time, and only
use static_libraries for NaCl.  This will result in subtle skew between NaCl
builds and non-NaCl builds.
2) Add --gc-sections to debug builds.
3) Provide stubs for all the functions that have been removed from the NaCl base
build.
4) Try to remove all of the files that transitively depend on symbols not
present in the NaCl build.  This is a yak shave that could fail for
unanticipated reasons.

[Dirk Pranke, 2015-03-03 20:17:30]

This one's for Brett :)

[brettw, 2015-03-03 20:33:05]

On 2015/03/03 20:17:30, Dirk Pranke wrote:
> This one's for Brett :)

I don't want to do this in general. GYP on Linux uses a "thin" static library
which are basically links to .o files from the .a file, rather than
concatenating them together. Doing this improved the build performance and we
were sad that Windows has no equivalent. Source sets solve this problem in a
cross-platform way. As a result, GN's static library is a "complete" static
library which will give worse performance than even GYP.

I think depending on the static library's symbol resolution rules is a very bad
practice. The vast majority of people don't even understand there is a
difference between putting something in a static library vs. just linking the
.o. Depending on this for successful builds leads to a lot of confusion for
changes that look benign.

I think it would be best to do some combination of 3 and 4, where we remove
files that obviously aren't necessary (there are a bunch), and write stubs for
the rest.

[Nick Bray (chromium), 2015-03-04 00:33:28]

OK, check this out, then.

[brettw, 2015-03-04 00:57:01]

I'm basically happy with this approach.

Normally we would split the BUILD.gn files apart in this case so there would be
base/process/BUILD.gn that would contain a "process" source_set. This will
reduce some redundancy in the base/BUILD.gn file too.

I would like to not regress "gn check" which this will do. Currently check more
is disabled due to a limitation of Android, but I spent a lot of time getting it
to this case. You should be able to edit the src/.gn file and uncomment the base
line, then run on the command line "gn check <outdir>" and it should pass.

This will fail with this change because of circular dependendencies
(base_process includes files from base, but base depends on base_process).
Fortunately, there is a feature to resolve this problem. Add a list in base
called allow_circular_includes_from = [ "//base/process" ]

[Nick Bray (chromium), 2015-03-04 21:50:32]

PTAL

[Nick Bray (chromium), 2015-03-05 22:27:40]

Ping.  This change is begging for merge conflicts, so I'd like to get it in
ASAP.

[brettw, 2015-03-05 23:44:27]

I think you missed this part:

"Normally we would split the BUILD.gn files apart in this case so there would be
base/process/BUILD.gn that would contain a "process" source_set."

[Nick Bray (chromium), 2015-03-05 23:56:46]

"Normally" is ambiguous and can be read as "but not in this case".  I will infer
from your statement that you wish these targets to be separated into files.

[brettw, 2015-03-06 00:09:42]

Sorry, yes.

[Nick Bray (chromium), 2015-03-07 01:14:01]

PTAL

[Nick Bray (chromium), 2015-03-10 00:45:07]

ping dpranke@ - sounds like you're the reviewer, now.

[Dirk Pranke, 2015-03-10 20:02:15]

lgtm.

Please add this line to the description to make sure we run it through all of
the GN trybots:

CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:android_chromium_gn_compile_dbg,android_chromium_gn_compile_rel;tryserver.chromium.win:win8_chromium_gn_rel,win8_chromium_gn_dbg;tryserver.chromium.mac:mac_chromium_gn_rel,mac_chromium_gn_dbg


You'll probably also still need to tbr brettw to get past the presubmit check.

[Nick Bray (chromium), 2015-03-10 20:48:07]

The CQ bit was checked by ncbray@chromium.org

[Nick Bray (chromium), 2015-03-10 20:48:07]

The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/972203003/#ps180001
(title: "Rebase again")

[commit-bot: I haz the power, 2015-03-10 20:49:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/972203003/180001

[commit-bot: I haz the power, 2015-03-10 22:14:56]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-03-10 22:15:28]

Patchset 10 (id:??) landed as
https://crrev.com/9099f49130311a760ce3fe88b7782a5574e6ba9d
Cr-Commit-Position: refs/heads/master@{#319987}