On Tue, Aug 24, 2021 at 01:27:34PM -0700, Junio C Hamano wrote: > Taylor Blau <me@xxxxxxxxxxxx> writes: > > > When writing a new multi-pack index, write_midx_internal() attempts to > > clean up any auxiliary files (currently just the MIDX's `.rev` file, but > > soon to include a `.bitmap`, too) corresponding to the MIDX it's > > replacing. > > > > This step should happen after the new MIDX is written into place, since > > doing so beforehand means that the old MIDX could be read without its > > corresponding .rev file. > > > > Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx> > > --- > > midx.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/midx.c b/midx.c > > index 321c6fdd2f..73b199ca49 100644 > > --- a/midx.c > > +++ b/midx.c > > @@ -1086,10 +1086,11 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index * > > > > if (flags & MIDX_WRITE_REV_INDEX) > > write_midx_reverse_index(midx_name, midx_hash, &ctx); > > - clear_midx_files_ext(the_repository, ".rev", midx_hash); > > > > commit_lock_file(&lk); > > > > + clear_midx_files_ext(the_repository, ".rev", midx_hash); > > This needs to take object_dir into account, no? Yes and no; clear_midx_files_ext() still takes a pointer to a 'struct repository' until we pick up [1]. I asked for some changes in the latest version that Johannes posted. So I'd be OK to live with this behavior for the time being, and then I can send another patch on top that fixes the new and existing callers (incorporating [1] with some new tests). Or we can hold one up and expedite the other. I would suggest that we pick up this series to next if you're otherwise happy with it and then I can send the trivial fixes on top. Thanks, Taylor [1]: https://lore.kernel.org/git/20210823171011.80588-1-johannes@xxxxxxxxxxxxxxxx/
