Re: How to replace a single corrupt, packed object?

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 11, 2008, 18:59 UTC
Message-ID: <alpine.DEB.1.00.0808111818460.24820@pacific.mpi-cbg.de.mpi-cbg.de>
In-Reply-To: <alpine.LFD.1.10.0808102146050.22892@xanadu.home>

Hi,

On Sun, 10 Aug 2008, Nicolas Pitre wrote:

Show 6 quoted lines

> On Fri, 8 Aug 2008, Johannes Schindelin wrote:
> 
> > In any case, the pack is too large for me to let my computer repack 
> > everything, when only one object needs repacking.
> 
> By that you mean you cannot/don't want to use repack -f, right?

Right. However, I had a relatively fast machine standing nearby today, so that scp was not too painful.

Show 12 quoted lines

> There _could_ be a way to hack pack-objects so not to reuse bad objects.  
> However I don't want that to impact the code too much for an event that 
> hopefully should almost never happens, especially if using -f does work 
> around it already.
> 
> Well, let's see.
> 
> [...]
> 
> OK, here's what the patch to allow repacking without -f and still using 
> redundant objects in presence of pack corruption might look like.  
> Please tell me if that works for you.

The testing took quite a while unfortunately, mainly because I followed Shawn's advice, and added not only a loose object, but also a single pack with the single object in it, and a newer timestamp.

This resulted in my CPU being hogged when Git tried to read the object. I do not know exactly what is happening, but I suspect an infinite loop due to the funny interaction between a valid and a corrupt pack containing the same object. Or maybe the issue described later in this mail.

Only when I removed the pack did things actually go further, so there is still a bug lurking.

Your patch worked _almost_:

Show 5 quoted lines

>  		offset += entry->in_pack_header_size;
>  		datalen -= entry->in_pack_header_size;
> +		if (!pack_to_stdout && p->index_version == 1 &&
> +		    check_pack_inflate(p, &w_curs, offset, datalen, entry->size)) {
> +			die("corrupt packed object for %s", sha1_to_hex(entry->idx.sha1));

This needs to be an error(), obviously.

Show 8 quoted lines

> +			if (entry->delta)
> +				reused_delta--;
> +			goto no_reuse;
> +		}
> +
>  		if (type == OBJ_OFS_DELTA) {
>  			off_t ofs = entry->idx.offset - entry->delta->idx.offset;
>  			unsigned pos = sizeof(dheader) - 1;

With that, it took quite a while, then it told me about the corrupt object.

And then it hangs in the loop sha1_file.c:1511. The function inflate() returns Z_BUF_ERROR, and nothing is read.

Oh, and it still tries to access the same corrupt pack.

Thanks, Dscho

P.S.: I have to wrap up my work at my current (interim) job, and will be moving in the next days, so do not expect too much from my side before Monday.