Make gyp/win32 compatible with upstream ninja.

Make gyp/win32 compatible with upstream ninja.

BUG=147451
Committed: https://code.google.com/p/gyp/source/detail?r=1501

[scottmg, 2012-09-17 19:25:25]

https://chromiumcodereview.appspot.com/10907263/diff/1/pylib/gyp/win_tool.py
File pylib/gyp/win_tool.py (left):

https://chromiumcodereview.appspot.com/10907263/diff/1/pylib/gyp/win_tool.py#...
pylib/gyp/win_tool.py:182: def ExecClWrapper(self, arch, depname, *args):
fyi, this function is unused, when you commit this you can just delete this
entirely

[scottmg, 2012-09-17 19:27:59]

https://chromiumcodereview.appspot.com/10907263/diff/1/pylib/gyp/generator/ni...
File pylib/gyp/generator/ninja.py (left):

https://chromiumcodereview.appspot.com/10907263/diff/1/pylib/gyp/generator/ni...
pylib/gyp/generator/ninja.py:1426: # Template for compile commands mostly shared
between compiling files
with the rest of this restored, lgtm

[Evan Martin, 2012-09-17 19:33:29]

LGTM

I wrote the identical patch locally this weekend while testing this out.


It's way slower than your depdb stuff.  :~(

https://chromiumcodereview.appspot.com/10907263/diff/4001/pylib/gyp/generator...
File pylib/gyp/generator/ninja.py (right):

https://chromiumcodereview.appspot.com/10907263/diff/4001/pylib/gyp/generator...
pylib/gyp/generator/ninja.py:1428: cc_template = ('ninja -t msvc -r . -o $out -e
$arch '
BTW, the reason this auto-appends the .d to the -o flag is because we needed the
.d file to mention the original output (without any .d).  It feels pretty gross
to me still that -o $out means "write to $out.d" though.  Suggestions welcome.

[scottmg, 2012-09-17 19:36:47]

On 2012/09/17 19:33:29, Evan Martin wrote:
> LGTM
> 
> I wrote the identical patch locally this weekend while testing this out.
> 
> 
> It's way slower than your depdb stuff.  :~(

Shucks. I was just building now to test times and hoping that
faster-machine-and-ssd might make it close-enough.

> 
>
https://chromiumcodereview.appspot.com/10907263/diff/4001/pylib/gyp/generator...
> File pylib/gyp/generator/ninja.py (right):
> 
>
https://chromiumcodereview.appspot.com/10907263/diff/4001/pylib/gyp/generator...
> pylib/gyp/generator/ninja.py:1428: cc_template = ('ninja -t msvc -r . -o $out
-e
> $arch '
> BTW, the reason this auto-appends the .d to the -o flag is because we needed
the
> .d file to mention the original output (without any .d).  It feels pretty
gross
> to me still that -o $out means "write to $out.d" though.  Suggestions welcome.

Yeah, was wondering at that, it feels weird to me too. But not needing to parse
to get the name for the .d to get the basename is probably a good enough reason.

Robbie suggested a flag to set the extension so at least is was explicit (and
default to -d), but... meh. I think just leave it, and perhaps add a runtime
check that the user doesn't include .d on the -o.

[scottmg, 2012-09-17 19:38:11]

On 2012/09/17 19:36:47, scottmg wrote:
> default to -d)

er, default to ".d" obviously.

[Evan Martin, 2012-09-17 19:38:47]

On 2012/09/17 19:36:47, scottmg wrote:
> I think just leave it, and perhaps add a runtime
> check that the user doesn't include .d on the -o.

I got this wrong at first.  You quickly discover it because all the .d files
(which are written out as foo.o.d.d) are then considered missing on the next
build, prompting a needless rebuild (due to your recent patch), and -d explain
then quickly points you at the problem.

[scottmg, 2012-09-17 19:45:25]

On 2012/09/17 19:38:47, Evan Martin wrote:
> On 2012/09/17 19:36:47, scottmg wrote:
> > I think just leave it, and perhaps add a runtime
> > check that the user doesn't include .d on the -o.
> 
> I got this wrong at first.  You quickly discover it because all the .d files
> (which are written out as foo.o.d.d) are then considered missing on the next
> build, prompting a needless rebuild (due to your recent patch), and -d explain
> then quickly points you at the problem.

Yup, me too, twice actually! :) At first I left it as .dl, then switched to .d
thinking it was appending because it wasn't already .d, and then the third time
I looked at the code. Oh well.

[iannucci, 2012-09-17 21:53:20]

Test fails are due to not having ninja in depot_tools yet. Have another cl for
that here: https://chromiumcodereview.appspot.com/10905312/

On 2012/09/17 19:45:25, scottmg wrote:
> On 2012/09/17 19:38:47, Evan Martin wrote:
> > On 2012/09/17 19:36:47, scottmg wrote:
> > > I think just leave it, and perhaps add a runtime
> > > check that the user doesn't include .d on the -o.
> > 
> > I got this wrong at first.  You quickly discover it because all the .d files
> > (which are written out as foo.o.d.d) are then considered missing on the next
> > build, prompting a needless rebuild (due to your recent patch), and -d
explain
> > then quickly points you at the problem.
> 
> Yup, me too, twice actually! :) At first I left it as .dl, then switched to .d
> thinking it was appending because it wasn't already .d, and then the third
time
> I looked at the code. Oh well.

[scottmg, 2012-09-18 16:13:20]

Looks like we're good to go on gyp roll now.