Merge branch 'tb/midx-incremental-custom-base' into seen

The 'git multi-pack-index write --incremental' command has been
corrected to properly honor the '--base' option.  Previously, the
custom base was ignored by the normal write path; packs from layers
above the selected base were incorrectly skipped by the pack exclusion
logic, and reachability closure for bitmaps was broken.

* tb/midx-incremental-custom-base:
  midx-write: include packs above custom incremental base
  midx: pass custom '--base' through incremental writes
  t5334: expose shared `nth_line()` helper

builtin/multi-pack-index.c

@@ -225,7 +225,8 @@ static int cmd_multi_pack_index_write(int argc, const char **argv,
 	}
 
 	ret = write_midx_file(source->packed, opts.preferred_pack,
-			      opts.refs_snapshot, opts.flags);
+			      opts.refs_snapshot, opts.incremental_base,
+			      opts.flags);
 
 	free(opts.refs_snapshot);
 	return ret;

builtin/repack.c

@@ -775,7 +775,7 @@ int cmd_repack(int argc,
 
 		if (git_env_bool(GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL, 0))
 			flags |= MIDX_WRITE_INCREMENTAL;
-		write_midx_file(files->packed, NULL, NULL, flags);
+		write_midx_file(files->packed, NULL, NULL, NULL, flags);
 	}
 
 cleanup:

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))
+	if (m && midx_contains_pack(m, file_name))
-		return 0;
-	else if (ctx->base_midx && midx_contains_pack(ctx->base_midx,
-						      file_name))
 		return 0;
 	else if (ctx->to_include &&
 		 !string_list_has_string(ctx->to_include, file_name))
@@ -1851,12 +1857,14 @@ cleanup:
 int write_midx_file(struct odb_source_packed *source,
 		    const char *preferred_pack_name,
 		    const char *refs_snapshot,
+		    const char *incremental_base,
 		    unsigned flags)
 {
 	struct write_midx_opts opts = {
 		.source = source,
 		.preferred_pack_name = preferred_pack_name,
 		.refs_snapshot = refs_snapshot,
+		.incremental_base = incremental_base,
 		.flags = flags,
 	};
 

midx.h

@@ -132,7 +132,7 @@ int prepare_multi_pack_index_one(struct odb_source_packed *source);
  */
 int write_midx_file(struct odb_source_packed *source,
 		    const char *preferred_pack_name, const char *refs_snapshot,
-		    unsigned flags);
+		    const char *incremental_base, unsigned flags);
 int write_midx_file_only(struct odb_source_packed *source,
 			 struct string_list *packs_to_include,
 			 const char *preferred_pack_name,

t/lib-midx.sh

@@ -34,3 +34,9 @@ compare_results_with_midx () {
 		midx_git_two_modes "cat-file --batch-all-objects --batch-check --unordered" sorted
 	'
 }
+
+nth_line() {
+	local n="$1"
+	shift
+	awk "NR==$n" "$@"
+}

t/t5334-incremental-multi-pack-index.sh

@@ -128,7 +128,12 @@ test_expect_success 'write MIDX layer with --base=none and --no-write-chain-file
 		--no-write-chain-file --base=none)" &&
 
 	test_cmp "$midx_chain.bak" "$midx_chain" &&
-	test_path_is_file "$midxdir/multi-pack-index-$layer.midx"
+	test_path_is_file "$midxdir/multi-pack-index-$layer.midx" &&
+
+	echo "$layer" >"$midx_chain" &&
+	test-tool read-midx --show-objects "$objdir" "$layer" >midx.objects &&
+	test_grep "^$(git rev-parse 2.2) " midx.objects &&
+	cp "$midx_chain.bak" "$midx_chain"
 '
 
 test_expect_success 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
@@ -136,11 +141,20 @@ test_expect_success 'write MIDX layer with --base=<hash> and --no-write-chain-fi
 	git repack -d &&
 
 	cp "$midx_chain" "$midx_chain.bak" &&
+	base="$(nth_line 1 "$midx_chain")" &&
 	layer="$(git multi-pack-index write --bitmap --incremental \
-		--no-write-chain-file --base="$(nth_line 1 "$midx_chain")")" &&
+		--no-write-chain-file --base="$base")" &&
 
 	test_cmp "$midx_chain.bak" "$midx_chain" &&
-	test_path_is_file "$midxdir/multi-pack-index-$layer.midx"
+	test_path_is_file "$midxdir/multi-pack-index-$layer.midx" &&
+
+	{
+		echo "$base" &&
+		echo "$layer"
+	} >"$midx_chain" &&
+	test-tool read-midx --show-objects "$objdir" "$layer" >midx.objects &&
+	test_grep "^$(git rev-parse 2.2) " midx.objects &&
+	cp "$midx_chain.bak" "$midx_chain"
 '
 
 for reuse in false single multi

t/t5335-compact-multi-pack-index.sh

@@ -3,6 +3,7 @@
 test_description='multi-pack-index compaction'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-midx.sh
 
 GIT_TEST_MULTI_PACK_INDEX=0
 GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0
@@ -13,12 +14,6 @@ packdir=$objdir/pack
 midxdir=$packdir/multi-pack-index.d
 midx_chain=$midxdir/multi-pack-index-chain
 
-nth_line() {
-	local n="$1"
-	shift
-	awk "NR==$n" "$@"
-}
-
 write_packs () {
 	for c in "$@"
 	do