On Mon, Mar 29, 2021 at 08:53:22AM -0400, Jeff King wrote:
> On Thu, Mar 11, 2021 at 12:05:38PM -0500, Taylor Blau wrote:
>
> > Implement the writing half of multi-pack reverse indexes. This is
> > nothing more than the format describe a few patches ago, with a new set
> > of helper functions that will be used to clear out stale .rev files
> > corresponding to old MIDXs.
>
> Looks good.
>
> > +struct clear_midx_data {
> > + char *keep;
> > + const char *ext;
> > +};
> > +
> > +static void clear_midx_file_ext(const char *full_path, size_t full_path_len,
> > + const char *file_name, void *_data)
>
> This will clean up _any_ stale midx .rev file. So even if we miss one
> when writing a new midx (due to a bug, race, power loss, etc), we'll
> catch it later.
>
> We _might_ want to also teach various tempfile-cleanup code run by gc to
> likewise look for unattached midx .rev files, but I don't think we
> necessarily have to do it now.
Agreed there on both counts.
> > void clear_midx_file(struct repository *r)
> > {
> > char *midx = get_midx_filename(r->objects->odb->path);
> > @@ -1049,6 +1162,8 @@ void clear_midx_file(struct repository *r)
> > if (remove_path(midx))
> > die(_("failed to clear multi-pack-index at %s"), midx);
> >
> > + clear_midx_files_ext(r, ".rev", NULL);
> > +
> > free(midx);
>
> The sole caller now doesn't pass the "keep" hash, so we'd always delete
> all of them. I guess we'll see that change once somebody starts actually
> writing them.
That's right. I hope that the benefits of splitting the MIDX bitmaps
topic into two series has generally outweighed the drawbacks, but in
instances like these it can be kind of annoying.
Thanks,
Taylor
