Re: [PATCH 2/7] repack: use die_for_incompatible_opt3() for -A/-k/--cruft

From: Taylor Blau <me@ttaylorr.com>
Date: Dec 6, 2023, 19:18 UTC
Message-ID: <ZXDJIXjKefNFhDdf@nand.local>
In-Reply-To: <20231206115215.94467-3-l.s.r@web.de>

On Wed, Dec 06, 2023 at 12:51:56PM +0100, René Scharfe wrote:

Show 14 quoted lines

> diff --git a/builtin/repack.c b/builtin/repack.c
> index edaee4dbec..c54777bbe5 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -1203,19 +1203,13 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>  	if (delete_redundant && repository_format_precious_objects)
>  		die(_("cannot delete packs in a precious-objects repo"));
>
> -	if (keep_unreachable &&
> -	    (unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE)))
> -		die(_("options '%s' and '%s' cannot be used together"), "--keep-unreachable", "-A");
> +	die_for_incompatible_opt3(unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE), "-A",
> +				  keep_unreachable, "-k/--keep-unreachable",
> +				  pack_everything & PACK_CRUFT, "--cruft");

Oof, thanks for cleaning this one up.

I had to read this hunk a couple of times to convince myself that it was doing the right thing, but I believe it is.

> -	if (pack_everything & PACK_CRUFT) {
> +	if (pack_everything & PACK_CRUFT)
>  		pack_everything |= ALL_INTO_ONE;

And adding the ALL_INTO_ONE bit here does not effect either of the below two checks, so moving them up is fine, too.

Show 5 quoted lines

> -		if (unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE))
> -			die(_("options '%s' and '%s' cannot be used together"), "--cruft", "-A");
> -		if (keep_unreachable)
> -			die(_("options '%s' and '%s' cannot be used together"), "--cruft", "-k");
> -	}

Thanks, Taylor