Re: fact-import: failed to apply delta

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
>> Daniel Barkalow <barkalow@iabervon.org> wrote:
>>> On Tue, 10 Feb 2009, Shawn O. Pearce wrote:
>>> > 
>>> > We should dump the cached_objects table in sha1_file.c during
>>> > a checkpoint in fast-import.
>>> 
>>> No, that one's keyed by sha1, and doesn't get collisions; it's the 
>>> delta_base_cache that's the issue; it's keyed by struct packed_git * and 
>>> offset.
>>
>> Uh, yea, I realize that after I sent the message.  Does this patch
>> fix it for you?
>>
>> --8<--
>> Clear the delta base cache during fast-import checkpoint
>>
>> Otherwise we may reuse the same memory address for a totally
>> different "struct packed_git", and a previously cached object from
>> the prior occupant might be returned when trying to unpack an object
>> from the new pack.
>
> Can this be made more automatic?
>
> For example if you do this every time a new pack is installed to
> sha1_file(), like in add_packed_git() perhaps, wouldn't that be much less
> error prone?

On second thought, I think fast-import is the only program that plays
funny games of feeding a packed_git that is *not* part of the real list of
packed_git installed in the system to unpack_entry(), so probably your
patch is a better idea.

We really need a better comment in the codepath from gfi_unpack_entry() to
unpack_entry() that there is a very unusual thing going on.

By the way, strictly speaking, you need to release the delta_base_cache
entries that is based on pack_data and nothing else, no?