Re: [PATCH v2 2/8] builtin/multi-pack-index.c: support `--stdin-packs` mode

[Josh Steadmon <steadmon@google.com>]

Thanks for the series! I have a couple of questions:


On 2021.09.15 14:24, Taylor Blau wrote:
> To power a new `--write-midx` mode, `git repack` will want to write a
> multi-pack index containing a certain set of packs in the repository.
> 
> This new option will be used by `git repack` to write a MIDX which
> contains only the packs which will survive after the repack (that is, it
> will exclude any packs which are about to be deleted).
> 
> This patch effectively exposes the function implemented in the previous
> commit via the `git multi-pack-index` builtin. An alternative approach
> would have been to call that function from the `git repack` builtin
> directly, but this introduces awkward problems around closing and
> reopening the object store, so the MIDX will be written out-of-process.

Could you elaborate a bit on the "awkward problems" here? I'm afraid I'm
missing the context here.


> diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
> index 73c0113b48..047647b5f2 100644
> --- a/builtin/multi-pack-index.c
> +++ b/builtin/multi-pack-index.c
> @@ -47,6 +47,7 @@ static struct opts_multi_pack_index {
>  	const char *preferred_pack;
>  	unsigned long batch_size;
>  	unsigned flags;
> +	int stdin_packs;
>  } opts;
>  
>  static struct option common_opts[] = {
> @@ -61,6 +62,16 @@ static struct option *add_common_options(struct option *prev)
>  	return parse_options_concat(common_opts, prev);
>  }
>  
> +static void read_packs_from_stdin(struct string_list *to)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	while (strbuf_getline(&buf, stdin) != EOF)
> +		string_list_append(to, buf.buf);
> +	string_list_sort(to);
> +
> +	strbuf_release(&buf);
> +}
> +

I'm presuming that the packfile list is going to be generated
automatically, but what happens if that becomes corrupt somehow, and we
skip a packfile that should have been included? Will that cause
incorrect behavior, or will we just miss out on some of the bitmap
performance benefits?