Fix copy steps for win32 ninja (issue 9422025)

[sco...@chromium.org]

Reviewers: Nico,

Message:
Few more tests pass.

Description:
- Another normpath (slightly concerned about this one for other platforms?
but
it makes the build files a lot more consistent on Windows)
- Fix copy rule for windows (mklink directory if file fails)

Please review this at https://chromiumcodereview.appspot.com/9422025/

SVN Base: http://gyp.googlecode.com/svn/trunk/

Affected files:
M pylib/gyp/generator/ninja.py

Index: pylib/gyp/generator/ninja.py
===================================================================
--- pylib/gyp/generator/ninja.py (revision 1211)
+++ pylib/gyp/generator/ninja.py (working copy)
@@ -213,7 +213,7 @@
path = path.replace(INTERMEDIATE_DIR,
os.path.join(product_dir or '', int_dir))

- return path
+ return os.path.normpath(path)

def ExpandRuleVariables(self, path, root, dirname, source, ext, name):
path = path.replace(generator_default_variables['RULE_INPUT_ROOT'],
root)
@@ -1183,7 +1183,7 @@
master_ninja.rule(
'copy',
description='COPY $in $out',
- command='mklink /h $out $in >nul')
+ command='cmd /c mklink /h $out $in >nul || mklink /h /j $out $in
>nul')
else:
master_ninja.rule(
'copy',

[tha...@chromium.org]

lgtm

https://chromiumcodereview.appspot.com/9422025/

[ev...@chromium.org]

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

https://chromiumcodereview.appspot.com/9422025/diff/1002/pylib/gyp/generator/ninja.py#newcode205
pylib/gyp/generator/ninja.py:205: path = path.replace(PRODUCT_DIR +
'\\', '')
The two above lines can now be removed, as the one below this point will
result in "./" which normpath will clean up.

https://chromiumcodereview.appspot.com/9422025/

[tha...@chromium.org]

According to go/gypbot, this broke ninja tests on mac.

https://chromiumcodereview.appspot.com/9422025/

[sco...@chromium.org]

On 2012/02/21 01:11:27, Nico wrote:
> According to go/gypbot, this broke ninja tests on mac.

Looks like some actions have an empty "" in their actions: [] as the first
argument. normpath converts that to '.' which happens to work for scripts
but
it's a bit weird anyway. (I'm actually not sure that that's the failure on
Mac,
but I'll revert for now and see if there's a better place to do the
normalize.)

https://chromiumcodereview.appspot.com/9422025/

[Evan Martin]

On Mon, Feb 20, 2012 at 6:08 PM, <sco...@chromium.org> wrote:
> On 2012/02/21 01:11:27, Nico wrote:
>>
>> According to go/gypbot, this broke ninja tests on mac.
>
> Looks like some actions have an empty "" in their actions: [] as the first
> argument. normpath converts that to '.' which happens to work for scripts
> but
> it's a bit weird anyway. (I'm actually not sure that that's the failure on
> Mac,
> but I'll revert for now and see if there's a better place to do the
> normalize.)

If the input files are wrong, another path is to fix the input files
(and make gyp sanity-check this problem so it doesn't reoccur).
See r1176 for an example.

[Scott Graham]

Thanks. I think the failure this was causing on Windows is just a sorta-bug in the input files so I'll add a sanity check for that case.

The cause of the Mac failure is that "./action.sh" is passed in to ExpandSpecial, but normpath removes the leading ./ so it's no longer a suitable argv[0] for a rule.

I was thinking maintaining a relative path maybe shouldn't be ExpandSpecial's job, but it's hard for the rule code to do it instead because it doesn't know if argv[0] is supposed to be in PATH or is a command in the current directory.

So, I guess it could either be a simple slash-replace on win32, since we don't need or want the path modification in this case?

[Scott Graham]

On Mon, Feb 20, 2012 at 10:12 PM, Evan Martin <ev...@chromium.org> wrote:

Fixed with sanity-check as suggested: https://chromiumcodereview.appspot.com/9451001/

I can't see any reason why that was there or should be allowed, but if anyone knows some magic historical reason please pipe up.