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!