Make crazy linker work in gn build

Make crazy linker work in gn build

This adds the configuration for the android_apk template to use the crazy
linker for an apk and adds a bunch of crazy linker related targets (including
crazy linker tests).

TBR=ben@
BUG=359249
Committed: https://crrev.com/aac45ed811d1f0fde1d8189c81813deebc8c9682
Cr-Commit-Position: refs/heads/master@{#300172}

[cjhopman, 2014-10-09 18:24:59]

cjhopman@chromium.org changed reviewers:
+ jamesr@chromium.org, simonb@chromium.org

[cjhopman, 2014-10-09 18:25:00]

simonb: *
jamesr: mojo/ OWNERS + gn readability

[jamesr, 2014-10-09 19:22:57]

https://codereview.chromium.org/640363002/diff/1/content/shell/android/BUILD.gn
File content/shell/android/BUILD.gn (right):

https://codereview.chromium.org/640363002/diff/1/content/shell/android/BUILD....
content/shell/android/BUILD.gn:219: group("chromium_linker_test_apk") {
this seems wrong - are people declaring a dependency on chromium_linker_test_apk
when they shouldn't?

[cjhopman, 2014-10-09 20:22:39]

https://codereview.chromium.org/640363002/diff/1/content/shell/android/BUILD.gn
File content/shell/android/BUILD.gn (right):

https://codereview.chromium.org/640363002/diff/1/content/shell/android/BUILD....
content/shell/android/BUILD.gn:219: group("chromium_linker_test_apk") {
On 2014/10/09 19:22:57, jamesr wrote:
> this seems wrong - are people declaring a dependency on
chromium_linker_test_apk
> when they shouldn't?

No, they aren't (afaik). The thing is, //BUILD.gn should sort of list all the
top-level targets (it really only needs to list enough that it discovers all the
relevant BUILD.gn files). It doesn't actually need to list
chromium_linker_test_apk (though it will if/when that gets moved into its own
BUILD.gn file). 

Then, the issue is, how do you make that work for a target that is disabled in
certain configurations. We could duplicate the condition enabling it here in
//BUILD.gn, but I don't really like that. We could hoist the condition into a
variable that can be used in both places (which is effectively what we do for
"is_android"-only targets). Or we could do this where we can unconditionally say
that chromium_linker_test_apk is a top-level target and just in some
configurations it is an empty group.

(the other problem that will come up for similar targets is that there might be
other places that we list top-level targets like build recipes, etc).

Alternatively, we could just ignore this for now and not list
chromium_linker_test_apk in the root build file.

Any preference? Other suggestions?

[jamesr, 2014-10-09 20:29:39]

On 2014/10/09 20:22:39, cjhopman wrote:
> Then, the issue is, how do you make that work for a target that is disabled in
> certain configurations. We could duplicate the condition enabling it here in
> //BUILD.gn

This if you must list it in //BUILD.gn or it won't happen at all

> 
> Alternatively, we could just ignore this for now and not list
> chromium_linker_test_apk in the root build file.

Or this, if you don't need it.

[cjhopman, 2014-10-09 20:47:38]

https://codereview.chromium.org/640363002/diff/1/content/shell/android/BUILD.gn
File content/shell/android/BUILD.gn (right):

https://codereview.chromium.org/640363002/diff/1/content/shell/android/BUILD....
content/shell/android/BUILD.gn:219: group("chromium_linker_test_apk") {
On 2014/10/09 20:22:39, cjhopman wrote:
> On 2014/10/09 19:22:57, jamesr wrote:
> > this seems wrong - are people declaring a dependency on
> chromium_linker_test_apk
> > when they shouldn't?
> 
> No, they aren't (afaik). The thing is, //BUILD.gn should sort of list all the
> top-level targets (it really only needs to list enough that it discovers all
the
> relevant BUILD.gn files). It doesn't actually need to list
> chromium_linker_test_apk (though it will if/when that gets moved into its own
> BUILD.gn file). 
> 
> Then, the issue is, how do you make that work for a target that is disabled in
> certain configurations. We could duplicate the condition enabling it here in
> //BUILD.gn, but I don't really like that. We could hoist the condition into a
> variable that can be used in both places (which is effectively what we do for
> "is_android"-only targets). Or we could do this where we can unconditionally
say
> that chromium_linker_test_apk is a top-level target and just in some
> configurations it is an empty group.
> 
> (the other problem that will come up for similar targets is that there might
be
> other places that we list top-level targets like build recipes, etc).
> 
> Alternatively, we could just ignore this for now and not list
> chromium_linker_test_apk in the root build file.
> 
> Any preference? Other suggestions?
> 

Done.

[jamesr, 2014-10-09 20:56:51]

lgtm

https://codereview.chromium.org/640363002/diff/200001/third_party/android_cra...
File third_party/android_crazy_linker/BUILD.gn (right):

https://codereview.chromium.org/640363002/diff/200001/third_party/android_cra...
third_party/android_crazy_linker/BUILD.gn:7: "src/include",
prefer one-line for short one-entry arrays:

include_dirs = [ "src/include" ]

https://codereview.chromium.org/640363002/diff/200001/third_party/android_cra...
third_party/android_crazy_linker/BUILD.gn:46: direct_dependent_configs = [
":crazy_config" ]
s/direct_dependent_configs/public_configs/

[simonb (inactive), 2014-10-14 10:21:06]

lgtm

[cjhopman, 2014-10-16 01:11:22]

cjhopman@chromium.org changed reviewers:
+ ben@chromium.org, yfriedman@chromium.org

[cjhopman, 2014-10-16 01:11:23]

yfriedman:     
    base/android/BUILD.gn
    content/shell/android/BUILD.gn
ben: TBRed for
    BUILD.gn
    base/base.gyp
    content/browser/BUILD.gn

https://codereview.chromium.org/640363002/diff/200001/third_party/android_cra...
File third_party/android_crazy_linker/BUILD.gn (right):

https://codereview.chromium.org/640363002/diff/200001/third_party/android_cra...
third_party/android_crazy_linker/BUILD.gn:7: "src/include",
On 2014/10/09 20:56:51, jamesr wrote:
> prefer one-line for short one-entry arrays:
> 
> include_dirs = [ "src/include" ]

Done.

https://codereview.chromium.org/640363002/diff/200001/third_party/android_cra...
third_party/android_crazy_linker/BUILD.gn:46: direct_dependent_configs = [
":crazy_config" ]
On 2014/10/09 20:56:51, jamesr wrote:
> s/direct_dependent_configs/public_configs/

Done.

[cjhopman, 2014-10-16 01:14:48]

Patchset #5 (id:330001) has been deleted

[cjhopman, 2014-10-16 01:15:05]

Patchset #5 (id:350001) has been deleted

[Yaron, 2014-10-17 19:16:27]

https://codereview.chromium.org/640363002/diff/310001/base/android/BUILD.gn
File base/android/BUILD.gn (right):

https://codereview.chromium.org/640363002/diff/310001/base/android/BUILD.gn#n...
base/android/BUILD.gn:3: assert(is_android && !is_android_webview_build)
will gn split out each value in the assert? if not, it's probably worth having
separate asserts.

https://codereview.chromium.org/640363002/diff/310001/base/android/BUILD.gn#n...
base/android/BUILD.gn:15: #'-finstrument-functions',
why is this commented out?

[cjhopman, 2014-10-17 21:05:20]

https://codereview.chromium.org/640363002/diff/310001/base/android/BUILD.gn
File base/android/BUILD.gn (right):

https://codereview.chromium.org/640363002/diff/310001/base/android/BUILD.gn#n...
base/android/BUILD.gn:15: #'-finstrument-functions',
On 2014/10/17 19:16:27, Yaron wrote:
> why is this commented out?

The "TODO(GYP)" above marks this as part of the GYP->GN conversion that still
needs to be done.

It's not clear how this will be implemented (note that it is removing a cflag).
This probably can't be done until we have the other parts of order profiling
supported (my understanding is that that is what uses the -finstrument-functions
cflag).

[cjhopman, 2014-10-17 21:05:37]

https://codereview.chromium.org/640363002/diff/310001/base/android/BUILD.gn
File base/android/BUILD.gn (right):

https://codereview.chromium.org/640363002/diff/310001/base/android/BUILD.gn#n...
base/android/BUILD.gn:3: assert(is_android && !is_android_webview_build)
On 2014/10/17 19:16:27, Yaron wrote:
> will gn split out each value in the assert? if not, it's probably worth having
> separate asserts.

Done.

[Yaron, 2014-10-17 21:06:58]

On 2014/10/17 21:05:37, cjhopman wrote:
> https://codereview.chromium.org/640363002/diff/310001/base/android/BUILD.gn
> File base/android/BUILD.gn (right):
> 
>
https://codereview.chromium.org/640363002/diff/310001/base/android/BUILD.gn#n...
> base/android/BUILD.gn:3: assert(is_android && !is_android_webview_build)
> On 2014/10/17 19:16:27, Yaron wrote:
> > will gn split out each value in the assert? if not, it's probably worth
having
> > separate asserts.
> 
> Done.

lgtm

[cjhopman, 2014-10-17 21:11:07]

The CQ bit was checked by cjhopman@chromium.org

[cjhopman, 2014-10-17 21:11:10]

The CQ bit was unchecked by cjhopman@chromium.org

[cjhopman, 2014-10-17 21:11:32]

The CQ bit was checked by cjhopman@chromium.org

[commit-bot: I haz the power, 2014-10-17 21:12:10]

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

[commit-bot: I haz the power, 2014-10-17 22:08:28]

Committed patchset #5 (id:370001)

[commit-bot: I haz the power, 2014-10-17 22:09:10]

Patchset 5 (id:??) landed as
https://crrev.com/aac45ed811d1f0fde1d8189c81813deebc8c9682
Cr-Commit-Position: refs/heads/master@{#300172}