Merge branch 'ps/repack-avoid-noop-midx-rewrite'

Even when there is no changes in the packfile and no need to
recompute bitmaps, "git repack" recomputed and updated the MIDX
file, which has been corrected.

* ps/repack-avoid-noop-midx-rewrite:
  midx-write: skip rewriting MIDX with `--stdin-packs` unless needed
  midx-write: extract function to test whether MIDX needs updating
  midx: fix `BUG()` when getting preferred pack without a reverse index
This commit is contained in:
Junio C Hamano
2025-12-30 12:58:19 +09:00
5 changed files with 194 additions and 21 deletions

View File

@@ -1014,6 +1014,65 @@ static void clear_midx_files(struct odb_source *source,
strbuf_release(&buf);
}
static bool midx_needs_update(struct multi_pack_index *midx, struct write_midx_context *ctx)
{
struct strset packs = STRSET_INIT;
struct strbuf buf = STRBUF_INIT;
bool needed = true;
/*
* Ignore incremental updates for now. The assumption is that any
* incremental update would be either empty (in which case we will bail
* out later) or it would actually cover at least one new pack.
*/
if (ctx->incremental)
goto out;
/*
* Otherwise, we need to verify that the packs covered by the existing
* MIDX match the packs that we already have. The logic to do so is way
* more complicated than it has any right to be. This is because:
*
* - We cannot assume any ordering.
*
* - The MIDX packs may not be loaded at all, and loading them would
* be wasteful. So we need to use the pack names tracked by the
* MIDX itself.
*
* - The MIDX pack names are tracking the ".idx" files, whereas the
* packs themselves are tracking the ".pack" files. So we need to
* strip suffixes.
*/
if (ctx->nr != midx->num_packs + midx->num_packs_in_base)
goto out;
for (uint32_t i = 0; i < ctx->nr; i++) {
strbuf_reset(&buf);
strbuf_addstr(&buf, pack_basename(ctx->info[i].p));
strbuf_strip_suffix(&buf, ".pack");
if (!strset_add(&packs, buf.buf))
BUG("same pack added twice?");
}
for (uint32_t i = 0; i < ctx->nr; i++) {
strbuf_reset(&buf);
strbuf_addstr(&buf, midx->pack_names[i]);
strbuf_strip_suffix(&buf, ".idx");
if (!strset_contains(&packs, buf.buf))
goto out;
strset_remove(&packs, buf.buf);
}
needed = false;
out:
strbuf_release(&buf);
strset_clear(&packs);
return needed;
}
static int write_midx_internal(struct odb_source *source,
struct string_list *packs_to_include,
struct string_list *packs_to_drop,
@@ -1031,6 +1090,7 @@ static int write_midx_internal(struct odb_source *source,
struct write_midx_context ctx = {
.preferred_pack_idx = NO_PREFERRED_PACK,
};
struct multi_pack_index *midx_to_free = NULL;
int bitmapped_packs_concat_len = 0;
int pack_name_concat_len = 0;
int dropped_packs = 0;
@@ -1111,27 +1171,39 @@ static int write_midx_internal(struct odb_source *source,
for_each_file_in_pack_dir(source->path, add_pack_to_midx, &ctx);
stop_progress(&ctx.progress);
if ((ctx.m && ctx.nr == ctx.m->num_packs + ctx.m->num_packs_in_base) &&
!ctx.incremental &&
!(packs_to_include || packs_to_drop)) {
struct bitmap_index *bitmap_git;
int bitmap_exists;
int want_bitmap = flags & MIDX_WRITE_BITMAP;
if (!packs_to_drop) {
/*
* If there is no MIDX then either it doesn't exist, or we're
* doing a geometric repack. Try to load it from the source to
* tell these two cases apart.
*/
struct multi_pack_index *midx = ctx.m;
if (!midx)
midx = midx_to_free = load_multi_pack_index(ctx.source);
bitmap_git = prepare_midx_bitmap_git(ctx.m);
bitmap_exists = bitmap_git && bitmap_is_midx(bitmap_git);
free_bitmap_index(bitmap_git);
if (midx && !midx_needs_update(midx, &ctx)) {
struct bitmap_index *bitmap_git;
int bitmap_exists;
int want_bitmap = flags & MIDX_WRITE_BITMAP;
if (bitmap_exists || !want_bitmap) {
/*
* The correct MIDX already exists, and so does a
* corresponding bitmap (or one wasn't requested).
*/
if (!want_bitmap)
clear_midx_files_ext(source, "bitmap", NULL);
result = 0;
goto cleanup;
bitmap_git = prepare_midx_bitmap_git(midx);
bitmap_exists = bitmap_git && bitmap_is_midx(bitmap_git);
free_bitmap_index(bitmap_git);
if (bitmap_exists || !want_bitmap) {
/*
* The correct MIDX already exists, and so does a
* corresponding bitmap (or one wasn't requested).
*/
if (!want_bitmap)
clear_midx_files_ext(source, "bitmap", NULL);
result = 0;
goto cleanup;
}
}
close_midx(midx_to_free);
midx_to_free = NULL;
}
if (ctx.incremental && !ctx.nr) {
@@ -1487,6 +1559,7 @@ cleanup:
free(keep_hashes);
}
strbuf_release(&midx_name);
close_midx(midx_to_free);
trace2_region_leave("midx", "write_midx_internal", r);