Fully qualify path to cl.exe for ninja windows, and support *_wrapper from environment

ninja windows: Fully qualify path to cl.exe

Part of upgrading to ninja trunk with more efficient dependency
tracking.

Does path search for correct cl.exe at generation time rather than
relying on the path being set properly.

Supports CC_wrapper/CXX_wrapper for goma/distcc. *_wrapper variables
are now supported from the environment on all platforms.

R=thakis@chromium.org
BUG=chromium:232421
Committed: https://code.google.com/p/gyp/source/detail?r=1612

[scottmg, 2013-04-15 21:37:37]

ukai, any suggestions on a way to get a full path to cl.exe at gyp time for
goma?

[Nico, 2013-04-16 23:02:59]

Looks fine.

Since it's part 1/N, maybe have a tracking bug?

ukai wrote
https://groups.google.com/forum/?fromgroups=#!topic/gyp-developer/L7o1Qzo7PqM
for making goma more automatic for ninja/make, but apparently that issue no
longer exists? :-/ Not sure what happened to it. Something like that could work
for windows goma too (it added a "compiler prefix" gyp thingy) maybe.

(also, wrap cl description)

https://codereview.chromium.org/13950005/diff/1/pylib/gyp/generator/ninja.py
File pylib/gyp/generator/ninja.py (left):

https://codereview.chromium.org/13950005/diff/1/pylib/gyp/generator/ninja.py#...
pylib/gyp/generator/ninja.py:1433: master_ninja.variable('use_dep_database',
'1')
Was this ever used for anything? (I guess it was used in your fork, but not in
version we launched?)

[scottmg, 2013-04-17 17:01:46]

On 2013/04/16 23:02:59, Nico wrote:
> Looks fine.
> 
> Since it's part 1/N, maybe have a tracking bug?

Done, http://crbug.com/232421

> 
> ukai wrote
>
https://groups.google.com/forum/?fromgroups=#%21topic/gyp-developer/L7o1Qzo7PqM
> for making goma more automatic for ninja/make, but apparently that issue no
> longer exists? :-/ Not sure what happened to it. Something like that could
work
> for windows goma too (it added a "compiler prefix" gyp thingy) maybe.

Ah, turns out that actually landed under another CL and name.
https://codereview.chromium.org/12300015 

Updated the patch the support CC_wrapper on Windows. Setting this in
include.gypi works as expected now:

{
  'make_global_settings': [
   ['CC_wrapper', 'c:\\goma\\goma-win\\gomacc.exe'],
   ['CXX_wrapper', 'c:\\goma\\goma-win\\gomacc.exe'],
  ],
}


Unfortunately, local goma setup will need to be modified (in particular on bots
I assume) as the recommended instructions use 'CC' override, not 'CC_wrapper'.
I'm not sure if there's a way to finesse that.


> (also, wrap cl description)

Done.

> 
> https://codereview.chromium.org/13950005/diff/1/pylib/gyp/generator/ninja.py
> File pylib/gyp/generator/ninja.py (left):
> 
>
https://codereview.chromium.org/13950005/diff/1/pylib/gyp/generator/ninja.py#...
> pylib/gyp/generator/ninja.py:1433: master_ninja.variable('use_dep_database',
> '1')
> Was this ever used for anything? (I guess it was used in your fork, but not in
> version we launched?)

Yeah, it was in a ninja binary that depot_tools had until it got unforked to
upstream ninja, but it was never on by default (had to also set a magical
environment variable). I don't think anyone other than me used it.

[scottmg, 2013-04-17 17:39:06]

> Updated the patch the support CC_wrapper on Windows. Setting this in
> include.gypi works as expected now:
> 
> {
>   'make_global_settings': [
>    ['CC_wrapper', 'c:\\goma\\goma-win\\gomacc.exe'],
>    ['CXX_wrapper', 'c:\\goma\\goma-win\\gomacc.exe'],
>   ],
> }
> 

Bah, include.gypi works, but apparently not chromium.gyp_env. Our various ways
of environment + settings files is a bit of a quagmire. Updated to make that
worse (support _wrapper variables from the environment), so that

{
  'CC_wrapper': 'C:\\goma\\goma-win\\gomacc.exe',
  'CXX_wrapper': 'C:\\goma\\goma-win\\gomacc.exe',
}

in chromium.gyp_env works... but only after
https://codereview.chromium.org/14093007 :(

[Nico, 2013-04-17 17:53:59]

lgtm

Maybe make it a little more clear that this adds support for *_wrapper coming
from the env on all platforms.

[scottmg, 2013-04-17 17:55:53]

On 2013/04/17 17:53:59, Nico wrote:
> lgtm
> 
> Maybe make it a little more clear that this adds support for *_wrapper coming
> from the env on all platforms.

You mean in the cl description, right? Done.

[scottmg, 2013-04-17 18:13:52]

Committed patchset #3 manually as r1612.

[Reid Kleckner, 2013-05-08 18:55:41]

https://codereview.chromium.org/13950005/diff/7001/pylib/gyp/generator/ninja.py
File pylib/gyp/generator/ninja.py (right):

https://codereview.chromium.org/13950005/diff/7001/pylib/gyp/generator/ninja....
pylib/gyp/generator/ninja.py:1417: if key.endswith('_wrapper'):
This doesn't seem to work for me, because Python stores the env var keys as all
uppercase, leading this comparison to always fail.  :(

Adding .lower() before .endswith might work.

Also, before this change (I think?) setting CC directly used to work.

[scottmg, 2013-05-08 19:23:33]

https://codereview.chromium.org/13950005/diff/7001/pylib/gyp/generator/ninja.py
File pylib/gyp/generator/ninja.py (right):

https://codereview.chromium.org/13950005/diff/7001/pylib/gyp/generator/ninja....
pylib/gyp/generator/ninja.py:1417: if key.endswith('_wrapper'):
On 2013/05/08 18:55:42, Reid Kleckner wrote:
> This doesn't seem to work for me, because Python stores the env var keys as
all
> uppercase, leading this comparison to always fail.  :(
> 
> Adding .lower() before .endswith might work.

Sorry. Are you using cygwin Python perhaps?

> 
> Also, before this change (I think?) setting CC directly used to work.

[scottmg, 2013-05-08 19:29:31]

On 2013/05/08 19:23:33, scottmg wrote:
>
https://codereview.chromium.org/13950005/diff/7001/pylib/gyp/generator/ninja.py
> File pylib/gyp/generator/ninja.py (right):
> 
>
https://codereview.chromium.org/13950005/diff/7001/pylib/gyp/generator/ninja....
> pylib/gyp/generator/ninja.py:1417: if key.endswith('_wrapper'):
> On 2013/05/08 18:55:42, Reid Kleckner wrote:
> > This doesn't seem to work for me, because Python stores the env var keys as
> all
> > uppercase, leading this comparison to always fail.  :(
> > 
> > Adding .lower() before .endswith might work.
> 
> Sorry. Are you using cygwin Python perhaps?
> 

Hmm, nope that's not it. Feel free to revert if it's causing problems for you.