My git repo is broken, how to fix it ?

My git repo is broken, how to fix it ?

[Alexander Litvinov]

Hello,

I use manualy compiled git under cygwin. Some time ago I have imported project 
from CVS and start to us it under git. To emulate 'separate remotes' schema 
for branches from CVS I clone imported git repo and work with it. From time 
to time I incrementaly update imported repo from cvs and sometimes use 
git-cvsexportcommit (from work repo) to export my changes and then get them 
using git-cvsimport.

Some times ago I descide to run fsck and found that by working repo is broken, 
while imported repo is correct. Is there way to fix it ? 

>git version
git version 1.5.0.GIT
(It was actualy compiled from e86d552 commit)
> git fsck
> git fsck --full
error: packed 7f5fed8131fb32972c602dede29b9257a053ba67 
from .git/objects/pack/pack-c4554978bbe079c9a43d6a13546a2fa314fe0884.pack is 
corrupt
sha1 mismatch 7f5fed8131fb32972c602dede29b9257a053ba67

(This is a blob, git cat-file blob 7f5fed813 shows me my c++ header file that 
is partialy broken with ^@ symbols)

The repo I get using git-cvsimport is correct and does not contains that blob. 
I also tried git-log -p for all by branches to force git to show me what is 
the commit was broken but git-log finished without errors.

By the way, several times I interrupt git's commands like commit and pull 
using Ctrl-C.

I tried to unpack all objects:
> git-unpack-objects -r 
< .git/objects/pack/pack-c4554978bbe079c9a43d6a13546a2fa314fe0884.pack; echo 
$?
Unpacking 12868 objects
 100% (12868/12868) done
0

No erorts here. But fsck find that broken blob:
> git fsck 
dangling blob beb992198d4d8813ea51fd1cbbf38313ef490c22

git-cat-file shows me this this is a broken object with correct sha1 sum.


As a cunclusion: my repo has broken file and I don't see there is the brakage. 
Can I reconstruct file by sha1 sum :-) or can I do something to stop fsck 
warn me ?

Re: My git repo is broken, how to fix it ?

[Linus Torvalds]

On Wed, 28 Feb 2007, Alexander Litvinov wrote:
> 
> Some times ago I descide to run fsck and found that by working repo is broken, 
> while imported repo is correct. Is there way to fix it ? 

Generally, the best way to fix things is (I've written this up at 
somewhat more length before, but I'm too lazy to find it):

 - back up all your state so that anything you do is re-doable if you 
   corrupt things more!

 - explode any corrupt pack-files

   See "man git-unpack-objects", and in particular the "-r" flag. Also, 
   please realize that it only unpacks objects that aren't already 
   available, so you need to move the pack-file away from its normal 
   location first (otherwise git-unpack-objects will find all objects 
   that are in the pack-file in the pack-file itself, and not unpack 
   anything at all)

 - replace any broken and/or missing objects

   This is the challenging part. Sometimes (hopefully often!) you can find 
   the missing objects in other copies of the repositories. At other 
   times, you may need to try to find the data some other way (for 
   example, maybe your checked-out copy contains the file content that 
   when hashed will be the missing object?).

 - make sure everything is happy with "git-fsck --full"

 - repack everything to get back to an efficient state again.

And remember: git does _not_ make backups pointless. It hopefully makes 
backups *easy* (since cloning and pulling is easy), but the basic need for 
backups does not go away!

> By the way, several times I interrupt git's commands like commit and pull 
> using Ctrl-C.

Shouldn't matter, at least as long as you are using the native git 
protocol: git will create objects fully under a temporary name, and then 
atomically rename things to their right names. 

Using rsync and/or http may not be as safe.

HOWEVER! I do not know how well Windows and/or cygwin does file renames. 
If cygwin does a rename as a copy + delete, a lot of the safety 
assumptions just fly out the window.

> I tried to unpack all objects:
>
> > git-unpack-objects -r < .git/objects/pack/pack-c4554978bbe079c9a43d6a13546a2fa314fe0884.pack; echo  $?
> Unpacking 12868 objects
>  100% (12868/12868) done

Ok, that's a good thing, but see above: I don't think anything should have 
gotten unpacked, because it found all objects already existing in the very 
pack-file you tried to unpack.

So you might well need to do

	mv .git/objects/pack/pack-c4554978bbe079c9a43d6a13546a2fa314fe0884.pack oldpack
	git-unpack-objects -r < oldpack

(or rename the .idx file instead).

Alternatively (and in many ways this migth be better when you're trying to 
recover something) just create a totally *new* git repo, by doing

	mkdir new-repo
	cd new-repo
	git init
	git unpack-objects -r < ../other-repo/.git/pack/pack-.....pack

and re-create the objects somewhere else - you can do all of this without 
at all disturbing the old repository (but you'd need to copy all the refs 
and all the loose objects by hand, of course!)

> No erorts here. But fsck find that broken blob:
> > git fsck 
> dangling blob beb992198d4d8813ea51fd1cbbf38313ef490c22
> 
> git-cat-file shows me this this is a broken object with correct sha1 sum.
> 
> As a cunclusion: my repo has broken file and I don't see there is the brakage. 
> Can I reconstruct file by sha1 sum :-) or can I do something to stop fsck 
> warn me ?

You didn't do "--full", so it's not looking inside your pack, so the fsck 
wasn't very interesting in this case.

And no, you cannot reconstruct the file by sha1 sum, although you may be 
able to reconstruct the file some *other* way (by looking at the other 
blobs and remembering what the missing case is), and then you can 
obviously use the sha1-sum to *confirm* that you reconstructed the file 
exactly as it was!

So yes, reconstruction of missing objects is possible, but no, you can't 
do it based purely based on SHA1, you need to base reconstruction on some 
other information. That's kind of what "cryptographically secure hash" 
means ;^p

			Linus

Re: My git repo is broken, how to fix it ?

[Alexander Litvinov]

В сообщении от Wednesday 28 February 2007 10:57 Linus Torvalds написал(a):
>  - replace any broken and/or missing objects
>
>    This is the challenging part. Sometimes (hopefully often!) you can find
>    the missing objects in other copies of the repositories. At other
>    times, you may need to try to find the data some other way (for
>    example, maybe your checked-out copy contains the file content that
>    when hashed will be the missing object?).

Thanks for answer. I have found this blob in cloned repo. I just copy it into 
objects subdir and repack repo again. fsck works without any errors.

Thanks again.

Re: My git repo is broken, how to fix it ?

[Linus Torvalds]

On Wed, 28 Feb 2007, Alexander Litvinov wrote:
>
> >  - replace any broken and/or missing objects
> >
> >    This is the challenging part. Sometimes (hopefully often!) you can find
> >    the missing objects in other copies of the repositories. At other
> >    times, you may need to try to find the data some other way (for
> >    example, maybe your checked-out copy contains the file content that
> >    when hashed will be the missing object?).
> 
> Thanks for answer. I have found this blob in cloned repo. I just copy it into 
> objects subdir and repack repo again. fsck works without any errors.

Good to hear.

It would probably be good to

 - try to figure out why things got corrupted in the first place.

   In particular, we should probably check our (my) assumption that the 
   file rename on cygwin is atomic. Your comment that you use ^C a lot 
   makes me worry that something basically caused an incomplete write or 
   other thing to happen..

   To be more precise, git will actually start off trying to do a "link + 
   unlink" pair, because that is the safest thing to do on a UNIX 
   filesystem: if the linked target already exists, we won't overwrite it 
   (and the git data consistency rules have always been: honor existing 
   data over new one, and *never* change anything that has already been 
   written).

   But I would not be shocked to hear that the "link + unlink" sequence 
   ends up being emulated under cygwin as a "copy + delete" due to lack of 
   hardlinks or something.

   Also, even if the link fails, and git then falls back to "rename()" 
   (since some filesystems don't do hardlinks at all, or limit them to one 
   particular directory), I would _still_ not be totally surprised if the 
   rename got emulated as a copy/delete for some strange Windows reason.

   There are other possibilities for corruption, of course: just plain 
   disk corruption, or (again) some other subtle cygwin emulation or 
   Windows issue could bite us. 

 - Even under UNIX, I'm not entirely sure about http/ftp/rsync transfers. 
   rsync in particular doesn't check anything at all, but last I looked, 
   the http fetcher was also doing things like checking the integrity of 
   the object *after* it had already moved it to its final resting place 
   (which is again unsafe with ^C).

   In general, I strongly suggest that people use the "native git" 
   pack-transfers. The "dumb protocol" transfers are called "dumb" for a 
   reason..

 - It would probably be good to write up the "How to recover" thing, 
   regardless of why any corruption happens. It doesn't matter if you're 
   under UNIX and using native protocols, and just being careful as hell: 
   disks get corrupted, sh*t happens, alpha-particles in the wrong place 
   do bad things to memory cells. And bugs _are_ inevitable, even if 
   we've been pretty damn good about these things.

   So it's important for people to know what the limits on corruption are, 
   and tell people that regardless of how stable the git data structures 
   are, if you care about your data, you need to have things in multiple 
   places (and no, RAID is _not_ the answer either, even if it can be a 
   small _part_ of doing things well).

Anybody?

		Linus

Re: My git repo is broken, how to fix it ?

[Alex Riesen]

On 2/28/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>  - try to figure out why things got corrupted in the first place.
> ...
>    But I would not be shocked to hear that the "link + unlink" sequence
>    ends up being emulated under cygwin as a "copy + delete" due to lack of
>    hardlinks or something.

Well, cygwin has a whole rename implementation using wincalls (which
may or maybe not syscalls). It's hard to tell whether micros$#%^ used
link+unlink: windows traditionally had no link(2). It has it since W2K,
allowed only on ntfs, and even there it spent some time undocumented :)

>    Also, even if the link fails, and git then falls back to "rename()"
>    (since some filesystems don't do hardlinks at all, or limit them to one
>    particular directory), I would _still_ not be totally surprised if the
>    rename got emulated as a copy/delete for some strange Windows reason.
>
>    There are other possibilities for corruption, of course: just plain
>    disk corruption, or (again) some other subtle cygwin emulation or
>    Windows issue could bite us.

It is very hard to tell: the rename function in cygwin is over 200 lines long,
has multiple calls down to the kernel (or whatever it is), and there are two
different wincalls used. It is hard to predict what path will be taken or
whether there were actually any data copying happened.
Of course, windows known to corrupt data just so...

>  - It would probably be good to write up the "How to recover" thing,
>    regardless of why any corruption happens. It doesn't matter if you're
>    under UNIX and using native protocols, and just being careful as hell:
>    disks get corrupted, sh*t happens, alpha-particles in the wrong place
>    do bad things to memory cells. And bugs _are_ inevitable, even if
>    we've been pretty damn good about these things.

It'd be a good start to link the recent disaster recoveries from
git.or.cz wiki from the title page (unless it is already).

Re: My git repo is broken, how to fix it ?

[Alexander Litvinov]

В сообщении от Wednesday 28 February 2007 22:19 Linus Torvalds написал(a):
> On Wed, 28 Feb 2007, Alexander Litvinov wrote:
> > Thanks for answer. I have found this blob in cloned repo. I just copy it
> > into objects subdir and repack repo again. fsck works without any errors.
>
> Good to hear.

Hello, its me again.

It is pity but my repo was corrupted again. I have WinXP + cygwin + 
git-1.5.0-572-ge86d552. I was doing 
git-apply/git-am/git-reset/git-cvsexportcommit and broke repo somehow. I have 
two broken blobs that should be done by my recent patches.

Will try to recover them and report the result.

Is there any way to catch and solve the problem ?
Thanks for help.

Re: My git repo is broken, how to fix it ?

[Linus Torvalds]

On Mon, 19 Mar 2007, Alexander Litvinov wrote:
> 
> It is pity but my repo was corrupted again. I have WinXP + cygwin + 
> git-1.5.0-572-ge86d552. I was doing 
> git-apply/git-am/git-reset/git-cvsexportcommit and broke repo somehow. I have 
> two broken blobs that should be done by my recent patches.

Ok, can you send me just the two broken blobs? I assume that they are 
loose objects, so when fsck complaines about a corrupt object xyzzy..., 
just take those objects from

	.git/objects/xy/zzy..

and tar the two broken ones up, and send it to me by email. I'll keep it 
private if need be, but if you don't care about that, it would be even 
better if you can send them to the list publicly so that others can see 
what the corruption looks like.

> Is there any way to catch and solve the problem ?

I'd like to see the objects to look at what the corruption looks like, but 
I suspect that it's cygwin and/or WinXP. I'm not at all convinced that 
Windows is all that safe in general when it comes to data consistency, and 
I suspect cygwin makes it much worse by making operations that *should* be 
atomic be non-atomic.

Here's a patch that is probably a good idea to try. It disables the 
hardlinking code for CYGWIN, and it also checks for errors from "close()". 
Those are the two most obvious issues that I could imagine causing 
problems..

		Linus
---
 sha1_file.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 372af60..d829dc7 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1760,6 +1760,14 @@ static void write_sha1_file_prepare(void *buf, unsigned long len,
 	SHA1_Final(sha1, &c);
 }
 
+#ifdef __CYGWIN__
+static int link(const char *old, const char *new)
+{
+	errno = ENOSYS;
+	return -1;
+}
+#endif
+
 /*
  * Link the tempfile to the final place, possibly creating the
  * last directory level as you do so.
@@ -1951,7 +1959,8 @@ int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned cha
 	if (write_buffer(fd, compressed, size) < 0)
 		die("unable to write sha1 file");
 	fchmod(fd, 0444);
-	close(fd);
+	if (close(fd))
+		die("error closing sha1 file (%s)", strerror(errno));
 	free(compressed);
 
 	return move_temp_to_file(tmpfile, filename);

Re: My git repo is broken, how to fix it ?

[Linus Torvalds]

On Tue, 20 Mar 2007, Alexander Litvinov wrote:
> 
> Actualy, I have packed that objects already, so fsck warn me:
> $ git fsck --full
> error: packed 8edc906985f00cf27180b1d9d4c3217ffd1896f8 from .git/objects/pack/pack-abc5cbabfc05c213e50c43ea07f43158bf1de236.pack is corrupt
> error: packed f6aca57bb30a12e9ac5d71558e0b6052d6fb67a8 from .git/objects/pack/pack-abc5cbabfc05c213e50c43ea07f43158bf1de236.pack is corrupt
> sha1 mismatch 8edc906985f00cf27180b1d9d4c3217ffd1896f8
> sha1 mismatch f6aca57bb30a12e9ac5d71558e0b6052d6fb67a8

Ok, this is different from what I expected. 

Since your pack-file seems to pass its own internal SHA1 checks, it means 
that it was likely corrupt already when it was written out in the pack. 
What's interesting is that it seems to unpack, but then the SHA1 of the 
unpacked object doesn't match.

The reason I say that's interesting is that it would seem to mean that the 
zlib CRC/adler check didn't trigger - which probably means that the object 
was corrupted *before* it was compressed (but after it was originally 
SHA1-summed), or the compression itself was corrupting (eg a libz 
problem).

And since the SHA1 of the pack-file matches, the thing was apparently 
also written out "correctly" after compression (but by that "correctly" I 
obviously mean that the *corrupted* data was written out). 

Sadly, by the time it's in a pack-file, it is *really* hard to figure out 
what went wrong: I see your unpacked data, but it's really the packed raw 
objects that I wanted to look at, in case there would be some pattern in 
the actual corruption (the corruption will then result in random crud when 
actually unpacking, which is why the unpacked data isn't that interesting, 
simple because there's no pattern left to analyze - it got inflated to 
bogus "data").

> I also use autocrlf feature:
> $ git config core.autocrlf
> true

I doubt autocrlf affects anything here, it's only used at checkin and 
checkout time, and it wouldn't affect the raw internal git objects.

More interesting might be if you might be using any of the other flags 
that actually affect internal git object packing: "use_legacy_headers" in 
particular? If we have a bug there, that could be nasty.

But to really look at this we should probably add a "really_careful" flag 
that actually re-verifies the SHA1 on read so that we'd catch these kinds 
of corruptions early. 

> This files are cpp code from our project and tham need to be private. Really.

Ok, no problem. I added back the git list (but not your attachments, 
obviously) but as explained above, there is not a lot I can do with the 
unpacked data, I'd like to see the actual "raw" stuff.

I'm hoping somebody has any ideas. We really *could* check the SHA1 on 
each read (and slow down git a lot) and that would catch corruption much 
faster and hopefully pinpoint it more quickly where exactly it happens. 
But maybe somebody has some other smart idea?

		Linus

Re: My git repo is broken, how to fix it ?

[Alexander Litvinov]

В сообщении от Tuesday 20 March 2007 11:34 Linus Torvalds написал:
> Ok, this is different from what I expected.

I will try to stop using git-gc for some time to find out broken loose 
objects.

> > I also use autocrlf feature:
> > $ git config core.autocrlf
> > true
>
> More interesting might be if you might be using any of the other flags
> that actually affect internal git object packing: "use_legacy_headers" in
> particular? If we have a bug there, that could be nasty.
This is the all my config options:
$ git config -l
user.name=Alexander Litvinov
user.email=XXX
core.logallrefupdates=true
core.filemode=false
core.autocrlf=true
diff.color=auto
status.color=auto
apply.whitespace=strip
core.repositoryformatversion=0
core.filemode=false
core.bare=false
remote.origin.url=/home/lan/src/XXX
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
branch.master.remote=origin
branch.master.merge=refs/heads/master
branch.XXX.remote=origin
branch.XXX.merge=refs/heads/XXX

> Ok, no problem. I added back the git list (but not your attachments,
> obviously) but as explained above, there is not a lot I can do with the
> unpacked data, I'd like to see the actual "raw" stuff.

I undertand your wish.

> I'm hoping somebody has any ideas. We really *could* check the SHA1 on
> each read (and slow down git a lot) and that would catch corruption much
> faster and hopefully pinpoint it more quickly where exactly it happens.

I can live with such slowdown as far as cygwin not fast and I am ready to wait 
right now. I don't think the situation become realy worser than now :-)

Re: My git repo is broken, how to fix it ?

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> But to really look at this we should probably add a "really_careful" flag 
> that actually re-verifies the SHA1 on read so that we'd catch these kinds 
> of corruptions early. 
> ...
> I'm hoping somebody has any ideas. We really *could* check the SHA1 on 
> each read (and slow down git a lot) and that would catch corruption much 
> faster and hopefully pinpoint it more quickly where exactly it happens. 

At least, we could do something like this to catch the breakage
when we (re)pack, to prevent damage from propagating.


diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 73d448b..5d0692a 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -65,6 +65,7 @@ static int no_reuse_delta;
 static int local;
 static int incremental;
 static int allow_ofs_delta;
+static int revalidate_sha1;
 
 static struct object_entry **sorted_by_sha, **sorted_by_type;
 static struct object_entry *objects;
@@ -974,8 +975,31 @@ static void add_preferred_base(unsigned char *sha1)
 	it->pcache.tree_size = size;
 }
 
-static void check_object(struct object_entry *entry)
+static void check_object(struct object_entry *entry, int ith, unsigned *last)
 {
+	if (revalidate_sha1) {
+		unsigned char sha1[20];
+		enum object_type type;
+		unsigned long size;
+		void *buf;
+
+		buf = read_sha1_file(entry->sha1, &type, &size);
+		hash_sha1_file(buf, size, typename(type), sha1);
+		if (hashcmp(sha1, entry->sha1))
+			die("'%s': hash mismatch", sha1_to_hex(entry->sha1));
+		free(buf);
+
+		if (progress) {
+			unsigned percent = ith * 100 / nr_objects;
+			if (percent != *last || progress_update) {
+				fprintf(stderr, "%4u%% (%u/%u) done\r",
+					percent, ith, nr_objects);
+				progress_update = 0;
+				*last = percent;
+			}
+		}
+	}
+
 	if (entry->in_pack && !entry->preferred_base) {
 		struct packed_git *p = entry->in_pack;
 		struct pack_window *w_curs = NULL;
@@ -1082,10 +1106,16 @@ static void get_object_details(void)
 {
 	uint32_t i;
 	struct object_entry *entry;
+	unsigned last_percent = 999;
+
+	if (progress && revalidate_sha1)
+		fprintf(stderr, "Revalidating %u objects.\n", nr_objects);
 
 	prepare_pack_ix();
 	for (i = 0, entry = objects; i < nr_objects; i++, entry++)
-		check_object(entry);
+		check_object(entry, i+1, &last_percent);
+	if (progress && revalidate_sha1)
+		fputc('\n', stderr);
 
 	if (nr_objects == nr_result) {
 		/*
@@ -1629,6 +1659,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			rp_av[1] = "--objects-edge";
 			continue;
 		}
+		if (!strcmp("--revalidate", arg)) {
+			revalidate_sha1 = 1;
+			continue;
+		}
 		usage(pack_usage);
 	}
 

Re: My git repo is broken, how to fix it ?

[Nicolas Pitre]

On Tue, 20 Mar 2007, Junio C Hamano wrote:

> Linus Torvalds <torvalds@linux-foundation.org> writes:
> 
> > But to really look at this we should probably add a "really_careful" flag 
> > that actually re-verifies the SHA1 on read so that we'd catch these kinds 
> > of corruptions early. 
> > ...
> > I'm hoping somebody has any ideas. We really *could* check the SHA1 on 
> > each read (and slow down git a lot) and that would catch corruption much 
> > faster and hopefully pinpoint it more quickly where exactly it happens. 
> 
> At least, we could do something like this to catch the breakage
> when we (re)pack, to prevent damage from propagating.

I think it would be better to retest the SHA1 when we're about to 
_write_ the object out to the pack, replacing check_pack_inflate() and 
revalidate_loose_object() with the full SHA1 check, and testing objects 
which data isn't reused from a pack too.  And make it conditional on 
!pack_to_stdout like we already do of course.


Nicolas

Re: My git repo is broken, how to fix it ?

[Linus Torvalds]

[ Sorry, I got sidetracked, have been looking at kernel bugs and git 
  optimizations. I added back the git mailing list, since there is no 
  private file data here any more, and I'd like others to follow this
  saga too ]

On Wed, 21 Mar 2007, Alexander Litvinov wrote:
>
> > Oh, btw, before I do that - do you by any chance have the *uncorrupt*
> > version of the file that should be this object? In other words, do you
> > have object 03312463e194d68d0d677b51e09b47cb29ca926a in another
> > repository? It should be a version of your file.
>
> It is pity, but I don't have that version. It was broken at my last commit and 
> I will redo it again. I remeber the changes I have done.
> 
> The new file has different sha1 sum but I attach it to show you the real file 
> content.

Sadly, I actually would need to compare it to the exact object it *should* 
have generated, and that means that I'm not actually all that interested 
in the "real file content" per se, I really would need to see the exact 
blob, so that I can generate the object it should have been, and then 
compare that against the corrupt one... 

It's the binary data I'd like to compare, so that I can tell (for example) 
if there is just a chunk missing in the middle, or something like that. 
But since even slight differences in the source data will lead to 
different binary data, and since the compressed and corrupt data I have 
from you earlier doesn't make sense on its own, I do care about the exact 
object that got corrupted.

> By the way, I now I am using git (3ba7a10) taken from next with tree your 
> patches:
> 1. disables the hardlinking code for CYGWIN, and it also checks for errors 
> from "close()"
> 2. Don't ever return corrupt objects from "parse_object()"
> 3. Be more careful about zlib return values.

Ok, apart from #1, those should be in current -git now, along with better 
validation checks (by Nico) when packing. So hopefully at least when there 
is corruption in a loose object, we will now always notice when we do a 
"git repack", and will never generate a broken pack-file. Knock wood.

Of course, I actually wonder if the bug might be in your version of zlib 
(miscompiled or some other thing), in which case *any* amount of 
pre-validation won't really help, because it will become corrupted when we 
deflate it prior to writing. For example, if "deflateBound()" sometimes 
doesn't give a valid upper bound and we allocate too little space..

> Yesterday brakage was made by git with only first patch.

I see that you seem to be able to reproduce it again - I'll answer that 
email separately just so that the git list sees that message too. But it 
boils down to: if it's reproducible, I'd *really* like to see the 
corrupted object and the exact file that it should have been generated 
from..

		Linus

Re: My git repo is broken, how to fix it ?

[Nicolas Pitre]

On Thu, 22 Mar 2007, Linus Torvalds wrote:

> Ok, apart from #1, those should be in current -git now, along with better 
> validation checks (by Nico) when packing. So hopefully at least when there 
> is corruption in a loose object, we will now always notice when we do a 
> "git repack", and will never generate a broken pack-file. Knock wood.

Not yet actually.  What I did do is to make index-pack perform more 
validation and ensure it never accept SHA1 collisions.

For the repack case... I think there should be a better way.  Either we 
revalidate the full SHA1 which would be expensive as we'd basically lose 
most advantages of direct pack data copy.

What I'm pondering is some sort of lightweight checksum like adler32 for 
object data in the pack but stored in the index.  Since index-pack 
already perform the full SHA1 already, it could as well provide a 
checksum for the raw pack object data for the repack case.  Currently we 
try to validate reused pack data by attempting an inflate pass on the 
object payload, but that doesn't validate the object type nor the 
reference SHA1 to delta base objects which could get corrupted and 
copied without noticing into another pack.

> Of course, I actually wonder if the bug might be in your version of zlib 
> (miscompiled or some other thing), in which case *any* amount of 
> pre-validation won't really help, because it will become corrupted when we 
> deflate it prior to writing. For example, if "deflateBound()" sometimes 
> doesn't give a valid upper bound and we allocate too little space..

Well, since we provide the size of the allocated output buffer to zlib 
it would be seriously broken if it overflowed it.  Also zlib perform a 
checksum verification of the deflated data if I remember correctly.  So 
it seems to me that zlib should be quite self validating already.


Nicolas

Re: My git repo is broken, how to fix it ?

[Linus Torvalds]

[ Git list cc'd again - I modified your branch names and commit header 
  line just in case you care about those ]

On Wed, 21 Mar 2007, Alexander Litvinov wrote:
> 
> I have a good news : I got the breakage again. And I can reproduce it :-)
> 
> This is a version of git with three your patches.
> 
> Here is the steps to broke my repo:

So does it break every time if you do this particular sequence with the 
particular state that it has?  If so, wonderful, since it should mean that 
you can also recreate the file that got corrupted as a blob.

> $ git prune
> $ git fsck --full
> dangling commit 50267ccaa820c456bd361db808f99d81714cbce8
> $ git rebase fix-autoxyzzy                                    
> First, rewinding head to replay your work on top of it...
> HEAD is now at 42af3b2... Replace ...
> Applying 'Show ...'
> 
> Wrote tree 851c5d8d2213c60efc1bd081b0012bfcc9e558b5
> Committed: e7117e5637e881368ff04e94a27dca2abdb12d38

And then..

> [lan@ac-7923bb4c6c14 navitel (debug-autoxyzzy)]$ git fsck --full
> error: corrupt loose object 'c01848491b53c3dcfd738149193a14d3c9abe107'
> error: c01848491b53c3dcfd738149193a14d3c9abe107: object corrupt or missing
> missing blob c01848491b53c3dcfd738149193a14d3c9abe107
> dangling commit 50267ccaa820c456bd361db808f99d81714cbce8
> 
> What can I do to debug this ?

Every time there's a corrupt object, if you can send it to me, that would 
be good. If you can tell the source for the corrupt object and can send 
that to me too, that's always even better, but even in the absense of 
that, the more corrupt objects I have, the better the chances that I see 
some pattern. And if it's always the same object that gets corrupted the 
same way when you start from a particular starting point, that would also 
be very interesting to know.

Considering that the "don't use hardlinks on cygwin" thing didn't matter 
for you (and really, I would have only expected it to matter if you used 
^C to kill a process in the middle or something), you migth also be better 
off just trackng the standard git, since it now has Nicos extra 
consistency checks over and beyond those I send you. 

It's also possible that the real bug is that we have some memory scribble 
internally in git, and that it shows up for you just because Cygwin and/or 
WinXp has different allocation patterns than other platforms. Do you know 
if there are any "debugging malloc" libraries for Cygwin? Something like 
ElectricFence/dmalloc under Linux, or running with valgrind.

Since it happens after a single rebase, if it's a git bug (as opposed 
to,for example, a zlib problem or simply a problem in your combination of 
vmware/winxp/cygwin), it would be the recursive merge that screws up. It 
*is* one of the more complex operations (especially if it also ends up 
doing file-level merging, which I assume it does), so some memory 
allocation problem there is not out of the question, although it's strange 
that you see it but the (many more) users on UNIX never seem to - it's not 
like rebase is an uncommon operation!

		Linus

Re: My git repo is broken, how to fix it ?

[Linus Torvalds]

On Thu, 22 Mar 2007, Linus Torvalds wrote:
> 
> It's also possible that the real bug is that we have some memory scribble 
> internally in git, and that it shows up for you just because Cygwin and/or 
> WinXp has different allocation patterns than other platforms. Do you know 
> if there are any "debugging malloc" libraries for Cygwin? Something like 
> ElectricFence/dmalloc under Linux, or running with valgrind.

Yeehaa! I think I'm on the right trail.

Git people: do this:

	yum install ElectricFence

(or similar, apt-get, whatever), and then apply this patch, and do

	make test

and it will fail in "git-apply"! Which (having read Alexander's corruption 
sequence once more) must have been what corrupted things for Alexander 
too!

I've not debugged it any more, but gdb on the core-dump shows:

	(gdb) where
	#0  0x0000003768462331 in SHA1_Update () from /lib64/libcrypto.so.6
	#1  0x0000000000461b0e in write_sha1_file_prepare (buf=0x2ba371737fd0, len=46, type=0x49a50b "blob", sha1=0x7fff395c84a0 "",
	    hdr=0x7fff395c8480 "blob 46", hdrlen=0x7fff395c847c) at sha1_file.c:1823
	#2  0x0000000000461e6f in write_sha1_file (buf=0x2ba371737fd0, len=46, type=0x49a50b "blob", returnsha1=0x2ba371733fe0 "")
	    at sha1_file.c:1962
	#3  0x000000000040aa9a in add_index_file (path=0x2ba371719ffc "one", mode=33188, buf=0x2ba371737fd0, size=46) at builtin-apply.c:2350
	#4  0x000000000040aeb5 in create_file (patch=0x2ba37170cf40) at builtin-apply.c:2451
	#5  0x000000000040af45 in write_out_one_result (patch=0x2ba37170cf40, phase=1) at builtin-apply.c:2475
	#6  0x000000000040b291 in write_out_results (list=0x2ba37170cf40, skipped_patch=0) at builtin-apply.c:2560
	#7  0x000000000040b71c in apply_patch (fd=6, filename=0x7fff395c96ec "patch.file", inaccurate_eof=0) at builtin-apply.c:2676
	#8  0x000000000040bd1b in cmd_apply (argc=3, argv=0x7fff395c8990, unused_prefix=0x0) at builtin-apply.c:2836
	#9  0x0000000000403fbb in handle_internal_command (argc=3, argv=0x7fff395c8990, envp=0x7fff395c89b0) at git.c:322
	#10 0x0000000000404193 in main (argc=3, argv=0x7fff395c8990, envp=0x7fff395c89b0) at git.c:391

so I thought I'd send out this email asap, in case somebody else finds the 
bug before I do.

Anyway, this looks like a real smoking gun..

		Linus
----
diff --git a/Makefile b/Makefile
index 51c1fed..7e20410 100644
--- a/Makefile
+++ b/Makefile
@@ -123,7 +123,7 @@ uname_P := $(shell sh -c 'uname -p 2>/dev/null || echo not')
 
 # CFLAGS and LDFLAGS are for the users to override from the command line.
 
-CFLAGS = -g -O2 -Wall
+CFLAGS = -g -Wall
 LDFLAGS =
 ALL_CFLAGS = $(CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
@@ -647,7 +647,7 @@ prefix_SQ = $(subst ','\'',$(prefix))
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 
-LIBS = $(GITLIBS) $(EXTLIBS)
+LIBS = $(GITLIBS) $(EXTLIBS) -lefence
 
 BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
 	-DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"' $(COMPAT_CFLAGS)

Re: My git repo is broken, how to fix it ?

[Linus Torvalds]

On Thu, 22 Mar 2007, Linus Torvalds wrote:
> 
> Yeehaa! I think I'm on the right trail.

.. and the reason only Alexander sees it, and nobody else does, is that 
this one is a bug in the CR/LF creation. 

Junio: I think it's your git-apply commit 67160271.

In "try_create_file()", we do:

	...
        if (convert_to_working_tree(path, &nbuf, &nsize)) {
                free((char *) buf);
                buf = nbuf;
                size = nsize;
        }
	...

but the thing is, the *caller* still uses the old "buf/nsize", so when you 
free it, the caller will now use the free'd data structure, and if it gets 
re-used by - for example - the zlib deflate() buffers, you'll get a 
corrupt object (if it gets re-used *before*, you'll get the *wrong* 
object!). Exactly Alexander's patterns.

Alexander - sorry for all the trouble, this was definitely our bad.

I think the easy temporary fix is to just remove that "free()" and leak a 
bit of memory. That gets it through that test with efence for me.

Does that fix it for you, Alexander?

I can't really say whether there are other problems too - electric fence 
has a few bugs in that it considers zero-length allocations to be 
"probably a bug" and aborts. This makes some of the tests fail with 
efence, when re_compile_internal wants to allocate a zero-length object.

(It also writes crap to stderr, which could make others fail, I didn't 
check).

		Linus

Re: My git repo is broken, how to fix it ?

[Nicolas Pitre]

On Thu, 22 Mar 2007, Linus Torvalds wrote:

> I can't really say whether there are other problems too - electric fence 
> has a few bugs in that it considers zero-length allocations to be 
> "probably a bug" and aborts. This makes some of the tests fail with 
> efence, when re_compile_internal wants to allocate a zero-length object.

You can tell it not to abort on zero-length allocations by setting the 
EF_ALLOW_MALLOC_0 environment variable to 1.


Nicolas

Re: My git repo is broken, how to fix it ?

[Linus Torvalds]

On Thu, 22 Mar 2007, Nicolas Pitre wrote:

> On Thu, 22 Mar 2007, Linus Torvalds wrote:
> 
> > I can't really say whether there are other problems too - electric fence 
> > has a few bugs in that it considers zero-length allocations to be 
> > "probably a bug" and aborts. This makes some of the tests fail with 
> > efence, when re_compile_internal wants to allocate a zero-length object.
> 
> You can tell it not to abort on zero-length allocations by setting the 
> EF_ALLOW_MALLOC_0 environment variable to 1.

Ahh,that gets me further, but then it bombs out on the added error 
messages. Is there something for that braindamage too?

(if it at least tested it with "isatty()" or something I'd understand it, 
as it is, it has made my life miserable in the past too.. I like efence, 
but bruce seems to think his ego is more important than the program 
you're debugging.)

		Linus

Re: My git repo is broken, how to fix it ?

[Nicolas Pitre]

On Thu, 22 Mar 2007, Linus Torvalds wrote:

> On Thu, 22 Mar 2007, Nicolas Pitre wrote:
> 
> > On Thu, 22 Mar 2007, Linus Torvalds wrote:
> > 
> > > I can't really say whether there are other problems too - electric fence 
> > > has a few bugs in that it considers zero-length allocations to be 
> > > "probably a bug" and aborts. This makes some of the tests fail with 
> > > efence, when re_compile_internal wants to allocate a zero-length object.
> > 
> > You can tell it not to abort on zero-length allocations by setting the 
> > EF_ALLOW_MALLOC_0 environment variable to 1.
> 
> Ahh,that gets me further, but then it bombs out on the added error 
> messages. Is there something for that braindamage too?

The efence man page doesn't mention any.


Nicolas

Re: My git repo is broken, how to fix it ?

[Jeff King]

On Thu, Mar 22, 2007 at 10:10:24AM -0700, Linus Torvalds wrote:

> Ahh,that gets me further, but then it bombs out on the added error 
> messages. Is there something for that braindamage too?

Try EF_DISABLE_BANNER=1

-Peff

Re: My git repo is broken, how to fix it ?

[Linus Torvalds]

On Thu, 22 Mar 2007, Jeff King wrote:
> 
> Try EF_DISABLE_BANNER=1

That does nothing for me. Nor does

	strings -- /usr/lib64/libefence.so | grep EF_

show that string or anything else half-way promising..

Googling for that shows that some versions of efence have had that flag 
(not necessarily as a environment variable, though). But certainly not the 
version I have.

		Linus

Re: My git repo is broken, how to fix it ?

[Bill Lear]

On Thursday, March 22, 2007 at 17:25:37 (-0700) Linus Torvalds writes:
>
>
>On Thu, 22 Mar 2007, Jeff King wrote:
>> 
>> Try EF_DISABLE_BANNER=1
>
>That does nothing for me. Nor does
>
>	strings -- /usr/lib64/libefence.so | grep EF_
>
>show that string or anything else half-way promising..
>
>Googling for that shows that some versions of efence have had that flag 
>(not necessarily as a environment variable, though). But certainly not the 
>version I have.

I just downloaded and installed the latest version (2.1.13):

% strings /usr/lib/libefence.a | grep BANNER
LLEF_DISABLE_BANNER
 EF_DISABLE_BANNER
EF_DISABLE_BANNER
EF_DISABLE_BANNER
EF_DISABLE_BANNER

http://perens.com/FreeSoftware/ElectricFence/electric-fence_2.1.13-0.1.tar.gz


Bill

Re: My git repo is broken, how to fix it ?

[Jeff King]

On Thu, Mar 22, 2007 at 05:25:37PM -0700, Linus Torvalds wrote:

> That does nothing for me. Nor does
> 
> 	strings -- /usr/lib64/libefence.so | grep EF_
> 
> show that string or anything else half-way promising..
> 
> Googling for that shows that some versions of efence have had that flag 
> (not necessarily as a environment variable, though). But certainly not the 
> version I have.

Hmm. It's in the latest debian package (2.1.14.1) and works as
advertised. I just poked at the FC6 version (2.2.2 -- but Bruce's last
version seemed to be the 2.1 series, so no idea who is responsible for
this brain damage), and it now unconditionally prints the banner.
Huzzah.

It at least goes to stderr, which might be redirectable; otherwise
you're stuck editing the source (see efence.c:initialize).

-Peff

[PATCH] git-apply: Do not free the wrong buffer when we convert the data for writeout

[Junio C Hamano]

When we write out the result of patch application, we sometimes
need to munge the data (e.g. under core.autocrlf).  After doing
so, what we should free is the temporary buffer that holds the
converted data returned from convert_to_working_tree(), not the
original one.

This patch also moves the call to open() up in the function, as
the caller expects us to fail cheaply if leading directories
need to be created (and then the caller creates them and calls
us again).  For that calling pattern, attempting conversion
before opening the file adds unnecessary overhead.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

  Linus Torvalds <torvalds@linux-foundation.org> writes:

  > In "try_create_file()", we do:
  >
  > 	...
  >         if (convert_to_working_tree(path, &nbuf, &nsize)) {
  >                 free((char *) buf);
  >                 buf = nbuf;
  >                 size = nsize;
  >         }
  > 	...
  >
  > but the thing is, the *caller* still uses the old "buf/nsize", so when you 
  > free it, the caller will now use the free'd data structure, and if it gets 
  > re-used by - for example - the zlib deflate() buffers, you'll get a 
  > corrupt object (if it gets re-used *before*, you'll get the *wrong* 
  > object!). Exactly Alexander's patterns.
  >
  > Alexander - sorry for all the trouble, this was definitely our bad.

  I concur.  Sorry for this, Alexander.

  git-apply in general is quite leaky (e.g. it never frees a
  finished patch, and freeing a patch is quite difficult as some
  strings like filenames are shared without refcounting), but
  this part deals with a large amount of data and I would rather
  not add a new one.

 builtin-apply.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index dfa1716..27a182b 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2355,7 +2355,7 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned
 
 static int try_create_file(const char *path, unsigned int mode, const char *buf, unsigned long size)
 {
-	int fd;
+	int fd, converted;
 	char *nbuf;
 	unsigned long nsize;
 
@@ -2364,17 +2364,18 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,
 		 * terminated.
 		 */
 		return symlink(buf, path);
+
+	fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 0100) ? 0777 : 0666);
+	if (fd < 0)
+		return -1;
+
 	nsize = size;
 	nbuf = (char *) buf;
-	if (convert_to_working_tree(path, &nbuf, &nsize)) {
-		free((char *) buf);
+	converted = convert_to_working_tree(path, &nbuf, &nsize);
+	if (converted) {
 		buf = nbuf;
 		size = nsize;
 	}
-
-	fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 0100) ? 0777 : 0666);
-	if (fd < 0)
-		return -1;
 	while (size) {
 		int written = xwrite(fd, buf, size);
 		if (written < 0)
@@ -2386,6 +2387,8 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,
 	}
 	if (close(fd) < 0)
 		die("closing file %s: %s", path, strerror(errno));
+	if (converted)
+		free(nbuf);
 	return 0;
 }
 

Re: [PATCH] git-apply: Do not free the wrong buffer when we convert the data for writeout

[Linus Torvalds]

On Thu, 22 Mar 2007, Junio C Hamano wrote:
> 
> This patch also moves the call to open() up in the function, as
> the caller expects us to fail cheaply if leading directories
> need to be created (and then the caller creates them and calls
> us again).  For that calling pattern, attempting conversion
> before opening the file adds unnecessary overhead.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>

Ack, looks good.

		Linus

Re: [PATCH] git-apply: Do not free the wrong buffer when we convert the data for writeout

[Alexander Litvinov]

В сообщении от Friday 23 March 2007 02:55 Linus Torvalds написал(a):
> On Thu, 22 Mar 2007, Junio C Hamano wrote:
> > This patch also moves the call to open() up in the function, as
> > the caller expects us to fail cheaply if leading directories
> > need to be created (and then the caller creates them and calls
> > us again).  For that calling pattern, attempting conversion
> > before opening the file adds unnecessary overhead.

I have applied this patch ontop of next (d06644b) and it also fix by repo 
breakage. 

Thanks for help !
Alexander Litvinov.

Re: My git repo is broken, how to fix it ?

[Alexander Litvinov]

В сообщении от Thursday 22 March 2007 22:48 Linus Torvalds написал(a):
> In "try_create_file()", we do:
>
> 	...
>         if (convert_to_working_tree(path, &nbuf, &nsize)) {
>                 free((char *) buf);
>                 buf = nbuf;
>                 size = nsize;
>         }
> 	...
>
> I think the easy temporary fix is to just remove that "free()" and leak a
> bit of memory. That gets it through that test with efence for me.
>
> Does that fix it for you, Alexander?

Yes, commenting out free fix repo breakage.

Thanks for help !

Re: My git repo is broken, how to fix it ?

[Johannes Sixt]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

Linus Torvalds wrote:
> Yeehaa! I think I'm on the right trail.
> 
> Git people: do this:
> 
>         yum install ElectricFence
> 
> (or similar, apt-get, whatever), and then apply this patch, and do
> 
>         make test
> 
> and it will fail in "git-apply"! Which (having read Alexander's corruption
> sequence once more) must have been what corrupted things for Alexander
> too!

Here is a repo, created with the MinGW port. It has a corrupted object,
that you can see even on Linux with just

$ git show :one

It was created with an obviously bogus "git apply" on Windows.
To restore the correct object, just do this on Linux:

$ git read-tree --reset -u HEAD
$ git apply --index patch.file

Now "git show :one" gives you the correct one.

HTH,
-- Hannes

[-- Attachment #2: trash.tgz --]
[-- Type: application/x-compressed, Size: 8823 bytes --]

Re: [PATCH] git-apply: Do not free the wrong buffer when we convert the data for writeout

[皐月]

iPhoneから送信