git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Elijah Newren <newren@gmail.com>
Date
Dec 4, 2021, 05:45 UTC
Message-ID
<CABPp-BHHqLvnMbR5Q+UmfTy7GPy4vz73zZdNc99PogCyxTFJHw@mail.gmail.com>
In-Reply-To
<pull.1092.git.1638586534.gitgitgadget@gmail.com>

On Fri, Dec 3, 2021 at 6:55 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 31 quoted lines
>
> 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>
Previous: Derrick Stolee via GitGitGadgetNext: Derrick Stolee via GitGitGadget
Message 6 of 9 in “Sparse Index: fix a checkout bug with deep sparse-checkout patterns”
  1. 0/2 Sparse Index: fix a checkout bug with deep sparse-checkout patternsDerrick Stolee via GitGitGadget, Dec 4, 2021
  2. 2/2 unpack-trees: use traverse_path instead of nameDerrick Stolee via GitGitGadget, Dec 4, 2021
  3. Elijah NewrenDec 4, 2021
  4. Derrick StoleeDec 6, 2021
  5. 1/2 t1092: add deeper changes during a checkoutDerrick Stolee via GitGitGadget, Dec 4, 2021
  6. Elijah NewrenDec 4, 2021
  7. 0/2 Sparse Index: fix a checkout bug with deep sparse-checkout patternsDerrick Stolee via GitGitGadget, Dec 6, 2021
  8. 1/2 t1092: add deeper changes during a checkoutDerrick Stolee via GitGitGadget, Dec 6, 2021
  9. 2/2 unpack-trees: use traverse_path instead of nameDerrick Stolee via GitGitGadget, Dec 6, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.