Re: [PATCH v2 1/3] ls-refs: report unborn targets of symrefs

From: Jonathan Tan <jonathantanmy@google.com>
To: peff@peff.net
Cc: jonathantanmy@google.com, git@vger.kernel.org,
	felipe.contreras@gmail.com, gitster@pobox.com, avarab@gmail.com
Subject: Re: [PATCH v2 1/3] ls-refs: report unborn targets of symrefs
Date: Wed, 16 Dec 2020 15:54:43 -0800	[thread overview]
Message-ID: <20201216235443.1674103-1-jonathantanmy@google.com> (raw)
In-Reply-To: <X9pQjHrRQQqB6AFI@coredump.intra.peff.net>

> On Tue, Dec 15, 2020 at 06:07:56PM -0800, Jonathan Tan wrote:
> 
> > +static int ls_refs_config(const char *var, const char *value, void *cb_data)
> > +{
> > +	struct ls_refs_data *data = cb_data;
> > +
> > +	if (!strcmp("lsrefs.unborn", var))
> > +		data->allow_unborn = !strcmp(value, "allow") ||
> > +			!strcmp(value, "advertise");
> 
> What's the reason we would want this to be configurable? I would think
> we would just want it always on for the server, and then clients can
> choose to make us of it or not (and probably not by omitting the
> capability; the question is what they want to do with the information
> about HEAD, but that is true whether it is unborn or not, and is
> controlled by options like "clone -b").

Firstly, this allows a staged rollout in load-balancing situations
wherein we turn on "allow" for all servers, then "advertise", so that we
don't end up with a client that sees the advertisement but then sends
the follow-up request to a server that has not received the latest
configuration yet.

Secondly, I wonder if some people purposely set HEAD to an unborn branch
just so that the repository would be presented as not having a HEAD.
Now, the name of the unborn branch would be revealed. I don't know if
that's a problem, though - but if it is, at least this configuration
variable is a way to solve that.

I'll include this in the documentation in a next version of this patch
set.