Re: [PATCH 0/2] Sparse Index: fix a checkout bug with deep sparse-checkout patterns

[Elijah Newren <newren@gmail.com>]

On Fri, Dec 3, 2021 at 6:55 PM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> This week, we rolled out the sparse index to a large internal monorepo. We
> got two very similar bug reports that dealt with a strange error that
> involved the same set of paths. One was during git pull (pull was a red
> herring) and the other was git checkout. The git checkout case gave enough
> of a reproduction to debug deep into unpack-trees.c and find the problem.
>
> This bug dates back to 523506d (unpack-trees: unpack sparse directory
> entries, 2021-07-14). The reason we didn't hit this before is because it
> requires the following:
>
>  1. The sparse-checkout definition needs to have recursive inclusion of deep
>     folders (depth 3 or more).
>  2. Adjacent to those deep folders, we need a deep sparse directory entry
>     that receives changes.
>  3. In this particular repo, deep directories are only added to the
>     sparse-checkout in rare occasions and those adjacent folders are rarely
>     updated. They happened to update this week and hit our sparse index
>     dogfooders in surprising ways.
>
> The first patch adds a test that fails without the fix. It requires
> modifying our test data to make adjacent, deep sparse directory entries
> possible. It's a rather simple test after we have that data change.
>
> The second patch includes the actual fix. It's really just an error of not
> understanding the difference between the name and traverse_path members of
> the struct traverse_info structure. name only stores a single tree entry
> while traverse_path actually includes the full name from root. The method we
> are editing also has an additional struct name_entry that fills in the tree
> entry on top of the traverse_path, which explains how this worked to depth
> two, but not depth three.

Thanks for the detailed explanation.  I looked around for similar
potential problems elsewhere, but only noted that the comment at the
top of the function is also wrong and should be updated (as I
commented on Patch 2).  After you fix the comment similarly, feel free
to add my

Reviewed-by: Elijah Newren <newren@gmail.com>