| Message ID | 60ec8b3466e7f94610a45bdd1c79feb06e439429.1627420428.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | multi-pack reachability bitmaps | expand |
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). > 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. -Peff
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
diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c index 7c2eb11a8e..cb0d27049a 100644 --- a/t/helper/test-read-midx.c +++ b/t/helper/test-read-midx.c @@ -60,12 +60,26 @@ static int read_midx_file(const char *object_dir, int show_objects) return 0; } +static int read_midx_checksum(const char *object_dir) +{ + struct multi_pack_index *m; + + setup_git_directory(); + m = load_multi_pack_index(object_dir, 1); + if (!m) + return 1; + printf("%s\n", hash_to_hex(get_midx_checksum(m))); + return 0; +} + int cmd__read_midx(int argc, const char **argv) { if (!(argc == 2 || argc == 3)) - usage("read-midx [--show-objects] <object-dir>"); + usage("read-midx [--show-objects|--checksum] <object-dir>"); if (!strcmp(argv[1], "--show-objects")) return read_midx_file(argv[2], 1); + else if (!strcmp(argv[1], "--checksum")) + return read_midx_checksum(argv[2]); return read_midx_file(argv[1], 0); } 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}" +}
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. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- t/helper/test-read-midx.c | 16 +++++++++++++++- t/lib-bitmap.sh | 4 ++++ 2 files changed, 19 insertions(+), 1 deletion(-)