On Tue, Jan 26, 2021 at 01:38:49PM -0800, Junio C Hamano wrote:
> > @@ -103,14 +131,31 @@ int ls_refs(struct repository *r, struct strvec *keys,
> > data.symrefs = 1;
> > else if (skip_prefix(arg, "ref-prefix ", &out))
> > strvec_push(&data.prefixes, out);
> > + else if (data.allow_unborn && !strcmp("unborn", arg))
> > + data.unborn = 1;
>
> I think the use of &&-cascade is iffy here. Even when we are *not*
> accepting request for unborn, we should still parse it as such.
> This does not matter in today's code, but it is a basic courtesy for
> future developers who may add more "else if" after it.
>
> IOW
>
> else if (!strcmp("unborn", arg)) {
> if (!data.allow_unborn)
> ; /* we are not accepting the request */
> else
> data.unborn = 1;
> }
>
> I wrote the above in longhand only for documentation purposes; in
> practice,
>
> else if (!strcmp("unborn", arg))
> data.unborn = data.allow_unborn;
>
> may suffice.
Doing it that way is friendlier, but loosens enforcement of:
Client will then send a space separated list of capabilities it wants
to be in effect. The client MUST NOT ask for capabilities the server
did not say it supports.
from Documentation/technical/protocol-capabilities.txt.
It does solve Jonathan's racy cluster-deploy problem, though. See the
discussion in the v4 thread (sorry, seems not to have hit the archive
yet, but hopefully this link will work soon):
https://lore.kernel.org/git/YBCitNb75rpnuW2L@xxxxxxxxxxxxxxxxxxxxxxx/
-Peff
