Re: [PATCH 1/3] traverse_trees(): allow pruning with pathspec

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 30, 2011, 17:44 UTC
Message-ID: <7v39gi3ib6.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CACsJy8CmZv7Fuhw+m6X2CVO4vWLYFQNmStu1jJmhVXSi4mPxJQ@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

Show 8 quoted lines

>> @@ -376,16 +396,22 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
>>                        mask |= 1ul << i;
>>                        if (S_ISDIR(entry[i].mode))
>>                                dirmask |= 1ul << i;
>> +                       e = &entry[i];
>>                }
>
> Why? "e" is not used in that loop or anywhere after that.

This is trying to find _a_ surviving entry to be fed to prune_traversal() which in turn uses tree_entry_interesting(). At this point in the code, we are stuffing the entries of the same name from the input trees (and if one tree is missing an entry of the chosen name, it will have NULL there), so any non-empty entry would do. It corresponds to "first" but that is just a simple string and not a name_entry tree_entry_interesting() wants.

Show 14 quoted lines

>>                if (!mask)
>>                        break;
>> -               ret = info->fn(n, mask, dirmask, entry, info);
>> -               if (ret < 0) {
>> -                       error = ret;
>> -                       if (!info->show_all_errors)
>> -                               break;
>> +               interesting = prune_traversal(e, info, &base, interesting);
>> +               if (interesting < 0)
>> +                       break;
>
> I don't really understand this function to comment. But I guess when
> interesting < 0, we only skip info->fn() and assume it returns "mask"
> (its user unpack_callback() only returns either "mask" or -1).

We consume the entries we have used in merging (which is actually "everything in entry[] array" as info->fn() returns "mask" itself) by saying "update_extended_entry()" and the purpose of doing so is to prepare to process the next entry of the tree we are traversing.

When tree_entry_interesting() returns negative, it tells us "no, and no subsequent entries will be either", meaning "we are done with this tree". As we are done, there is nothing to prepare for the next round; we are not walking the remaining entries in the trees we are looking at. Is there any point in calling update_extended_entry() I am missing?