Re: [PATCH v3 17/25] t/helper/test-read-midx.c: add --checksum mode

From: Jeff King <peff@peff.net>
Date: Aug 12, 2021, 20:31 UTC
Message-ID: <YRWFFgfXnyiRKdC1@coredump.intra.peff.net>
In-Reply-To: <60ec8b3466e7f94610a45bdd1c79feb06e439429.1627420428.git.me@ttaylorr.com>

On Tue, Jul 27, 2021 at 05:20:07PM -0400, Taylor Blau wrote:

Show 10 quoted lines

> Subsequent tests will want to check for the existence of a multi-pack
> bitmap which matches the multi-pack-index stored in the pack directory.
> 
> The multi-pack bitmap includes the hex checksum of the MIDX it
> corresponds to in its filename (for example,
> '$packdir/multi-pack-index-<checksum>.bitmap'). As a result, some tests
> want a way to learn what '<checksum>' is.
> 
> This helper addresses that need by printing the checksum of the
> repository's multi-pack-index.

Makes sense. It might be nice to have a generic tool for pulling hashes out of checksum files. Perhaps even a tool that is shipped with Git for operating on such files (for in-the-field debugging and diagnosis). But that can definitely be separate from this series (if ever).

>  t/helper/test-read-midx.c | 16 +++++++++++++++-
>  t/lib-bitmap.sh           |  4 ++++
>  2 files changed, 19 insertions(+), 1 deletion(-)

The patch itself looks fine to me. One curiosity:

Show 12 quoted lines

> diff --git a/t/lib-bitmap.sh b/t/lib-bitmap.sh
> index ecb5d0e05d..09cd036f4d 100644
> --- a/t/lib-bitmap.sh
> +++ b/t/lib-bitmap.sh
> @@ -260,3 +260,7 @@ have_delta () {
>  	echo $1 | git cat-file --batch-check="%(deltabase)" >actual &&
>  	test_cmp expect actual
>  }
> +
> +midx_checksum () {
> +	test-tool read-midx --checksum "${1:-.git/objects}"
> +}

This default ".git/objects" will only _usually_ be the right thing. :) If the actual C code accepted a missing object-dir, it could use the correct object directory discovered by setup_git_directory().

Probably not a big deal either way, though.

-Peff