Detect and warn on missing vcvarsall.bat

Detect and warn on missing vcvarsall.bat

VC++ 2015 defaults to *not* installing the C++ toolchain. Several
developers have failed to notice this and then hit cryptic errors when
building Chrome. This detects the problem and gives a more informative
error message for gn builds.

crrev.com/1832593002 does this for gyp builds.

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

[brucedawson, 2016-03-23 19:20:14]

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

[brucedawson, 2016-03-23 19:20:14]

A bit more hardening to more politely detect what is a somewhat common mistake.
GYP change will land separately.

[scottmg, 2016-03-23 19:30:13]

lgtm

https://codereview.chromium.org/1822383002/diff/1/build/toolchain/win/setup_t...
File build/toolchain/win/setup_toolchain.py (right):

https://codereview.chromium.org/1822383002/diff/1/build/toolchain/win/setup_t...
build/toolchain/win/setup_toolchain.py:121:
os.environ['GYP_MSVS_OVERRIDE_PATH'],
+indent here, they're arguments to join() not normpath().

https://codereview.chromium.org/1822383002/diff/1/build/toolchain/win/setup_t...
build/toolchain/win/setup_toolchain.py:125: % script_path)
% on previous line

[brucedawson, 2016-03-23 19:35:22]

https://codereview.chromium.org/1822383002/diff/1/build/toolchain/win/setup_t...
File build/toolchain/win/setup_toolchain.py (right):

https://codereview.chromium.org/1822383002/diff/1/build/toolchain/win/setup_t...
build/toolchain/win/setup_toolchain.py:121:
os.environ['GYP_MSVS_OVERRIDE_PATH'],
On 2016/03/23 19:30:13, scottmg wrote:
> +indent here, they're arguments to join() not normpath().

I'm unclear about what is desired here. Indenting to join( pushes the line
length beyond 80. Just indent an extra few characters? "git cl format" doesn't
help unfortunately :-(

https://codereview.chromium.org/1822383002/diff/1/build/toolchain/win/setup_t...
build/toolchain/win/setup_toolchain.py:125: % script_path)
On 2016/03/23 19:30:13, scottmg wrote:
> % on previous line

So, increase indent by one? Done.

[scottmg, 2016-03-23 19:41:53]

https://codereview.chromium.org/1822383002/diff/20001/build/toolchain/win/set...
File build/toolchain/win/setup_toolchain.py (right):

https://codereview.chromium.org/1822383002/diff/20001/build/toolchain/win/set...
build/toolchain/win/setup_toolchain.py:121:
os.environ['GYP_MSVS_OVERRIDE_PATH'],
Yeah, I wish we had a good python formatter that wasn't terribly slow and didn't
reformat the whole file. :(

I just meant that this alignment makes it look like you're passing 3 arguments
to normpath(). I would indent os.environ[...] and 'VC/...' 4 spaces to make it
clear they're subordinate to join(), and there's only one argument being passed
to normpath().

https://codereview.chromium.org/1822383002/diff/20001/build/toolchain/win/set...
build/toolchain/win/setup_toolchain.py:125: % script_path)
Sorry, I should have been clearer. The binary operator goes on the preceding
line, not the same line as the RHS of the binary operator, so:

raise Exception('%s is missing - make sure VC++ tools are installed.' %
    script_path)

but you had script_path aligned properly before, it should be under the '
following Exception(.

[brucedawson, 2016-03-23 19:48:25]

Description was changed from

==========
Detect and warn on missing vcvarsall.bat

VC++ 2015 defaults to *not* installing the C++ toolchain. Several
developers have failed to notice this and then hit cryptic errors when
building Chrome. This detects the problem and gives a more informative
error message for gn builds.

A separate change can do this for gyp builds.

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

to

==========
Detect and warn on missing vcvarsall.bat

VC++ 2015 defaults to *not* installing the C++ toolchain. Several
developers have failed to notice this and then hit cryptic errors when
building Chrome. This detects the problem and gives a more informative
error message for gn builds.

crrev.com/1832593002 does this for gyp builds.

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

[brucedawson, 2016-03-23 19:48:51]

https://codereview.chromium.org/1822383002/diff/20001/build/toolchain/win/set...
File build/toolchain/win/setup_toolchain.py (right):

https://codereview.chromium.org/1822383002/diff/20001/build/toolchain/win/set...
build/toolchain/win/setup_toolchain.py:121:
os.environ['GYP_MSVS_OVERRIDE_PATH'],
On 2016/03/23 19:41:53, scottmg wrote:
> Yeah, I wish we had a good python formatter that wasn't terribly slow and
didn't
> reformat the whole file. :(
> 
> I just meant that this alignment makes it look like you're passing 3 arguments
> to normpath(). I would indent os.environ[...] and 'VC/...' 4 spaces to make it
> clear they're subordinate to join(), and there's only one argument being
passed
> to normpath().

Done.

https://codereview.chromium.org/1822383002/diff/20001/build/toolchain/win/set...
build/toolchain/win/setup_toolchain.py:125: % script_path)
On 2016/03/23 19:41:53, scottmg wrote:
> Sorry, I should have been clearer. The binary operator goes on the preceding
> line, not the same line as the RHS of the binary operator, so:
> 
> raise Exception('%s is missing - make sure VC++ tools are installed.' %
>     script_path)
> 
> but you had script_path aligned properly before, it should be under the '
> following Exception(.

Done.

[scottmg, 2016-03-23 19:50:26]

best-python-indenting-ever-lgtm

hopefully it even works

[brucedawson, 2016-03-23 19:51:25]

> hopefully it even works

Hmmm. Good point. Now that the indenting is perfect I should test it.

#notsureifserious

[brucedawson, 2016-03-23 19:52:40]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2016-03-23 19:52:57]

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

[commit-bot: I haz the power, 2016-03-23 20:32:57]

Description was changed from

==========
Detect and warn on missing vcvarsall.bat

VC++ 2015 defaults to *not* installing the C++ toolchain. Several
developers have failed to notice this and then hit cryptic errors when
building Chrome. This detects the problem and gives a more informative
error message for gn builds.

crrev.com/1832593002 does this for gyp builds.

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

to

==========
Detect and warn on missing vcvarsall.bat

VC++ 2015 defaults to *not* installing the C++ toolchain. Several
developers have failed to notice this and then hit cryptic errors when
building Chrome. This detects the problem and gives a more informative
error message for gn builds.

crrev.com/1832593002 does this for gyp builds.

BUG=440500

Committed: https://crrev.com/13856ebb7cb3b75dd9d40bb394ffa567ab4a524a
Cr-Commit-Position: refs/heads/master@{#382915}
==========

[commit-bot: I haz the power, 2016-03-23 20:32:57]

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