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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 12, 2021 at 04:31:18PM -0400, Jeff King wrote:
> On Tue, Jul 27, 2021 at 05:20:07PM -0400, Taylor Blau wrote:
>
> > 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).

Yeah. That would definitely be in the spirit of "we should have more
test-tool-like helpers exposed via user-facing plumbing". And I agree
that it would be nice, but I definitely agree that it's a topic for a
later date ;).

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

Yeah. We could just sidestep the whole thing by not having the
`.git/objects` default, since all callers of midx_checksum pass an
argument, so that fallback is dead code anyway. Thanks for noting, I'll
remove it for the next round.

Thanks,
Taylor



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux