Tweaks to vs_toolchain to simplify it, and support gn VS 2015

Tweaks to vs_toolchain to simplify it, and support gn VS 2015

This change updates and simplifies vs_toolchain.py so that it supports
VS 2015 in gn builds. The gn and gyp code now shares DLL copying code so
that they should more naturally stay in sync in the future. The
out_release_nacl64 directory now gets a PGO DLL that it didn't used to.

The six warnings globally disabled are globally disabled in gyp builds
as well. The equivalent warnings (narrowing conversions and variable
shadowing) are disabled in gcc/clang so there is no bug for enabling
them at this time.

BUG=440500
Committed: https://crrev.com/aaff8dcb6ee993a18ab6e7236711839e72e81cc3
Cr-Commit-Position: refs/heads/master@{#360988}

[brucedawson, 2015-11-20 22:30:44]

Description was changed from

==========
Tweaks to vs_toolchain to simplify, and support gn VS 2015

BUG=440500
==========

to

==========
Tweaks to vs_toolchain to simplify it, and support gn VS 2015

This change updates and simplifies vs_toolchain.py so that it supports
VS 2015 in gn builds. The gn and gyp code now shares DLL copying code so
that they should more naturally stay in sync in the future. The
out_release_nacl64 directory now gets a PGO DLL that it didn't used to.

The six warnings globally disabled are globally disabled in gyp builds
as well. The equivalent warnings (narrowing conversions and variable
shadowing) are disabled in gcc/clang so there is no bug for enabling
them at this time.

BUG=440500
==========

[brucedawson, 2015-11-20 22:30:44]

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

[brucedawson, 2015-11-20 22:31:53]

PTAL.

Unlike my 64-bit fixes I'm going to try to land small changes as I get them
polished, in hopes that this will go more smoothly. This one has to be first.

[scottmg, 2015-11-20 23:04:42]

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

[scottmg, 2015-11-20 23:04:43]

lgtm

https://codereview.chromium.org/1468643002/diff/40001/build/vs_toolchain.py
File build/vs_toolchain.py (right):

https://codereview.chromium.org/1468643002/diff/40001/build/vs_toolchain.py#n...
build/vs_toolchain.py:114: def _CopyRuntime(target_dir, source_dir, target_cpu,
debug=False):
It's OK to pass debug=True/False at the callsite if you want make the calls more
clear, but don't specify a default value here.

https://codereview.chromium.org/1468643002/diff/40001/build/vs_toolchain.py#n...
build/vs_toolchain.py:139: assert(0)
This is non-idiomatic in Python (because they're compiled out when optimizations
are on, but no one really knows whether they're running a .py script optimized
or not).

Maybe `raise ValueError(target_cpu)`.

[Dirk Pranke, 2015-11-20 23:16:33]

lgtm as well. 

+mcgrathr just for the odd FYI about minor tweaks to how the vs toolchains are
set up (which you probably don't care about but might).

https://codereview.chromium.org/1468643002/diff/40001/build/vs_toolchain.py
File build/vs_toolchain.py (right):

https://codereview.chromium.org/1468643002/diff/40001/build/vs_toolchain.py#n...
build/vs_toolchain.py:139: assert(0)
On 2015/11/20 23:04:43, scottmg wrote:
> This is non-idiomatic in Python (because they're compiled out when
optimizations
> are on, but no one really knows whether they're running a .py script optimized
> or not).
> 
> Maybe `raise ValueError(target_cpu)`.

or `raise NotImplementedError("Unexpected target_cpu value:" + target_cpu)` or
some such.

[Roland McGrath, 2015-11-20 23:19:52]

On 2015/11/20 23:16:33, Dirk Pranke wrote:
> +mcgrathr just for the odd FYI about minor tweaks to how the vs toolchains are
> set up (which you probably don't care about but might).

Nothing here looks like it would affect what I've been doing, except that I
already uglified some NaCl code to avoid the narrowing conversion warnings and I
could have been saved the trouble if this had been done earlier.

[brucedawson, 2015-11-21 00:28:01]

Suggestions addressed.

> I already uglified some NaCl code to avoid the narrowing conversion warnings

Well, those warnings are required by C++ 11 and I think they offer some value -
I've seen them catch some gnarly unwanted conversions. So, I think we'll
eventually want to fix them.

https://codereview.chromium.org/1468643002/diff/40001/build/vs_toolchain.py
File build/vs_toolchain.py (right):

https://codereview.chromium.org/1468643002/diff/40001/build/vs_toolchain.py#n...
build/vs_toolchain.py:114: def _CopyRuntime(target_dir, source_dir, target_cpu,
debug=False):
On 2015/11/20 23:04:43, scottmg wrote:
> It's OK to pass debug=True/False at the callsite if you want make the calls
more
> clear, but don't specify a default value here.

Oops - I forgot to remove that. Good catch. I do like the ability to name the
parameters when passing True/False since it is otherwise a non-obvious
anti-pattern.

https://codereview.chromium.org/1468643002/diff/40001/build/vs_toolchain.py#n...
build/vs_toolchain.py:139: assert(0)
On 2015/11/20 23:16:33, Dirk Pranke wrote:
> On 2015/11/20 23:04:43, scottmg wrote:
> > This is non-idiomatic in Python (because they're compiled out when
> optimizations
> > are on, but no one really knows whether they're running a .py script
optimized
> > or not).
> > 
> > Maybe `raise ValueError(target_cpu)`.
> 
> or `raise NotImplementedError("Unexpected target_cpu value:" + target_cpu)` or
> some such.

Done.

[brucedawson, 2015-11-21 00:28:49]

The CQ bit was checked by brucedawson@chromium.org

[brucedawson, 2015-11-21 00:28:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/1468643002/#ps80001
(title: "Remove default value for argument")

[commit-bot: I haz the power, 2015-11-21 00:30:46]

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

[commit-bot: I haz the power, 2015-11-21 02:21:59]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-11-21 02:22:58]

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