Re: [PATCH 12/17] builtin/repack.c: support generating a cruft pack

[Derrick Stolee <stolee@gmail.com>]

On 11/29/2021 5:25 PM, Taylor Blau wrote:

> +static int write_cruft_pack(const struct pack_objects_args *args,
> +			    const char *pack_prefix,
> +			    struct string_list *names,
> +			    struct string_list *existing_packs,
> +			    struct string_list *existing_kept_packs)
> +{
> +	struct child_process cmd = CHILD_PROCESS_INIT;
> +	struct strbuf line = STRBUF_INIT;
> +	struct string_list_item *item;
> +	FILE *in, *out;
> +	int ret;
> +
> +	prepare_pack_objects(&cmd, args);
> +
> +	strvec_push(&cmd.args, "--cruft");
> +	if (cruft_expiration)
> +		strvec_pushf(&cmd.args, "--cruft-expiration=%s",
> +			     cruft_expiration);
> +
> +	strvec_push(&cmd.args, "--honor-pack-keep");
> +	strvec_push(&cmd.args, "--non-empty");
> +	strvec_push(&cmd.args, "--max-pack-size=0");

This --max-pack-size is meaningless, right? The config that would change
this is already ignored by 'git pack-objects'.

> +		OPT_BIT(0, "cruft", &pack_everything,
> +				N_("same as -a, pack unreachable cruft objects separately"),
> +				   PACK_CRUFT | ALL_INTO_ONE),

I can understand the use of OPT_BIT here. Keep in mind that --no-cruft would
remove the '-a' option, if it already existed. Perhaps we should just use
OPT_BOOL and update to add the ALL_INTO_ONE if PACK_CRUFT exists?

> +		OPT_STRING(0, "cruft-expiration", &cruft_expiration, N_("approxidate"),
> +				N_("with -C, expire objects older than this")),

Here, --no-cruft-expiration will set cruft_expiration to NULL and not overwrite
the --cruft option, as expected. Just pointing out that this is different than
the option in 'git pack-objects'.

> --- a/t/t5327-pack-objects-cruft.sh
> +++ b/t/t5327-pack-objects-cruft.sh
> @@ -358,4 +358,157 @@ test_expect_success 'expired objects are pruned' '
>  	)
>  '
>  
> +test_expect_success 'repack --cruft generates a cruft pack' '
> +	git init repo &&
> +	test_when_finished "rm -fr repo" &&
> +	(
> +		cd repo &&
> +
> +		test_commit reachable &&
> +		git branch -M main &&
> +		git checkout --orphan other &&

Here is a way to make objects unreachable!

> +		test_commit unreachable &&
> +
> +		git checkout main &&
> +		git branch -D other &&
> +		git tag -d unreachable &&

Thanks,
-Stolee