On Wed, May 29, 2024 at 06:55:42PM -0400, Taylor Blau wrote:
> Commit f894081deae (pack-revindex: read multi-pack reverse indexes,
> 2021-03-30) introduced the `get_midx_rev_filename()` helper (later
> modified by commit 60980aed786 (midx.c: write MIDX filenames to
> strbuf, 2021-10-26)).
>
> This function returns the location of the classic ".rev" files we used
> to write for MIDXs (prior to 95e8383bac1 (midx.c: make changing the
> preferred pack safe, 2022-01-25)), which is always of the form:
>
> $GIT_DIR/objects/pack/multi-pack-index-$HASH.rev
>
> Replace this function with a generic helper that populates a strbuf with
> the above form, replacing the ".rev" extension with a caller-provided
> argument.
Makes sense, as we have similar routines for packfiles.
> +void get_midx_filename_ext(struct strbuf *out, const char *object_dir,
> + const unsigned char *hash, const char *ext)
> {
> strbuf_addf(out, "%s/pack/multi-pack-index", object_dir);
> -}
> -
> -void get_midx_rev_filename(struct strbuf *out, struct multi_pack_index *m)
> -{
> - get_midx_filename(out, m->object_dir);
> - strbuf_addf(out, "-%s.rev", hash_to_hex(get_midx_checksum(m)));
> + if (ext)
> + strbuf_addf(out, "-%s.%s", hash_to_hex(hash), ext);
> }
This bare "const unsigned char *hash" caught my eye, as we've mostly
been removing them. But it was present in the original, too; it was just
hidden in the return from get_midx_checksum().
And I'm not sure what the non-ugly version is. We implicitly use
the_hash_algo for these kinds of trailer checksums, and calling them
"struct object_id" is probably even more confusing. I guess they could
be "struct csum_file_trailer" or something, but I'm not sure that would
actually make anything more clear. Anyway, none of this is new in your
patch and we can ignore it for now.
-Peff
