Abhijeet Sonar <abhijeet.nkt@xxxxxxxxx> writes: > t/t8101-show-index-hash-function.sh | 15 +++++++++++++++ > 2 files changed, 18 insertions(+) > create mode 100755 t/t8101-show-index-hash-function.sh Thanks. But let's not waste the scarce resource that is a test number for a single oddball test (and as t/README says t8xxx series is for forensics Porcelains). > +test_expect_success 'show-index: should not fail outside a repository' ' > + git init --object-format=sha1 && ( > + echo "" | git hash-object -w --stdin | git pack-objects test && Our tests run in an already initialized repository, and some test configuration would use sha256 to initialize that repository. So the above is not a good idea, unless you use a new directory. We often create a new directory inside the initial directory the test begins in, and then in a subshell chdir into the directory. We frown upon a pipeline that has "git" as an upstream, because the exit status from such invocation of "git" will be hidden. git init --object-format=sha1 sample && ( cd sample && O=$(git hash-object -w /dev/null) && T=$(echo "$O" | git pack-objects test) && would give you a pair of files "test-$T.idx" and "test-$T.pack" and it will notice if hash-object or pack-objects fail. > + rm -rf .git && This alone does *not* necessarily make the directory you are using for test completely unassociated with any repository. If you are working with the source code of Git from a repository (as opposed to extracted tar archive), with that "rm -fr", you may have made the directory not a Git repository, but then that directory is now a mere subdirectory "t/trash directory.t8101-show-index-hash-function" of the repository that houses the Git source code (unless you are using the --root=<directory> option to run the tests). When we test behaviour of commands outside a repository, we use the GIT_CEILING_DIRECTORIES feature, often via the nongit helper function that is defined in t/test-lib-functions.sh (which becomes available to tests by doing ". ./test-lib.sh". In t5300-pack-object.sh we see these bits already. test_expect_success 'index-pack --stdin complains of non-repo' ' nongit test_must_fail git index-pack \ --object-format=$(test_oid algo) --stdin <foo.pack && test_path_is_missing non-repo/.git ' test_expect_success 'index-pack <pack> works in non-repo' ' nongit git index-pack \ --object-format=$(test_oid algo) ../foo.pack && test_path_is_file foo.idx ' I wonder if it is sufficient to add a new test after these two steps, something like test_expect_success SHA1 'show-index works OK outside a repository' ' nongit git show-index <foo.idx ' perhaps? With that, your patch would become like so: ------------ >8 ----------------------- >8 ------------ From: Abhijeet Sonar <abhijeet.nkt@xxxxxxxxx> Date: Mon, 15 Jul 2024 15:53:43 +0530 Subject: [PATCH] show-index: fix uninitialized hash function As stated in the docs, show-index should use SHA1 as the default hash algorithm when run outside a repository. However, 'the_hash_algo' is left uninitialized if we are not in a repository and no explicit hash function is specified, causing a crash. Fix it by falling back to SHA1 when it is found uninitialized. Also add test that verifies this behaviour. Signed-off-by: Abhijeet Sonar <abhijeet.nkt@xxxxxxxxx> [jc: fixed up the test] Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> --- builtin/show-index.c | 3 +++ t/t5300-pack-object.sh | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/builtin/show-index.c b/builtin/show-index.c index 540dc3dad1..bb6d9e3c40 100644 --- a/builtin/show-index.c +++ b/builtin/show-index.c @@ -35,6 +35,9 @@ int cmd_show_index(int argc, const char **argv, const char *prefix) repo_set_hash_algo(the_repository, hash_algo); } + if (!the_hash_algo) + repo_set_hash_algo(the_repository, GIT_HASH_SHA1); + hashsz = the_hash_algo->rawsz; if (fread(top_index, 2 * 4, 1, stdin) != 1) diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh index 4ad023c846..83933eca5d 100755 --- a/t/t5300-pack-object.sh +++ b/t/t5300-pack-object.sh @@ -523,6 +523,10 @@ test_expect_success 'index-pack --strict <pack> works in non-repo' ' test_path_is_file foo.idx ' +test_expect_success SHA1 'show-index works OK outside a repository' ' + nongit git show-index <foo.idx +' + test_expect_success !PTHREADS,!FAIL_PREREQS \ 'index-pack --threads=N or pack.threads=N warns when no pthreads' ' test_must_fail git index-pack --threads=2 2>err && -- 2.46.0-rc0-140-g824782812f