On Wed, Jan 15, 2020 at 12:25 PM Derrick Stolee via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > > b9660c1 (dir: fix checks on common prefix directory, 2019-12-19) > modified the way pathspecs are handled when handling a directory > during "git clean -f <path>". While this improved the behavior > for known test breakages, it also regressed in how the clean > command handles cleaning a specified file. > > Add a test case that demonstrates this behavior. This test passes > before b9660c1 then fails after. > > Helped-by: Kevin Willford <Kevin.Willford@xxxxxxxxxxxxx> > Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > --- > clean: demonstrate a bug with pathspecs > > While integrating v2.25.0 into the microsoft/git fork, one of our VFS > for Git functional tests started failing. Looking into it, the only > possible place could have been where one of our integration points with > the virtualfilesystem hook was moved by c5c4edd (dir: break part of > read_directory_recursive() out for reuse, 2019-12-10) and then used in > the following two commits. > > By reverting these two commits, we stopped the failure, but it took a > while before figuring out that it was a regression in Git and not a > failure in our integration to the new logic. Thanks to Kevin Willford > for producing a test case. > > b9660c1 (dir: fix checks on common prefix directory, 2019-12-19) is the > culprit, so this patch is based on that. If rebased to c5c4edd, then the > test passes. > > As for actually fixing this regression, I don't know how. This code is > pretty dense and I don't have a firm grasp of what is happening in both > b9660c1 and the following 777b420 (dir: synchronize tread_leading_path() > and read_directory_recursive()). Elijah is CC'd in case he still has > context on this area. > > Thanks, -Stolee > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-526%2Fderrickstolee%2Fclean-bug-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-526/derrickstolee/clean-bug-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/526 > > t/t7300-clean.sh | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh > index 6e6d24c1c3..782e125c89 100755 > --- a/t/t7300-clean.sh > +++ b/t/t7300-clean.sh > @@ -737,4 +737,13 @@ test_expect_success MINGW 'handle clean & core.longpaths = false nicely' ' > test_i18ngrep "too long" .git/err > ' > > +test_expect_failure 'clean untracked paths by pathspec' ' > + git init untracked && > + mkdir untracked/dir && > + echo >untracked/dir/file.txt && > + git -C untracked clean -f dir/file.txt && > + ls untracked/dir >actual && > + test_must_be_empty actual > +' > + > test_done > > base-commit: b9670c1f5e6b98837c489a03ac0d343d30e08505 > -- > gitgitgadget Is there an inverted phrase corresponding to "the gift that keeps on giving", something like "the punishment that keeps on punishing"? If so, it would be a very appropriate description of dir.c. Yeah, I still have context. I even think I've got an idea about what the fix might be, though with dir.c my ideas about fixes usually just serve as starting points for debugging before I find the real fix. I'll try to dig in.
