Thread (8 messages) 8 messages, 5 authors, 2020-01-16

Re: [PATCH] clean: demonstrate a bug with pathspecs

flat view

From: Elijah Newren <hidden>
Date: 2020-01-16 00:38:30

On Wed, Jan 15, 2020 at 12:25 PM Derrick Stolee via GitGitGadget
[off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Derrick Stolee <redacted>

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 [off-list ref]
Signed-off-by: Derrick Stolee <redacted>
---
    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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help