Re: [PATCH v4 4/6] clone: add tags refspec earlier to fetch refspec

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 3, 2025, 07:51 UTC
Message-ID: <Z6B1f1jB8KoTp8P8@pks.im>
In-Reply-To: <20250131-toon-clone-refs-v4-4-2a4ff851498f@iotcl.com>

On Fri, Jan 31, 2025 at 04:30:32PM +0100, Toon Claes wrote:

> In clone.c we call refspec_ref_prefixes() to copy the fetch refspecs
> from the `remote->fetch` refspec into `ref_prefixes` of
> `transport_ls_refs_options`. Afterward we add the tags prefix

s/Afterward/&s/

Show 11 quoted lines

> diff --git a/builtin/clone.c b/builtin/clone.c
> index d652682494d0d27dd73cd0585e28b23f2883786d..7ab156ac00240de89baca6533ed2541839286fc4 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1375,14 +1365,15 @@ int cmd_clone(int argc,
>  		transport->smart_options->check_self_contained_and_connected = 1;
>  
>  	strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
> +
> +	if (option_tags || option_branch)
> +		refspec_append(&remote->fetch, TAG_REFSPEC);

It's a bit surprising that we also do this with `option_branch`, which only seems to indicate which branch git-clone(1) is supposed to check out. But in fact, the documentation mentions that it may also be used to check out a tag. Principle of least surprise at its best.

In any case, I think it would be nice to have a comment here explaining why this is the correct thing to do.

Patrick