{"thread":{"id":"61770","subject":"[PATCH] show-index: fix uninitialized hash function","startedAt":"2024-07-12T14:24:26Z","lastAt":"2024-12-16T16:21:23Z","messageCount":27,"participants":["Abhijeet Sonar","Junio C Hamano","Eric Sunshine","brian m. carlson","Taylor Blau","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"498581","messageId":"20240712142326.266533-1-abhijeet.nkt@gmail.com","threadId":"61770","inReplyTo":null,"subject":"[PATCH] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-07-12T14:23:26Z","receivedAt":"2024-07-12T14:24:26Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"As stated in the docs, show-index should use SHA1 as the default hash algorithm\nwhen run outsize of a repository.  However, 'the_hash_algo' is currently left\nuninitialized if we are not in a repository and no explicit hash funciton is\nspecified, causing a crash.  Fix it by falling back to SHA1 when it is found\nuninitialized.\n\nSigned-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n---\n builtin/show-index.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/builtin/show-index.c b/builtin/show-index.c\nindex 540dc3dad1..bb6d9e3c40 100644\n--- a/builtin/show-index.c\n+++ b/builtin/show-index.c\n@@ -35,6 +35,9 @@ int cmd_show_index(int argc, const char **argv, const char *prefix)\n \t\trepo_set_hash_algo(the_repository, hash_algo);\n \t}\n \n+\tif (!the_hash_algo)\n+\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n+\n \thashsz = the_hash_algo->rawsz;\n \n \tif (fread(top_index, 2 * 4, 1, stdin) != 1)\n-- \n2.45.2.827.g557ae147e6\n\n"},{"id":"498587","messageId":"xmqqbk32oc7g.fsf@gitster.g","threadId":"61770","inReplyTo":"20240712142326.266533-1-abhijeet.nkt@gmail.com","subject":"Re: [PATCH] show-index: fix uninitialized hash function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-12T15:35:15Z","receivedAt":"2024-07-12T15:35:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Abhijeet Sonar <abhijeet.nkt@gmail.com> writes:\n\n> As stated in the docs, show-index should use SHA1 as the default hash algorithm\n> when run outsize of a repository.  However, 'the_hash_algo' is currently left\n> uninitialized if we are not in a repository and no explicit hash funciton is\n> specified, causing a crash.  Fix it by falling back to SHA1 when it is found\n> uninitialized.\n>\n> Signed-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n> ---\n>  builtin/show-index.c | 3 +++\n>  1 file changed, 3 insertions(+)\n\nNicely described.\n\nWe'd probably want to protect this with a new test, so that\nregardless of the choice of GIT_TEST_DEFAULT_HASH, the command\nshould behave as advertised.\n\nHaving said that, I am not sure if --object-format specified on the\ncommand line, or picked up from the repository, makes much sense in\nthe context of the command, especially for the longer term [*].  The\ncommand is designed to read from its standard input a byte-stream,\nwhich is assumed to be an .idx file of _any_ origin, so ideally it\nshould be able to tell what hash the incoming data uses and use that\nhash algorithm, without being told from the command line?\n\nBut that longer-term worry has nothing to do with the validity of\nthis patch (but the lack of test does).  Thanks.\n\n[Footnote]\n\n * Perhaps the file format does not make it obvious what hash\n   algorithm it uses, so it may be hard to auto-detect without\n   additional code.  But if that is the case, it would be something\n   we may want to eventually fix.\n\n"},{"id":"498589","messageId":"CAPig+cT+X2k4RfTb_mjErQ6reXk44SzbTaXpzQdgLJ+TugtiXQ@mail.gmail.com","threadId":"61770","inReplyTo":"20240712142326.266533-1-abhijeet.nkt@gmail.com","subject":"Re: [PATCH] show-index: fix uninitialized hash function","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-07-12T16:53:58Z","receivedAt":"2024-07-12T16:54:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 12, 2024 at 10:24 AM Abhijeet Sonar <abhijeet.nkt@gmail.com> wrote:\n> As stated in the docs, show-index should use SHA1 as the default hash algorithm\n> when run outsize of a repository.  However, 'the_hash_algo' is currently left\n> uninitialized if we are not in a repository and no explicit hash funciton is\n\ns/funciton/function/\n\n> specified, causing a crash.  Fix it by falling back to SHA1 when it is found\n> uninitialized.\n>\n> Signed-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n"},{"id":"498719","messageId":"20240715102344.182388-1-abhijeet.nkt@gmail.com","threadId":"61770","inReplyTo":"xmqqbk32oc7g.fsf@gitster.g","subject":"[PATCH v2] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-07-15T10:23:43Z","receivedAt":"2024-07-15T10:23:52Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"As stated in the docs, show-index should use SHA1 as the default hash algorithm\nwhen run outsize of a repository.  However, 'the_hash_algo' is currently left\nuninitialized if we are not in a repository and no explicit hash function is\nspecified, causing a crash.  Fix it by falling back to SHA1 when it is found\nuninitialized. Also add test that verifies this behaviour.\n\nSigned-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n---\n builtin/show-index.c                |  3 +++\n t/t8101-show-index-hash-function.sh | 15 +++++++++++++++\n 2 files changed, 18 insertions(+)\n create mode 100755 t/t8101-show-index-hash-function.sh\n\ndiff --git a/builtin/show-index.c b/builtin/show-index.c\nindex 540dc3dad1..bb6d9e3c40 100644\n--- a/builtin/show-index.c\n+++ b/builtin/show-index.c\n@@ -35,6 +35,9 @@ int cmd_show_index(int argc, const char **argv, const char *prefix)\n \t\trepo_set_hash_algo(the_repository, hash_algo);\n \t}\n \n+\tif (!the_hash_algo)\n+\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n+\n \thashsz = the_hash_algo->rawsz;\n \n \tif (fread(top_index, 2 * 4, 1, stdin) != 1)\ndiff --git a/t/t8101-show-index-hash-function.sh b/t/t8101-show-index-hash-function.sh\nnew file mode 100755\nindex 0000000000..2e9308f73c\n--- /dev/null\n+++ b/t/t8101-show-index-hash-function.sh\n@@ -0,0 +1,15 @@\n+#!/bin/sh\n+\n+test_description='git show-index'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'show-index: should not fail outside a repository' '\n+    git init --object-format=sha1 && (\n+        echo \"\" | git hash-object -w --stdin | git pack-objects test &&\n+        rm -rf .git &&\n+        cat test-*.idx | git show-index\n+    )\n+'\n+\n+test_done\n-- \n2.45.2.827.g557ae147e6\n\n"},{"id":"498720","messageId":"c61a88c0-a45f-4d8c-b9f5-bb5853362709@gmail.com","threadId":"61770","inReplyTo":"xmqqbk32oc7g.fsf@gitster.g","subject":"Re: [PATCH] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-07-15T10:31:07Z","receivedAt":"2024-07-15T10:31:12Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"On 12/07/24 21:05, Junio C Hamano wrote:\n> Abhijeet Sonar <abhijeet.nkt@gmail.com> writes:\n> \n>> As stated in the docs, show-index should use SHA1 as the default hash algorithm\n>> when run outsize of a repository.  However, 'the_hash_algo' is currently left\n>> uninitialized if we are not in a repository and no explicit hash funciton is\n>> specified, causing a crash.  Fix it by falling back to SHA1 when it is found\n>> uninitialized.\n>>\n>> Signed-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n>> ---\n>>  builtin/show-index.c | 3 +++\n>>  1 file changed, 3 insertions(+)\n> \n> Nicely described.\n> \n> We'd probably want to protect this with a new test, so that\n> regardless of the choice of GIT_TEST_DEFAULT_HASH, the command\n> should behave as advertised.\n\nI wrote a test which build an index file using a `hash-object |\npack-objects` chain.  I am not sure if its the best way to do this, I\nwould appreciate some guidance on this.\n\nAnother way I can think of is having an index file sit along with the\ntests in the codebase which will be read by `show-index` instead of\ngenerating one on the fly.  Thoughts?\n\nThanks.\n\n> \n> Having said that, I am not sure if --object-format specified on the\n> command line, or picked up from the repository, makes much sense in\n> the context of the command, especially for the longer term [*].  The\n> command is designed to read from its standard input a byte-stream,\n> which is assumed to be an .idx file of _any_ origin, so ideally it\n> should be able to tell what hash the incoming data uses and use that\n> hash algorithm, without being told from the command line?\n> \n> But that longer-term worry has nothing to do with the validity of\n> this patch (but the lack of test does).  Thanks.\n> \n> [Footnote]\n> \n>  * Perhaps the file format does not make it obvious what hash\n>    algorithm it uses, so it may be hard to auto-detect without\n>    additional code.  But if that is the case, it would be something\n>    we may want to eventually fix.\n> \n\n\n\n"},{"id":"498735","messageId":"xmqqzfqi4oc6.fsf_-_@gitster.g","threadId":"61770","inReplyTo":"20240715102344.182388-1-abhijeet.nkt@gmail.com","subject":"Re* [PATCH v2] show-index: fix uninitialized hash function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-15T16:22:33Z","receivedAt":"2024-07-15T16:22:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Abhijeet Sonar <abhijeet.nkt@gmail.com> writes:\n\n>  t/t8101-show-index-hash-function.sh | 15 +++++++++++++++\n>  2 files changed, 18 insertions(+)\n>  create mode 100755 t/t8101-show-index-hash-function.sh\n\nThanks.  But let's not waste the scarce resource that is a test\nnumber for a single oddball test (and as t/README says t8xxx series\nis for forensics Porcelains).\n\n> +test_expect_success 'show-index: should not fail outside a repository' '\n> +    git init --object-format=sha1 && (\n> +        echo \"\" | git hash-object -w --stdin | git pack-objects test &&\n\nOur tests run in an already initialized repository, and some test\nconfiguration would use sha256 to initialize that repository.  So\nthe above is not a good idea, unless you use a new directory.  We\noften create a new directory inside the initial directory the test\nbegins in, and then in a subshell chdir into the directory.\n\nWe frown upon a pipeline that has \"git\" as an upstream, because the\nexit status from such invocation of \"git\" will be hidden.\n\n\tgit init --object-format=sha1 sample &&\n\t(\n\t\tcd sample &&\n\t\tO=$(git hash-object -w /dev/null) &&\n\t\tT=$(echo \"$O\" | git pack-objects test) &&\n\nwould give you a pair of files \"test-$T.idx\" and \"test-$T.pack\" and\nit will notice if hash-object or pack-objects fail.\n\n> +        rm -rf .git &&\n\nThis alone does *not* necessarily make the directory you are using\nfor test completely unassociated with any repository.  If you are\nworking with the source code of Git from a repository (as opposed to\nextracted tar archive), with that \"rm -fr\", you may have made the\ndirectory not a Git repository, but then that directory is now a\nmere subdirectory \"t/trash directory.t8101-show-index-hash-function\"\nof the repository that houses the Git source code (unless you are\nusing the --root=<directory> option to run the tests).\n\nWhen we test behaviour of commands outside a repository, we use the\nGIT_CEILING_DIRECTORIES feature, often via the nongit helper function\nthat is defined in t/test-lib-functions.sh (which becomes available\nto tests by doing \". ./test-lib.sh\".\n\nIn t5300-pack-object.sh we see these bits already.\n\n    test_expect_success 'index-pack --stdin complains of non-repo' '\n            nongit test_must_fail git index-pack \\\n                    --object-format=$(test_oid algo) --stdin <foo.pack &&\n            test_path_is_missing non-repo/.git\n    '\n\n    test_expect_success 'index-pack <pack> works in non-repo' '\n            nongit git index-pack \\\n                    --object-format=$(test_oid algo) ../foo.pack &&\n            test_path_is_file foo.idx\n    '\n\nI wonder if it is sufficient to add a new test after these two\nsteps, something like\n\n    test_expect_success SHA1 'show-index works OK outside a repository' '\n\t    nongit git show-index <foo.idx\n    '\n\nperhaps?\n\nWith that, your patch would become like so:\n\n------------ >8 ----------------------- >8 ------------\nFrom: Abhijeet Sonar <abhijeet.nkt@gmail.com>\nDate: Mon, 15 Jul 2024 15:53:43 +0530\nSubject: [PATCH] show-index: fix uninitialized hash function\n\nAs stated in the docs, show-index should use SHA1 as the default\nhash algorithm when run outside a repository.\n\nHowever, 'the_hash_algo' is left uninitialized if we are not in a\nrepository and no explicit hash function is specified, causing a\ncrash.\n\nFix it by falling back to SHA1 when it is found uninitialized. Also\nadd test that verifies this behaviour.\n\nSigned-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n[jc: fixed up the test]\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/show-index.c   | 3 +++\n t/t5300-pack-object.sh | 4 ++++\n 2 files changed, 7 insertions(+)\n\ndiff --git a/builtin/show-index.c b/builtin/show-index.c\nindex 540dc3dad1..bb6d9e3c40 100644\n--- a/builtin/show-index.c\n+++ b/builtin/show-index.c\n@@ -35,6 +35,9 @@ int cmd_show_index(int argc, const char **argv, const char *prefix)\n \t\trepo_set_hash_algo(the_repository, hash_algo);\n \t}\n \n+\tif (!the_hash_algo)\n+\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n+\n \thashsz = the_hash_algo->rawsz;\n \n \tif (fread(top_index, 2 * 4, 1, stdin) != 1)\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 4ad023c846..83933eca5d 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' '\n \ttest_path_is_file foo.idx\n '\n \n+test_expect_success SHA1 'show-index works OK outside a repository' '\n+\tnongit git show-index <foo.idx\n+'\n+\n test_expect_success !PTHREADS,!FAIL_PREREQS \\\n \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n \ttest_must_fail git index-pack --threads=2 2>err &&\n-- \n2.46.0-rc0-140-g824782812f\n\n\n"},{"id":"498761","messageId":"ZpWdkq2GBqIBI8Lr@tapette.crustytoothpaste.net","threadId":"61770","inReplyTo":"20240715102344.182388-1-abhijeet.nkt@gmail.com","subject":"Re: [PATCH v2] show-index: fix uninitialized hash function","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2024-07-15T22:07:14Z","receivedAt":"2024-07-15T22:07:16Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2024-07-15 at 10:23:43, Abhijeet Sonar wrote:\n> diff --git a/t/t8101-show-index-hash-function.sh b/t/t8101-show-index-hash-function.sh\n> new file mode 100755\n> index 0000000000..2e9308f73c\n> --- /dev/null\n> +++ b/t/t8101-show-index-hash-function.sh\n> @@ -0,0 +1,15 @@\n> +#!/bin/sh\n> +\n> +test_description='git show-index'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'show-index: should not fail outside a repository' '\n> +    git init --object-format=sha1 && (\n> +        echo \"\" | git hash-object -w --stdin | git pack-objects test &&\n> +        rm -rf .git &&\n> +        cat test-*.idx | git show-index\n> +    )\n> +'\n\nI don't think this change is going to work.  If you run with\n`GIT_TEST_DEFAULT_HASH=sha256`, as I do, you see this error:\n\n  fatal: attempt to reinitialize repository with different hash\n\nThat's because the repository is already initialized as SHA-256 when you\ndo `git init`.\n\nThe reason that the `--object-format` option was added was to make this\nconfiguration work outside a repository.  It would probably be better to\nrequire the user to specify that option if we're outside of a repository\nrather than just try to guess.  We want to have _fewer_ dependencies on\nSHA-1 as the implicit algorithm, not more.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"506139","messageId":"20241026120950.72727-1-abhijeet.nkt@gmail.com","threadId":"61770","inReplyTo":"xmqqzfqi4oc6.fsf_-_@gitster.g","subject":"[PATCH v3] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-10-26T12:09:50Z","receivedAt":"2024-10-26T12:09:58Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"As stated in the docs, show-index should use SHA1 as the default hash algorithm\nwhen run outsize of a repository.  However, 'the_hash_algo' is currently left\nuninitialized if we are not in a repository and no explicit hash function is\nspecified, causing a crash.  Fix it by falling back to SHA1 when it is found\nuninitialized. Also add test that verifies this behaviour.\n\nSigned-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n---\n builtin/show-index.c   | 3 +++\n t/t5300-pack-object.sh | 4 ++++\n 2 files changed, 7 insertions(+)\n\ndiff --git a/builtin/show-index.c b/builtin/show-index.c\nindex f164c01bbe..978ae70470 100644\n--- a/builtin/show-index.c\n+++ b/builtin/show-index.c\n@@ -38,6 +38,9 @@ int cmd_show_index(int argc,\n \t\trepo_set_hash_algo(the_repository, hash_algo);\n \t}\n \n+\tif (!the_hash_algo)\n+\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n+\n \thashsz = the_hash_algo->rawsz;\n \n \tif (fread(top_index, 2 * 4, 1, stdin) != 1)\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 3b9dae331a..51fed26cc4 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' '\n \ttest_path_is_file foo.idx\n '\n \n+test_expect_success SHA1 'show-index works OK outside a repository' '\n+\tnongit git show-index <foo.idx\n+'\n+\n test_expect_success !PTHREADS,!FAIL_PREREQS \\\n \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n \ttest_must_fail git index-pack --threads=2 2>err &&\n-- \n2.47.0.107.g34b6ce9b30\n\n"},{"id":"506140","messageId":"03c602c4-8b53-485a-9a42-c989258acc57@gmail.com","threadId":"61770","inReplyTo":"xmqqzfqi4oc6.fsf_-_@gitster.g","subject":"Re: Re* [PATCH v2] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-10-26T12:17:14Z","receivedAt":"2024-10-26T12:17:21Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"Please excuse the neco-bump.\n\nOn 15/07/24 21:52, Junio C Hamano wrote:\n\n> With that, your patch would become like so:\n> \n> ------------ >8 ----------------------- >8 ------------\n\nI misunderstood this as \"I have made these changes to your patch on your\nbehalf\". But looking at how this was never queued and the commit does\nnot appear in any upstream branch, I realised that I was supposed to\nsend another iteration. Apologies.\n\nI have sent another iteration (v3) just before writing this.\n\nThanks\n\n"},{"id":"506180","messageId":"Zx7WaEn6nvtjhs/B@nand.local","threadId":"61770","inReplyTo":"20241026120950.72727-1-abhijeet.nkt@gmail.com","subject":"Re: [PATCH v3] show-index: fix uninitialized hash function","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-28T00:10:16Z","receivedAt":"2024-10-28T00:10:20Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Sat, Oct 26, 2024 at 05:39:50PM +0530, Abhijeet Sonar wrote:\n> As stated in the docs, show-index should use SHA1 as the default hash algorithm\n> when run outsize of a repository.  However, 'the_hash_algo' is currently left\n> uninitialized if we are not in a repository and no explicit hash function is\n> specified, causing a crash.  Fix it by falling back to SHA1 when it is found\n> uninitialized. Also add test that verifies this behaviour.\n\nThis commit description is good, and would benefit further from a\nbisection showing where the regression began. I don't think that it is a\nprerequisite for us moving this patch forward, though.\n\n> Signed-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n> ---\n>  builtin/show-index.c   | 3 +++\n>  t/t5300-pack-object.sh | 4 ++++\n>  2 files changed, 7 insertions(+)\n>\n> diff --git a/builtin/show-index.c b/builtin/show-index.c\n> index f164c01bbe..978ae70470 100644\n> --- a/builtin/show-index.c\n> +++ b/builtin/show-index.c\n> @@ -38,6 +38,9 @@ int cmd_show_index(int argc,\n>  \t\trepo_set_hash_algo(the_repository, hash_algo);\n>  \t}\n>\n> +\tif (!the_hash_algo)\n> +\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n> +\n>  \thashsz = the_hash_algo->rawsz;\n>\n>  \tif (fread(top_index, 2 * 4, 1, stdin) != 1)\n> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\n> index 3b9dae331a..51fed26cc4 100755\n> --- a/t/t5300-pack-object.sh\n> +++ b/t/t5300-pack-object.sh\n> @@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' '\n>  \ttest_path_is_file foo.idx\n>  '\n>\n> +test_expect_success SHA1 'show-index works OK outside a repository' '\n> +\tnongit git show-index <foo.idx\n> +'\n> +\n>  test_expect_success !PTHREADS,!FAIL_PREREQS \\\n>  \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n>  \ttest_must_fail git index-pack --threads=2 2>err &&\n> --\n> 2.47.0.107.g34b6ce9b30\n\nThese all look reasonable and as-expected to me. Patrick (CC'd) has been\nreviewing similar changes elsewhere, so I'd like him to chime in as well\non whether or not this looks good to go.\n\nThanks,\nTaylor\n"},{"id":"506193","messageId":"Zx8ijtkn7y6eBQ-n@pks.im","threadId":"61770","inReplyTo":"Zx7WaEn6nvtjhs/B@nand.local","subject":"Re: [PATCH v3] show-index: fix uninitialized hash function","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-28T05:35:15Z","receivedAt":"2024-10-28T05:35:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Oct 27, 2024 at 08:10:16PM -0400, Taylor Blau wrote:\n> On Sat, Oct 26, 2024 at 05:39:50PM +0530, Abhijeet Sonar wrote:\n> > diff --git a/builtin/show-index.c b/builtin/show-index.c\n> > index f164c01bbe..978ae70470 100644\n> > --- a/builtin/show-index.c\n> > +++ b/builtin/show-index.c\n> > @@ -38,6 +38,9 @@ int cmd_show_index(int argc,\n> >  \t\trepo_set_hash_algo(the_repository, hash_algo);\n> >  \t}\n> >\n> > +\tif (!the_hash_algo)\n> > +\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n\nLet's add a todo-comment here. The behaviour with this patch is somewhat\nbroken as you cannot inspect indices that use any other object hash than\nSHA256 outside of a repository. This is fine from my point of view and\nnothing that you have to fix here, as you simply fix up the broken\nbehaviour. But in the future, we should either:\n\n  - Add logic to detect the format of the passed-in index and set that\n    up as the hash algorithm.\n\n  - If that is impossible, add a command line option to pick the hash\n    algo.\n\n> >  \thashsz = the_hash_algo->rawsz;\n> >\n> >  \tif (fread(top_index, 2 * 4, 1, stdin) != 1)\n> > diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\n> > index 3b9dae331a..51fed26cc4 100755\n> > --- a/t/t5300-pack-object.sh\n> > +++ b/t/t5300-pack-object.sh\n> > @@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' '\n> >  \ttest_path_is_file foo.idx\n> >  '\n> >\n> > +test_expect_success SHA1 'show-index works OK outside a repository' '\n> > +\tnongit git show-index <foo.idx\n> > +'\n\nSo how does this behave with SHA256? Does it raise an error? Does it\nsegfault?\n\nI think it's okay to fail with SHA256 for now, but I'd like the\nfailure behaviour to be cleanish. So I'd prefer to not skip the test\ncompletely, but adapt our expectations based on the hash algo. Or have\ntwo separate tests, one for each hash, that explicitly init the repo\nwith `git init --ref-format=$hash`, and then exercise the behaviour for\neach of them.\n\n> >  test_expect_success !PTHREADS,!FAIL_PREREQS \\\n> >  \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n> >  \ttest_must_fail git index-pack --threads=2 2>err &&\n> > --\n> > 2.47.0.107.g34b6ce9b30\n> \n> These all look reasonable and as-expected to me. Patrick (CC'd) has been\n> reviewing similar changes elsewhere, so I'd like him to chime in as well\n> on whether or not this looks good to go.\n\nAh, thanks. I've missed this topic somehow.\n\nPatrick\n"},{"id":"506238","messageId":"Zx/NE/9HFNr9V2H7@nand.local","threadId":"61770","inReplyTo":"Zx8ijtkn7y6eBQ-n@pks.im","subject":"Re: [PATCH v3] show-index: fix uninitialized hash function","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-28T17:42:43Z","receivedAt":"2024-10-28T17:42:46Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 28, 2024 at 06:35:15AM +0100, Patrick Steinhardt wrote:\n> > >  test_expect_success !PTHREADS,!FAIL_PREREQS \\\n> > >  \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n> > >  \ttest_must_fail git index-pack --threads=2 2>err &&\n> > > --\n> > > 2.47.0.107.g34b6ce9b30\n> >\n> > These all look reasonable and as-expected to me. Patrick (CC'd) has been\n> > reviewing similar changes elsewhere, so I'd like him to chime in as well\n> > on whether or not this looks good to go.\n>\n> Ah, thanks. I've missed this topic somehow.\n\nNot a problem at all. Thanks for a very helpful review.\n\nAbhijeet: I've gone ahead and marked this in my notes as \"expecting\nanother round\" to address the feedback from Patrick. I'll keep my eyes\nout for the new version of this patch. Thanks!\n\nThanks,\nTaylor\n"},{"id":"506276","messageId":"00094bf3-61df-44ec-a469-a3b33399a306@gmail.com","threadId":"61770","inReplyTo":"Zx7WaEn6nvtjhs/B@nand.local","subject":"Re: [PATCH v3] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-10-29T10:30:57Z","receivedAt":"2024-10-29T10:31:05Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"On 28/10/24 05:40, Taylor Blau wrote:\n> On Sat, Oct 26, 2024 at 05:39:50PM +0530, Abhijeet Sonar wrote:\n>> As stated in the docs, show-index should use SHA1 as the default hash algorithm\n>> when run outsize of a repository.  However, 'the_hash_algo' is currently left\n>> uninitialized if we are not in a repository and no explicit hash function is\n>> specified, causing a crash.  Fix it by falling back to SHA1 when it is found\n>> uninitialized. Also add test that verifies this behaviour.\n> \n> This commit description is good, and would benefit further from a\n> bisection showing where the regression began. I don't think that it is a\n> prerequisite for us moving this patch forward, though.\n\nOn bisecting, the offending commit appears to be c8aed5e8da (repository:\nstop setting SHA1 as the default object hash).\n\nAnother related commit is ab274909d4 (builtin/diff: explicitly set hash\nalgo when there is no repo) which did something very similar to my patch.\n\nThanks\n\n\n"},{"id":"506280","messageId":"26d1bd3c-4f90-4406-8a1f-2eb085c46bab@gmail.com","threadId":"61770","inReplyTo":"Zx8ijtkn7y6eBQ-n@pks.im","subject":"Re: [PATCH v3] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-10-29T11:54:59Z","receivedAt":"2024-10-29T11:55:07Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"On 28/10/24 11:05, Patrick Steinhardt wrote:\n> On Sun, Oct 27, 2024 at 08:10:16PM -0400, Taylor Blau wrote:\n>> On Sat, Oct 26, 2024 at 05:39:50PM +0530, Abhijeet Sonar wrote:\n>>> diff --git a/builtin/show-index.c b/builtin/show-index.c\n>>> index f164c01bbe..978ae70470 100644\n>>> --- a/builtin/show-index.c\n>>> +++ b/builtin/show-index.c\n>>> @@ -38,6 +38,9 @@ int cmd_show_index(int argc,\n>>>  \t\trepo_set_hash_algo(the_repository, hash_algo);\n>>>  \t}\n>>>\n>>> +\tif (!the_hash_algo)\n>>> +\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n> \n> Let's add a todo-comment here. The behaviour with this patch is somewhat\n> broken as you cannot inspect indices that use any other object hash than\n> SHA256 outside of a repository. This is fine from my point of view and\n> nothing that you have to fix here, as you simply fix up the broken\n> behaviour. But in the future, we should either:\n\nI will add those comments, thanks.\n\n> \n>   - Add logic to detect the format of the passed-in index and set that\n>     up as the hash algorithm\n\nI am very interested in hacking around and trying to implement this.\nI read about the index format here:\nhttps://git-scm.com/docs/index-format and (assuming I am looking at the\nright thing) it does not seem like index files contain information about\nthe hash algorithm used in their headers or anywhere else. Are there\nother leads I should follow?\n\n> \n>   - If that is impossible, add a command line option to pick the hash\n>     algo.\n>\nActually there is already an option (--object-format) that allows\nchanging the hash algo used, it's just that default format is SHA1 when\none is not specified. (or at least will be, after this patch)\n\n>>>  \thashsz = the_hash_algo->rawsz;\n>>>\n>>>  \tif (fread(top_index, 2 * 4, 1, stdin) != 1)\n>>> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\n>>> index 3b9dae331a..51fed26cc4 100755\n>>> --- a/t/t5300-pack-object.sh\n>>> +++ b/t/t5300-pack-object.sh\n>>> @@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' '\n>>>  \ttest_path_is_file foo.idx\n>>>  '\n>>>\n>>> +test_expect_success SHA1 'show-index works OK outside a repository' '\n>>> +\tnongit git show-index <foo.idx\n>>> +'\n> \n> So how does this behave with SHA256? Does it raise an error? Does it\n> segfault?\n> \n> I think it's okay to fail with SHA256 for now, but I'd like the\n> failure behaviour to be cleanish. So I'd prefer to not skip the test\n> completely, but adapt our expectations based on the hash algo. Or have\n> two separate tests, one for each hash, that explicitly init the repo\n> with `git init --ref-format=$hash`, and then exercise the behaviour for\n> each of the>\n\nRunning show-index outside of a repository with a SHA256 based index\nfile gives:\n\n$ git show-index <foo.idx\nfatal: inconsistent 64b offset index\n\nAt the very least, it does not crash.\n\n>>>  test_expect_success !PTHREADS,!FAIL_PREREQS \\\n>>>  \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n>>>  \ttest_must_fail git index-pack --threads=2 2>err &&\n>>> --\n>>> 2.47.0.107.g34b6ce9b30\n>>\n>> These all look reasonable and as-expected to me. Patrick (CC'd) has been\n>> reviewing similar changes elsewhere, so I'd like him to chime in as well\n>> on whether or not this looks good to go.\n> \n> Ah, thanks. I've missed this topic somehow.\n> \n> Patrick\n\nThanks\n"},{"id":"506455","messageId":"20241101172800.21997-1-abhijeet.nkt@gmail.com","threadId":"61770","inReplyTo":"Zx/NE/9HFNr9V2H7@nand.local","subject":"[PATCH v4] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-11-01T17:28:00Z","receivedAt":"2024-11-01T17:28:13Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"In c8aed5e8da (repository: stop setting SHA1 as the default object\nhash), we got rid of the default hash algorithm for the_repository.\nDue to this change, it is now the responsibility of the callers to set\nthier own default when this is not present.\n\nAs stated in the docs, show-index should use SHA1 as the default hash\nalgorithm when ran outsize of a repository. Make sure this promise is\nmet by falling back to SHA1 when the_hash_algo is not present (i.e.\nwhen the command is ran outside of a repository). Also add a test that\nverifies this behaviour.\n\nSigned-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n---\n builtin/show-index.c   | 6 ++++++\n rm                     | 3 +++\n t/t5300-pack-object.sh | 4 ++++\n 3 files changed, 13 insertions(+)\n create mode 100755 rm\n\ndiff --git a/builtin/show-index.c b/builtin/show-index.c\nindex f164c01bbe..645c2548fb 100644\n--- a/builtin/show-index.c\n+++ b/builtin/show-index.c\n@@ -38,6 +38,12 @@ int cmd_show_index(int argc,\n \t\trepo_set_hash_algo(the_repository, hash_algo);\n \t}\n \n+\t// Fallback to SHA1 if we are running outside of a repository.\n+\t// TODO: Figure out and implement a way to detect the hash algorithm in use by the\n+\t//       the index file passed in and use that instead.\n+\tif (!the_hash_algo)\n+\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n+\n \thashsz = the_hash_algo->rawsz;\n \n \tif (fread(top_index, 2 * 4, 1, stdin) != 1)\ndiff --git a/rm b/rm\nnew file mode 100755\nindex 0000000000..2237506bf2\n--- /dev/null\n+++ b/rm\n@@ -0,0 +1,3 @@\n+#!/bin/sh\n+\n+echo rm $@\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 3b9dae331a..51fed26cc4 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' '\n \ttest_path_is_file foo.idx\n '\n \n+test_expect_success SHA1 'show-index works OK outside a repository' '\n+\tnongit git show-index <foo.idx\n+'\n+\n test_expect_success !PTHREADS,!FAIL_PREREQS \\\n \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n \ttest_must_fail git index-pack --threads=2 2>err &&\n-- \n2.47.0.107.g34b6ce9b30\n\n"},{"id":"506481","messageId":"xmqq1pzuylm6.fsf@gitster.g","threadId":"61770","inReplyTo":"20241101172800.21997-1-abhijeet.nkt@gmail.com","subject":"Re: [PATCH v4] show-index: fix uninitialized hash function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-11-02T10:29:37Z","receivedAt":"2024-11-02T10:29:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Abhijeet Sonar <abhijeet.nkt@gmail.com> writes:\n\n> In c8aed5e8da (repository: stop setting SHA1 as the default object\n> hash), we got rid of the default hash algorithm for the_repository.\n> Due to this change, it is now the responsibility of the callers to set\n> thier own default when this is not present.\n\n\"their own default\".\n\n> As stated in the docs, show-index should use SHA1 as the default hash\n> algorithm when ran outsize of a repository. Make sure this promise is\n\n\"outside a repository\".\n\n> met by falling back to SHA1 when the_hash_algo is not present (i.e.\n> when the command is ran outside of a repository). Also add a test that\n> verifies this behaviour.\n>\n> Signed-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n> ---\n>  builtin/show-index.c   | 6 ++++++\n>  rm                     | 3 +++\n\nHuh?\n\n>  t/t5300-pack-object.sh | 4 ++++\n>  3 files changed, 13 insertions(+)\n>  create mode 100755 rm\n>\n> diff --git a/builtin/show-index.c b/builtin/show-index.c\n> index f164c01bbe..645c2548fb 100644\n> --- a/builtin/show-index.c\n> +++ b/builtin/show-index.c\n> @@ -38,6 +38,12 @@ int cmd_show_index(int argc,\n>  \t\trepo_set_hash_algo(the_repository, hash_algo);\n>  \t}\n>  \n> +\t// Fallback to SHA1 if we are running outside of a repository.\n> +\t// TODO: Figure out and implement a way to detect the hash algorithm in use by the\n> +\t//       the index file passed in and use that instead.\n\n\t/*\n\t * A multi-line comment in our codebase looks\n\t * like this; slash-asterisk and asterisk-slash\n\t * are placed on their own lines.  We do not do\n\t * double-slash comments.\n\t */\n\n> +\tif (!the_hash_algo)\n> +\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n\nOK.  This is in line with how the command is documented to behave.\n\nHaving said that, I am not sure if it was an omission by mistake\nwhen 8e42eb0e (doc: sha256 is no longer experimental, 2023-07-31)\nmarked SHA-256 as non-experimental, or it was deliberate.  It would\nhave been an equally plausible, if not more sensible, position to\ntake to say that, since SHA-1 and SHA-256 are now on equal footing,\nwe won't \"default\" to SHA-1 anymore, when 8e42eb0e declared that\nSHA-256 is no longer a second-class citizen.\n\nIn any case, we can further remedy that, if we really wanted to, by\ntweaking the documentation to require the option outside a\nrepository without any default, for example, and then change this to\ndie().\n\nOf course, we may want to use the hash that is used in the index\nfile we are reading, if we can, as your comment said.\n\nThese incremental improvements can be left outside the scope of this\nchange.\n\n> diff --git a/rm b/rm\n> new file mode 100755\n> index 0000000000..2237506bf2\n> --- /dev/null\n> +++ b/rm\n> @@ -0,0 +1,3 @@\n> +#!/bin/sh\n> +\n> +echo rm $@\n\nPlease don't.\n\n> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\n> index 3b9dae331a..51fed26cc4 100755\n> --- a/t/t5300-pack-object.sh\n> +++ b/t/t5300-pack-object.sh\n> @@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' '\n>  \ttest_path_is_file foo.idx\n>  '\n>  \n> +test_expect_success SHA1 'show-index works OK outside a repository' '\n> +\tnongit git show-index <foo.idx\n> +'\n\nIf we are not using a hash that is not SHA-1, we should then be able\nto do the same check with\n\n    nongit git show-index --object-format=<hash> <foo.idx\n\ni.e., with an explicit argument.  I do not think we have any hits\nin the t/ directory from\n\n    $ git grep -e 'show-index .*--object-format' t/\n\nso such a test might be worth adding, either as a part of this\nchange or as a separate patch.\n   \n>  test_expect_success !PTHREADS,!FAIL_PREREQS \\\n>  \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n>  \ttest_must_fail git index-pack --threads=2 2>err &&\n\n\nExcept for these minor nits, everything else looks great.\n\nThanks.\n"},{"id":"506486","messageId":"74c0eddf-8bf9-4fb7-a0cd-edea8acaa938@gmail.com","threadId":"61770","inReplyTo":"xmqq1pzuylm6.fsf@gitster.g","subject":"Re: [PATCH v4] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-11-02T16:26:20Z","receivedAt":"2024-11-02T16:26:28Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"On 02/11/24 15:59, Junio C Hamano wrote:\n> Abhijeet Sonar <abhijeet.nkt@gmail.com> writes:\n> \n>> In c8aed5e8da (repository: stop setting SHA1 as the default object\n>> hash), we got rid of the default hash algorithm for the_repository.\n>> Due to this change, it is now the responsibility of the callers to set\n>> thier own default when this is not present.\n> \n> \"their own default\".\n> \n>> As stated in the docs, show-index should use SHA1 as the default hash\n>> algorithm when ran outsize of a repository. Make sure this promise is\n> \n> \"outside a repository\".\n> \n\nI will address those in v5, thanks\n\n>> met by falling back to SHA1 when the_hash_algo is not present (i.e.\n>> when the command is ran outside of a repository). Also add a test that\n>> verifies this behaviour.\n>>\n>> Signed-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n>> ---\n>>  builtin/show-index.c   | 6 ++++++\n>>  rm                     | 3 +++\n> \n> Huh?\n> \n>>  t/t5300-pack-object.sh | 4 ++++\n>>  3 files changed, 13 insertions(+)\n>>  create mode 100755 rm\n>>\n>> diff --git a/builtin/show-index.c b/builtin/show-index.c\n>> index f164c01bbe..645c2548fb 100644\n>> --- a/builtin/show-index.c\n>> +++ b/builtin/show-index.c\n>> @@ -38,6 +38,12 @@ int cmd_show_index(int argc,\n>>  \t\trepo_set_hash_algo(the_repository, hash_algo);\n>>  \t}\n>>  \n>> +\t// Fallback to SHA1 if we are running outside of a repository.\n>> +\t// TODO: Figure out and implement a way to detect the hash algorithm in use by the\n>> +\t//       the index file passed in and use that instead.\n> \n> \t/*\n> \t * A multi-line comment in our codebase looks\n> \t * like this; slash-asterisk and asterisk-slash\n> \t * are placed on their own lines.  We do not do\n> \t * double-slash comments.\n> \t */\n> \n>> +\tif (!the_hash_algo)\n>> +\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n> \n> OK.  This is in line with how the command is documented to behave.\n> \n> Having said that, I am not sure if it was an omission by mistake\n> when 8e42eb0e (doc: sha256 is no longer experimental, 2023-07-31)\n> marked SHA-256 as non-experimental, or it was deliberate.  It would\n> have been an equally plausible, if not more sensible, position to\n> take to say that, since SHA-1 and SHA-256 are now on equal footing,\n> we won't \"default\" to SHA-1 anymore, when 8e42eb0e declared that\n> SHA-256 is no longer a second-class citizen.>\n> In any case, we can further remedy that, if we really wanted to, by\n> tweaking the documentation to require the option outside a\n> repository without any default, for example, and then change this to\n> die().\n> \n> Of course, we may want to use the hash that is used in the index\n> file we are reading, if we can, as your comment said.\n> \n> These incremental improvements can be left outside the scope of this\n> change.\n>\n\nI see. So while this behavior not completely ideal, we are at least able\nto resolve a segfault. I take it that it is OK to leave it like this in\nthis patch and address it separately after.\n\n>> diff --git a/rm b/rm\n>> new file mode 100755\n>> index 0000000000..2237506bf2\n>> --- /dev/null\n>> +++ b/rm\n>> @@ -0,0 +1,3 @@\n>> +#!/bin/sh\n>> +\n>> +echo rm $@\n> \n> Please don't.\n> \n\nOops, this is embarrassing, that probably slipped in from a different\nthing I was experimenting with which is unrelated to this patch. I will\nverify that my patches are free of such errors in future before sending\nthem, apologies.\n\n>> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\n>> index 3b9dae331a..51fed26cc4 100755\n>> --- a/t/t5300-pack-object.sh\n>> +++ b/t/t5300-pack-object.sh\n>> @@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' '\n>>  \ttest_path_is_file foo.idx\n>>  '\n>>  \n>> +test_expect_success SHA1 'show-index works OK outside a repository' '\n>> +\tnongit git show-index <foo.idx\n>> +'\n> \n> If we are not using a hash that is not SHA-1, we should then be able\n> to do the same check with\n> \n>     nongit git show-index --object-format=<hash> <foo.idx\n> \n> i.e., with an explicit argument.  I do not think we have any hits\n> in the t/ directory from\n> \n>     $ git grep -e 'show-index .*--object-format' t/\n> \n\nWould that look something like this?\n\n```\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 51fed26cc4..78047604e4 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -527,6 +527,22 @@ test_expect_success SHA1 'show-index works OK\noutside a repository' '\n        nongit git show-index <foo.idx\n '\n\n+for hash in sha1 sha256\n+do\n+       test_expect_success 'show-index works OK outside a repository\nwith hash algo passed in via --object-format' '\n+               git init --object-format=$hash $hash-repo &&\n+               echo foo >$hash-repo/foo &&\n+               git -C $hash-repo add foo &&\n+               git -C $hash-repo commit -m \"commit foo\" &&\n+               oid=$(git -C $hash-repo rev-parse HEAD) &&\n+               echo $oid | git -C $hash-repo pack-objects $hash &&\n+               mv $hash-repo/$hash-*.idx $hash.idx &&\n+               nongit git show-index --object-format=$hash <$hash.idx &&\n+               wow &&\n+               rm -fr $hash/ $hash.idx\n+       '\n+done\n+\n test_expect_success !PTHREADS,!FAIL_PREREQS \\\n        'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n        test_must_fail git index-pack --threads=2 2>err &&\n```\n\n> so such a test might be worth adding, either as a part of this\n> change or as a separate patch.\n>    \n>>  test_expect_success !PTHREADS,!FAIL_PREREQS \\\n>>  \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n>>  \ttest_must_fail git index-pack --threads=2 2>err &&\n> \n> \n> Except for these minor nits, everything else looks great.\n> \n> Thanks.\n\n\n\n"},{"id":"506558","messageId":"20241104192958.64310-1-abhijeet.nkt@gmail.com","threadId":"61770","inReplyTo":"xmqq1pzuylm6.fsf@gitster.g","subject":"[PATCH v5 0/2] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-11-04T19:29:56Z","receivedAt":"2024-11-04T19:30:28Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"In this iteration, I have fixed some typos along with a stylistisc issue\nthat were noted in v4.\n\nI have also added an additional patch that adds a test for --object-format\noption in show-index as it was noted we don't already have any.\n\nAbhijeet Sonar (2):\n  show-index: fix uninitialized hash function\n  t5300: add test for 'show-index --object-format'\n\n builtin/show-index.c   |  9 +++++++++\n t/t5300-pack-object.sh | 26 ++++++++++++++++++++++++++\n 2 files changed, 35 insertions(+)\n\nRange-diff against v4:\n1:  c75175ad9b ! 1:  05ee1e2ea5 show-index: fix uninitialized hash function\n    @@ Commit message\n         In c8aed5e8da (repository: stop setting SHA1 as the default object\n         hash), we got rid of the default hash algorithm for the_repository.\n         Due to this change, it is now the responsibility of the callers to set\n    -    thier own default when this is not present.\n    +    their own default when this is not present.\n     \n         As stated in the docs, show-index should use SHA1 as the default hash\n    -    algorithm when ran outsize of a repository. Make sure this promise is\n    +    algorithm when run outside a repository. Make sure this promise is\n         met by falling back to SHA1 when the_hash_algo is not present (i.e.\n    -    when the command is ran outside of a repository). Also add a test that\n    -    verifies this behaviour.\n    +    when the command is run outside a repository). Also add a test that\n    +    verifies this behavior.\n     \n         Signed-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n     \n    @@ builtin/show-index.c: int cmd_show_index(int argc,\n      \t\trepo_set_hash_algo(the_repository, hash_algo);\n      \t}\n      \n    -+\t// Fallback to SHA1 if we are running outside of a repository.\n    -+\t// TODO: Figure out and implement a way to detect the hash algorithm in use by the\n    -+\t//       the index file passed in and use that instead.\n    ++\t/*\n    ++\t * Fallback to SHA1 if we are running outside of a repository.\n    ++\t *\n    ++\t * TODO: Figure out and implement a way to detect the hash algorithm in use by the\n    ++\t *       the index file passed in and use that instead.\n    ++\t */\n     +\tif (!the_hash_algo)\n     +\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n     +\n    @@ builtin/show-index.c: int cmd_show_index(int argc,\n      \n      \tif (fread(top_index, 2 * 4, 1, stdin) != 1)\n     \n    - ## rm (new) ##\n    -@@\n    -+#!/bin/sh\n    -+\n    -+echo rm $@\n    -\n      ## t/t5300-pack-object.sh ##\n     @@ t/t5300-pack-object.sh: test_expect_success 'index-pack --strict <pack> works in non-repo' '\n      \ttest_path_is_file foo.idx\n-:  ---------- > 2:  c8a28aae55 t5300: add test for 'show-index --object-format'\n-- \n2.47.0.107.g34b6ce9b30\n\n"},{"id":"506559","messageId":"20241104192958.64310-2-abhijeet.nkt@gmail.com","threadId":"61770","inReplyTo":"20241104192958.64310-1-abhijeet.nkt@gmail.com","subject":"[PATCH v5 1/2] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-11-04T19:29:57Z","receivedAt":"2024-11-04T19:30:32Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"In c8aed5e8da (repository: stop setting SHA1 as the default object\nhash), we got rid of the default hash algorithm for the_repository.\nDue to this change, it is now the responsibility of the callers to set\ntheir own default when this is not present.\n\nAs stated in the docs, show-index should use SHA1 as the default hash\nalgorithm when run outside a repository. Make sure this promise is\nmet by falling back to SHA1 when the_hash_algo is not present (i.e.\nwhen the command is run outside a repository). Also add a test that\nverifies this behavior.\n\nSigned-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n---\n builtin/show-index.c   | 9 +++++++++\n t/t5300-pack-object.sh | 4 ++++\n 2 files changed, 13 insertions(+)\n\ndiff --git a/builtin/show-index.c b/builtin/show-index.c\nindex f164c01bbe..b5e337869d 100644\n--- a/builtin/show-index.c\n+++ b/builtin/show-index.c\n@@ -38,6 +38,15 @@ int cmd_show_index(int argc,\n \t\trepo_set_hash_algo(the_repository, hash_algo);\n \t}\n \n+\t/*\n+\t * Fallback to SHA1 if we are running outside of a repository.\n+\t *\n+\t * TODO: Figure out and implement a way to detect the hash algorithm in use by the\n+\t *       the index file passed in and use that instead.\n+\t */\n+\tif (!the_hash_algo)\n+\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n+\n \thashsz = the_hash_algo->rawsz;\n \n \tif (fread(top_index, 2 * 4, 1, stdin) != 1)\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 3b9dae331a..51fed26cc4 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' '\n \ttest_path_is_file foo.idx\n '\n \n+test_expect_success SHA1 'show-index works OK outside a repository' '\n+\tnongit git show-index <foo.idx\n+'\n+\n test_expect_success !PTHREADS,!FAIL_PREREQS \\\n \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n \ttest_must_fail git index-pack --threads=2 2>err &&\n-- \n2.47.0.107.g34b6ce9b30\n\n"},{"id":"506560","messageId":"20241104192958.64310-3-abhijeet.nkt@gmail.com","threadId":"61770","inReplyTo":"20241104192958.64310-1-abhijeet.nkt@gmail.com","subject":"[PATCH v5 2/2] t5300: add test for 'show-index --object-format'","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-11-04T19:29:58Z","receivedAt":"2024-11-04T19:30:35Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"In 88a09a557c (builtin/show-index: provide options to determine hash\nalgo), the flag --object-format was added to show-index builtin as a way\nto provide a hash algorithm explicitly. However, we do not have tests in\nplace for that functionality. Add them.\n\nSigned-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n---\n t/t5300-pack-object.sh | 22 ++++++++++++++++++++++\n 1 file changed, 22 insertions(+)\n\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 51fed26cc4..301d5f1b61 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -527,6 +527,28 @@ test_expect_success SHA1 'show-index works OK outside a repository' '\n \tnongit git show-index <foo.idx\n '\n \n+for hash in sha1 sha256\n+do\n+\ttest_expect_success 'setup: show-index works OK outside a repository with hash algo passed in via --object-format' '\n+\t\tgit init explicit-hash-$hash --object-format=$hash &&\n+\t\ttest_commit -C explicit-hash-$hash one &&\n+\n+\t\tcat >in <<-EOF &&\n+\t\t$(git -C explicit-hash-$hash rev-parse one)\n+\t\tEOF\n+\n+\t\tgit -C explicit-hash-$hash pack-objects explicit-hash-$hash <in\n+\t'\n+\n+\ttest_expect_success 'show-index works OK outside a repository with hash algo passed in via --object-format' '\n+\t\tidx=$(echo explicit-hash-$hash/explicit-hash-$hash*.idx) &&\n+\t\tnongit git show-index --object-format=$hash <\"$idx\" >actual &&\n+\t\ttest_line_count = 1 actual &&\n+\n+\t\trm -rf explicit-hash-$hash\n+\t'\n+done\n+\n test_expect_success !PTHREADS,!FAIL_PREREQS \\\n \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n \ttest_must_fail git index-pack --threads=2 2>err &&\n-- \n2.47.0.107.g34b6ce9b30\n\n"},{"id":"506584","messageId":"xmqq4j4mv5o6.fsf@gitster.g","threadId":"61770","inReplyTo":"20241104192958.64310-3-abhijeet.nkt@gmail.com","subject":"Re: [PATCH v5 2/2] t5300: add test for 'show-index --object-format'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-11-05T01:19:05Z","receivedAt":"2024-11-05T01:19:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Abhijeet Sonar <abhijeet.nkt@gmail.com> writes:\n\n> In 88a09a557c (builtin/show-index: provide options to determine hash\n> algo), the flag --object-format was added to show-index builtin as a way\n> to provide a hash algorithm explicitly. However, we do not have tests in\n> place for that functionality. Add them.\n>\n> Signed-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n> ---\n>  t/t5300-pack-object.sh | 22 ++++++++++++++++++++++\n>  1 file changed, 22 insertions(+)\n\nNicely described.\n\n\n> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\n> index 51fed26cc4..301d5f1b61 100755\n> --- a/t/t5300-pack-object.sh\n> +++ b/t/t5300-pack-object.sh\n> @@ -527,6 +527,28 @@ test_expect_success SHA1 'show-index works OK outside a repository' '\n>  \tnongit git show-index <foo.idx\n>  '\n>  \n> +for hash in sha1 sha256\n> +do\n> +\ttest_expect_success 'setup: show-index works OK outside a repository with hash algo passed in via --object-format' '\n> +\t\tgit init explicit-hash-$hash --object-format=$hash &&\n\n\"git help cli\"; dashed options first and then other arguments.\n\n> +\t\ttest_commit -C explicit-hash-$hash one &&\n> +\n> +\t\tcat >in <<-EOF &&\n> +\t\t$(git -C explicit-hash-$hash rev-parse one)\n> +\t\tEOF\n\nHmph, is the above a roundabout way to say\n\n\t\tgit -C explicit-hash-$hash rev-parse one >in &&\n\nor am I missing some subtlety?\n\n> +\t\tgit -C explicit-hash-$hash pack-objects explicit-hash-$hash <in\n\n> +\t'\n> +\n> +\ttest_expect_success 'show-index works OK outside a repository with hash algo passed in via --object-format' '\n> +\t\tidx=$(echo explicit-hash-$hash/explicit-hash-$hash*.idx) &&\n> +\t\tnongit git show-index --object-format=$hash <\"$idx\" >actual &&\n> +\t\ttest_line_count = 1 actual &&\n> +\n> +\t\trm -rf explicit-hash-$hash\n\nWhen this test fails (e.g., the number of lines in the show-index\noutput is not 1), explicit-hash-$hash is not removed, because &&-\nchain short-circuits.\n\nPerhaps join thw two into one and use test_when_finished, like this?\n\n\ttest_expect_success 'show-index with explicit --object-format=$hash outside repo' '\n\t\ttest_when_finished \"rm -fr explicit-hash-$hash\" &&\n\t\tgit init --object-format=$hash explicit-hash-$hash &&\n\t\t...\n                nongit git show-index --object-format=$hash <\"$idx\" >actual &&\n\t\ttest_line_count 1 actual\n\t'\n\nOther than that, very nicely done.\n\nThanks.\n"},{"id":"506892","messageId":"20241109092739.14276-1-abhijeet.nkt@gmail.com","threadId":"61770","inReplyTo":"xmqq4j4mv5o6.fsf@gitster.g","subject":"[PATCH v6 0/2] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-11-09T09:27:37Z","receivedAt":"2024-11-09T09:28:39Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"> Nicely described.\n\nThanks!\n\n> \"git help cli\"; dashed options first and then other arguments.\n\nApplied.\n\n> Hmph, is the above a roundabout way to say\n>   git -C explicit-hash-$hash rev-parse one >in &&\n\nApplied.\n\n> or am I missing some subtlety?\n\nNo, I don't think you are. However, I would like to point out that the code\nwhich I used as the inspiration also does thing the same way:\n\n\ttest_expect_success 'pack-object <stdin parsing: [|--revs] with --stdin' '\n\t\tcat >in <<-EOF &&\n\t\t$(git -C pack-object-stdin rev-parse one)\n\t\t$(git -C pack-object-stdin rev-parse two)\n\t\tEOF\n\n\n> When this test fails (e.g., the number of lines in the show-index\n> output is not 1), explicit-hash-$hash is not removed, because &&-\n> chain short-circuits.\n> \n> Perhaps join thw two into one and use test_when_finished, like this?\n> \n> \ttest_expect_success 'show-index with explicit --object-format=$hash outside repo' '\n> \t\ttest_when_finished \"rm -fr explicit-hash-$hash\" &&\n> \t\tgit init --object-format=$hash explicit-hash-$hash &&\n> \t\t...\n>                 nongit git show-index --object-format=$hash <\"$idx\" >actual &&\n> \t\ttest_line_count 1 actual\n> \t'\n\nThat makes sense, applied.\n\n\nAbhijeet Sonar (2):\n  show-index: fix uninitialized hash function\n  t5300: add test for 'show-index --object-format'\n\n builtin/show-index.c   |  9 +++++++++\n t/t5300-pack-object.sh | 18 ++++++++++++++++++\n 2 files changed, 27 insertions(+)\n\nRange-diff against v5:\n1:  05ee1e2ea5 = 1:  05ee1e2ea5 show-index: fix uninitialized hash function\n2:  c8a28aae55 ! 2:  778f3ca18e t5300: add test for 'show-index --object-format'\n    @@ t/t5300-pack-object.sh: test_expect_success SHA1 'show-index works OK outside a\n      \n     +for hash in sha1 sha256\n     +do\n    -+\ttest_expect_success 'setup: show-index works OK outside a repository with hash algo passed in via --object-format' '\n    -+\t\tgit init explicit-hash-$hash --object-format=$hash &&\n    -+\t\ttest_commit -C explicit-hash-$hash one &&\n    -+\n    -+\t\tcat >in <<-EOF &&\n    -+\t\t$(git -C explicit-hash-$hash rev-parse one)\n    -+\t\tEOF\n    -+\n    -+\t\tgit -C explicit-hash-$hash pack-objects explicit-hash-$hash <in\n    -+\t'\n    -+\n     +\ttest_expect_success 'show-index works OK outside a repository with hash algo passed in via --object-format' '\n    ++\t\ttest_when_finished \"rm -rf explicit-hash-$hash\" &&\n    ++\t\tgit init --object-format=$hash explicit-hash-$hash &&\n    ++\t\ttest_commit -C explicit-hash-$hash one &&\n    ++\t\tgit -C explicit-hash-$hash rev-parse one >in &&\n    ++\t\tgit -C explicit-hash-$hash pack-objects explicit-hash-$hash <in &&\n     +\t\tidx=$(echo explicit-hash-$hash/explicit-hash-$hash*.idx) &&\n     +\t\tnongit git show-index --object-format=$hash <\"$idx\" >actual &&\n    -+\t\ttest_line_count = 1 actual &&\n    -+\n    -+\t\trm -rf explicit-hash-$hash\n    ++\t\ttest_line_count = 1 actual\n     +\t'\n     +done\n     +\n-- \n2.47.0.107.g34b6ce9b30\n\n"},{"id":"506893","messageId":"20241109092739.14276-2-abhijeet.nkt@gmail.com","threadId":"61770","inReplyTo":"20241109092739.14276-1-abhijeet.nkt@gmail.com","subject":"[PATCH v6 1/2] show-index: fix uninitialized hash function","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-11-09T09:27:38Z","receivedAt":"2024-11-09T09:28:51Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"In c8aed5e8da (repository: stop setting SHA1 as the default object\nhash), we got rid of the default hash algorithm for the_repository.\nDue to this change, it is now the responsibility of the callers to set\ntheir own default when this is not present.\n\nAs stated in the docs, show-index should use SHA1 as the default hash\nalgorithm when run outside a repository. Make sure this promise is\nmet by falling back to SHA1 when the_hash_algo is not present (i.e.\nwhen the command is run outside a repository). Also add a test that\nverifies this behavior.\n\nSigned-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n---\n builtin/show-index.c   | 9 +++++++++\n t/t5300-pack-object.sh | 4 ++++\n 2 files changed, 13 insertions(+)\n\ndiff --git a/builtin/show-index.c b/builtin/show-index.c\nindex f164c01bbe..b5e337869d 100644\n--- a/builtin/show-index.c\n+++ b/builtin/show-index.c\n@@ -38,6 +38,15 @@ int cmd_show_index(int argc,\n \t\trepo_set_hash_algo(the_repository, hash_algo);\n \t}\n \n+\t/*\n+\t * Fallback to SHA1 if we are running outside of a repository.\n+\t *\n+\t * TODO: Figure out and implement a way to detect the hash algorithm in use by the\n+\t *       the index file passed in and use that instead.\n+\t */\n+\tif (!the_hash_algo)\n+\t\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n+\n \thashsz = the_hash_algo->rawsz;\n \n \tif (fread(top_index, 2 * 4, 1, stdin) != 1)\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 3b9dae331a..51fed26cc4 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' '\n \ttest_path_is_file foo.idx\n '\n \n+test_expect_success SHA1 'show-index works OK outside a repository' '\n+\tnongit git show-index <foo.idx\n+'\n+\n test_expect_success !PTHREADS,!FAIL_PREREQS \\\n \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n \ttest_must_fail git index-pack --threads=2 2>err &&\n-- \n2.47.0.107.g34b6ce9b30\n\n"},{"id":"506894","messageId":"20241109092739.14276-3-abhijeet.nkt@gmail.com","threadId":"61770","inReplyTo":"20241109092739.14276-1-abhijeet.nkt@gmail.com","subject":"[PATCH v6 2/2] t5300: add test for 'show-index --object-format'","fromName":"Abhijeet Sonar","fromEmail":"abhijeet.nkt@gmail.com","sentAt":"2024-11-09T09:27:39Z","receivedAt":"2024-11-09T09:28:56Z","isPatch":true,"sender":{"key":"abhijeet.nkt@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40241646?v=4"},"body":"In 88a09a557c (builtin/show-index: provide options to determine hash\nalgo), the flag --object-format was added to show-index builtin as a way\nto provide a hash algorithm explicitly. However, we do not have tests in\nplace for that functionality. Add them.\n\nSigned-off-by: Abhijeet Sonar <abhijeet.nkt@gmail.com>\n---\n t/t5300-pack-object.sh | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 51fed26cc4..bb6a22b438 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -527,6 +527,20 @@ test_expect_success SHA1 'show-index works OK outside a repository' '\n \tnongit git show-index <foo.idx\n '\n \n+for hash in sha1 sha256\n+do\n+\ttest_expect_success 'show-index works OK outside a repository with hash algo passed in via --object-format' '\n+\t\ttest_when_finished \"rm -rf explicit-hash-$hash\" &&\n+\t\tgit init --object-format=$hash explicit-hash-$hash &&\n+\t\ttest_commit -C explicit-hash-$hash one &&\n+\t\tgit -C explicit-hash-$hash rev-parse one >in &&\n+\t\tgit -C explicit-hash-$hash pack-objects explicit-hash-$hash <in &&\n+\t\tidx=$(echo explicit-hash-$hash/explicit-hash-$hash*.idx) &&\n+\t\tnongit git show-index --object-format=$hash <\"$idx\" >actual &&\n+\t\ttest_line_count = 1 actual\n+\t'\n+done\n+\n test_expect_success !PTHREADS,!FAIL_PREREQS \\\n \t'index-pack --threads=N or pack.threads=N warns when no pthreads' '\n \ttest_must_fail git index-pack --threads=2 2>err &&\n-- \n2.47.0.107.g34b6ce9b30\n\n"},{"id":"506961","messageId":"xmqqzfm6h33w.fsf@gitster.g","threadId":"61770","inReplyTo":"20241109092739.14276-1-abhijeet.nkt@gmail.com","subject":"Re: [PATCH v6 0/2] show-index: fix uninitialized hash function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-11-11T03:16:19Z","receivedAt":"2024-11-11T03:16:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Abhijeet Sonar <abhijeet.nkt@gmail.com> writes:\n\n> No, I don't think you are. However, I would like to point out that the code\n> which I used as the inspiration also does thing the same way:\n>\n> \ttest_expect_success 'pack-object <stdin parsing: [|--revs] with --stdin' '\n> \t\tcat >in <<-EOF &&\n> \t\t$(git -C pack-object-stdin rev-parse one)\n> \t\t$(git -C pack-object-stdin rev-parse two)\n> \t\tEOF\n\nThere is a huge difference between one and two, though ;-)  If you\nexpect that your new thing may later have to expect more than one\nline of output, the way you wrote may be easier to extend, but if\nyou know it won't gain any more lines to its output, output from a\nsingle command is better written without cat around it.\n\n"},{"id":"509144","messageId":"Z1_gnA2kwRSyCF02@pks.im","threadId":"61770","inReplyTo":"20241109092739.14276-1-abhijeet.nkt@gmail.com","subject":"Re: [PATCH v6 0/2] show-index: fix uninitialized hash function","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-16T08:11:08Z","receivedAt":"2024-12-16T08:11:27Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Nov 09, 2024 at 02:57:37PM +0530, Abhijeet Sonar wrote:\n> That makes sense, applied.\n> \n> Abhijeet Sonar (2):\n>   show-index: fix uninitialized hash function\n>   t5300: add test for 'show-index --object-format'\n> \n>  builtin/show-index.c   |  9 +++++++++\n>  t/t5300-pack-object.sh | 18 ++++++++++++++++++\n>  2 files changed, 27 insertions(+)\n> \n> Range-diff against v5:\n> 1:  05ee1e2ea5 = 1:  05ee1e2ea5 show-index: fix uninitialized hash function\n> 2:  c8a28aae55 ! 2:  778f3ca18e t5300: add test for 'show-index --object-format'\n>     @@ t/t5300-pack-object.sh: test_expect_success SHA1 'show-index works OK outside a\n>       \n>      +for hash in sha1 sha256\n>      +do\n>     -+\ttest_expect_success 'setup: show-index works OK outside a repository with hash algo passed in via --object-format' '\n>     -+\t\tgit init explicit-hash-$hash --object-format=$hash &&\n>     -+\t\ttest_commit -C explicit-hash-$hash one &&\n>     -+\n>     -+\t\tcat >in <<-EOF &&\n>     -+\t\t$(git -C explicit-hash-$hash rev-parse one)\n>     -+\t\tEOF\n>     -+\n>     -+\t\tgit -C explicit-hash-$hash pack-objects explicit-hash-$hash <in\n>     -+\t'\n>     -+\n>      +\ttest_expect_success 'show-index works OK outside a repository with hash algo passed in via --object-format' '\n>     ++\t\ttest_when_finished \"rm -rf explicit-hash-$hash\" &&\n>     ++\t\tgit init --object-format=$hash explicit-hash-$hash &&\n>     ++\t\ttest_commit -C explicit-hash-$hash one &&\n>     ++\t\tgit -C explicit-hash-$hash rev-parse one >in &&\n>     ++\t\tgit -C explicit-hash-$hash pack-objects explicit-hash-$hash <in &&\n>      +\t\tidx=$(echo explicit-hash-$hash/explicit-hash-$hash*.idx) &&\n>      +\t\tnongit git show-index --object-format=$hash <\"$idx\" >actual &&\n>     -+\t\ttest_line_count = 1 actual &&\n>     -+\n>     -+\t\trm -rf explicit-hash-$hash\n>     ++\t\ttest_line_count = 1 actual\n>      +\t'\n>      +done\n>      +\n\nThanks, this version looks good to me.\n\nPatrick\n"},{"id":"509167","messageId":"xmqqjzbz7g5b.fsf@gitster.g","threadId":"61770","inReplyTo":"Z1_gnA2kwRSyCF02@pks.im","subject":"Re: [PATCH v6 0/2] show-index: fix uninitialized hash function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-16T16:21:20Z","receivedAt":"2024-12-16T16:21:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Sat, Nov 09, 2024 at 02:57:37PM +0530, Abhijeet Sonar wrote:\n>> That makes sense, applied.\n>> \n>> Abhijeet Sonar (2):\n>>   show-index: fix uninitialized hash function\n>>   t5300: add test for 'show-index --object-format'\n>> \n>>  builtin/show-index.c   |  9 +++++++++\n>>  t/t5300-pack-object.sh | 18 ++++++++++++++++++\n>>  2 files changed, 27 insertions(+)\n>> \n>> Range-diff against v5:\n>> 1:  05ee1e2ea5 = 1:  05ee1e2ea5 show-index: fix uninitialized hash function\n>> 2:  c8a28aae55 ! 2:  778f3ca18e t5300: add test for 'show-index --object-format'\n>>     @@ t/t5300-pack-object.sh: test_expect_success SHA1 'show-index works OK outside a\n>>       \n>>      +for hash in sha1 sha256\n>>      +do\n>>     -+\ttest_expect_success 'setup: show-index works OK outside a repository with hash algo passed in via --object-format' '\n>>     -+\t\tgit init explicit-hash-$hash --object-format=$hash &&\n>>     -+\t\ttest_commit -C explicit-hash-$hash one &&\n>>     -+\n>>     -+\t\tcat >in <<-EOF &&\n>>     -+\t\t$(git -C explicit-hash-$hash rev-parse one)\n>>     -+\t\tEOF\n>>     -+\n>>     -+\t\tgit -C explicit-hash-$hash pack-objects explicit-hash-$hash <in\n>>     -+\t'\n>>     -+\n>>      +\ttest_expect_success 'show-index works OK outside a repository with hash algo passed in via --object-format' '\n>>     ++\t\ttest_when_finished \"rm -rf explicit-hash-$hash\" &&\n>>     ++\t\tgit init --object-format=$hash explicit-hash-$hash &&\n>>     ++\t\ttest_commit -C explicit-hash-$hash one &&\n>>     ++\t\tgit -C explicit-hash-$hash rev-parse one >in &&\n>>     ++\t\tgit -C explicit-hash-$hash pack-objects explicit-hash-$hash <in &&\n>>      +\t\tidx=$(echo explicit-hash-$hash/explicit-hash-$hash*.idx) &&\n>>      +\t\tnongit git show-index --object-format=$hash <\"$idx\" >actual &&\n>>     -+\t\ttest_line_count = 1 actual &&\n>>     -+\n>>     -+\t\trm -rf explicit-hash-$hash\n>>     ++\t\ttest_line_count = 1 actual\n>>      +\t'\n>>      +done\n>>      +\n>\n> Thanks, this version looks good to me.\n\nThanks, both.  Let me mark it for 'next', then.\n"}]}