Re: [PATCH v2 4/8] builtin/pack-objects.c: respect 'pack.writeReverseIndex'
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Jan 25, 2021, 20:21 UTC
- Message-ID
- <YA8oPcvS0n8pUmcB@nand.local>
- In-Reply-To
- <YAto/ndOVfKSkizS@coredump.intra.peff.net>
On Fri, Jan 22, 2021 at 07:08:30PM -0500, Jeff King wrote:
Show 22 quoted lines
> On Fri, Jan 22, 2021 at 06:57:35PM -0500, Jeff King wrote:
>
> > > + if (!strcmp(k, "pack.writereverseindex")) {
> > > + if (git_config_bool(k, v))
> > > + pack_idx_opts.flags |= WRITE_REV;
> > > + else
> > > + pack_idx_opts.flags &= ~WRITE_REV;
> > > + return 0;
> > > + }
> >
> > This turned out delightfully simple. And I guess this is the "why is
> > WRITE_REV" caller I asked about from patch 2. It is
> > finish_tmp_packfile() where the magic happens. That unconditionally
> > calls write_rev_file(), but it's a noop if WRITE_REV isn't specified.
> >
> > Makes sense.
>
> Oh, one subtlety here: this is in pack-objects itself, _not_ in
> git-repack. This has bit us before with options like
> repack.writebitmaps, which was originally pack.writebitmaps and
> introduced all sorts of awkwardness (because pack-objects serves many
> other purposes besides repacks).I'd think that we'd want a single option to control whether or not reverse indexes are written to disk. I briefly considered (and I believe that you and I even discussed) having options to control this behavior per command, but it got out of hand quickly.
And that might have been OK, but I don't think the complexity was even warranted, because really on-disk reverse indexes are an all-or-nothing thing. That is: either you want to have revindexes accompanying .packs, or you don't. IOW, it doesn't matter whether those packs were pushed to us, or generated during repack, or from another pack-objects or what have you.
Thanks, Taylor