git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] cache-tree: invalidate i-t-a paths after writing trees

From
Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date
Nov 10, 2012, 11:04 UTC
Message-ID
<CACsJy8DEwpg0gY1o6gSB747W5fAYYxz97e-qnkQthSut3B7Eag@mail.gmail.com>
In-Reply-To
<7vy5ibouo4.fsf@alter.siamese.dyndns.org>
On Fri, Nov 9, 2012 at 6:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 29 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..30a8018 100644
>> --- a/cache-tree.c
>> +++ b/cache-tree.c
>> @@ -381,6 +381,9 @@ int cache_tree_update(struct cache_tree *it,
>>       i = update_one(it, cache, entries, "", 0, flags);
>>       if (i < 0)
>>               return i;
>> +     for (i = 0; i < entries; i++)
>> +             if (cache[i]->ce_flags & CE_INTENT_TO_ADD)
>> +                     cache_tree_invalidate_path(it, cache[i]->name);
>>       return 0;
>>  }
>
> I notice there is another special case for CE_REMOVE but there is
> nothing that adjusts the cache-tree for these entries in the current
> codebase.
>
> I suspect the original code before we (perhaps incorrectly) updated
> the code not to error out upon I-T-A entries was fine only because
> we do not attempt to fully populate the cache-tree during a merge in
> the unpack-trees codepath, which will mark the index entries that
> are to be removed with CE_REMOVE in the resulting index.
>
> The solution implemented with this patch will break if we start
> updating the cache tree after a successful merge in unpack-trees, I
> suspect.

I don't understand. I thought we handled CE_REMOVE correctly (i.e. no CE_REMOVE entries in cache tree even after a successful merge). Or should we keep CE_REMOVE in cache tree after a successful merge?

Show 7 quoted lines
> An alternative might be to add a "phoney" bit next to "used" in the
> cache_tree structure, mark the cache tree as phoney when we skip an
> entry marked as CE_REMOVE or CE_ITA, and make the postprocessing
> loop this patch adds aware of that bit, instead of iterating over
> the index entries; instead, it would recurse the resulting cache
> tree and invalidate parts of the tree that have subtrees with the
> "phoney" bit set, or something.
Yeah, that sounds better.
-- 
Duy
Previous: Junio C HamanoNext: Junio C Hamano
Message 5 of 16 in “Bug: write-tree corrupts intent-to-add index state”
  1. Jonathon MahNov 6, 2012
  2. Nguyen Thai Ngoc DuyNov 6, 2012
  3. cache-tree: invalidate i-t-a paths after writing treesNguyễn Thái Ngọc Duy, Nov 9, 2012
  4. Junio C HamanoNov 9, 2012
  5. Nguyen Thai Ngoc DuyNov 10, 2012
  6. Junio C HamanoNov 30, 2012
  7. Nguyen Thai Ngoc DuyNov 30, 2012
  8. cache-tree: invalidate i-t-a paths after generating treesNguyễn Thái Ngọc Duy, Dec 8, 2012
  9. Junio C HamanoDec 10, 2012
  10. Nguyen Thai Ngoc DuyDec 10, 2012
  11. Junio C HamanoDec 10, 2012
  12. 1/2 cache-tree: invalidate i-t-a paths after generating treesNguyễn Thái Ngọc Duy, Dec 13, 2012
  13. 2/2 cache-tree: remove dead i-t-a code in verify_cache()Nguyễn Thái Ngọc Duy, Dec 13, 2012
  14. Junio C HamanoDec 13, 2012
  15. Junio C HamanoDec 13, 2012
  16. Nguyen Thai Ngoc DuyDec 15, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.