Re: [PATCH v2 1/8] midx: expose `write_midx_file_only()` publicly

From: Jonathan Tan <jonathantanmy@google.com>
Date: Sep 22, 2021, 23:14 UTC
Message-ID: <20210922231423.2610917-1-jonathantanmy@google.com>
In-Reply-To: <03c1b2c4d3d9d10a6768684e9c4d1c1cc8019844.1631730270.git.me@ttaylorr.com>

Show 9 quoted lines

> @@ -1237,7 +1253,7 @@ static int write_midx_internal(const char *object_dir,
>  
>  	QSORT(ctx.info, ctx.nr, pack_info_compare);
>  
> -	if (packs_to_drop && packs_to_drop->nr) {
> +	if (ctx.m && packs_to_drop && packs_to_drop->nr) {
>  		int drop_index = 0;
>  		int missing_drops = 0;
>

I couldn't figure out why this requires ctx.m now.

Show 8 quoted lines

> @@ -62,6 +63,10 @@ int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name)
>  int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local);
>  
>  int write_midx_file(const char *object_dir, const char *preferred_pack_name, unsigned flags);
> +int write_midx_file_only(const char *object_dir,
> +			 struct string_list *packs_to_include,
> +			 const char *preferred_pack_name,
> +			 unsigned flags);

It took me a while to figure out that this function doesn't only write a MIDX file, but writes an MIDX file only for certain packs. Maybe worth adding a comment here (e.g. "Write an MIDX file only for the given packs").

Other than that, this patch looks good.