From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got: allow clone without symrefs
To: Martijn van Duren <openbsd+got@list.imperialat.at>
Cc: gameoftrees@openbsd.org
Date: Sat, 14 Jun 2025 17:19:37 +0200

On Sat, Jun 14, 2025 at 01:17:19AM +0200, Martijn van Duren wrote:
> Hello all,
> 
> Some time ago I got pointed to this repo[0] which doesn't want to clone
> without specifying the specific branch. I have no particular usecase
> for this repo, but the fact that it couldn't be checked out by default
> kept irking me.
> 
> After some digging I found out that we rely on symrefs to determine
> the default branch. According to the protocol spec[1]:
>   Clients MAY use the parameters from this capability to select the
>   proper initial branch when cloning a repository.
> So it's definitely not a bad thing to do. However, this particular host
> doesn't send a symrefs and we're left hanging until you use -l to see
> what's available, and then -b to manually specify the branch to check
> out.
> 
> The diff below adds a workaround for this situation by using the first
> refs/heads/ whose id_str matches that of HEAD. It's definitely not
> fool proof, but it at least covers this case.

I am fine with this approach. This server does not advertise any symref
capability, so what you are proposing is the best we can do.

I see one wrinkle: your patch is blindly assuming that the ref advertised
on the first line received from the server points at the "HEAD".

The Git docs imply that HEAD doesn't necessarily need to exist (why that
is I don't know -- probably their usual disregard of conventions in favour
of extreme flexibility). Quoting gitprotocol-pack(5):

     If HEAD is a valid ref, HEAD MUST appear as the first advertised ref. If
     HEAD is not a valid ref, HEAD MUST NOT appear in the advertisement list
     at all, but other refs may still appear.

To be honest, I expect that every useful repository out there will
contain HEAD in practice, so your code will probably just work as it is.
But in theory your patch makes us fetch some arbitrary branch by default,
whichever the server announces first. If some server implementation were
to advertise all refs in alphabetical order and omit HEAD, and people were
adding new branches all day long, we could end up cloning a different
default branch every time.

I would suggest to check whether the variable refname equals "HEAD" in
the code quoted below, and leave default_branch set to NULL if the refname
is not "HEAD". This would keep the current behaviour intact in the edge
case where no HEAD is advertised at all. We do want people to use the
-b option in that case, rather than fetching something arbitrary.

> +				if (default_branch == NULL) {
> +					default_id_str = strdup(id_str);
> +					if (default_id_str == NULL) {
> +						err = got_error_from_errno(
> +						    "strdup");
> +						goto done;
> +					}
> +				}