{"thread":{"id":"17667","subject":"\"repack\" when a needed commit object missing \"kills\" the repo","startedAt":"2009-02-09T02:26:42Z","lastAt":"2009-02-11T09:27:43Z","messageCount":4,"participants":["Björn Steinbrink","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"103808","messageId":"20090209022642.GA19413@atjola.homenet","threadId":"17667","inReplyTo":null,"subject":"\"repack\" when a needed commit object missing \"kills\" the repo","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2009-02-09T02:26:42Z","receivedAt":"2009-02-09T02:26:42Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"Hi,\n\nI have a git-svn repo with a large grafts file to recreate merges done\nin svn and in one case to simply change a commit's parent. I don't\nrecall the reason for the latter, it's been there for almost a year now,\nprobably some crap from the svn repo that I didn't want to have in my\ngit history. So the original history was like:\n\nA---B---C\n\nAnd the graft turns that into:\n\nA---C\n\nEarlier repacks seem to have already pruned the B commit object, so\nremoving the grafts correctly causes git to complain about that missing\nobject.\n\nNow I decided to finally run filter-branch on that repo to get rid of\nthe huge grafts file. So I ran \"git filter-branch -- --all\", removed the\ngrafts file, dropped all the refs/original/ stuff and went on to \"git\nrepack -adf\". And when that finished, a _lot_ of objects were suddenly\nmissing, including some of the rewritten commits --> repo busted.\nFortunately I had made a backup. :-)\n\nThe pack-objects process complained about the B commit object that was\nalready missing earlier, and after some time I realised that I didn't\nexpire the reflogs, so the \"broken\" commit was still reachable through\nthem. So I retried with the reflogs empty, and yup, that worked well. So\nmissing objects seem to confuse pack-objects enough to stop it from\npacking all of objects reachable through non-broken chains.\n\nA backtrace from the point at which pack-objects complains about the\nmissing object shows this:\n\n#0  error (err=0x4bff35 \"Could not read %s\") at usage.c:64\n#1  0x0000000000459e3c in parse_commit (item=0x15f8c50) at commit.c:304\n#2  0x000000000048d2ea in add_parents_to_list (revs=0x7fff5f8bd7c0, \n    commit=0x15f8a10, list=0x7fff5f8bd7c0, cache_ptr=0x0) at revision.c:514\n#3  0x000000000048d75d in get_revision_1 (revs=0x7fff5f8bd7c0)\n    at revision.c:1740\n#4  0x000000000048d7cb in get_revision_internal (revs=0x7fff5f8bd7c0)\n    at revision.c:1843\n#5  0x000000000048da31 in get_revision (revs=0x7fff5f8bd7c0) at revision.c:1924\n#6  0x0000000000471f0c in traverse_commit_list (revs=0x7fff5f8bd7c0, \n    show_commit=0x437ec0 <show_commit>, show_object=0x437e80 <show_object>)\n    at list-objects.c:147\n#7  0x0000000000437fc8 in get_object_list (ac=<value optimized out>, \n    av=<value optimized out>) at builtin-pack-objects.c:2059\n#8  0x000000000043aa4e in cmd_pack_objects (argc=6, argv=0x7fff5f8bde38, \n    prefix=<value optimized out>) at builtin-pack-objects.c:2288\n#9  0x0000000000404983 in handle_internal_command (argc=6, argv=0x7fff5f8bde38)\n    at git.c:244\n#10 0x0000000000404eb6 in main (argc=6, argv=0x7fff5f8bde38) at git.c:473\n\nMaybe get_revision_internal() switching into boundary mode is bad there?\nThe failing parse_commit() call causes get_revision_1() to return NULL,\nso get_revision_internal() assumes it ran out of commits, but in this\ncase, it \"just\" hit a bad commit.\n\nUp to now, I don't have a small reproduction case yet and unfortunately\nI can't make the repo with which I've seen the failure available.\n\nThanks,\nBjörn\n"},{"id":"103827","messageId":"20090209091303.GA1320@atjola.homenet","threadId":"17667","inReplyTo":"20090209022642.GA19413@atjola.homenet","subject":"Re: repack might cause having when commit objects are missing","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2009-02-09T09:13:03Z","receivedAt":"2009-02-09T09:13:03Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"[topic fixed to resemble something sane...]\n\nOn 2009.02.09 03:26:42 +0100, Björn Steinbrink wrote:\n> Up to now, I don't have a small reproduction case yet and unfortunately\n> I can't make the repo with which I've seen the failure available.\n\nNot a self-contained reproduction test, but at least it shows the\nproblem with a public repo. Assuming you have no refs referencing\na0325eb^\n\nIn a clone of git.git add to .git/info/grafts:\na0325eb2e982e25376e4cb89ebcac5d8d703548d 604dd0a0782b3e33bcc397c27e811ebc019c9f5a\n\nThen:\ngit reflog expire --expire=0 --all\ngit repack -ad\nrm .git/info/grafts\ngit repack -ad\n\nThe second repack wrecks the repo:\n\ndoener@atjola:git2 (master) $ cp /home/doener/grafts .git/info/\ndoener@atjola:git2 (master) $ git reflog expire --expire=0 --all\ndoener@atjola:git2 (master) $ git repack -ad\nCounting objects: 92161, done.\nDelta compression using 4 threads.\nCompressing objects: 100% (23481/23481), done.\nWriting objects: 100% (92161/92161), done.\nTotal 92161 (delta 67185), reused 92061 (delta 67092)\ndoener@atjola:git2 (master) $ rm .git/info/grafts\ndoener@atjola:git2 (master) $ git repack -ad\nerror: Could not read c0e2e12fa0babcc4ab28b95bc5ad4f86e139d6b4\nCounting objects: 3195, done.\nDelta compression using 4 threads.\nCompressing objects: 100% (2245/2245), done.\nWriting objects: 100% (3195/3195), done.\nTotal 3195 (delta 1263), reused 2418 (delta 886)\nerror: refs/heads/rebase does not point to a valid object!\n[dozens of more broken refs]\n\nNote how in the second repack, the walking machniery finds only 3195\nobjects, as opposed to more than 92k before.\n\nI've also tried with threaded delta search disabled (pack.threads=1),\nno difference.\n\nBjörn\n"},{"id":"103828","messageId":"20090209092113.GB1320@atjola.homenet","threadId":"17667","inReplyTo":"20090209091303.GA1320@atjola.homenet","subject":"Re: repack might cause having when commit objects are missing","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2009-02-09T09:21:13Z","receivedAt":"2009-02-09T09:21:13Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2009.02.09 10:13:03 +0100, Björn Steinbrink wrote:\n> [topic fixed to resemble something sane...]\n\nI'll just give up... s/having/havoc/ FWIW\n\nBjörn, too stupid to type\n"},{"id":"104158","messageId":"7vfxilgw6o.fsf_-_@gitster.siamese.dyndns.org","threadId":"17667","inReplyTo":"20090209092113.GB1320@atjola.homenet","subject":"[PATCH] revision traversal and pack: notice and die on missing commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-11T09:27:43Z","receivedAt":"2009-02-11T09:27:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"cc0e6c5 (Handle return code of parse_commit in revision machinery,\n2007-05-04) attempted to tighten error checking in the revision machinery,\nbut it wasn't enough.  When get_revision_1() was asked for the next commit\nto return, it tries to read and simplify the parents of the commit to be\nreturned, but an error while doing so was silently ignored and reported as\na truncated history to the caller instead.\n\nThis resulted in an early end of \"git log\" output or a pack that lacks\nolder commits from \"git pack-objects\", without any error indication in the\nexit status from these commands, even though the underlying parse_commit()\nissues an error message to the end user.\n\nNote that the codepath in add_parents_list() that paints parents of an\nUNINTERESTING commit UNINTERESTING silently ignores the error when\nparse_commit() fails; this is deliberate and in line with aeeae1b\n(revision traversal: allow UNINTERESTING objects to be missing,\n2009-01-27).\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * Dscho, do not complain that I did not use test_commit, because this is\n   meant to apply against maint-1.5.4, which happens to be the oldest\n   maintenance track I am still keeping around.  The breakage actually\n   goes even older; cc0e6c5 is *not* the culprit, but merely an incomplete\n   attempt to fix this issue.\n\n   There is a fairly obvious comflict when this patch is applied to maint\n   and master, but I'll be merging the commit upward after a bit more\n   testing.\n\n revision.c                     |    6 ++++--\n t/t5307-pack-missing-commit.sh |   39 +++++++++++++++++++++++++++++++++++++++\n 2 files changed, 43 insertions(+), 2 deletions(-)\n create mode 100755 t/t5307-pack-missing-commit.sh\n\ndiff --git a/revision.c b/revision.c\nindex d79f2b3..b8c015e 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1484,14 +1484,16 @@ static struct commit *get_revision_1(struct rev_info *revs)\n \t\t\t    (commit->date < revs->max_age))\n \t\t\t\tcontinue;\n \t\t\tif (add_parents_to_list(revs, commit, &revs->commits) < 0)\n-\t\t\t\treturn NULL;\n+\t\t\t\tdie(\"Failed to traverse parents of commit %s\",\n+\t\t\t\t    sha1_to_hex(commit->object.sha1));\n \t\t}\n \n \t\tswitch (simplify_commit(revs, commit)) {\n \t\tcase commit_ignore:\n \t\t\tcontinue;\n \t\tcase commit_error:\n-\t\t\treturn NULL;\n+\t\t\tdie(\"Failed to simplify parents of commit %s\",\n+\t\t\t    sha1_to_hex(commit->object.sha1));\n \t\tdefault:\n \t\t\treturn commit;\n \t\t}\ndiff --git a/t/t5307-pack-missing-commit.sh b/t/t5307-pack-missing-commit.sh\nnew file mode 100755\nindex 0000000..ae52a18\n--- /dev/null\n+++ b/t/t5307-pack-missing-commit.sh\n@@ -0,0 +1,39 @@\n+#!/bin/sh\n+\n+test_description='pack should notice missing commit objects'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tfor i in 1 2 3 4 5\n+\tdo\n+\t\techo \"$i\" >\"file$i\" &&\n+\t\tgit add \"file$i\" &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"$i\" &&\n+\t\tgit tag \"tag$i\"\n+\tdone &&\n+\tobj=$(git rev-parse --verify tag3) &&\n+\tfanout=$(expr \"$obj\" : \"\\(..\\)\") &&\n+\tremainder=$(expr \"$obj\" : \"..\\(.*\\)\") &&\n+\trm -f \".git/objects/$fanout/$remainder\"\n+'\n+\n+test_expect_success 'check corruption' '\n+\ttest_must_fail git fsck\n+'\n+\n+test_expect_success 'rev-list notices corruption (1)' '\n+\ttest_must_fail git rev-list HEAD\n+'\n+\n+test_expect_success 'rev-list notices corruption (2)' '\n+\ttest_must_fail git rev-list --objects HEAD\n+'\n+\n+test_expect_success 'pack-objects notices corruption' '\n+\techo HEAD |\n+\ttest_must_fail git pack-objects --revs pack\n+'\n+\n+test_done\n-- \n1.6.2.rc0.50.g0a27\n"}]}