Re: [PATCH] MIDX: revert the default version to v1

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Thu, Apr 16, 2026 at 11:45:49AM -0700, Junio C Hamano wrote:
>
>> From: Jeff King <peff@peff.net>
>> 
>> Dscho reports that fetching into a repository with Git 2.54-rc2 made
>> it unusable with Git 2.53, as the recent code by default writes
>> version 2 multi-pack-index files.  Version 2 is the base of more
>> advanced features to come, but using it where these features are not
>> used is a strict regression.
>> 
>> As deployed versions of Git and its reimplementations may not be
>> ready for the format bump, let's revert the default version of MIDX
>> file we write back to V1, and adjust tests that exercise V2-specific
>> features to explicitly request V2.
>> 
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> This looks fine to me, and you can add my S-o-b if you want. But let me
> propose a slight alternative that reduces the test churn and may make
> things easier going forward.

I did merge it to 'jch' (which I also tested externally) and then
'next', but haven't pushed out the 'next' integration out to test it
externally, so I'll chuck it and replace it with this version, as
the "compact is the only thing that needs v2" sound like a better
workaround.

> diff --git a/midx-write.c b/midx-write.c
> index 0ff2e45aa7..a89ac53045 100644
> --- a/midx-write.c
> +++ b/midx-write.c
> @@ -1259,7 +1259,6 @@ static int write_midx_internal(struct write_midx_opts *opts)
>  	struct tempfile *incr;
>  	struct write_midx_context ctx = {
>  		.preferred_pack_idx = NO_PREFERRED_PACK,
> -		.version = MIDX_VERSION_V2,
>  	 };
>  	struct multi_pack_index *midx_to_free = NULL;
>  	int bitmapped_packs_concat_len = 0;
> @@ -1275,6 +1274,9 @@ static int write_midx_internal(struct write_midx_opts *opts)
>  	ctx.repo = r;
>  	ctx.source = opts->source;
>  
> +	ctx.version = opts->flags & MIDX_WRITE_COMPACT ?
> +		MIDX_VERSION_V2 :
> +		MIDX_VERSION_V1;

Just a sytle thing but I'd prefer to see this written like this:

+	ctx.version = ((opts->flags & MIDX_WRITE_COMPACT)
+		       ? MIDX_VERSION_V2
+		       : MIDX_VERSION_V1);

because you do not have to worry about the precedence between & and
?: with an extra (parentheses), and a ternary split into multiple
lines is easier to spot if you write it at the beginning of the
line, with ?  and : aligned.