git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] add--interactive: fix external command invocation on Windows

From
Duy Nguyen <pclouds@gmail.com>
Date
Sep 4, 2013, 12:02 UTC
Message-ID
<CACsJy8DbkNz8rTFDV3u9j3+mmzF0JVdO+TNVBX6uRQPCCfis_g@mail.gmail.com>
In-Reply-To
<5226E03F.208@viscovery.net>
On Wed, Sep 4, 2013 at 2:24 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
Show 25 quoted lines
> From: Johannes Sixt <j6t@kdbg.org>
>
> Back in 21e9757e (Hack git-add--interactive to make it work with
> ActiveState Perl, 2007-08-01), the invocation of external commands was
> changed to use qx{} on Windows. The rationale was that the command
> interpreter on Windows is not a POSIX shell, but rather Windows's CMD.
> That patch was wrong to include 'msys' in the check whether to use qx{}
> or not: 'msys' identifies MSYS perl as shipped with Git for Windows,
> which does not need the special treatment; qx{} should be used only with
> ActiveState perl, which is identified by 'MSWin32'.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> Am 9/2/2013 13:56, schrieb Duy Nguyen:
>> On Mon, Sep 2, 2013 at 5:41 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> Which features do we lose on Windows with the previous patch and this fixup?
>>
>> New pathspec magic :(glob), :(literal) and :(icase). You can still use
>> them via --*-pathspecs or equivalent env variables. You just can't
>> enable them per individual pathspec.
>
> I think this here is the correct "solution" rather than the special cases
> for Windows that you proposed. ActiveState perl users would still suffer,
> but that is a problem of the run_cmd_pipe implementation, which would
> need a smarter method to quote the arguments before it feeds them to qx{}.

I admit this is cleaner than my patch. Let's see any users using ActiveState Perl yell up, then we'll deal with the problem.

Show 19 quoted lines
>  git-add--interactive.perl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 75a991f..5156384 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -169,7 +169,7 @@ my %patch_modes = (
>  my %patch_mode_flavour = %{$patch_modes{stage}};
>
>  sub run_cmd_pipe {
> -       if ($^O eq 'MSWin32' || $^O eq 'msys') {
> +       if ($^O eq 'MSWin32') {
>                 my @invalid = grep {m/[":*]/} @_;
>                 die "$^O does not support: @invalid\n" if @invalid;
>                 my @args = map { m/ /o ? "\"$_\"": $_ } @_;
> --
> 1.8.4.1549.gc059550.dirty
>
-- 
Duy
Previous: Johannes Sixt
Message 10 of 10 in “nd/magic-pathspec exposes breakage in git-add--interactive on Windows”
  1. Johannes SixtAug 29, 2013
  2. Duy NguyenAug 29, 2013
  3. Alex RiesenAug 29, 2013
  4. Turn off pathspec magic on "{checkout,reset,add} -p" on native Windows buildsNguyễn Thái Ngọc Duy, Sep 1, 2013
  5. Johannes SixtSep 2, 2013
  6. Duy NguyenSep 2, 2013
  7. Johannes SixtSep 2, 2013
  8. Duy NguyenSep 2, 2013
  9. add--interactive: fix external command invocation on WindowsJohannes Sixt, Sep 4, 2013
  10. Duy NguyenSep 4, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.