Re: [PATCH] Turn off pathspec magic on "{checkout,reset,add} -p" on native Windows builds
Am 9/2/2013 11:30, schrieb Duy Nguyen:
Show 56 quoted lines
> 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