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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]

On Mon, Apr 01, 2024 at 05:16:44PM -0400, Taylor Blau wrote:
[snip]
> @@ -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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]