Don't disable incremental linking for chrome.dll in component

Don't disable incremental linking for chrome.dll in component

R=brettw@chromium.org
BUG=636468
Committed: https://crrev.com/3a480c53288755252ea84683e6166e1619d85bb1
Cr-Commit-Position: refs/heads/master@{#411259}

[scottmg, 2016-08-10 23:11:36]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 23:12:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[brettw, 2016-08-10 23:29:48]

lgtm

https://codereview.chromium.org/2239533002/diff/1/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2239533002/diff/1/chrome/BUILD.gn#newcode354
chrome/BUILD.gn:354: # This is a large module that can't do incremental linking
in some cases.
80 col rapping.

[scottmg, 2016-08-10 23:40:39]

https://codereview.chromium.org/2239533002/diff/1/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2239533002/diff/1/chrome/BUILD.gn#newcode354
chrome/BUILD.gn:354: # This is a large module that can't do incremental linking
in some cases.
On 2016/08/10 23:29:48, brettw (ping on IM after 24h) wrote:
> 80 col rapping.

I really need to fix that in gn format some day. Some penalty doesn't quite work
or something, I forget.

[scottmg, 2016-08-10 23:40:54]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2016-08-10 23:40:55]

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

[commit-bot: I haz the power, 2016-08-10 23:42:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-08-11 00:17:45]

The CQ bit was unchecked by scottmg@chromium.org

[scottmg, 2016-08-11 00:21:30]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2016-08-11 00:22:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-08-11 00:22:42]

On 2016/08/10 23:42:20, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

Oh, wait. Do you know what this is about?

https://cs.chromium.org/chromium/src/build/config/win/BUILD.gn?rcl=0&l=339

https://codereview.chromium.org/1027543003/

In particular, why is it disabled for x86 only? Should we change that to be

  if (symbol_level > 0 && !is_component_build) {

instead of changing it in chrome/BUILD.gn? Or is the limit actually different
for x64 ilks?

[brettw, 2016-08-11 00:28:26]

I messed with the incremental linking flags so much...

The only way to know is to try a full build with that config.

[scottmg, 2016-08-11 00:30:58]

On 2016/08/11 00:28:26, brettw (ping on IM after 24h) wrote:
> I messed with the incremental linking flags so much...
> 
> The only way to know is to try a full build with that config.

OK, I'll let this go in and try that separately then.

[commit-bot: I haz the power, 2016-08-11 01:49:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-11 01:49:20]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[scottmg, 2016-08-11 03:03:03]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2016-08-11 03:03:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 04:07:39]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-11 04:09:57]

Description was changed from

==========
Don't disable incremental linking for chrome.dll in component

R=brettw@chromium.org
BUG=636468
==========

to

==========
Don't disable incremental linking for chrome.dll in component

R=brettw@chromium.org
BUG=636468

Committed: https://crrev.com/3a480c53288755252ea84683e6166e1619d85bb1
Cr-Commit-Position: refs/heads/master@{#411259}
==========

[commit-bot: I haz the power, 2016-08-11 04:09:58]

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

[Nico, 2017-08-16 20:07:25]

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

[Nico, 2017-08-16 20:07:26]

https://codereview.chromium.org/2239533002/diff/20001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2239533002/diff/20001/chrome/BUILD.gn#newcode359
chrome/BUILD.gn:359: }
This basically reverts
https://codereview.chromium.org/1930073002/diff/20001/chrome/BUILD.gn

I think the idea here was that default_large_module_incremental_linking does the
checking so that its clients don't have to do any itself. (But then it didn't do
it quite right, https://chromium-review.googlesource.com/c/617764/). Once
https://chromium-review.googlesource.com/c/617764/ is in, I think this should be
reverted -- does that sound right?

[scottmg, 2017-08-16 20:22:57]

https://codereview.chromium.org/2239533002/diff/20001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2239533002/diff/20001/chrome/BUILD.gn#newcode359
chrome/BUILD.gn:359: }
On 2017/08/16 20:07:26, Nico wrote:
> This basically reverts
> https://codereview.chromium.org/1930073002/diff/20001/chrome/BUILD.gn
> 
> I think the idea here was that default_large_module_incremental_linking does
the
> checking so that its clients don't have to do any itself. (But then it didn't
do
> it quite right, https://chromium-review.googlesource.com/c/617764/). Once
> https://chromium-review.googlesource.com/c/617764/ is in, I think this should
be
> reverted -- does that sound right?

Yeah.