Re: [PATCH v3 4/8] t7300: add testcase showing unnecessary traversal into ignored directory

From: Elijah Newren <newren@gmail.com>
Date: May 11, 2021, 17:45 UTC
Message-ID: <CABPp-BFV6R9GNeZVbiKx_Tazq+6LuBNCzLcsEq63MfG5YD0hFQ@mail.gmail.com>
In-Reply-To: <xmqq7dk7rwcc.fsf@gitster.g>

On Sun, May 9, 2021 at 10:28 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 35 quoted lines

>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > +test_expect_failure 'avoid traversing into ignored directories' '
> > +     test_when_finished rm -f output error trace.* &&
> > +     test_create_repo avoid-traversing-deep-hierarchy &&
> > +     (
> > +             cd avoid-traversing-deep-hierarchy &&
> > +
> > +             mkdir -p untracked/subdir/with/a &&
> > +             >untracked/subdir/with/a/random-file.txt &&
> > +
> > +             GIT_TRACE2_PERF="$TRASH_DIRECTORY/trace.output" \
> > +             git clean -ffdxn -e untracked
> > +     ) &&
> > +
> > +     grep data.*read_directo.*visited trace.output \
> > +             | cut -d "|" -f 9 >trace.relevant &&
> > +     cat >trace.expect <<-EOF &&
> > +      directories-visited:1
> > +      paths-visited:4
>
> Are the origins of '1' and '4' trivially obvious to those who are
> reading the test, or do these deserve comments?
>
> We create an empty test repository, go there and create a untracked/
> hierarchy with a junk file, and tell "clean" that 'untracked' is
> "also" in the exclude pattern (but since there is no other exclude
> pattern, that is the only one), so everything underneath untracked/
> we have no reason to inspect.
>
> So, we do not visit 'untracked' directory.  Which ones do we visit?
> Is '1' coming from the top-level of the working tree '.'?  What
> about the number of visited paths '4' (the trace is stored outside
> this new test repository, so that's not it).

Good points. I'll make a comment that directories-visited:1 is about ensuring we only went into the toplevel directory, and I'll removed the paths-visited check.

But to answer your question, the paths we visit are '.', '..', '.git', and 'untracked', the first three of which we mark as path_none and don't recurse into because of special rules for those paths, and the last of which we shouldn't recurse into since it is ignored. There weren't any non-directory files in the toplevel directory, or those would also be included in the paths-visited count. A later patch in the series will fix the code to not recurse into the 'untracked' directory, fixing this test.