On Tue, May 24 2022, Taylor Blau wrote:
> On Tue, May 24, 2022 at 12:39:11PM -0700, Jonathan Nieder wrote:
>> Ævar Arnfjörð Bjarmason wrote:
>> > On Fri, May 20 2022, Taylor Blau wrote:
>> > On Mon, Nov 29 2021, Taylor Blau wrote:
>>
>> > > +static void write_mtimes_header(struct hashfile *f)
>> > > +{
>> > > + hashwrite_be32(f, MTIMES_SIGNATURE);
>> > > + hashwrite_be32(f, MTIMES_VERSION);
>> > > + hashwrite_be32(f, oid_version(the_hash_algo));
>> > > +}
>> [...]
>> > But since this is a new format I think it's worth considering not using
>> > the 1 or 2 you get from oid_version(), but the "format_id",
>> > i.e. GIT_SHA1_FORMAT_ID or GIT_SHA256_FORMAT_ID.
>> >
>> > You'll use the same space in the format for it, but we'll end up with
>> > something more obvious (as the integer encodes the sha1 or sha256 name).
>>
>> Agreed.
>
> I know we recommend using the format_id for on-disk formats, but I think
> there is enough existing uses of "1" or "2" that either are acceptable
> in practice.
>
> E.g., grepping around for "hashwrite.*oid_version", there are three
> existing formats that use "1" or "2" instead of the format_id. They are:
>
> - the commit-graph format
> - the midx format
> - the .rev format
>
> Moreover, I can't seem to find any formats that _don't_ use that
> convention.
It's used in the reftable format.
> So I have a vague preference towards using the values "1"
> and "2" as we currently do in these patches.
I suspect that's less "vague" and more "c'mon, I'm using it in
production already" :)
Anyway, I'm fine with leaving it be as you have it currently. I'd first
encountered this magic with reftable I think, and only recently found
that we use 1 and 2 in these other more recent places.
> (TBH, I don't find "sha1"
> significantly more interpretable than just "1", so I would be just as
> happy leaving it as-is).
Hrm, I'd think having it sha1 or s256 in big-endian would be a bit more
self-explanatory. I.e. SHA-256 is 2, not 256, and our 3 (if that ever
arrives) is likely not to be SHA-3 (but probably some successor).
