Re: [PATCH] Turn off pathspec magic on "{checkout,reset,add} -p" on native Windows builds

[Johannes Sixt <j.sixt@viscovery.net>]

Am 9/2/2013 11:30, schrieb Duy Nguyen:
> On Mon, Sep 02, 2013 at 08:42:18AM +0200, Johannes Sixt wrote:
>> Am 9/1/2013 4:08, schrieb Nguyễn Thái Ngọc Duy:
>>> git-add--interactive.perl rejects arguments with colons in 21e9757
>>> (Hack git-add--interactive to make it work with ActiveState Perl -
>>> 2007-08-01). Pathspec magic starts with a colon, so it won't work if
>>> these pathspecs are passed to git-add--interactive.perl running with
>>> ActiveState Perl. Make sure we only pass plain paths in this case.
>>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>> ---
>>>  Johannes, can you check the test suite passes for you with this
>>>  patch? I assume that Cygwin Perl behaves differently and does not hit
>>>  this limit. So I keep the special case to GIT_WINDOWS_NATIVE only.
>>>  I'll resend the patch with a few others on the same topic if it works
>>>  for you.
>>
>> It does not help. The error in git-add--interactive is avoided, but the
>> failure in t2016-checkout-patch.sh is now:
>>
>> 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
>>
>> No changes.
>> not ok 13 - path limiting works: foo inside dir
>>
>> and the same "No changes." happens in t7105-reset-patch.sh
> 
> Right. Because I got rid of ':(prefix)foo' form but I passed 'foo'
> instead of 'dir/foo'. How about this on top?
> 
> -- 8< --
> diff --git a/builtin/add.c b/builtin/add.c
> index 3402239..a138360 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -257,9 +257,15 @@ int run_add_interactive(const char *revision, const char *patch_mode,
>  	if (revision)
>  		args[ac++] = revision;
>  	args[ac++] = "--";
> +#ifdef GIT_WINDOWS_NATIVE
> +	GUARD_PATHSPEC(pathspec, PATHSPEC_FROMTOP);
> +	for (i = 0; i < pathspec->nr; i++)
> +		args[ac++] = pathspec->items[i].match;
> +#else
>  	for (i = 0; i < pathspec->nr; i++)
>  		/* pass original pathspec, to be re-parsed */
>  		args[ac++] = pathspec->items[i].original;
> +#endif
>  
>  	status = run_command_v_opt(args, RUN_GIT_CMD);
>  	free(args);
> -- 8< --

With this patch, the two tests pass.

Which features do we lose on Windows with the previous patch and this fixup?

-- Hannes