Re: [PATCH v3 07/12] unpack-trees: stop recursing into sparse directories

On 5/17/2021 10:06 PM, Elijah Newren wrote:

Sorry, I spoke too soon...

On Mon, May 17, 2021 at 7:03 PM Elijah Newren [off-list ref] wrote:

quoted

quoted

diff --git a/unpack-trees.c b/unpack-trees.c
index ef6a2b1c951c..703b0bdc9dfd 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c

@@ -1261,6 +1261,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;

Here, you set unpack_tree to 1.

quoted

quoted

        /* Find first entry with a real name (we could use "mask" too) */
        while (!p->mode)

@@ -1307,7 +1308,8 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str
                }
        }

-       if (unpack_nondirectories(n, mask, dirmask, src, names, info) < 0)
+       if (unpack_tree &&

You check it's value here...

quoted

quoted

+           unpack_nondirectories(n, mask, dirmask, src, names, info) < 0)
                return -1;

        if (o->merge && src[0]) {

@@ -1337,7 +1339,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 &&

...and here....

quoted

quoted

+                   traverse_trees_recursive(n, dirmask, mask & ~dirmask,
                                             names, info) < 0)
                        return -1;
                return mask;

but you never set unpack_tree to 0, so this is wasted effort and you
always recurse.  The previous iteration had a case where it'd set
unpack_tree to 0 in a certain case, but you deleted that code in this
version.  Why?

It appears that the changes to unpack-trees.c are no longer relevant,
and instead the changes to diff-lib.c (which were already out of place)
should instead be the focus. In fact, those changes to diff-lib.c can
be simplified and moved to path 10, so I will do that.

Thanks,
-Stolee