On Tue, Apr 04, 2017 at 11:16:56AM +0200, Patrick Steinhardt wrote: > Previous to commit 5d8f084a5 (pathspec: simpler logic to prefix original > pathspec elements, 2017-01-04), we were always using the computed > `match` variable to perform pathspec matching whenever > `PATHSPEC_PREFIX_ORIGIN` is set. This is for example useful when passing > the parsed pathspecs to other commands, as the computed `match` may > contain a pathspec relative to the repository root. The commit changed > this logic to only do so when we do have an actual prefix and when > literal pathspecs are deactivated. > > But this change may actually break some commands which expect passed > pathspecs to be relative to the repository root. One such case is `git > add --patch`, which now fails when using relative paths from a > subdirectory. For example if executing "git add -p ../foo.c" in a > subdirectory, the `git-add--interactive` command will directly pass > "../foo.c" to `git-ls-files`. As ls-files is executed at the > repository's root, the command will notice that "../foo.c" is outside > the repository and fail. > > Fix the issue by again using the computed `match` variable when > `PATHSPEC_PREFIX_ORIGIN` is set and global literal pathspecs are > deactivated. Note that in contrast to previous behavior, we will now > always call `prefix_magic` regardless of whether a prefix is actually > set. But this is the right thing to do: when the `match` variable has > been resolved to the repository's root, it will be set to an empty > string. When passing the empty string directly to other commands, it > will result in a warning regarding deprecated empty pathspecs. By always > adding the prefix magic, we will end up with at least the string > ":(prefix:0)" and thus avoid the warning. > > Signed-off-by: Patrick Steinhardt <ps@xxxxxx> > --- Just a short reminder on this patch, as I haven't seen it or v1 being picked up by the What's Cooking reports. Am I simply being too eager or was this an oversight? Thanks Patrick > > This is the second version of [1]. It fixes a bug catched by > Brandon when the pathspec is resolved to the empty string and > improves the test a bit to actually catch this issue. > > [1]: http://public-inbox.org/git/1556910880cfce391bdca2d8f0cbcb8c71371691.1491206540.git.ps@xxxxxx/T/#u > > > pathspec.c | 2 +- > t/t3701-add-interactive.sh | 22 ++++++++++++++++++++++ > 2 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/pathspec.c b/pathspec.c > index 303efda83..3079a817a 100644 > --- a/pathspec.c > +++ b/pathspec.c > @@ -505,7 +505,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags, > * original. Useful for passing to another command. > */ > if ((flags & PATHSPEC_PREFIX_ORIGIN) && > - prefixlen && !get_literal_global()) { > + !get_literal_global()) { > struct strbuf sb = STRBUF_INIT; > > /* Preserve the actual prefix length of each pattern */ > diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh > index f9528fa00..2ecb43a61 100755 > --- a/t/t3701-add-interactive.sh > +++ b/t/t3701-add-interactive.sh > @@ -436,6 +436,28 @@ test_expect_success 'add -p handles globs' ' > test_cmp expect actual > ' > > +test_expect_success 'add -p handles relative paths' ' > + git reset --hard && > + > + echo base >relpath.c && > + git add "*.c" && > + git commit -m relpath && > + > + echo change >relpath.c && > + mkdir -p subdir && > + git -C subdir add -p .. 2>error <<-\EOF && > + y > + EOF > + > + test_must_be_empty error && > + > + cat >expect <<-\EOF && > + relpath.c > + EOF > + git diff --cached --name-only >actual && > + test_cmp expect actual > +' > + > test_expect_success 'add -p does not expand argument lists' ' > git reset --hard && > > -- > 2.12.2
Attachment:
signature.asc
Description: PGP signature