Re: [PATCH v4 05/25] midx: clear auxiliary .rev after replacing the MIDX

Taylor Blau [off-list ref] writes:

quoted

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 was hoping that [1] will become part of this series as a trivial
clean-up and bugfix, perhaps in its early part.

On Tue, Aug 24, 2021 at 02:12:42PM -0700, Junio C Hamano wrote:

Taylor Blau [off-list ref] writes:

quoted

quoted

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 was hoping that [1] will become part of this series as a trivial
clean-up and bugfix, perhaps in its early part.

Sure, that works even better. I'll send a reroll incorporating it as
soon as I finish re-testing.

Thanks,
Taylor

On Tue, Aug 24, 2021 at 05:24:45PM -0400, Taylor Blau wrote:

On Tue, Aug 24, 2021 at 02:12:42PM -0700, Junio C Hamano wrote:

quoted

Taylor Blau [off-list ref] writes:

quoted

quoted

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 was hoping that [1] will become part of this series as a trivial
clean-up and bugfix, perhaps in its early part.

Sure, that works even better. I'll send a reroll incorporating it as
soon as I finish re-testing.

Hmm, this got me wondering: what should be the behavior be when we run
the multi-pack-index command outside of a Git repository? For example,
in patch 15 we do:

    for (cur = get_multi_pack_index(the_repository); cur; cur = cur->next) {
      if (!strcmp(object_dir, cur->object_dir)) {
        ctx.m = cur;
        break;
      }
    }

but obviously get_multi_pack_index(the_repository) will fail when there
is no repository to begin with.

The real question is whether we should allow munging arbitrary MIDXs, or
restrict the ones we can modify to just our alternates. If we allow the
former, then that code needs to be tweaked. If not, and we only allow
touching alternates, then we need to require a repository for the
multi-pack-index builtin.

Thanks,
Taylor