Re: fact-import: failed to apply delta

From

Shawn O. Pearce <spearce@spearce.org>

Date

Feb 10, 2009, 23:09 UTC

Message-ID

<20090210230950.GX30949@spearce.org>

In-Reply-To

<7vfxillxiu.fsf@gitster.siamese.dyndns.org>

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

> 
> 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.

Right, that was my thought.
 

> 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.

diff --git a/fast-import.c b/fast-import.c
index 03b13e0..7bfb563 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1204,6 +1204,12 @@ static void *gfi_unpack_entry(
 		 */
 		p->pack_size = pack_size + 20;
 	}
+	/* DANGER, WILL ROBINSON DANGER !!!!
+	 *
+	 * unpack_entry() wasn't meant to be called the way we are
+	 * about to call it right here.  Be very careful, any sort
+	 * of assumption is probably wrong.
+	 */
 	return unpack_entry(p, oe->offset, &type, sizep);
 }
 
 
> By the way, strictly speaking, you need to release the delta_base_cache
> entries that is based on pack_data and nothing else, no?

Right.

But the hiccup of a checkpoint in terms of overall performance is
such a huge amount (due to needing to re-read the entire pack to
compute its final checksum) that the loss of the delta_base_cache
is pretty much a drop in the bucket here.

I can go back and add in a struct packed_git* and filter to only
those entries in the cache, but it doesn't seem worth it to me.

-- 
Shawn.