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 <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Author: ttaylorr

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))

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=<hash> and --no-write-chain-file' '
+test_expect_success 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
 	test_commit base-hash &&
 	git repack -d &&