Re* [PATCH v2] show-index: fix uninitialized hash function

Abhijeet Sonar <abhijeet.nkt@gmail.com> writes:

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).

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.

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@gmail.com>
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@gmail.com>
[jc: fixed up the test]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/show-index.c   | 3 +++
 t/t5300-pack-object.sh | 4 ++++
 2 files changed, 7 insertions(+)