Re: [PATCH v3 05/17] pack-mtimes: support writing pack .mtimes files
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Mar 4, 2022, 10:40 UTC
- Message-ID
- <220304.86ee3if13q.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <YiFQxsmkcqb63azh@nand.local>
On Thu, Mar 03 2022, Taylor Blau wrote:
Show 30 quoted lines
> 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!