Re: [PATCH v3 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries

[Duy Nguyen <pclouds@gmail.com>]

On Tue, Jul 12, 2016 at 10:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> diff --git a/cache-tree.c b/cache-tree.c
>> index c2676e8..2d50640 100644
>> --- a/cache-tree.c
>> +++ b/cache-tree.c
>> @@ -380,6 +380,13 @@ static int update_one(struct cache_tree *it,
>>                       continue;
>>               }
>>
>> +             /*
>> +              * "sub" can be an empty tree if subentries are i-t-a.
>> +              */
>> +             if (sub && sub->cache_tree->entry_count < 0 &&
>> +                 !hashcmp(sha1, EMPTY_TREE_SHA1_BIN))
>> +                     continue;
>> +
>
> Looks sensible, except that it is unclear if it is correct to assume
> that "subentries are ita" always equals to "entry_count < 0" here.
> I _think_ it is OK, as the function itself does use the latter as
> the sign that it hit to_invalidate=1 case when it returns.

I had the same concern and double checked it. If one day we have a new
entry_count < 0 case that's not i-t-a, we'll need to refactor this
code a bit.
-- 
Duy