[Top][All Lists]

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

Re: quotearg.c's shell_quoting_style and MinGW

From: Eli Zaretskii
Subject: Re: quotearg.c's shell_quoting_style and MinGW
Date: Sun, 06 May 2012 19:41:54 +0300

> From: Bruno Haible <address@hidden>
> Cc: address@hidden, address@hidden
> Bcc: address@hidden
> Date: Sun, 06 May 2012 13:49:14 +0200
> diff3 receives the arguments as expected

Yes, of course: it receives them from the command line, correctly
> But on native Windows, there are two ways to execute commands:
>   a) through the Windows function CreateProcess(); this is what gnulib's
>      modules 'execute' and 'spawn-pipe' do.
>   b) through the cmd.exe program; this is what popen() and system() do.
> Both need different quoting.
>   - For a), you find an implementation in gnulib/lib/w32spawn.h.
>   - For b), it will be even more complicated, because arguments have to
>     be passed through a first CreateProcess() (from diff3 to cmd.exe),
>     are then subject to parsing by cmd.exe, and are then passed through
>     a second CreateProcess() call (from cmd.exe to diff).

In practice, the processing by cmd.exe doesn't change anything,
because cmd.exe almost always leaves the quotes unaltered.  The rare
situation where it doesn't (described by "cmd /?") will at most cause
removal of quotes around the 'diff' program itself, which is harmless,
because cmd will invoke it correctly, and we are always interested in
quoting the rest of arguments.

> Modifying quotearg.c, like you propose, would have an effect
>   - on quotearg, but as Paul said, quotearg's behaviour assumes a POSIX sh,

I don't see why quotearg should assume a Posix shell when compiled
into a MinGW program.  I'd like to hear what kinds of use cases
require that on MS-Windows.

>   - on sh-quote, but here too the comments in sh-quote.h indicate a POSIX
>     shell.

Comments should certainly be synchronized with code.  I didn't do that
because I wanted to hear agreement with the idea and/or instructions
for additional changes.

> In particular, there is another use of shell_quote() in src/diff.c,
> function option_list(). It is used here:
> $ mkdir foo ; echo Hi > foo/text
> $ mkdir gaz ; echo Hello > gaz/text
> $ diff -r -x foo\ bar foo gaz
> diff -r -x 'foo bar' foo/text gaz/text
> 1c1
> < Hi
> ---
> > Hello
> Your proposed patch would lead to a different 'diff' output on native Windows.
> Which is obviously undesirable.

A command line such as

  diff -r -x foo\ bar foo gaz

is impossible on MS-Windows, because whitespace cannot be
escape-protected with a backslash there, as you well know.  The
equivalent of this for Windows is

  diff -r -x "foo bar" foo gaz

which will produce the same result as on Posix platforms (unless I'm
missing something).

> I can see two possible actions:
> 1) Change the three locations in src/diff3.c, src/sdiff.c, src/util.c to
>    use the gnulib modules 'execute' and 'spawn-pipe'.
>    This will have the following advantages:
>      - It will fix the quoting problem at the right place.
>      - It will speed up the subprocess invocation, by bypassing cmd.exe.
>      - If you apply these changes also to the HAVE_WORKING_FORK code branch,
>        it will speed up the subprocess invocation also on Unix, by
>        bypassing /bin/sh.

Unfortunately, I lack knowledge and tools to make this happen.  I
don't know how to import a gnulib module into a program, and lack some
of the auto-tools to do that.  But if you or someone else provide a
patch, I can surely try it.


reply via email to

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