Re: [PATCH v2 2/8] builtin/multi-pack-index.c: support `--stdin-packs` mode
- From
Josh Steadmon <steadmon@google.com>
- Date
- Sep 22, 2021, 22:29 UTC
- Message-ID
- <YUuuYW3RktlDzskH@google.com>
- In-Reply-To
- <59556e554565120929549239f1aee5a76d80ac8d.1631730270.git.me@ttaylorr.com>
Thanks for the series! I have a couple of questions:
On 2021.09.15 14:24, Taylor Blau wrote:
Show 12 quoted lines
> 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.
Show 26 quoted lines
> 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?