From 6afc679fa62362ddaab955778d973c85a6cbf004 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 12 Jun 2026 16:07:14 -0400 Subject: [PATCH] midx-write: include packs above custom incremental base The previous commit made '--base' take effect on the normal incremental write path, which exposed an existing assumption in our helper function `should_include_pack()`, which is that any pack already present in `ctx->m` was skipped. That is only correct for non-incremental writes. For incremental writes, `ctx->base_midx` is the boundary that should be excluded from the new layer. If the caller selects an older base, or no base at all, then packs from layers above that base have to be included in the detached layer so that its bitmap has reachability closure. Teach `should_include_pack()` to choose the MIDX used for pack exclusion based on whether or not we are performing an incremental write. When doing so, use `ctx->base_midx`, and use `ctx->m` otherwise. The t5334 cases from the previous commit can now be marked as successful. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- midx-write.c | 16 +++++++++++----- t/t5334-incremental-multi-pack-index.sh | 4 ++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/midx-write.c b/midx-write.c index aa438775eb..c50fdb5c6d 100644 --- a/midx-write.c +++ b/midx-write.c @@ -133,8 +133,17 @@ static uint32_t midx_pack_perm(struct write_midx_context *ctx, static int should_include_pack(const struct write_midx_context *ctx, const char *file_name) { + struct multi_pack_index *m = ctx->m; /* - * Note that at most one of ctx->m and ctx->to_include are set, + * When writing incrementally, ctx->m may contain layers above + * the selected base MIDX, which must be included in the new + * layer. + */ + if (ctx->incremental) + m = ctx->base_midx; + + /* + * Note that at most one of m and ctx->to_include are set, * so we are testing midx_contains_pack() and * string_list_has_string() independently (guarded by the * appropriate NULL checks). @@ -148,10 +157,7 @@ static int should_include_pack(const struct write_midx_context *ctx, * should be performed independently (likely checking * to_include before the existing MIDX). */ - if (ctx->m && midx_contains_pack(ctx->m, file_name)) - return 0; - else if (ctx->base_midx && midx_contains_pack(ctx->base_midx, - file_name)) + if (m && midx_contains_pack(m, file_name)) return 0; else if (ctx->to_include && !string_list_has_string(ctx->to_include, file_name)) diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh index 69e96bf8d9..84ff612097 100755 --- a/t/t5334-incremental-multi-pack-index.sh +++ b/t/t5334-incremental-multi-pack-index.sh @@ -119,7 +119,7 @@ test_expect_success 'write MIDX layer with --base without --no-write-chain-file' test_grep "cannot use --base without --no-write-chain-file" err ' -test_expect_failure 'write MIDX layer with --base=none and --no-write-chain-file' ' +test_expect_success 'write MIDX layer with --base=none and --no-write-chain-file' ' test_commit base-none && git repack -d && @@ -136,7 +136,7 @@ test_expect_failure 'write MIDX layer with --base=none and --no-write-chain-file cp "$midx_chain.bak" "$midx_chain" ' -test_expect_failure 'write MIDX layer with --base= and --no-write-chain-file' ' +test_expect_success 'write MIDX layer with --base= and --no-write-chain-file' ' test_commit base-hash && git repack -d &&