Re: [PATCH v2] cache-tree: invalidate i-t-a paths after generating trees
- From
Nguyen Thai Ngoc Duy <pclouds@gmail.com>
- Date
- Dec 10, 2012, 11:53 UTC
- Message-ID
- <CACsJy8CW1UGgQcgHa11XP71XZGaTxytrGmJmrtdNvy=N7cUyMw@mail.gmail.com>
- In-Reply-To
- <7v7goqcsdy.fsf@alter.siamese.dyndns.org>
On Mon, Dec 10, 2012 at 1:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 34 quoted lines
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> diff --git a/cache-tree.c b/cache-tree.c
>> index 28ed657..989a7ff 100644
>> --- a/cache-tree.c
>> +++ b/cache-tree.c
>> @@ -248,6 +248,7 @@ static int update_one(struct cache_tree *it,
>> int missing_ok = flags & WRITE_TREE_MISSING_OK;
>> int dryrun = flags & WRITE_TREE_DRY_RUN;
>> int i;
>> + int to_invalidate = 0;
>>
>> if (0 <= it->entry_count && has_sha1_file(it->sha1))
>> return it->entry_count;
>> @@ -324,7 +325,13 @@ static int update_one(struct cache_tree *it,
>> if (!sub)
>> die("cache-tree.c: '%.*s' in '%s' not found",
>> entlen, path + baselen, path);
>> - i += sub->cache_tree->entry_count - 1;
>> + i--; /* this entry is already counted in "sub" */
>> + if (sub->cache_tree->entry_count < 0) {
>> + i -= sub->cache_tree->entry_count;
>> + to_invalidate = 1;
>> + }
>> + else
>> + i += sub->cache_tree->entry_count;
>
> Hrm. update_one() is prepared to see a cache-tree whose entry count
> is zero (see the context lines in the previous hunk) and the
> invariant for the rest of the code is "if 0 <= entry_count, the
> cached tree is valid; invalid cache-tree has -1 in entry_count.
> More importantly, entry_count negated does not in general express
> how many entries there are in the subtree and does not tell us how
> many index entries to skip.Yeah I use entry_count for two different things here. In the previous (unsent) version of the patch I had "entry_count = -1" right after "i -= entry_count"
Show 5 quoted lines
>> + if (sub->cache_tree->entry_count < 0) {
>> + i -= sub->cache_tree->entry_count;
>> + sub->cache_tree->entry_count = -1;
>> + to_invalidate = 1;
>> + }which makes it clearer that the use of negative entry count is only valid within update_one. Then I changed my mind because it says 'negative means "invalid"' in cache-tree.h. So, put "entry_count = -1" back or just add another field to struct cache_tree for this?
-- Duy