Re: [PATCH v2 4/4] midx-write.c: use `--stdin-packs` when repacking

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 2, 2024, 11:47 UTC
Message-ID: <ZgvwXyMMUKxfP9ni@tanuki>
In-Reply-To: <b5d6ba5802aef6ddf1542f1b0efffe43c22436ba.1712006190.git.me@ttaylorr.com>

On Mon, Apr 01, 2024 at 05:16:44PM -0400, Taylor Blau wrote: [snip]

Show 22 quoted lines

> @@ -1498,16 +1499,15 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,
>  	}
>  
>  	cmd_in = xfdopen(cmd.in, "w");
> -
> -	for (i = 0; i < m->num_objects; i++) {
> -		struct object_id oid;
> -		uint32_t pack_int_id = nth_midxed_pack_int_id(m, i);
> -
> -		if (!include_pack[pack_int_id])
> +	for (i = 0; i < m->num_packs; i++) {
> +		struct packed_git *p = m->packs[i];
> +		if (!p)
>  			continue;
>  
> -		nth_midxed_object_oid(&oid, m, i);
> -		fprintf(cmd_in, "%s\n", oid_to_hex(&oid));
> +		if (include_pack[i])
> +			fprintf(cmd_in, "%s\n", pack_basename(p));
> +		else
> +			fprintf(cmd_in, "^%s\n", pack_basename(p));
>  	}

The patch looks straight-forward to me overall, but this last part confused me a bit. Why do we explicitly exclude those packs instead of merely skipping over them? Is this so that we don't repack objects which are part of both included and not-included packs?

I was mostly wondering whether it can ever happen here that we decide to not include a pack because all of its objects are already contained in other packs. In that case, explicitly excluding it would lead to repo corruption if we deleted such a not-included pack.

Patrick