{"thread":{"id":"62029","subject":"git 2.46.0 crashes when trying to verify-pack outside of a repo","startedAt":"2024-08-31T06:53:04Z","lastAt":"2024-09-04T06:26:29Z","messageCount":7,"participants":["Ilya K","Junio C Hamano","Patrick Steinhardt","brian m. carlson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"501950","messageId":"14ec9394-a7ea-456c-800a-6a84c52e5cda@0upti.me","threadId":"62029","inReplyTo":null,"subject":"git 2.46.0 crashes when trying to verify-pack outside of a repo","fromName":"Ilya K","fromEmail":"me@0upti.me","sentAt":"2024-08-31T06:46:10Z","receivedAt":"2024-08-31T06:53:04Z","isPatch":false,"sender":{"key":"me@0upti.me","avatar":null},"body":"Hello folks!\n\nWe've updated to Git 2.46.0 in NixOS, and encountered an issue with Dulwich (a Python Git implementation) tests failing[0] because it attempts to call `git verify-pack` on a bare pack, with no surrounding repo. This used to work in Git 2.45.x, but in 2.46 it simply prints \"error: index-pack died of signal 11\". This seems to happen on any pack, including ones generated by Git itself (at least on a random pack from a random checkout I tested).\n\nHere's a traceback from the crash:\n\n    (gdb) bt\n    #0  0x00000000005d93ef in add_packed_git (\n        path=path@entry=0x7dd8f0 \"pack-a94057e69dcef307b749cd32232e845c48bb32c9.idx\", path_len=45,\n        local=local@entry=1) at packfile.c:754\n    754             if (path_len < the_hash_algo->hexsz ||\n    #1  0x000000000046bd38 in read_idx_option (\n        pack_name=0x7dd8f0 \"pack-a94057e69dcef307b749cd32232e845c48bb32c9.idx\", opts=0x7fffffffcd00)\n        at builtin/index-pack.c:1652\n    1652            struct packed_git *p = add_packed_git(pack_name, strlen(pack_name), 1);\n    #2  cmd_index_pack (argc=3, argv=<optimized out>, prefix=<optimized out>)\n        at builtin/index-pack.c:1883\n    1883                    read_idx_option(&opts, index_name);\n    #3  0x000000000040651d in run_builtin (argv=0x7fffffffd270, argc=3, p=0x7a0560 <commands+1344>)\n        at git.c:476\n    476             status = p->fn(argc, argv, prefix);\n    #4  handle_builtin (argc=3, argv=0x7fffffffd270) at git.c:732\n    732                     exit(run_builtin(builtin, argc, argv));\n    #5  0x0000000000407690 in run_argv (argcp=argcp@entry=0x7fffffffd04c,\n        argv=argv@entry=0x7fffffffd040) at git.c:796\n    796                             handle_builtin(*argcp, *argv);\n    #6  0x000000000040812c in cmd_main (argc=<optimized out>, argc@entry=4, argv=<optimized out>,\n        argv@entry=0x7fffffffd268) at git.c:931\n    931                     int was_alias = run_argv(&argc, &argv);\n    #7  0x00000000004061fd in main (argc=4, argv=0x7fffffffd268) at common-main.c:64\n    64              result = cmd_main(argc, argv);\n\nIt seems like `the_hash_algo` (which, AFAIUI, is macroed to `the_repository->hash_algo`) may be uninitialized here, but I'm not familiar enough with the codebase to really tell if that's the issue.\n\nIf you want to reproduce it, simply copy any pack file from a repo's .git/objects/pack/ to a directory that's not a repo, then `git verify-pack <the-file-you-just-copied>`.\n\nIf you need any more information, please contact me.\n\nThanks in advance!\n\n[0]: https://github.com/jelmer/dulwich/issues/1359\n\n"},{"id":"501956","messageId":"xmqq7cbvpf8v.fsf@gitster.g","threadId":"62029","inReplyTo":"14ec9394-a7ea-456c-800a-6a84c52e5cda@0upti.me","subject":"Re: git 2.46.0 crashes when trying to verify-pack outside of a repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-01T15:26:08Z","receivedAt":"2024-09-01T15:26:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ilya K <me@0upti.me> writes:\n\n> We've updated to Git 2.46.0 in NixOS, and encountered an issue\n> with Dulwich (a Python Git implementation) tests failing[0]\n> because it attempts to call `git verify-pack` on a bare pack, with\n> no surrounding repo. This used to work in Git 2.45.x, but in 2.46\n> it simply prints \"error: index-pack died of signal 11\".\n\nThanks.  This is a fallout from code-wide clean-up in 2.46.0 where\nwe do not assume that everybody runs SHA-1.\n\n------- >8 -------\nSubject: verify-pack: fall back to SHA-1 outside a repo\n\nIn c8aed5e8da (repository: stop setting SHA1 as the default object hash,\n2024-05-07), we have stopped setting the default hash algorithm for\n`the_repository`. Consequently, code that relies on `the_hash_algo` will\nnow crash when it hasn't explicitly been initialized, which may be the\ncase when running outside of a Git repository.\n\nAs the verify-pack command ought to be able to infer what algorithm\nis used in the input file (and if the input file does not have such\nan information, that by itself is a problem), and the command allows\nan option to explicitly tell what algorithm to use in case it cannot\nbe guessed from the input file, in theory we shouldn't have to use\nthe default algorithm anywhere in the operation of the command, but\nwe fail fairly early in the process when run outside a repository\nwithout any default algorithm set.\n\nResurrect the setting of the default algorithm just like we used to\ndo before 2.46.0\n\nReported-by: Ilya K <me@0upti.me>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/verify-pack.c  | 4 ++++\n t/t5300-pack-object.sh | 4 ++++\n 2 files changed, 8 insertions(+)\n\ndiff --git c/builtin/verify-pack.c w/builtin/verify-pack.c\nindex 011dddd2dc..5b663905ae 100644\n--- c/builtin/verify-pack.c\n+++ w/builtin/verify-pack.c\n@@ -1,6 +1,7 @@\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"gettext.h\"\n+#include \"hash.h\"\n #include \"run-command.h\"\n #include \"parse-options.h\"\n #include \"strbuf.h\"\n@@ -77,6 +78,9 @@ int cmd_verify_pack(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n+\tif (!the_hash_algo)\n+\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n+\n \tgit_config(git_default_config, NULL);\n \targc = parse_options(argc, argv, prefix, verify_pack_options,\n \t\t\t     verify_pack_usage, 0);\ndiff --git c/t/t5300-pack-object.sh w/t/t5300-pack-object.sh\nindex 4ad023c846..d6f45d8923 100755\n--- c/t/t5300-pack-object.sh\n+++ w/t/t5300-pack-object.sh\n@@ -322,6 +322,10 @@ test_expect_success 'verify-pack catches a corrupted sum of the index file itsel\n \tfi\n '\n \n+test_expect_success 'verify-pack outside a repository' '\n+\tnongit git verify-pack -v \"$(pwd)/test-1-${packname_1}.idx\"\n+'\n+\n test_expect_success 'build pack index for an existing pack' '\n \tcat test-1-${packname_1}.pack >test-3.pack &&\n \tgit index-pack -o tmp.idx test-3.pack &&\n"},{"id":"501967","messageId":"ZtT8p06fdTwXO7iX@tanuki","threadId":"62029","inReplyTo":"xmqq7cbvpf8v.fsf@gitster.g","subject":"Re: git 2.46.0 crashes when trying to verify-pack outside of a repo","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-09-01T23:45:43Z","receivedAt":"2024-09-01T23:45:49Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Sep 01, 2024 at 08:26:08AM -0700, Junio C Hamano wrote:\n> Ilya K <me@0upti.me> writes:\n> \n> > We've updated to Git 2.46.0 in NixOS, and encountered an issue\n> > with Dulwich (a Python Git implementation) tests failing[0]\n> > because it attempts to call `git verify-pack` on a bare pack, with\n> > no surrounding repo. This used to work in Git 2.45.x, but in 2.46\n> > it simply prints \"error: index-pack died of signal 11\".\n> \n> Thanks.  This is a fallout from code-wide clean-up in 2.46.0 where\n> we do not assume that everybody runs SHA-1.\n\nYup, indeed. The problem lies deeper than what the below patch fixes\nthough. The issue isn't in git-verify-pack(1), but in git-index-pack(1),\nand can be fixed like this:\n\n    diff --git a/builtin/index-pack.c b/builtin/index-pack.c\n    index fd968d673d2..e6edd96d099 100644\n    --- a/builtin/index-pack.c\n    +++ b/builtin/index-pack.c\n    @@ -1733,7 +1733,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n        unsigned char pack_hash[GIT_MAX_RAWSZ];\n        unsigned foreign_nr = 1;\t/* zero is a \"good\" value, assume bad */\n        int report_end_of_input = 0;\n    -\tint hash_algo = 0;\n    +\tint hash_algo = GIT_HASH_UNKNOWN;;\n     \n        /*\n         * index-pack never needs to fetch missing objects except when\n    @@ -1857,6 +1857,9 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n            pack_name = arg;\n        }\n     \n    +\tif (!the_repository->hash_algo && hash_algo == GIT_HASH_UNKNOWN)\n    +\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n    +\n        if (!pack_name && !from_stdin)\n            usage(index_pack_usage);\n        if (fix_thin_pack && !from_stdin)\n\nUnfortunately, this once again uncovers a deeper issue: neither the\npackfile nor their index encode the object format they use. So while\nfalling back to SHA1 papers over the issue, it means that we misparse\nSHA256 indices. Also, we misparse SHA1 indices if we happen to be in a\nSHA256 repository. E.g. when parsing a SHA256 file in a SHA1 repo:\n\n    $ git index-pack --verify '/tmp/git-tests/trash directory.t5300-pack-object/repo/.git/objects/pack/pack-aa45f7f08f043c9f0388f1844a2a797587254e249919b35ac9dc2b52c1aada29.pack'\n    error: wrong index v2 file size in /tmp/git-tests/trash directory.t5300-pack-object/repo/.git/objects/pack/pack-aa45f7f08f043c9f0388f1844a2a797587254e249919b35ac9dc2b52c1aada29.idx\n    fatal: Cannot open existing pack idx file for '/tmp/git-tests/trash directory.t5300-pack-object/repo/.git/objects/pack/pack-aa45f7f08f043c9f0388f1844a2a797587254e249919b35ac9dc2b52c1aada29.idx'\n\nThe error message isn't even properly indicating what the actual issue\nis.\n\nOne potential solution would be to try and derive the object format from\nthe hash that the packfile index name has. But that is quite roundabout\nand rather ugly, and packfiles may not necessarily have that hash in the\nfirst place. It would also become potentially ambiguous in the future if\nwe were to ever adopt another hash that has the same length as either\nSHA1 or SHA256.\n\nSo we basically have three different options:\n\n  - Accept that we just don't handle this case correctly and let the\n    code error out. This pessimizes all hashes but SHA256.\n\n  - Bail out when outside of a repository when `--object-format=` wasn't\n    given. This pessimizes all hashes, but gives a clear indicator to\n    the user why things don't work.\n\n  - Introduce packfiles v3 and encode the object format into the header.\n    Then do either (1) or (2) on top.\n\nThe last option is of course the cleanest, but also the most involved.\n\nPatrick\n\n> ------- >8 -------\n> Subject: verify-pack: fall back to SHA-1 outside a repo\n> \n> In c8aed5e8da (repository: stop setting SHA1 as the default object hash,\n> 2024-05-07), we have stopped setting the default hash algorithm for\n> `the_repository`. Consequently, code that relies on `the_hash_algo` will\n> now crash when it hasn't explicitly been initialized, which may be the\n> case when running outside of a Git repository.\n> \n> As the verify-pack command ought to be able to infer what algorithm\n> is used in the input file (and if the input file does not have such\n> an information, that by itself is a problem), and the command allows\n> an option to explicitly tell what algorithm to use in case it cannot\n> be guessed from the input file, in theory we shouldn't have to use\n> the default algorithm anywhere in the operation of the command, but\n> we fail fairly early in the process when run outside a repository\n> without any default algorithm set.\n> \n> Resurrect the setting of the default algorithm just like we used to\n> do before 2.46.0\n> \n> Reported-by: Ilya K <me@0upti.me>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  builtin/verify-pack.c  | 4 ++++\n>  t/t5300-pack-object.sh | 4 ++++\n>  2 files changed, 8 insertions(+)\n> \n> diff --git c/builtin/verify-pack.c w/builtin/verify-pack.c\n> index 011dddd2dc..5b663905ae 100644\n> --- c/builtin/verify-pack.c\n> +++ w/builtin/verify-pack.c\n> @@ -1,6 +1,7 @@\n>  #include \"builtin.h\"\n>  #include \"config.h\"\n>  #include \"gettext.h\"\n> +#include \"hash.h\"\n>  #include \"run-command.h\"\n>  #include \"parse-options.h\"\n>  #include \"strbuf.h\"\n> @@ -77,6 +78,9 @@ int cmd_verify_pack(int argc, const char **argv, const char *prefix)\n>  \t\tOPT_END()\n>  \t};\n>  \n> +\tif (!the_hash_algo)\n> +\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n> +\n>  \tgit_config(git_default_config, NULL);\n>  \targc = parse_options(argc, argv, prefix, verify_pack_options,\n>  \t\t\t     verify_pack_usage, 0);\n> diff --git c/t/t5300-pack-object.sh w/t/t5300-pack-object.sh\n> index 4ad023c846..d6f45d8923 100755\n> --- c/t/t5300-pack-object.sh\n> +++ w/t/t5300-pack-object.sh\n> @@ -322,6 +322,10 @@ test_expect_success 'verify-pack catches a corrupted sum of the index file itsel\n>  \tfi\n>  '\n>  \n> +test_expect_success 'verify-pack outside a repository' '\n> +\tnongit git verify-pack -v \"$(pwd)/test-1-${packname_1}.idx\"\n> +'\n> +\n>  test_expect_success 'build pack index for an existing pack' '\n>  \tcat test-1-${packname_1}.pack >test-3.pack &&\n>  \tgit index-pack -o tmp.idx test-3.pack &&\n"},{"id":"501980","messageId":"ZtW7LtQEobPpVB99@tapette.crustytoothpaste.net","threadId":"62029","inReplyTo":"ZtT8p06fdTwXO7iX@tanuki","subject":"Re: git 2.46.0 crashes when trying to verify-pack outside of a repo","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2024-09-02T13:18:38Z","receivedAt":"2024-09-02T13:18:46Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2024-09-01 at 23:45:43, Patrick Steinhardt wrote:\n> Unfortunately, this once again uncovers a deeper issue: neither the\n> packfile nor their index encode the object format they use. So while\n> falling back to SHA1 papers over the issue, it means that we misparse\n> SHA256 indices. Also, we misparse SHA1 indices if we happen to be in a\n> SHA256 repository. E.g. when parsing a SHA256 file in a SHA1 repo:\n> \n>     $ git index-pack --verify '/tmp/git-tests/trash directory.t5300-pack-object/repo/.git/objects/pack/pack-aa45f7f08f043c9f0388f1844a2a797587254e249919b35ac9dc2b52c1aada29.pack'\n>     error: wrong index v2 file size in /tmp/git-tests/trash directory.t5300-pack-object/repo/.git/objects/pack/pack-aa45f7f08f043c9f0388f1844a2a797587254e249919b35ac9dc2b52c1aada29.idx\n>     fatal: Cannot open existing pack idx file for '/tmp/git-tests/trash directory.t5300-pack-object/repo/.git/objects/pack/pack-aa45f7f08f043c9f0388f1844a2a797587254e249919b35ac9dc2b52c1aada29.idx'\n> \n> The error message isn't even properly indicating what the actual issue\n> is.\n\nYes, this is also true of other formats like the index as well, but\nthere we know it must be of the same format as the rest of the\nrepository.\n\nI noticed this during writing the SHA-256 series, and it's inconvenient.\nIf you blame some of the tests that add the `--object-format` entry,\nI wrote them.\n\n> One potential solution would be to try and derive the object format from\n> the hash that the packfile index name has. But that is quite roundabout\n> and rather ugly, and packfiles may not necessarily have that hash in the\n> first place. It would also become potentially ambiguous in the future if\n> we were to ever adopt another hash that has the same length as either\n> SHA1 or SHA256.\n\nYes, we've decided not to derive things by their length except in the\ndumb HTTP protocol for this reason.\n\n> So we basically have three different options:\n> \n>   - Accept that we just don't handle this case correctly and let the\n>     code error out. This pessimizes all hashes but SHA256.\n> \n>   - Bail out when outside of a repository when `--object-format=` wasn't\n>     given. This pessimizes all hashes, but gives a clear indicator to\n>     the user why things don't work.\n\nThis is what I would recommend.\n\n>   - Introduce packfiles v3 and encode the object format into the header.\n>     Then do either (1) or (2) on top.\n\nI think we have pack v3 already (which is the same as v2), and v4 was\nfor an experimental format that never landed fully.  Maybe v5?\n\nIf you wanted to do this, you could add support for arbitrary chunks,\nlike with multi-pack indexes, that would allow for extensibility in the\nfuture.  However, you'd also need some protocol capabilities if you\nwant to send pack v5 or certain chunks over the protocol.\n\n> The last option is of course the cleanest, but also the most involved.\n\nI'd personally recommend just requiring the `--object-format=` option,\nbut of course if you want to write pack v5, don't let me stop you.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"501985","messageId":"ZtXCBcn6WZIHr65b@tanuki","threadId":"62029","inReplyTo":"ZtW7LtQEobPpVB99@tapette.crustytoothpaste.net","subject":"Re: git 2.46.0 crashes when trying to verify-pack outside of a repo","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-09-02T13:47:55Z","receivedAt":"2024-09-02T13:47:59Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Sep 02, 2024 at 01:18:38PM +0000, brian m. carlson wrote:\n> On 2024-09-01 at 23:45:43, Patrick Steinhardt wrote:\n> > So we basically have three different options:\n> > \n> >   - Accept that we just don't handle this case correctly and let the\n> >     code error out. This pessimizes all hashes but SHA256.\n> > \n> >   - Bail out when outside of a repository when `--object-format=` wasn't\n> >     given. This pessimizes all hashes, but gives a clear indicator to\n> >     the user why things don't work.\n> \n> This is what I would recommend.\n\nI'm also leaning into this direction. I want us to move to a world where\nSHA1 and SHA256 are equal citizens. We won't ever get folks to move on\nto SHA256 repositories when it continues to be an afterthought that\nbehaves worse than SHA1 repositories.\n\nThat's basically where this whole exercise to stop setting the default\nhash came from. GitLab nowadays has support for SHA256 repositories, so\nI want to ensure that it can be used as a drop-in replacement for SHA1\nin basically all ways.\n\n> >   - Introduce packfiles v3 and encode the object format into the header.\n> >     Then do either (1) or (2) on top.\n> \n> I think we have pack v3 already (which is the same as v2), and v4 was\n> for an experimental format that never landed fully.  Maybe v5?\n\nAh, fair enough.\n\n> If you wanted to do this, you could add support for arbitrary chunks,\n> like with multi-pack indexes, that would allow for extensibility in the\n> future.  However, you'd also need some protocol capabilities if you\n> want to send pack v5 or certain chunks over the protocol.\n> \n> > The last option is of course the cleanest, but also the most involved.\n> \n> I'd personally recommend just requiring the `--object-format=` option,\n> but of course if you want to write pack v5, don't let me stop you.\n\nWell, in the context of this issue I'd definitely aim for the easier fix\nfirst. Regardless of whether or not we introduce v5, we'd still have to\naddress the underlying issue for repositories that do not (yet) have v5\npackfiles.\n\nPatrick\n"},{"id":"502041","messageId":"xmqqv7zcn39t.fsf@gitster.g","threadId":"62029","inReplyTo":"ZtXCBcn6WZIHr65b@tanuki","subject":"Re: git 2.46.0 crashes when trying to verify-pack outside of a repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-03T15:52:14Z","receivedAt":"2024-09-03T15:52:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n>> I'd personally recommend just requiring the `--object-format=` option,\n>> but of course if you want to write pack v5, don't let me stop you.\n>\n> Well, in the context of this issue I'd definitely aim for the easier fix\n> first. Regardless of whether or not we introduce v5, we'd still have to\n> address the underlying issue for repositories that do not (yet) have v5\n> packfiles.\n\nI would be hesitant to reroactively tightening the rules, though.\nIf a tool has worked well for those who can and wants to assume a\nhash function (because it has been the default, or perhaps because\nthe user configured the tool as such) even outside a repository,\nsuch a tightening is a regression.  If such a change is done to\navoid triggering a data corrupting bug, we may be able to come up\nwith a valid justification, but on the other hand, to discourage\ncertain uses of the tool, even if the discouraged use is an insecure\none, feels a bit too opinionated for a tool.  It has the same smell\nas updating the \"md5sum\" tool to discourage the use of the function\nso that it always exits with 1 (after computing and showing the\nhash) or something silly like that.\n\nI am not saying that it is bad for a tool to be opinionated.  When\nwe design a new feature, it is part of the design process to make\nsure that use of the feature encourages use of better workflows.\n\nBut disabling what has worked to user's expectation so far is a bit\ndifferent story.\n\n"},{"id":"502089","messageId":"9a4267b8854312351f82286b6025d0a3d0e66743.1725429169.git.ps@pks.im","threadId":"62029","inReplyTo":"14ec9394-a7ea-456c-800a-6a84c52e5cda@0upti.me","subject":"[PATCH] builtin/index-pack: fix segfaults when running outside of a repo","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-09-04T06:26:24Z","receivedAt":"2024-09-04T06:26:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"It was reported that git-verify-pack(1) has started to crash with Git\nv2.46.0 when run outside of a repository. This is another fallout from\nc8aed5e8da (repository: stop setting SHA1 as the default object hash,\n2024-05-07), where we have stopped setting the default hash algorithm\nfor `the_repository`. Consequently, code that relies on `the_hash_algo`\nwill now crash when it hasn't explicitly been initialized, which may be\nthe case when running outside of a Git repository.\n\nThe crash is not in git-verify-pack(1) but instead in git-index-pack(1),\nwhich gets called by the former. Ideally, both of these programs should\nbe able to identify the hash algorithm used by the packfile and index\nwithout having to rely on external information. But unfortunately, the\nformat for neither of them is completely self-describing, so it is not\npossible to derive that information. This is a design issue that we\nshould address by introducing a new packfile version that encodes its\nobject hash.\n\nFor now though the more important fix is to not make either of these\nprograms crash anymore, which we do by falling back to SHA1 when the\nobject hash is unconfigured. This pessimizes reading packfiles which\nuse a different hash than SHA1, but restores previous behaviour.\n\nReported-by: Ilya K <me@0upti.me>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n builtin/index-pack.c   |  9 +++++++++\n t/t5300-pack-object.sh | 39 +++++++++++++++++++++++++++++++++++++++\n 2 files changed, 48 insertions(+)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex fd968d673d2..763b01372aa 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1868,6 +1868,15 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tif (!index_name && pack_name)\n \t\tindex_name = derive_filename(pack_name, \"pack\", \"idx\", &index_name_buf);\n \n+\t/*\n+\t * Packfiles and indices do not carry enough information to be able to\n+\t * identify their object hash. So when we are neither in a repository\n+\t * nor has the user told us which object hash to use we have no other\n+\t * choice but to guess the object hash.\n+\t */\n+\tif (!the_repository->hash_algo)\n+\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n+\n \topts.flags &= ~(WRITE_REV | WRITE_REV_VERIFY);\n \tif (rev_index) {\n \t\topts.flags |= verify ? WRITE_REV_VERIFY : WRITE_REV;\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 4ad023c846d..3b9dae331a5 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -635,4 +635,43 @@ test_expect_success 'negative window clamps to 0' '\n \tcheck_deltas stderr = 0\n '\n \n+for hash in sha1 sha256\n+do\n+\ttest_expect_success \"verify-pack with $hash packfile\" '\n+\t\ttest_when_finished \"rm -rf repo\" &&\n+\t\tgit init --object-format=$hash repo &&\n+\t\ttest_commit -C repo initial &&\n+\t\tgit -C repo repack -ad &&\n+\t\tgit -C repo verify-pack \"$(pwd)\"/repo/.git/objects/pack/*.idx &&\n+\t\tif test $hash = sha1\n+\t\tthen\n+\t\t\tnongit git verify-pack \"$(pwd)\"/repo/.git/objects/pack/*.idx\n+\t\telse\n+\t\t\t# We have no way to identify the hash used by packfiles\n+\t\t\t# or indices, so we always fall back to SHA1.\n+\t\t\tnongit test_must_fail git verify-pack \"$(pwd)\"/repo/.git/objects/pack/*.idx &&\n+\t\t\t# But with an explicit object format we should succeed.\n+\t\t\tnongit git verify-pack --object-format=$hash \"$(pwd)\"/repo/.git/objects/pack/*.idx\n+\t\tfi\n+\t'\n+\n+\ttest_expect_success \"index-pack outside of a $hash repository\" '\n+\t\ttest_when_finished \"rm -rf repo\" &&\n+\t\tgit init --object-format=$hash repo &&\n+\t\ttest_commit -C repo initial &&\n+\t\tgit -C repo repack -ad &&\n+\t\tgit -C repo index-pack --verify \"$(pwd)\"/repo/.git/objects/pack/*.pack &&\n+\t\tif test $hash = sha1\n+\t\tthen\n+\t\t\tnongit git index-pack --verify \"$(pwd)\"/repo/.git/objects/pack/*.pack\n+\t\telse\n+\t\t\t# We have no way to identify the hash used by packfiles\n+\t\t\t# or indices, so we always fall back to SHA1.\n+\t\t\tnongit test_must_fail git index-pack --verify \"$(pwd)\"/repo/.git/objects/pack/*.pack 2>err &&\n+\t\t\t# But with an explicit object format we should succeed.\n+\t\t\tnongit git index-pack --object-format=$hash --verify \"$(pwd)\"/repo/.git/objects/pack/*.pack\n+\t\tfi\n+\t'\n+done\n+\n test_done\n-- \n2.46.0.421.g159f2d50e7.dirty\n\n"}]}