How to replace a single corrupt, packed object?

> my auto gc kicked in, and shows this:
> 
> fatal: corrupt packed object for 2c1e128aa51e3a64bd61556c0cd488628b423ccf
> error: failed to run repack
> 
> Fortunately, I have the uncorrupted object somewhere else.  So I copy the 
> single object as a loose one, and all is fine.  Right?
> 
> Wrong.
> 
> Repack still picks up the corrupt object instead of the good one.  What's 
> the best way out?

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > my auto gc kicked in, and shows this:
> > 
> > fatal: corrupt packed object for 2c1e128aa51e3a64bd61556c0cd488628b423ccf
> > error: failed to run repack
> > 
> > Fortunately, I have the uncorrupted object somewhere else.  So I copy the 
> > single object as a loose one, and all is fine.  Right?
> > 
> > Wrong.
> > 
> > Repack still picks up the corrupt object instead of the good one.  What's 
> > the best way out?
> 
> Use a 1.6.0 rc?  Nico I thought fixed this in 1.6 to automatically
> try and find another copy of an object if the first copy considered
> for inclusion into the pack was corrupt.

> On Fri, 8 Aug 2008, Shawn O. Pearce wrote:
> 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > my auto gc kicked in, and shows this:
> > > 
> > > fatal: corrupt packed object for 2c1e128aa51e3a64bd61556c0cd488628b423ccf
> > > error: failed to run repack
> > > 
> > > Fortunately, I have the uncorrupted object somewhere else.  So I copy the 
> > > single object as a loose one, and all is fine.  Right?
> > > 
> > > Wrong.
> > > 
> > > Repack still picks up the corrupt object instead of the good one.  What's 
> > > the best way out?
> > 
> > Use a 1.6.0 rc?  Nico I thought fixed this in 1.6 to automatically
> > try and find another copy of an object if the first copy considered
> > for inclusion into the pack was corrupt.
> 
> Thanks for the quick reply, but no joy (You should know me better than 
> that ;-) :
> 
> $ git --version
> git version 1.6.0.rc1.112.gebbe4

> On Fri, 8 Aug 2008, Johannes Schindelin wrote:
> > On Fri, 8 Aug 2008, Shawn O. Pearce wrote:
> > 
> > > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > > my auto gc kicked in, and shows this:
> > > > 
> > > > fatal: corrupt packed object for 2c1e128aa51e3a64bd61556c0cd488628b423ccf
> > > > error: failed to run repack

> fatal: corrupt packed object for  
> 2c1e128aa51e3a64bd61556c0cd488628b423ccf
> error: failed to run repack
>
> Fortunately, I have the uncorrupted object somewhere else.  So I  
> copy the
> single object as a loose one, and all is fine.  Right?
>
> Wrong.
>
> Repack still picks up the corrupt object instead of the good one.   
> What's
> the best way out?

> On 8 aug 2008, at 16:41, Johannes Schindelin wrote:
>
>> fatal: corrupt packed object for  
>> 2c1e128aa51e3a64bd61556c0cd488628b423ccf
>> error: failed to run repack
>>
>> What's
>> the best way out?
>
> I don't know how to replace the object, but you can expand the pack,  
> replace
> the loose file, delete the old pack, and then create a new one

> Hi,
> 
> my auto gc kicked in, and shows this:
> 
> fatal: corrupt packed object for 2c1e128aa51e3a64bd61556c0cd488628b423ccf
> error: failed to run repack
> 
> Fortunately, I have the uncorrupted object somewhere else.  So I copy the 
> single object as a loose one, and all is fine.  Right?
> 
> Wrong.

> On Fri, 8 Aug 2008, Johannes Schindelin wrote:
> 
> > my auto gc kicked in, and shows this:
> > 
> > fatal: corrupt packed object for 2c1e128aa51e3a64bd61556c0cd488628b423ccf
> > error: failed to run repack
> > 
> > Fortunately, I have the uncorrupted object somewhere else.  So I copy the 
> > single object as a loose one, and all is fine.  Right?
> > 
> > Wrong.
> 
> Well, to be sure things are then right or wrong, just do a 
> 
> 	git show 2c1e128aa51e3a64bd61556c0cd488628b423ccf
> 
> If you can't see the object before, and are able to see it once it has 
> been copied over, then things are "right".
> 
> > Repack still picks up the corrupt object instead of the good one.  
> > What's the best way out?
> 
> How do you repack?  The only way to get rid of a corrupted object in 
> that case is to 'git repack -a -f'.

> On Sun, 10 Aug 2008, Nicolas Pitre wrote:
> 
> > On Fri, 8 Aug 2008, Johannes Schindelin wrote:
> > 
> > > my auto gc kicked in, and shows this:
> > > 
> > > fatal: corrupt packed object for 2c1e128aa51e3a64bd61556c0cd488628b423ccf
> > > error: failed to run repack
> > > 
> > > Fortunately, I have the uncorrupted object somewhere else.  So I copy the 
> > > single object as a loose one, and all is fine.  Right?
> > > 
> > > Wrong.
> > 
> > Well, to be sure things are then right or wrong, just do a 
> > 
> > 	git show 2c1e128aa51e3a64bd61556c0cd488628b423ccf
> > 
> > If you can't see the object before, and are able to see it once it has 
> > been copied over, then things are "right".
> > 
> > > Repack still picks up the corrupt object instead of the good one.  
> > > What's the best way out?
> > 
> > How do you repack?  The only way to get rid of a corrupted object in 
> > that case is to 'git repack -a -f'.
> 
> Turns out I am a complete, utter moron.  And I am sure René will quote me 
> on that.
> 
> Git would probably have taken the copied-over object, and now took the 
> copied-over pack (finally!).
> 
> My mistake was to keep the .keep file.  And the corrupt object was -- you 
> guessed it -- in the corresponding .pack file.

> On Mon, 15 Sep 2008, Johannes Schindelin wrote:
> 
> > On Sun, 10 Aug 2008, Nicolas Pitre wrote:
> > 
> > > On Fri, 8 Aug 2008, Johannes Schindelin wrote:
> > > 
> > > > my auto gc kicked in, and shows this:
> > > > 
> > > > fatal: corrupt packed object for 2c1e128aa51e3a64bd61556c0cd488628b423ccf
> > > > error: failed to run repack
> > > > 
> > > > Fortunately, I have the uncorrupted object somewhere else.  So I copy the 
> > > > single object as a loose one, and all is fine.  Right?
> > > > 
> > > > Wrong.
> > > 
> > > Well, to be sure things are then right or wrong, just do a 
> > > 
> > > 	git show 2c1e128aa51e3a64bd61556c0cd488628b423ccf
> > > 
> > > If you can't see the object before, and are able to see it once it has 
> > > been copied over, then things are "right".
> > > 
> > > > Repack still picks up the corrupt object instead of the good one.  
> > > > What's the best way out?
> > > 
> > > How do you repack?  The only way to get rid of a corrupted object in 
> > > that case is to 'git repack -a -f'.
> > 
> > Turns out I am a complete, utter moron.  And I am sure René will quote me 
> > on that.
> > 
> > Git would probably have taken the copied-over object, and now took the 
> > copied-over pack (finally!).
> > 
> > My mistake was to keep the .keep file.  And the corrupt object was -- you 
> > guessed it -- in the corresponding .pack file.
> 
> OK.  Then I'll dig my patch out and write a test for it before 
> submitting it to Junio.

> On 8 aug 2008, at 18:19, Shawn O. Pearce wrote:
>
>> The unpack-objects process will
>> fail when it finds this bad object, and everything after that in
>> the pack file will be dropped on the floor and not get unpacked.
>
> Even with the -r switch?
>
>        -r     When unpacking a corrupt packfile, the command dies at the 
> first corruption. This flag tells it to keep going and make
>               the best effort to recover as many objects as possible.

> On 8 aug 2008, at 18:19, Shawn O. Pearce wrote:
> 
> >The unpack-objects process will fail when it finds this bad object, and 
> >everything after that in the pack file will be dropped on the floor and 
> >not get unpacked.
> 
> Even with the -r switch?
> 
>       -r     When unpacking a corrupt packfile, the command dies at the first
>       corruption. This flag tells it to keep going and make
>              the best effort to recover as many objects as possible.

> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index 2dadec1..88e73f3 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -277,6 +277,7 @@ static unsigned long write_object(struct sha1file *f,
>  				 */
>  
>  	if (!to_reuse) {
> +		no_reuse:
>  		if (!usable_delta) {
>  			buf = read_sha1_file(entry->idx.sha1, &type, &size);
>  			if (!buf)
> @@ -364,14 +365,28 @@ static unsigned long write_object(struct sha1file *f,
>  			reused_delta++;
>  		}
>  		hdrlen = encode_header(type, entry->size, header);
> +
>  		offset = entry->in_pack_offset;
>  		revidx = find_pack_revindex(p, offset);
>  		datalen = revidx[1].offset - offset;
>  		if (!pack_to_stdout && p->index_version > 1 &&
> -		    check_pack_crc(p, &w_curs, offset, datalen, revidx->nr))
> -			die("bad packed object CRC for %s", sha1_to_hex(entry->idx.sha1));
> +		    check_pack_crc(p, &w_curs, offset, datalen, revidx->nr)) {
> +			error("bad packed object CRC for %s", sha1_to_hex(entry->idx.sha1));
> +			if (entry->delta)
> +				reused_delta--;
> +			goto no_reuse;
> +		}
> +
>  		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));
> +			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;
> @@ -394,10 +409,6 @@ static unsigned long write_object(struct sha1file *f,
>  				return 0;
>  			sha1write(f, header, hdrlen);
>  		}
> -
> -		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));
>  		copy_pack_data(f, p, &w_curs, offset, datalen);
>  		unuse_pack(&w_curs);
>  		reused++;
> 
> 
> Nicolas

> Nicolas Pitre <nico@cam.org> wrote:
> > 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.
> 
> Aside from goto being considered harmful by some really smart people,

> The actual downside I see with this patch is the fact that real data 
> corruptions might be "fixed" automagically with user unaware of it.  
> This could be a serious sign that the hardware is going bad and 
> requiring the user to consciously use -f to fix things is good.  However 
> it is most unlikely that redundant objects will be kept around in the 
> normal case, hence manual intervention will be needed anyway to bring a 
> copy of bad object into the repository.  So not having to use -f might 
> not be such an issue.

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

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

>  		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));

> +			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;