Subject: Re: [PATCH v3 05/17] pack-mtimes: support writing pack .mtimes files

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 04 Mar 2022 11:40:10 +0100

On Thu, Mar 03 2022, Taylor Blau wrote:

> On Thu, Mar 03, 2022 at 05:45:23PM +0100, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Wed, Mar 02 2022, Taylor Blau wrote:
>>
>> > Now that the `.mtimes` format is defined, supplement the pack-write API
>> > to be able to conditionally write an `.mtimes` file along with a pack by
>> > setting an additional flag and passing an oidmap that contains the
>> > timestamps corresponding to each object in the pack.
>> > [...]
>> >  void write_promisor_file(const char *promisor_name, struct ref **sought, int nr_sought)
>> > diff --git a/pack.h b/pack.h
>> > index fd27cfdfd7..01d385903a 100644
>> > --- a/pack.h
>> > +++ b/pack.h
>> > @@ -44,6 +44,7 @@ struct pack_idx_option {
>> >  #define WRITE_IDX_STRICT 02
>> >  #define WRITE_REV 04
>> >  #define WRITE_REV_VERIFY 010
>> > +#define WRITE_MTIMES 020
>> >
>> >  	uint32_t version;
>> >  	uint32_t off32_limit;
>>
>> Why the hardcoding? The 010 was added in your 8ef50d9958f (pack-write.c:
>> prepare to write 'pack-*.rev' files, 2021-01-25). That would be the same
>> as 8|2, but there's no 8 there., ditto this new 020 that's the same as
>> 1<<4 | 1<<2, but there's no "16", just WRITE_REV=4.
>
> I'm not sure I understand. These are octals, so octal "20" (or decimal
> 16) just gives us bit 5 -- the next available -- by itself.

Urgh, tired/rushed eyes yesterday. I managed to read these as decimals,
sorry.

I see from:

    git grep 'define[^0-9]*(\b020\b|\b16\b|1.*<<.*\b4\b)[^0-9]*$'

That I managed to patch what seems to be one of two other places in the
codebase using it recently (that goes >=020) in 245b9488150 (cat-file:
use GET_OID_ONLY_TO_DIE in --(textconv|filters), 2021-12-28).

Anyway, I think nothing needs to be done here. If you ever feel like
some churn here I think converting it to the almost ubiquitous "1 << N"
style we use almost everywhere else would be an improvement :)

Sorry!