On Fri, Apr 23, 2021 at 2:34 PM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>
> When walking trees using traverse_trees_recursive() and
> unpack_callback(), we must not attempt to walk into a sparse directory
> entry. There are no index entries within that directory to compare to
> the tree object at that position, so skip over the entries of that tree.
>
> This code is used in many places, so the only way to test it is to start
> removing the command_requres_full_index option from one builtin at a
> time and carefully test that its use of unpack_trees() behaves correctly
> with a sparse-index. Such tests will be added by later changes.
>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
> unpack-trees.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 3af797093095..67777570f829 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1256,6 +1256,7 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str
> struct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, };
> struct unpack_trees_options *o = info->data;
> const struct name_entry *p = names;
> + unsigned unpack_tree = 1;
>
> /* Find first entry with a real name (we could use "mask" too) */
> while (!p->mode)
> @@ -1297,12 +1298,16 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str
> }
> }
> src[0] = ce;
> +
> + if (S_ISSPARSEDIR(ce->ce_mode))
> + unpack_tree = 0;
> }
> break;
> }
> }
>
> - if (unpack_nondirectories(n, mask, dirmask, src, names, info) < 0)
> + if (unpack_tree &&
> + unpack_nondirectories(n, mask, dirmask, src, names, info) < 0)
> return -1;
>
> if (o->merge && src[0]) {
> @@ -1332,7 +1337,8 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str
> }
> }
>
> - if (traverse_trees_recursive(n, dirmask, mask & ~dirmask,
> + if (unpack_tree &&
> + traverse_trees_recursive(n, dirmask, mask & ~dirmask,
> names, info) < 0)
> return -1;
> return mask;
> --
> gitgitgadget
The splitting of the previous patch looks really good here too, and
the variable rename makes it flow nicely. Looking good.
