On Mon, 20 Jul 2020 at 17:25, Elijah Newren <newren@xxxxxxxxx> wrote:
>
> 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@xxxxxxxxx>
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@xxxxxxxxxxxxxx>
Reviewed-by: Elijah Newren <newren@xxxxxxxxx>
Signed-off-by: Martin Ågren <martin.agren@xxxxxxxxx>
---
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
