{"thread":{"id":"4595","subject":"Odd behavior with git and cairo-devel repo","startedAt":"2006-06-21T01:00:30Z","lastAt":"2006-06-21T19:43:12Z","messageCount":7,"participants":["Art Haas","Andre Noll","Junio C Hamano","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"22170","messageId":"20060621010030.GP2820@artsapartment.org","threadId":"4595","inReplyTo":null,"subject":"Odd behavior with git and cairo-devel repo","fromName":"Art Haas","fromEmail":"ahaas@airmail.net","sentAt":"2006-06-21T01:00:30Z","receivedAt":"2006-06-21T01:00:30Z","isPatch":false,"sender":{"key":"ahaas@airmail.net","avatar":null},"body":"Hi.\n\nI've been seeing some odd errors with git when it is dealing with\nthe Cairo graphics library repo. The errors include git failing\na 'fsck-objects' with a 'Floating point exception' error message\nand 'git prune' mangling the repo.\n\nA fresh cloning of the repo is needed to see the FPE error. The\nrepo is about 13M in size, btw.\n\n$ git clone git://git.cairographics.org/git/cairo cairo\n[ ... git clones the repo without problem ... ]\n$ cd cairo\n$ git fsck-objects\nFloating point exception\n$\n\nThe strace of this shows that things bomb after the '.git/index' file is\nread. Here's the tail end of the strace output ...\n\nclose(3)                                = 0\nopen(\".git/index\", O_RDONLY|O_LARGEFILE) = 3\nfstat64(3, {st_mode=S_IFREG|0644, st_size=51472, ...}) = 0\nmmap2(NULL, 51472, PROT_READ|PROT_WRITE, MAP_PRIVATE, 3, 0) = 0xb7f4b000\nclose(3)                                = 0\n--- SIGFPE (Floating point exception) @ 0 (0) ---\n+++ killed by SIGFPE +++\n\nNow, if the repo is added to and you 'git pull' to update your copy,\nthe 'git fsck-objects' command completes without error.\n\nAs for the problem where 'git prune' mangles the local copy, it would\nhappen after a 'git fsck-objects' run succeeds without incident.\nThen 'git prune' would run, seemingly without problems, and I would\ntry and repack my repo with 'git repack -a -d'. Here, things go\nboom with SHA1 errors, and subsequent 'git fsck-objects' runs\nproduce errors.\n\nMy local 'git' build is current with the 'master' branch. I've observed\nthese problems with the Cairo repo on a machine running Debian unstable,\nFedora Core 4, and Fedora Rawhide. The Debian machine uses git built\nby the current gcc-4.1 package, FC4 uses its current gcc-4.0 package,\nand rawhide uses its current gcc-4.1 package. All these machines are\nkept up-to-date with the respective current packages versions for\nthat distro.\n\nCan anyone out in git-land seeing problems with the Cairo\ngit repo, or able to duplicate these problems?\n\nThanks in advance,\n\nArt Haas\n-- \nMan once surrendering his reason, has no remaining guard against absurdities\nthe most monstrous, and like a ship without rudder, is the sport of every wind.\n\n-Thomas Jefferson to James Smith, 1822\n"},{"id":"22171","messageId":"20060621024605.GO11245@skl-net.de","threadId":"4595","inReplyTo":"20060621010030.GP2820@artsapartment.org","subject":"Re: Odd behavior with git and cairo-devel repo","fromName":"Andre Noll","fromEmail":"maan@systemlinux.org","sentAt":"2006-06-21T02:46:05Z","receivedAt":"2006-06-21T02:46:05Z","isPatch":false,"sender":{"key":"maan@systemlinux.org","avatar":null},"body":"On 20:00, Art Haas wrote:\n\n> $ git clone git://git.cairographics.org/git/cairo cairo\n> [ ... git clones the repo without problem ... ]\n> $ cd cairo\n> $ git fsck-objects\n> Floating point exception\n\nThis is due to refs_hash_size being zero in mark_reachable().\nBoth \"git fsck-objects --full\" and \"git repack -a -d\" seem to work\nfine with the patch below (tested by cloning your repo).\n\n---\nCurrently, we don't check refs_hash_size size and happily call\nlookup_object_refs() even if refs_hash_size is zero which leads to\na division by zero in hash_obj().\n\nSigned-off-by: Andre Noll <maan@systemlinux.org>\n---\ndiff --git a/object-refs.c b/object-refs.c\nindex 8afa227..a7d49c6 100644\n--- a/object-refs.c\n+++ b/object-refs.c\n@@ -127,6 +127,9 @@ void mark_reachable(struct object *obj, \n \n \tif (!track_object_refs)\n \t\tdie(\"cannot do reachability with object refs turned off\");\n+\t/* nothing to lookup */\n+\tif (!refs_hash_size)\n+\t\treturn;\n \t/* If we've been here already, don't bother */\n \tif (obj->flags & mask)\n \t\treturn;\n-- \nThe only person who always got his work done by Friday was Robinson Crusoe\n"},{"id":"22196","messageId":"7vy7vqzsa7.fsf@assigned-by-dhcp.cox.net","threadId":"4595","inReplyTo":"20060621024605.GO11245@skl-net.de","subject":"Re: Odd behavior with git and cairo-devel repo","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-21T11:01:36Z","receivedAt":"2006-06-21T11:01:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andre Noll <maan@systemlinux.org> writes:\n\n> On 20:00, Art Haas wrote:\n>\n>> $ git clone git://git.cairographics.org/git/cairo cairo\n>> [ ... git clones the repo without problem ... ]\n>> $ cd cairo\n>> $ git fsck-objects\n>> Floating point exception\n>\n> This is due to refs_hash_size being zero in mark_reachable().\n> Both \"git fsck-objects --full\" and \"git repack -a -d\" seem to work\n> fine with the patch below (tested by cloning your repo).\n\nGood catch.  Thanks both!\n"},{"id":"22201","messageId":"20060621120618.GR2820@artsapartment.org","threadId":"4595","inReplyTo":"20060621024605.GO11245@skl-net.de","subject":"Re: Odd behavior with git and cairo-devel repo","fromName":"Art Haas","fromEmail":"ahaas@airmail.net","sentAt":"2006-06-21T12:06:18Z","receivedAt":"2006-06-21T12:06:18Z","isPatch":false,"sender":{"key":"ahaas@airmail.net","avatar":null},"body":"On Wed, Jun 21, 2006 at 04:46:05AM +0200, Andre Noll wrote:\n> On 20:00, Art Haas wrote:\n> \n> > $ git clone git://git.cairographics.org/git/cairo cairo\n> > [ ... git clones the repo without problem ... ]\n> > $ cd cairo\n> > $ git fsck-objects\n> > Floating point exception\n> \n> This is due to refs_hash_size being zero in mark_reachable().\n> Both \"git fsck-objects --full\" and \"git repack -a -d\" seem to work\n> fine with the patch below (tested by cloning your repo).\n> \n> [ ... snip ... ]\n\nHi.\n\nI see this patch has made it into git now, and it fixes the problem\ndescribed above. Thanks!\n\nHowever, I'm still seeing the problem where 'git prune' leaves the\nrepo in an invalid state. In the case above, running 'git prune' on a\nfreshly checked-out repo works without problems; when the repo has a\nnumber of unpacked objects, however, things go bad. On the FC4 machine\nI have, I updated my git repo, rebuilt, and installed the build with\nthe patch above, then updated my cairo repo. The 'git pull' retrieved\na handful of objects, and 'git fsck-objects' ran without problem.\nThen 'git prune' was run, seemingly without problem, and when I tried\n'git repack -a -d' things went boom. A subsequent 'git fsck-object'\nrun indicated the repo was missing tree and commit objects.\n\nIs anyone else seeing a similar problem with 'git prune'?\n\nArt Haas\n-- \nMan once surrendering his reason, has no remaining guard against absurdities\nthe most monstrous, and like a ship without rudder, is the sport of every wind.\n\n-Thomas Jefferson to James Smith, 1822\n"},{"id":"22220","messageId":"20060621174632.GP11245@skl-net.de","threadId":"4595","inReplyTo":"20060621120618.GR2820@artsapartment.org","subject":"Re: Odd behavior with git and cairo-devel repo","fromName":"Andre Noll","fromEmail":"maan@systemlinux.org","sentAt":"2006-06-21T17:46:32Z","receivedAt":"2006-06-21T17:46:32Z","isPatch":false,"sender":{"key":"maan@systemlinux.org","avatar":null},"body":"On 07:06, Art Haas wrote:\n> I see this patch has made it into git now, and it fixes the problem\n> described above. Thanks!\n> \n> However, I'm still seeing the problem where 'git prune' leaves the\n> repo in an invalid state. In the case above, running 'git prune' on a\n> freshly checked-out repo works without problems; when the repo has a\n> number of unpacked objects, however, things go bad. On the FC4 machine\n> I have, I updated my git repo, rebuilt, and installed the build with\n> the patch above, then updated my cairo repo. The 'git pull' retrieved\n> a handful of objects, and 'git fsck-objects' ran without problem.\n> Then 'git prune' was run, seemingly without problem, and when I tried\n> 'git repack -a -d' things went boom. A subsequent 'git fsck-object'\n> run indicated the repo was missing tree and commit objects.\n> \n> Is anyone else seeing a similar problem with 'git prune'?\n\nYeah. This is due to a thinko in grow_refs_hash() which was introduced in\n\n\tcommit 3e4339e6f96e8c4f38a9c6607b98d3e96a2ed783\n\tAuthor: Linus Torvalds <torvalds@osdl.org>\n\tDate:   Sun Jun 18 11:45:02 2006 -0700\n\n\t    Remove \"refs\" field from \"struct object\"\n\nFix below.\n---\nFix grow_refs_hash()\n\nAs the hash values depend on the hash size, they have to be\nrecalulated when growing the hash. It's possible (though unlikely)\nthat there are duplicates even with the new, larger hash size, so\nmake grow_refs_hash() check for duplicates.\n\nSigned-off-by: Andre Noll <maan@systemlinux.org>\n---\ndiff --git a/object-refs.c b/object-refs.c\nindex 8afa227..fa1d3c1 100644\n--- a/object-refs.c\n+++ b/object-refs.c\n@@ -25,6 +25,8 @@ static void grow_refs_hash(void)\n \t\tif (!ref)\n \t\t\tcontinue;\n \t\tj = hash_obj(ref->base, new_hash_size);\n+\t\twhile (new_hash[j])\n+\t\t\tj = (j + 1) % new_hash_size;\n \t\tnew_hash[j] = ref;\n \t}\n \tfree(refs_hash);\n-- \nThe only person who always got his work done by Friday was Robinson Crusoe\n"},{"id":"22221","messageId":"Pine.LNX.4.64.0606211054500.5498@g5.osdl.org","threadId":"4595","inReplyTo":"20060621174632.GP11245@skl-net.de","subject":"Re: Odd behavior with git and cairo-devel repo","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-06-21T18:01:12Z","receivedAt":"2006-06-21T18:01:12Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 21 Jun 2006, Andre Noll wrote:\n>\n> Fix grow_refs_hash()\n\nOuch. \n\nActually, the alternate patch is the one I had intended to do but for some \nreasons didn't.\n\nThis one also removes two more lines than it adds, but it's obviously a \nbigger patch.\n\nJunio, depending on which one you prefer, add the appropriate\n\n\tAcked-by: Linus Torvalds <torvalds@osdl.org>\n\n(for Andre's one) or\n\n\tSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n\nif you take this one.\n\n\t\tLinus\n---\n object-refs.c |   30 ++++++++++++++----------------\n 1 files changed, 14 insertions(+), 16 deletions(-)\n\ndiff --git a/object-refs.c b/object-refs.c\nindex a7d49c6..b1b8065 100644\n--- a/object-refs.c\n+++ b/object-refs.c\n@@ -12,6 +12,18 @@ static unsigned int hash_obj(struct obje\n \treturn hash % n;\n }\n \n+static void insert_ref_hash(struct object_refs *ref, struct object_refs **hash, unsigned int size)\n+{\n+\tint j = hash_obj(ref->base, size);\n+\n+\twhile (hash[j]) {\n+\t\tj++;\n+\t\tif (j >= size)\n+\t\t\tj = 0;\n+\t}\n+\thash[j] = ref;\n+}\n+\n static void grow_refs_hash(void)\n {\n \tint i;\n@@ -20,30 +32,16 @@ static void grow_refs_hash(void)\n \n \tnew_hash = calloc(new_hash_size, sizeof(struct object_refs *));\n \tfor (i = 0; i < refs_hash_size; i++) {\n-\t\tint j;\n \t\tstruct object_refs *ref = refs_hash[i];\n \t\tif (!ref)\n \t\t\tcontinue;\n-\t\tj = hash_obj(ref->base, new_hash_size);\n-\t\tnew_hash[j] = ref;\n+\t\tinsert_ref_hash(ref, new_hash, new_hash_size);\n \t}\n \tfree(refs_hash);\n \trefs_hash = new_hash;\n \trefs_hash_size = new_hash_size;\n }\n \n-static void insert_ref_hash(struct object_refs *ref)\n-{\n-\tint j = hash_obj(ref->base, refs_hash_size);\n-\n-\twhile (refs_hash[j]) {\n-\t\tj++;\n-\t\tif (j >= refs_hash_size)\n-\t\t\tj = 0;\n-\t}\n-\trefs_hash[j] = ref;\n-}\n-\n static void add_object_refs(struct object *obj, struct object_refs *ref)\n {\n \tint nr = nr_object_refs + 1;\n@@ -51,7 +49,7 @@ static void add_object_refs(struct objec\n \tif (nr > refs_hash_size * 2 / 3)\n \t\tgrow_refs_hash();\n \tref->base = obj;\n-\tinsert_ref_hash(ref);\n+\tinsert_ref_hash(ref, refs_hash, refs_hash_size);\n \tnr_object_refs = nr;\n }\n \n"},{"id":"22222","messageId":"7v3bdyz44v.fsf@assigned-by-dhcp.cox.net","threadId":"4595","inReplyTo":"Pine.LNX.4.64.0606211054500.5498@g5.osdl.org","subject":"Re: Odd behavior with git and cairo-devel repo","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-21T19:43:12Z","receivedAt":"2006-06-21T19:43:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> Ouch. \n\nOuch indeed ;-).\n\n> Actually, the alternate patch is the one I had intended to do but for some \n> reasons didn't.\n>\n> This one also removes two more lines than it adds, but it's obviously a \n> bigger patch.\n\nI'll take this, since I think keeping the hash population\n(collision maintenance) code in one place like your version does\nseems cleaner for me to read three months down the road.\n\nAgain, thanks Andre, Art and Linus.\n"}]}