git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: How to replace a single corrupt, packed object?
  2008-08-08 14:41 How to replace a single corrupt, packed object? Johannes Schindelin
@ 2008-08-08 14:39 ` Shawn O. Pearce
  2008-08-08 14:50   ` Johannes Schindelin
  2008-08-08 15:30 ` Pieter de Bie
  2008-08-11  1:43 ` Nicolas Pitre
  2 siblings, 1 reply; 18+ messages in thread
From: Shawn O. Pearce @ 2008-08-08 14:39 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, nico

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.

-- 
Shawn.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* How to replace a single corrupt, packed object?
@ 2008-08-08 14:41 Johannes Schindelin
  2008-08-08 14:39 ` Shawn O. Pearce
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Johannes Schindelin @ 2008-08-08 14:41 UTC (permalink / raw)
  To: git, nico

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.

Repack still picks up the corrupt object instead of the good one.  What's 
the best way out?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-08-08 14:39 ` Shawn O. Pearce
@ 2008-08-08 14:50   ` Johannes Schindelin
  2008-08-08 15:21     ` Johannes Schindelin
  0 siblings, 1 reply; 18+ messages in thread
From: Johannes Schindelin @ 2008-08-08 14:50 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, nico

Hi,

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

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-08-08 14:50   ` Johannes Schindelin
@ 2008-08-08 15:21     ` Johannes Schindelin
  2008-08-08 16:23       ` Shawn O. Pearce
  0 siblings, 1 reply; 18+ messages in thread
From: Johannes Schindelin @ 2008-08-08 15:21 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, nico

Hi,

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

Just for bullocks, I gave it a try with git version 1.6.0.rc1.141.g0d7ea.
That is rc2+26 patches, in case you are interested.

Unfortunately, each test costs me

630.25user 19.74system 22:01.43elapsed 49%CPU (0avgtext+0avgdata 0maxresident)k
7533688inputs+3154520outputs (55275major+2011583minor)pagefaults 0swaps

(including the inability to work properly because of all the swapping) 
which is not really nice, as the corrupt packed object occurs during the 
last 20% of the _writing_ phase of 60104 objects.

Oh, and they are big objects.  The corrupt one is 65 megabyte, for 
example.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-08-08 14:41 How to replace a single corrupt, packed object? Johannes Schindelin
  2008-08-08 14:39 ` Shawn O. Pearce
@ 2008-08-08 15:30 ` Pieter de Bie
  2008-08-08 16:19   ` Shawn O. Pearce
  2008-08-11  1:43 ` Nicolas Pitre
  2 siblings, 1 reply; 18+ messages in thread
From: Pieter de Bie @ 2008-08-08 15:30 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, nico

On 8 aug 2008, at 16:41, Johannes Schindelin wrote:

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

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, as  
also explained
in this thread:

	http://thread.gmane.org/gmane.comp.version-control.git/40893/focus=40896

- Pieter

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-08-08 15:30 ` Pieter de Bie
@ 2008-08-08 16:19   ` Shawn O. Pearce
       [not found]     ` <90E12BC7-1950-41DF-8BE5-C6B63CE060D9@ai.rug.nl>
  0 siblings, 1 reply; 18+ messages in thread
From: Shawn O. Pearce @ 2008-08-08 16:19 UTC (permalink / raw)
  To: Pieter de Bie; +Cc: Johannes Schindelin, git, nico

Pieter de Bie <pdebie@ai.rug.nl> wrote:
> 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

I don't think that will work here.  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.

-- 
Shawn.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-08-08 15:21     ` Johannes Schindelin
@ 2008-08-08 16:23       ` Shawn O. Pearce
  0 siblings, 0 replies; 18+ messages in thread
From: Shawn O. Pearce @ 2008-08-08 16:23 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, nico

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 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

Here's another idea.

You have the good object loose.  So do something like this:

  $ mkdir foo
  $ cd foo
  $ git init
  $ cp ../../the_good_loose_obj .git/objects/??/....

  $ cd ../corrupt_repo
  $ (
   cd ../foo;
   echo blob; 
   echo data $(git cat-file -s the_good_loose_obj);
   git cat-file blob the_good_loose_obj;
  ) | git fast-import
  $ git repack -a -d

The new pack file created by gfi will have a newer timestamp than the
one with the corruption.  This will cause it to sort higher in the
pack list, and we'll take objects from the first pack we find it in.

-- 
Shawn.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
       [not found]     ` <90E12BC7-1950-41DF-8BE5-C6B63CE060D9@ai.rug.nl>
@ 2008-08-08 16:36       ` Shawn O. Pearce
  2008-08-08 16:48       ` Johannes Schindelin
  1 sibling, 0 replies; 18+ messages in thread
From: Shawn O. Pearce @ 2008-08-08 16:36 UTC (permalink / raw)
  To: Pieter de Bie; +Cc: Johannes Schindelin, git, nico

Pieter de Bie <pdebie@ai.rug.nl> wrote:
> 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.

Oh, thanks for reminding me.  I had forgotten that Linus added -r
when someone else had corruption in a packed object.  Yea, if you
use -r it may be able to resume and pick up where it left off.

I haven't studied the -r code so I'm not sure how it knows where
its safe to restart unpacking from.  But if you have a .idx file
we probably could make a really good guess based on the offsets
it stores.  I doubt unpack-objects makes use of the .idx.

-- 
Shawn.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
       [not found]     ` <90E12BC7-1950-41DF-8BE5-C6B63CE060D9@ai.rug.nl>
  2008-08-08 16:36       ` Shawn O. Pearce
@ 2008-08-08 16:48       ` Johannes Schindelin
  2008-08-11  2:55         ` Nicolas Pitre
  1 sibling, 1 reply; 18+ messages in thread
From: Johannes Schindelin @ 2008-08-08 16:48 UTC (permalink / raw)
  To: Pieter de Bie; +Cc: Shawn O. Pearce, git, nico

Hi,

On Fri, 8 Aug 2008, Pieter de Bie wrote:

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

In any case, the pack is too large for me to let my computer repack 
everything, when only one object needs repacking.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-08-08 14:41 How to replace a single corrupt, packed object? Johannes Schindelin
  2008-08-08 14:39 ` Shawn O. Pearce
  2008-08-08 15:30 ` Pieter de Bie
@ 2008-08-11  1:43 ` Nicolas Pitre
  2008-09-15 14:05   ` Johannes Schindelin
  2 siblings, 1 reply; 18+ messages in thread
From: Nicolas Pitre @ 2008-08-11  1:43 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git


[ sorry for the delay -- I just returned from vacation ]

On Fri, 8 Aug 2008, Johannes Schindelin wrote:

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

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


Nicolas

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-08-08 16:48       ` Johannes Schindelin
@ 2008-08-11  2:55         ` Nicolas Pitre
  2008-08-11  3:07           ` Shawn O. Pearce
  2008-08-11 18:59           ` Johannes Schindelin
  0 siblings, 2 replies; 18+ messages in thread
From: Nicolas Pitre @ 2008-08-11  2:55 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Pieter de Bie, Shawn O. Pearce, git

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.

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-08-11  2:55         ` Nicolas Pitre
@ 2008-08-11  3:07           ` Shawn O. Pearce
  2008-08-11  3:40             ` Nicolas Pitre
  2008-08-11 18:59           ` Johannes Schindelin
  1 sibling, 1 reply; 18+ messages in thread
From: Shawn O. Pearce @ 2008-08-11  3:07 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Johannes Schindelin, Pieter de Bie, git

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,
this patch makes a lot of sense.  Its only downside is a backwards
goto within this function, but the code is actually still quite
clear to me.

If this allows git to magically fix Dscho's bad pack, it may be
worth including in the core tree.

 
> 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

-- 
Shawn.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-08-11  3:07           ` Shawn O. Pearce
@ 2008-08-11  3:40             ` Nicolas Pitre
  2008-08-11  3:46               ` Shawn O. Pearce
  0 siblings, 1 reply; 18+ messages in thread
From: Nicolas Pitre @ 2008-08-11  3:40 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Johannes Schindelin, Pieter de Bie, git

On Sun, 10 Aug 2008, Shawn O. Pearce wrote:

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

Well, other really smart people consider gotos perfectly fine when used 
judiciously.  So this ends up being a question of belief and taste.

> this patch makes a lot of sense.  Its only downside is a backwards
> goto within this function, but the code is actually still quite
> clear to me.

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.

> If this allows git to magically fix Dscho's bad pack, it may be
> worth including in the core tree.

Yep.


Nicolas

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-08-11  3:40             ` Nicolas Pitre
@ 2008-08-11  3:46               ` Shawn O. Pearce
  0 siblings, 0 replies; 18+ messages in thread
From: Shawn O. Pearce @ 2008-08-11  3:46 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Johannes Schindelin, Pieter de Bie, git

Nicolas Pitre <nico@cam.org> wrote:
> 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.

Yup, I agree completely.

Duplicates should be rare, and likely are only the fault of a
dumb transport fetch, or the user trying to fix their repository
by placing copies of corrupt objects obtained from elsewhere.
Requiring -f to fix such cases is heavy-handed.  Some trees can
take many hours to repack with -f; think gcc or OOo.

-- 
Shawn.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-08-11  2:55         ` Nicolas Pitre
  2008-08-11  3:07           ` Shawn O. Pearce
@ 2008-08-11 18:59           ` Johannes Schindelin
  1 sibling, 0 replies; 18+ messages in thread
From: Johannes Schindelin @ 2008-08-11 18:59 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Pieter de Bie, Shawn O. Pearce, git

Hi,

On Sun, 10 Aug 2008, Nicolas Pitre wrote:

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

> 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_:

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

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-08-11  1:43 ` Nicolas Pitre
@ 2008-09-15 14:05   ` Johannes Schindelin
  2008-09-15 16:26     ` Nicolas Pitre
  0 siblings, 1 reply; 18+ messages in thread
From: Johannes Schindelin @ 2008-09-15 14:05 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1311 bytes --]

Hi,

On Sun, 10 Aug 2008, Nicolas Pitre wrote:

> [ sorry for the delay -- I just returned from vacation ]

No need to be sorry; my delay is even worse...

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

Aaaaargh,
Dscho

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-09-15 14:05   ` Johannes Schindelin
@ 2008-09-15 16:26     ` Nicolas Pitre
  2008-09-15 16:34       ` Johannes Schindelin
  0 siblings, 1 reply; 18+ messages in thread
From: Nicolas Pitre @ 2008-09-15 16:26 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1399 bytes --]

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.


Nicolas

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: How to replace a single corrupt, packed object?
  2008-09-15 16:26     ` Nicolas Pitre
@ 2008-09-15 16:34       ` Johannes Schindelin
  0 siblings, 0 replies; 18+ messages in thread
From: Johannes Schindelin @ 2008-09-15 16:34 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1569 bytes --]

Hi,

On Mon, 15 Sep 2008, Nicolas Pitre wrote:

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

Thank you very much & sorry for the trouble,
Dscho

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2008-09-15 16:35 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-08 14:41 How to replace a single corrupt, packed object? Johannes Schindelin
2008-08-08 14:39 ` Shawn O. Pearce
2008-08-08 14:50   ` Johannes Schindelin
2008-08-08 15:21     ` Johannes Schindelin
2008-08-08 16:23       ` Shawn O. Pearce
2008-08-08 15:30 ` Pieter de Bie
2008-08-08 16:19   ` Shawn O. Pearce
     [not found]     ` <90E12BC7-1950-41DF-8BE5-C6B63CE060D9@ai.rug.nl>
2008-08-08 16:36       ` Shawn O. Pearce
2008-08-08 16:48       ` Johannes Schindelin
2008-08-11  2:55         ` Nicolas Pitre
2008-08-11  3:07           ` Shawn O. Pearce
2008-08-11  3:40             ` Nicolas Pitre
2008-08-11  3:46               ` Shawn O. Pearce
2008-08-11 18:59           ` Johannes Schindelin
2008-08-11  1:43 ` Nicolas Pitre
2008-09-15 14:05   ` Johannes Schindelin
2008-09-15 16:26     ` Nicolas Pitre
2008-09-15 16:34       ` Johannes Schindelin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).