Implement arflags in the GN build.

Implement arflags in the GN build.

This flag was recently added in GN. This patch hooks it up to the toolchains
and moves some flags to use it that previously had to be hardcoded on the
tool command itself.

Remove concurrent_links variable from gcc_toolchain which was unused.

BUG=598599
Committed: https://crrev.com/c33bfab4e4dc2702bc6ccc36038defd850837bd2
Cr-Commit-Position: refs/heads/master@{#388554}

[brettw, 2016-04-19 17:29:21]

Description was changed from

==========
Implement arflags in the GN build.

This flag was recently added in GN. This patch hooks it up to the toolchains
and moves some flags to use it that previously had to be hardcoded on the
tool command itself.

BUG=1904473002
==========

to

==========
Implement arflags in the GN build.

This flag was recently added in GN. This patch hooks it up to the toolchains
and moves some flags to use it that previously had to be hardcoded on the
tool command itself.

BUG=598599
==========

[brettw, 2016-04-19 17:30:22]

brettw@chromium.org changed reviewers:
+ brucedawson@chromium.org

[brucedawson, 2016-04-19 18:36:08]

Aside: our concurrent links calculation doesn't work as well as it should, due
to highly variant linker memory requirements. Separate issue, but it would be
nice to address sometime, somehow.

https://codereview.chromium.org/1896163003/diff/1/build/toolchain/gcc_toolcha...
File build/toolchain/gcc_toolchain.gni (left):

https://codereview.chromium.org/1896163003/diff/1/build/toolchain/gcc_toolcha...
build/toolchain/gcc_toolchain.gni:11: # This value will be inherited in the
toolchain below.
Why is this being deleted? This seems unrelated and isn't mentioned in the
description.

[brettw, 2016-04-19 19:10:34]

Description was changed from

==========
Implement arflags in the GN build.

This flag was recently added in GN. This patch hooks it up to the toolchains
and moves some flags to use it that previously had to be hardcoded on the
tool command itself.

BUG=598599
==========

to

==========
Implement arflags in the GN build.

This flag was recently added in GN. This patch hooks it up to the toolchains
and moves some flags to use it that previously had to be hardcoded on the
tool command itself.

Remove concurrent_links variable from gcc_toolchain which was unused.

BUG=598599
==========

[brettw, 2016-04-19 19:10:43]

https://codereview.chromium.org/1896163003/diff/1/build/toolchain/gcc_toolcha...
File build/toolchain/gcc_toolchain.gni (left):

https://codereview.chromium.org/1896163003/diff/1/build/toolchain/gcc_toolcha...
build/toolchain/gcc_toolchain.gni:11: # This value will be inherited in the
toolchain below.
It was unused. I'll add a line to the CL description.

[brucedawson, 2016-04-19 20:59:32]

lgtm with one question.

https://codereview.chromium.org/1896163003/diff/1/build/config/compiler/BUILD.gn
File build/config/compiler/BUILD.gn (right):

https://codereview.chromium.org/1896163003/diff/1/build/config/compiler/BUILD...
build/config/compiler/BUILD.gn:424: ]
This arflags initialization is under "if (!is_debug &&
(allow_posix_link_time_opt || is_cfi) && !is_nacl)" whereas before the
equivalent check was under "if ((allow_posix_link_time_opt || is_cfi) &&
invoker.toolchain_os != "nacl")" - the "if (!is_debug" check is new.

Can you confirm that this is intentional or benign?

[brettw, 2016-04-19 21:02:19]

On 2016/04/19 20:59:32, brucedawson wrote:
> lgtm with one question.
> 
>
https://codereview.chromium.org/1896163003/diff/1/build/config/compiler/BUILD.gn
> File build/config/compiler/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1896163003/diff/1/build/config/compiler/BUILD...
> build/config/compiler/BUILD.gn:424: ]
> This arflags initialization is under "if (!is_debug &&
> (allow_posix_link_time_opt || is_cfi) && !is_nacl)" whereas before the
> equivalent check was under "if ((allow_posix_link_time_opt || is_cfi) &&
> invoker.toolchain_os != "nacl")" - the "if (!is_debug" check is new.
> 
> Can you confirm that this is intentional or benign?

I knew I did that. I decided that these conditions are likely meant to be the
same, and it doesn't make sense to run link-time-optimization for debug builds
anyway.

[brucedawson, 2016-04-19 21:48:35]

Okay, just double checking. Go for it.

[brettw, 2016-04-20 18:11:21]

merge

[brettw, 2016-04-20 18:11:21]

The CQ bit was checked by brettw@chromium.org

[brettw, 2016-04-20 18:11:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
brucedawson@chromium.org
Link to the patchset: https://codereview.chromium.org/1896163003/#ps20001
(title: "merge")

[commit-bot: I haz the power, 2016-04-20 18:11:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1896163003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1896163003/20001

[commit-bot: I haz the power, 2016-04-20 19:38:59]

Description was changed from

==========
Implement arflags in the GN build.

This flag was recently added in GN. This patch hooks it up to the toolchains
and moves some flags to use it that previously had to be hardcoded on the
tool command itself.

Remove concurrent_links variable from gcc_toolchain which was unused.

BUG=598599
==========

to

==========
Implement arflags in the GN build.

This flag was recently added in GN. This patch hooks it up to the toolchains
and moves some flags to use it that previously had to be hardcoded on the
tool command itself.

Remove concurrent_links variable from gcc_toolchain which was unused.

BUG=598599
==========

[commit-bot: I haz the power, 2016-04-20 19:39:00]

Committed patchset #2 (id:20001)

[jbudorick, 2016-04-21 00:20:55]

jbudorick@chromium.org changed reviewers:
+ jbudorick@chromium.org

[jbudorick, 2016-04-21 00:20:55]

Looking at this CL as being possibly responsible for linker failures on multiple
android bots, e.g.
https://build.chromium.org/p/chromium.android/builders/Android%20arm64%20Buil...

[jbudorick, 2016-04-21 01:02:08]

On 2016/04/21 00:20:55, jbudorick wrote:
> Looking at this CL as being possibly responsible for linker failures on
multiple
> android bots, e.g.
>
https://build.chromium.org/p/chromium.android/builders/Android%20arm64%20Buil...

... also e.g. both GPU bots:

https://uberchromegw.corp.google.com/i/chromium.gpu/builders/Android%20Debug%...
https://uberchromegw.corp.google.com/i/chromium.gpu/builders/Android%20Debug%...

speculatively reverting.

[jbudorick, 2016-04-21 01:02:56]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1903363003/ by jbudorick@chromium.org.

The reason for reverting is: variety of compile failures on Android bots that
have been switched to GN, see https://codereview.chromium.org/1896163003/#msg18.

[commit-bot: I haz the power, 2016-04-22 19:25:39]

Description was changed from

==========
Implement arflags in the GN build.

This flag was recently added in GN. This patch hooks it up to the toolchains
and moves some flags to use it that previously had to be hardcoded on the
tool command itself.

Remove concurrent_links variable from gcc_toolchain which was unused.

BUG=598599
==========

to

==========
Implement arflags in the GN build.

This flag was recently added in GN. This patch hooks it up to the toolchains
and moves some flags to use it that previously had to be hardcoded on the
tool command itself.

Remove concurrent_links variable from gcc_toolchain which was unused.

BUG=598599

Committed: https://crrev.com/c33bfab4e4dc2702bc6ccc36038defd850837bd2
Cr-Commit-Position: refs/heads/master@{#388554}
==========

[commit-bot: I haz the power, 2016-04-22 19:25:40]

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