From: Johannes Sixt <j.sixt@viscovery.net>
To: Alex Riesen <raa.lkml@gmail.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: nd/magic-pathspec exposes breakage in git-add--interactive on Windows
Date: Thu, 29 Aug 2013 08:54:34 +0200 [thread overview]
Message-ID: <521EF02A.2020300@viscovery.net> (raw)
With nd/magic-pathspec I get the following failure on Windows in
t2016-checkout-patch.sh:
expecting success:
set_state dir/foo work head &&
# the third n is to get out in case it mistakenly does not apply
(echo y; echo n; echo n) | (cd dir && git checkout -p foo) &&
verify_saved_state bar &&
verify_state dir/foo head head
==== xx.
==== 1d.
==== 10e.
msys does not support: :(prefix:4)dir/foo
not ok 13 - path limiting works: foo inside dir
The error message 'msys does not support...' originates from this function
in git-add--interactive.perl (which is invoked from checkout -p):
sub run_cmd_pipe {
if ($^O eq 'MSWin32' || $^O eq 'msys') {
my @invalid = grep {m/[":*]/} @_;
die "$^O does not support: @invalid\n" if @invalid;
my @args = map { m/ /o ? "\"$_\"": $_ } @_;
return qx{@args};
} else {
my $fh = undef;
open($fh, '-|', @_) or die;
return <$fh>;
}
}
It looks like on Windows we disallow arguments that contain double-quote,
colon, or asterisk, and otherwise wrap arguments in double-quotes if they
contain space. Then pass them through qx{}, which I can only guess what it
does.
This code was introduced in 21e9757e (Hack git-add--interactive to make it
work with ActiveState Perl, 2007-08-01). The commit message has the
general statement "It wont work for arguments with special characters
(like ", : or *)), which I do not know how to interpret: Does ActiveState
Perl not work with special charactoers, or Windows? Because the latter is
definitely not true.
Can we be more permissive in the 'my @invalid' check without breaking
ActiveState Perl?
BTW, there is a similar failure in t7105-reset-patch.sh, which invokes
'git reset -p', but I haven't investigate further.
-- Hannes
next reply other threads:[~2013-08-29 6:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-29 6:54 Johannes Sixt [this message]
2013-08-29 9:47 ` nd/magic-pathspec exposes breakage in git-add--interactive on Windows Duy Nguyen
2013-08-29 13:01 ` Alex Riesen
2013-09-01 2:08 ` [PATCH] Turn off pathspec magic on "{checkout,reset,add} -p" on native Windows builds Nguyễn Thái Ngọc Duy
2013-09-02 6:42 ` Johannes Sixt
2013-09-02 9:30 ` Duy Nguyen
2013-09-02 10:41 ` Johannes Sixt
2013-09-02 11:56 ` Duy Nguyen
2013-09-04 7:24 ` [PATCH] add--interactive: fix external command invocation on Windows Johannes Sixt
2013-09-04 12:02 ` Duy Nguyen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=521EF02A.2020300@viscovery.net \
--to=j.sixt@viscovery.net \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.com \
--cc=raa.lkml@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.