[PATCH] dir: check pathspecs before returning `path_excluded`
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Jul 20, 2020, 18:45 UTC
- Message-ID
- <20200720184529.22449-1-martin.agren@gmail.com>
- In-Reply-To
- <CABPp-BH6xYb8a5hkV_68vbXhH4kApCxw0WG9oveXOk5zzU==1A@mail.gmail.com>
On Mon, 20 Jul 2020 at 17:25, Elijah Newren <newren@gmail.com> wrote:
Show 6 quoted lines
> > Awesome, thanks Andreas for the bisected report and Martin for finding > and fixing the bug. As for the reason that the old patch injected the > pathspec check between the path_excluded and the path_untracked cases, > that appears to me to just be "I'm good at making boneheaded > mistakes". Your changes here are the right fix.
Ok, here it is as a proper patch.
> Reviewed-by: Elijah Newren <newren@gmail.com>
Thanks. I've included your reviewed-by below. The log message is obviously new, but the diff is identical to what I posted earlier.
BTW, this bug first appeared in v2.27.0, so this is not a regression during the v2.28.0 cycle.
Martin
-- >8 -- In 95c11ecc73 ("Fix error-prone fill_directory() API; make it only return matches", 2020-04-01), we taught `fill_directory()`, or more specifically `treat_path()`, to check against any pathspecs so that we could simplify the callers.
But in doing so, we added a slightly-to-early return for the "excluded" case. We end up not checking the pathspecs, meaning we return `path_excluded` when maybe we should return `path_none`. As a result, `git status --ignored -- pathspec` might show paths that don't actually match "pathspec".
Move the "excluded" check down to after we've checked any pathspecs.
Reported-by: Andreas Schwab <schwab@linux-m68k.org> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- dir.c | 4 ++-- t/t7061-wtstatus-ignore.sh | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/dir.c b/dir.c index 1045cc9c6f..fe64be30ed 100644 --- a/dir.c +++ b/dir.c @@ -2209,13 +2209,13 @@ static enum path_treatment treat_path(struct dir_struct *dir, baselen, excluded, pathspec); case DT_REG: case DT_LNK: - if (excluded) - return path_excluded; if (pathspec && !match_pathspec(istate, pathspec, path->buf, path->len, 0 /* prefix */, NULL /* seen */, 0 /* is_dir */)) return path_none; + if (excluded) + return path_excluded; return path_untracked; } } diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh index e4cf5484f9..2f9bea9793 100755 --- a/t/t7061-wtstatus-ignore.sh +++ b/t/t7061-wtstatus-ignore.sh @@ -30,6 +30,31 @@ test_expect_success 'same with gitignore starting with BOM' ' test_cmp expected actual ' +test_expect_success 'status untracked files --ignored with pathspec (no match)' ' + git status --porcelain --ignored -- untracked/i >actual && + test_must_be_empty actual && + git status --porcelain --ignored -- untracked/u >actual && + test_must_be_empty actual +' + +test_expect_success 'status untracked files --ignored with pathspec (literal match)' ' + git status --porcelain --ignored -- untracked/ignored >actual && + echo "!! untracked/ignored" >expected && + test_cmp expected actual && + git status --porcelain --ignored -- untracked/uncommitted >actual && + echo "?? untracked/uncommitted" >expected && + test_cmp expected actual +' + +test_expect_success 'status untracked files --ignored with pathspec (glob match)' ' + git status --porcelain --ignored -- untracked/i\* >actual && + echo "!! untracked/ignored" >expected && + test_cmp expected actual && + git status --porcelain --ignored -- untracked/u\* >actual && + echo "?? untracked/uncommitted" >expected && + test_cmp expected actual +' + cat >expected <<\EOF ?? .gitignore ?? actual
-- 2.28.0.rc1