Re: [PATCH] builtin/repack.c: invalidate MIDX only when necessary

From: Son Luong Ngoc <sluongng@gmail.com>
Date: Aug 25, 2020, 07:55 UTC
Message-ID: <CB6B70D3-5FC6-43FE-8460-33F6CFC123E6@gmail.com>
In-Reply-To: <ef9186a8df0d712c2ecccbe62cb43a7abadb9c96.1598320716.git.me@ttaylorr.com>

Hi Taylor,

Thanks for working on this.

Show 10 quoted lines

> On Aug 25, 2020, at 04:01, Taylor Blau <me@ttaylorr.com> wrote:
> 
> In 525e18c04b (midx: clear midx on repack, 2018-07-12), 'git repack'
> learned to remove a multi-pack-index file if it added or removed a pack
> from the object store.
> 
> This mechanism is a little over-eager, since it is only necessary to
> drop a MIDX if 'git repack' removes a pack that the MIDX references.
> Adding a pack outside of the MIDX does not require invalidating the
> MIDX, and likewise for removing a pack the MIDX does not know about.

I wonder if its worth to trigger write_midx_file() to update the midx instead of just removing MIDX?

That is already the direction we are taking in the 'maintenance' patch series whenever the multi-pack-index file was deemed invalid.

Or perhaps, we can check for 'core.multiPackIndex' value (which recently is 'true' by default) and determine whether we should remove the MIDX or rewrite it?

Show 7 quoted lines

> 
> Teach 'git repack' to check for this by loading the MIDX, and checking
> whether the to-be-removed pack is known to the MIDX. This requires a
> slightly odd alternation to a test in t5319, which is explained with a
> comment.
> 
> Signed-off-by: Taylor Blau <me@ttaylorr.com>

Cheers, Son Luong.