Use the /WX flag to have link warnings treated as errors.

Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can indicate dependency issues in the codebase, slow down builds, and otherwise just make it harder to find more relevant output in build logs.

This CL also adds exceptions to ignore certain linker warnings under specific configurations and targets.

BUG=659007, 676418, 676417, 654776
Committed: https://crrev.com/c868aae8f9b8d51461afff79064c75339e6ce30c
Cr-Commit-Position: refs/heads/master@{#440600}

[Wez, 2016-12-16 02:11:06]

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

[commit-bot: I haz the power, 2016-12-16 02:12:23]

Dry run: 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-12-16 03:42:50]

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

[commit-bot: I haz the power, 2016-12-16 03:42:51]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Wez, 2016-12-20 00:47:37]

Description was changed from

==========
Use the /WX flag to have link warnings treated as errors.

BUG=659007
==========

to

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

BUG=659007
==========

[Wez, 2016-12-20 00:47:37]

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

[Wez, 2016-12-20 00:48:40]

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

[commit-bot: I haz the power, 2016-12-20 00:49:10]

Dry run: 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-12-20 01:58:30]

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

[commit-bot: I haz the power, 2016-12-20 01:58:30]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Wez, 2016-12-21 00:15:14]

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

[commit-bot: I haz the power, 2016-12-21 00:15:57]

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

[Wez, 2016-12-21 00:18:55]

Description was changed from

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

BUG=659007
==========

to

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

In addition to treating link warnings and errors in all targets, via the
sdk_link and common_linker_setup base targets, explicit exceptions are added for
several targets which currently raise link warnings.

BUG=659007
==========

[Wez, 2016-12-21 00:21:42]

wez@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[Wez, 2016-12-21 00:21:43]

Bruce suggested having '/WX' expressed via a configuration target which would be
included in all targets by default, and explicitly omitted from problem targets.

This CL instead applies /WX universally, and suppresses individual warnings
under specific configurations of targets. This has the advantage of being
fine-grained, but the disadvantage of causing those warnings not even to be
logged.

Would be interested on your input on the two approaches :)

[commit-bot: I haz the power, 2016-12-21 00:54:54]

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

[commit-bot: I haz the power, 2016-12-21 00:54:55]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[DaleCurtis, 2016-12-21 00:55:54]

I would have guessed we already had this. I seem to recall failures on POSIX at
least. Seems fine to me.

[DaleCurtis, 2016-12-21 00:56:47]

lgtm

[Wez, 2016-12-21 02:18:28]

Description was changed from

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

In addition to treating link warnings and errors in all targets, via the
sdk_link and common_linker_setup base targets, explicit exceptions are added for
several targets which currently raise link warnings.

BUG=659007
==========

to

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

In addition to treating link warnings and errors in all targets, via the
sdk_link and common_linker_setup base targets, explicit exceptions are added for
several targets which currently raise link warnings.

BUG=659007, 654776
==========

[Wez, 2016-12-21 08:07:25]

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

[commit-bot: I haz the power, 2016-12-21 08:07:35]

Dry run: 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-12-21 08:40:49]

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

[commit-bot: I haz the power, 2016-12-21 08:40:50]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Wez, 2016-12-21 18:55:04]

Description was changed from

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

In addition to treating link warnings and errors in all targets, via the
sdk_link and common_linker_setup base targets, explicit exceptions are added for
several targets which currently raise link warnings.

BUG=659007, 654776
==========

to

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

BUG=659007, 654776
==========

[Wez, 2016-12-21 18:55:16]

Description was changed from

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

BUG=659007, 654776
==========

to

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

BUG=659007
==========

[Wez, 2016-12-21 19:14:17]

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

[commit-bot: I haz the power, 2016-12-21 19:14:58]

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

[Wez, 2016-12-21 19:15:45]

The CQ bit was unchecked by wez@chromium.org

[Wez, 2016-12-21 19:16:17]

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

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

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

[Wez, 2016-12-21 19:18:43]

brucedawson: PTAL - the exceptions could all be removed, thanks to a dependency
change in //ui/aura which just landed!

[commit-bot: I haz the power, 2016-12-21 20:09:19]

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

[commit-bot: I haz the power, 2016-12-21 20:09:20]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Wez, 2016-12-21 20:20:26]

Description was changed from

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

BUG=659007
==========

to

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

This CL also adds exceptions to ignore certain linker warnings under specific
configurations and targets.

BUG=659007, 676418, 676417, 654776
==========

[Wez, 2016-12-21 20:24:08]

On 2016/12/21 19:18:43, Wez wrote:
> brucedawson: PTAL - the exceptions could all be removed, thanks to a
dependency
> change in //ui/aura which just landed!

Scratch that; turns out that changing ldflags for a target doesn't force it to
re-link.

[Wez, 2016-12-21 21:41:18]

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

[commit-bot: I haz the power, 2016-12-21 21:41:57]

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

[Wez, 2016-12-21 22:02:50]

wez@chromium.org changed reviewers:
+ jochen@chromium.org

[Wez, 2016-12-21 22:02:51]

jochen: PTAL the build/ and third_party/WebKit changes, for OWNERS.

[brucedawson, 2016-12-21 22:22:53]

lgtm with one fix needed.

https://codereview.chromium.org/2581893002/diff/140001/build/config/win/BUILD.gn
File build/config/win/BUILD.gn (right):

https://codereview.chromium.org/2581893002/diff/140001/build/config/win/BUILD...
build/config/win/BUILD.gn:105: ldflags += [
This is wrong. "+=" should only be allowed if ldflags was set earlier - the
conditional setting of ldflags at line 97 doesn't count.

TL;DR - this needs to be "=" instead of "+=".

https://codereview.chromium.org/2581893002/diff/140001/build/config/win/BUILD...
build/config/win/BUILD.gn:251: ldflags += [
Odd that this is separate. For historical reasons I assume (since it is VC++
2015 only). Feel free to merge to clean this up. Or ignore.

[commit-bot: I haz the power, 2016-12-21 22:50:21]

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

[commit-bot: I haz the power, 2016-12-21 22:50:22]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Wez, 2016-12-22 00:03:08]

https://codereview.chromium.org/2581893002/diff/140001/build/config/win/BUILD.gn
File build/config/win/BUILD.gn (right):

https://codereview.chromium.org/2581893002/diff/140001/build/config/win/BUILD...
build/config/win/BUILD.gn:105: ldflags += [
On 2016/12/21 22:22:53, brucedawson wrote:
> This is wrong. "+=" should only be allowed if ldflags was set earlier - the
> conditional setting of ldflags at line 97 doesn't count.
> 
> TL;DR - this needs to be "=" instead of "+=".

Done.

https://codereview.chromium.org/2581893002/diff/140001/build/config/win/BUILD...
build/config/win/BUILD.gn:251: ldflags += [
On 2016/12/21 22:22:53, brucedawson wrote:
> Odd that this is separate. For historical reasons I assume (since it is VC++
> 2015 only). Feel free to merge to clean this up. Or ignore.

Done.

[Wez, 2016-12-22 00:03:24]

The CQ bit was checked by wez@chromium.org

[Wez, 2016-12-22 00:03:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, brucedawson@chromium.org
Link to the patchset: https://codereview.chromium.org/2581893002/#ps160001
(title: "Address review comments")

[commit-bot: I haz the power, 2016-12-22 00:03:49]

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-12-22 00:12:25]

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

[commit-bot: I haz the power, 2016-12-22 00:12:26]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[jam, 2016-12-22 00:25:36]

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

[jam, 2016-12-22 00:25:37]

lgtm

[jochen (gone - plz use gerrit), 2016-12-22 16:00:09]

lgtm

[Wez, 2016-12-22 19:18:34]

FWIW win_clang failure isn't Clang-specific, but we only build "all" (and
thereby build v8:cctest) under Clang.

I have a CL to suppress that warning in-flight, will land this as soon as V8
rolls with it.

[Wez, 2016-12-22 21:29:29]

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

[commit-bot: I haz the power, 2016-12-22 21:29:55]

Dry run: 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-12-22 23:12:50]

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

[commit-bot: I haz the power, 2016-12-22 23:12:51]

Dry run: This issue passed the CQ dry run.

[Wez, 2016-12-23 08:05:25]

The CQ bit was checked by wez@chromium.org

[Wez, 2016-12-23 08:05:25]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
jam@chromium.org, dalecurtis@chromium.org, brucedawson@chromium.org
Link to the patchset: https://codereview.chromium.org/2581893002/#ps180001
(title: "Move /WX setter into :compiler and respect fatal_linker_warnings")

[commit-bot: I haz the power, 2016-12-23 08:05:39]

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-12-23 08:09:08]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1482480325228130,
"parent_rev": "e8fe54fe0e3dd9a6f655fa4bef972ff752dece38", "commit_rev":
"504a932ae269517ad06ca69d4cf0fd507e02e508"}

[commit-bot: I haz the power, 2016-12-23 08:13:37]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1482480325228130,
"parent_rev": "881ffad4116adb90699669eb944d5551cf6ac06c", "commit_rev":
"b226c71fbd3c5d558caf79647e52f422eb262b27"}

[commit-bot: I haz the power, 2016-12-23 08:14:04]

Description was changed from

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

This CL also adds exceptions to ignore certain linker warnings under specific
configurations and targets.

BUG=659007, 676418, 676417, 654776
==========

to

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

This CL also adds exceptions to ignore certain linker warnings under specific
configurations and targets.

BUG=659007, 676418, 676417, 654776

Review-Url: https://codereview.chromium.org/2581893002
==========

[commit-bot: I haz the power, 2016-12-23 08:14:06]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-12-23 08:15:47]

Description was changed from

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

This CL also adds exceptions to ignore certain linker warnings under specific
configurations and targets.

BUG=659007, 676418, 676417, 654776

Review-Url: https://codereview.chromium.org/2581893002
==========

to

==========
Use the /WX flag to have link warnings treated as errors.

Link steps with warnings typically lead to working binaries, but warnings can
indicate dependency issues in the codebase, slow down builds, and otherwise just
make it harder to find more relevant output in build logs.

This CL also adds exceptions to ignore certain linker warnings under specific
configurations and targets.

BUG=659007, 676418, 676417, 654776

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

[commit-bot: I haz the power, 2016-12-23 08:15:49]

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