make-w32
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Bug: make fails to execute .BAT files with space in the path, with S


From: Eli Zaretskii
Subject: Re: Bug: make fails to execute .BAT files with space in the path, with SHELL=cmd.exe
Date: Thu, 02 May 2013 19:50:25 +0300

> Date: Thu, 2 May 2013 10:26:50 +0200
> From: Erik Carstensen <address@hidden>
> Cc: address@hidden
> 
> I think this fix deserves an item in NEWS.

Yes, I already did that.

> The change breaks makefiles like
> this one (succeeds with 3.82, fails with master):
> 
> SHELL=cmd.exe
> X=C:\Program\ Files
> default: $(X)
>        ls.exe $(X)

Make on Windows cannot support escaping with backslash when the shell
is cmd.exe.  And Make in general doesn't support file names with
whitespace too well, you need to introduce blanks via variables, for
it to work reliably.  The only place where it works well is in the
command line itself, but not in variables and certainly not in
dependency lists.

> It does not! With the following makefile:
> 
> SHELL=cmd.exe
> default:
>         "a b\x.bat"
> 
> I get this most of the time:
> make/gnumake.exe --debug=j
> "a b\x.bat"
> CreateProcess(NULL,"C:\cygwin\tmp\a b\x.bat",...)
> (and make exits with code 127)
> 
> Sometimes (1 in 5), the bat file is actually executed before make exits
> with 127.

That's strange, I cannot reproduce this.  I get 100% success and zero
exit code all the time.

> The machine runs Windows 7 Enterprise.

I tried both on Windows 7 and on XP SP3 here, and it works on both for
me.  I build Make with MinGW32, while yours is a 64-bit build AFAIU,
so maybe this could explain that -- perhaps there's some quirk in
64-bit executables.  Or maybe it's something specific to your system.
Can you test on another box?

It is also strange that it sometimes works.  Perhaps there's some
subtle Heisenbug, but I cannot debug something I cannot reproduce.
And the command you should under --debug=j looks correct to me.

If you can run Make under a debugger and look what's going on there,
I'd appreciate.

> I don't like calling CreateProcess with NULL as the first parameter - it will
> actually first look for cmd.exe in cwd, with interesting results.

You mean, if there's (an incompatible) cmd.exe in the current
directory?  Why else would that be "interesting"?  Windows always
looks in cwd for a program, that's nothing new.

> Using COMSPEC directly is more likely to be correct. The
> CreateProcess documentation suggests:
> 
>   To run a batch file, you must start the command interpreter; set
>   lpApplicationName to cmd.exe and set lpCommandLine to the following
>   arguments: /c plus the name of the batch file.
> 
> Unfortunately this says nothing at all about how to quote the batch file name
> and its arguments.

Exactly!  And that's why this way is impractical.  Even CreateProcess
itself manages to trigger some bugs related with excess quoting of
cmd.exe commands -- do you really think someone like me, who is not a
Microsoft employee, can do that better?

> This means that the right way to run a batch file with multiple arguments is 
> to
> run cmd.exe with the command line
> 
>   /c "batch_file arg1 arg2 ..."
> 
> where any or all of batch_file and the arguments may be quoted in turn, such 
> as
> 
>   /c ""my file.bat" "one arg" "another arg""
> 
> since the outermost quotes are stripped away. This seems to be what
> CreateProcess does when given a NULL first argument, although it also adds a
> dummy "cmd" before the /c. This can be seen quite clearly in Process Monitor.

You forget the use cases where you need to pass quotes into the
programs invoked by the batch files.  That complicates things quite a
bit.  And we already know that the above won't work anyway: it's the
original reason of the problem that you reported.  Going there means
going back; I don't want to do that.  Using NULL fixed the problem
entirely in my testing.  I hope you could find why it fails for you,
perhaps there's some other more subtle problem.

Anyway, I think I will install that patch soon, because I cannot see
anything wrong with it.

Thanks.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]