{"thread":{"id":"15160","subject":"[BUG?] 'git verify-pack -v' on multiple packs fails for all but the first pack","startedAt":"2008-08-22T12:39:31Z","lastAt":"2008-08-22T21:57:04Z","messageCount":6,"participants":["Johan Herland","Nicolas Pitre"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"88122","messageId":"200808221439.31219.johan@herland.net","threadId":"15160","inReplyTo":null,"subject":"[BUG?] 'git verify-pack -v' on multiple packs fails for all but the first pack","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-08-22T12:39:31Z","receivedAt":"2008-08-22T12:39:31Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"(I don't have time to look into this right now, but will do so later if \nnobody comes up with a solution in the meantime...)\n\nWhen running 'git verify-pack -v' on multiple packs (.idx files), it \nfails for all packs, except the first, with exit code 128, and the \nfollowing single line:\n\n\tfatal: internal error: pack revindex fubar\n\nThis does not happen when given only a single pack, or when given \nmultiple packs, but without '-v' option.\n\nTo reproduce, simply do:\n\n\tgit verify-pack -v .git/objects/pack/*.idx\n\nin any repo with more than one pack file.\n\nThis happens with a fairly current 'next' (1.6.0.96.g2fad1). AFAICS, it \nalso happens in v1.6.0.\n\n\n...Johan\n\n\nPS: Does someone knows a better way to generate a sorted list of all the \nblob objects (by SHA1 sum) in a packed repo?\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"88125","messageId":"200808221527.21971.johan@herland.net","threadId":"15160","inReplyTo":"200808221439.31219.johan@herland.net","subject":"Re: [BUG?] 'git verify-pack -v' on multiple packs fails for all but the first pack","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-08-22T13:27:21Z","receivedAt":"2008-08-22T13:27:21Z","isPatch":false,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Friday 22 August 2008, Johan Herland wrote:\n> (I don't have time to look into this right now, but will do so later\n> if nobody comes up with a solution in the meantime...)\n>\n> When running 'git verify-pack -v' on multiple packs (.idx files), it\n> fails for all packs, except the first, with exit code 128, and the\n> following single line:\n>\n> \tfatal: internal error: pack revindex fubar\n>\n> This does not happen when given only a single pack, or when given\n> multiple packs, but without '-v' option.\n>\n> To reproduce, simply do:\n>\n> \tgit verify-pack -v .git/objects/pack/*.idx\n>\n> in any repo with more than one pack file.\n>\n> This happens with a fairly current 'next' (1.6.0.96.g2fad1). AFAICS,\n> it also happens in v1.6.0.\n\nBisection point to this commit:\n\ncommit 1f5c74f6cf918d317c73b328dcd4cf6f55c44d8a\nAuthor: Nicolas Pitre \nDate: Mon Jun 23 21:22:14 2008 -0400\n\n call init_pack_revindex() lazily\n\n This makes life much easier for next patch, as well as being more efficient\n when the revindex is actually not used.\n\n Signed-off-by: Nicolas Pitre \n Signed-off-by: Junio C Hamano \n\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"88129","messageId":"200808221545.55709.johan@herland.net","threadId":"15160","inReplyTo":"200808221527.21971.johan@herland.net","subject":"[PATCH] Selftest for verifying 'git verify-pack -v' with multiple arguments","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-08-22T13:45:55Z","receivedAt":"2008-08-22T13:45:55Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"This is set to expect failure. Please update when fixing the associated bug.\n\nSigned-off-by: Johan Herland \n---\n t/t5300-pack-object.sh | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 645583f..339375a 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -186,6 +186,12 @@ test_expect_success \\\n \t\t\ttest-2-${packname_2}.idx \\\n \t\t\ttest-3-${packname_3}.idx'\n \n+test_expect_failure \\\n+ 'verify pack -v' \\\n+ 'git verify-pack -v\ttest-1-${packname_1}.idx \\\n+\t\t\ttest-2-${packname_2}.idx \\\n+\t\t\ttest-3-${packname_3}.idx'\n+\n test_expect_success \\\n 'verify-pack catches mismatched .idx and .pack files' \\\n 'cat test-1-${packname_1}.idx >test-3.idx &&\n-- \n1.6.0.96.g2fad1\n"},{"id":"88130","messageId":"alpine.LFD.1.10.0808220949080.1624@xanadu.home","threadId":"15160","inReplyTo":"200808221527.21971.johan@herland.net","subject":"Re: [BUG?] 'git verify-pack -v' on multiple packs fails for all but the first pack","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-08-22T13:49:50Z","receivedAt":"2008-08-22T13:49:50Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 22 Aug 2008, Johan Herland wrote:\n\n> On Friday 22 August 2008, Johan Herland wrote:\n> > (I don't have time to look into this right now, but will do so later\n> > if nobody comes up with a solution in the meantime...)\n> >\n> > When running 'git verify-pack -v' on multiple packs (.idx files), it\n> > fails for all packs, except the first, with exit code 128, and the\n> > following single line:\n> >\n> > \tfatal: internal error: pack revindex fubar\n> >\n> > This does not happen when given only a single pack, or when given\n> > multiple packs, but without '-v' option.\n> >\n> > To reproduce, simply do:\n> >\n> > \tgit verify-pack -v .git/objects/pack/*.idx\n> >\n> > in any repo with more than one pack file.\n> >\n> > This happens with a fairly current 'next' (1.6.0.96.g2fad1). AFAICS,\n> > it also happens in v1.6.0.\n> \n> Bisection point to this commit:\n> \n> commit 1f5c74f6cf918d317c73b328dcd4cf6f55c44d8a\n> Author: Nicolas Pitre \n> Date: Mon Jun 23 21:22:14 2008 -0400\n> \n> call init_pack_revindex() lazily\n\nI'll have a look later today.\n\n\nNicolas\n"},{"id":"88182","messageId":"alpine.LFD.1.10.0808221540290.1624@xanadu.home","threadId":"15160","inReplyTo":"200808221527.21971.johan@herland.net","subject":"[PATCH] discard revindex data when pack list changes","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-08-22T19:45:53Z","receivedAt":"2008-08-22T19:45:53Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"This is needed to fix verify-pack -v with multiple pack arguments.\n\nAlso, in theory, revindex data (if any) must be discarded whenever \nreprepare_packed_git() is called. In practice this is hard to trigger \nthough.\n\nSigned-off-by: Nicolas Pitre \n---\n\nOn Fri, 22 Aug 2008, Johan Herland wrote:\n\n> On Friday 22 August 2008, Johan Herland wrote:\n> > (I don't have time to look into this right now, but will do so later\n> > if nobody comes up with a solution in the meantime...)\n> >\n> > When running 'git verify-pack -v' on multiple packs (.idx files), it\n> > fails for all packs, except the first, with exit code 128, and the\n> > following single line:\n> >\n> > \tfatal: internal error: pack revindex fubar\n> >\n> > This does not happen when given only a single pack, or when given\n> > multiple packs, but without '-v' option.\n> >\n> > To reproduce, simply do:\n> >\n> > \tgit verify-pack -v .git/objects/pack/*.idx\n> >\n> > in any repo with more than one pack file.\n> >\n> > This happens with a fairly current 'next' (1.6.0.96.g2fad1). AFAICS,\n> > it also happens in v1.6.0.\n> \n> Bisection point to this commit:\n> \n> commit 1f5c74f6cf918d317c73b328dcd4cf6f55c44d8a\n\nThis patch should fix it. Lightly tested, but appears to work for me.\n\nI'm leaving for the weekend now so hopefully I've got it right.\n\ndiff --git a/builtin-verify-pack.c b/builtin-verify-pack.c\nindex f4ac595..c88ca18 100644\n--- a/builtin-verify-pack.c\n+++ b/builtin-verify-pack.c\n@@ -129,6 +129,7 @@ int cmd_verify_pack(int argc, const char **argv, const char *prefix)\n \t\telse {\n \t\t\tif (verify_one_pack(argv[1], verbose))\n \t\t\t\terr = 1;\n+\t\t\tdiscard_revindex();\n \t\t\tnothing_done = 0;\n \t\t}\n \t\targc--; argv++;\ndiff --git a/pack-revindex.c b/pack-revindex.c\nindex cd300bd..6096b62 100644\n--- a/pack-revindex.c\n+++ b/pack-revindex.c\n@@ -142,3 +142,15 @@ struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs)\n \t} while (lo < hi);\n \tdie(\"internal error: pack revindex corrupt\");\n }\n+\n+void discard_revindex(void)\n+{\n+\tif (pack_revindex_hashsz) {\n+\t\tint i;\n+\t\tfor (i = 0; i < pack_revindex_hashsz; i++)\n+\t\t\tif (pack_revindex[i].revindex)\n+\t\t\t\tfree(pack_revindex[i].revindex);\n+\t\tfree(pack_revindex);\n+\t\tpack_revindex_hashsz = 0;\n+\t}\n+}\ndiff --git a/pack-revindex.h b/pack-revindex.h\nindex 36a514a..8d5027a 100644\n--- a/pack-revindex.h\n+++ b/pack-revindex.h\n@@ -7,5 +7,6 @@ struct revindex_entry {\n };\n \n struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs);\n+void discard_revindex(void);\n \n #endif\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 2aff59b..9ee1ed1 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -990,6 +990,7 @@ void prepare_packed_git(void)\n \n void reprepare_packed_git(void)\n {\n+\tdiscard_revindex();\n \tprepare_packed_git_run_once = 0;\n \tprepare_packed_git();\n }\n"},{"id":"88214","messageId":"200808222357.04912.johan@herland.net","threadId":"15160","inReplyTo":"alpine.LFD.1.10.0808221540290.1624@xanadu.home","subject":"Re: [PATCH] discard revindex data when pack list changes","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2008-08-22T21:57:04Z","receivedAt":"2008-08-22T21:57:04Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Friday 22 August 2008, Nicolas Pitre wrote:\n> This is needed to fix verify-pack -v with multiple pack arguments.\n>\n> Also, in theory, revindex data (if any) must be discarded whenever\n> reprepare_packed_git() is called. In practice this is hard to trigger\n> though.\n>\n> Signed-off-by: Nicolas Pitre \n\nTested-by: Johan Herland \n\nThanks. Works for me :)\n\nJunio: if you added the my testcase, please squash the following into this \npatch before applying:\n\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 339375a..83abe5f 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -186,7 +186,7 @@ test_expect_success \\\n \t\t\ttest-2-${packname_2}.idx \\\n \t\t\ttest-3-${packname_3}.idx'\n \n-test_expect_failure \\\n+test_expect_success \\\n 'verify pack -v' \\\n 'git verify-pack -v\ttest-1-${packname_1}.idx \\\n \t\t\ttest-2-${packname_2}.idx \\\n-- \n1.6.0.96.g2fad1\n\n\nHave fun! :)\n\n...Johan\n\n\n-- \nJohan Herland, \nwww.herland.net\n"}]}