Re: [PATCH] fetch/push: allow refs/*:refs/*

[Junio C Hamano <gitster@pobox.com>]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> * Full refnames like "refs/foo" or "refs/bar/baz" (starting with "refs/").

When reading any of these out of existing set of ref candidates (i.e. read
from readdir() recursing down from ".git/refs"), we shouldn't be rejecting
with format checks, but we may want to warn.

When creating, we have full refnames in this case, so we can just check.

> * Top-level special ALL_CAPS reference names like "MERGE_HEAD".

Similar.  These are "full refname" as far as ref format checking code is
concerned, and the format logic allows only ALL_CAPS at the top-level.
When reading any single-level out of existing set of ref candidates
(i.e. read from readdir() scanning ".git/"), malformed candidates should
be ignored without even warning (otherwise you will incorrectly complain
upon seeing ".git/config").  When creating, we have full refnames in this
case, so we can just check.

> * Refspec-style patterns with wildcards, like "refs/heads/*" or
> "refs/foo/*/bar".

As I said in the message you are replying to, the format check for these
should be a separate function (which might happen to share some code with
ref, but that is an implementation detail).

> * Short branch names with an implied "refs/heads/" prefix omitted;
> e.g., "master" meaning "refs/heads/master".
>
> * Short tag names with an implied "refs/tags/" prefix omitted; e.g.,
> "v1.2.3" meaning "refs/tags/v1.2.3".

Checking for the above two classes should conceptually happen with the
implied prefix.  When creating (i.e. git branch/git checkout -b/git tag),
you know what full refname the thing will become if you allow it to be
created.

As an optimization measure at the implementation level, being able to
check "frotz", knowing that the calling code wants to know that the name
is usable as a branch name, without having to do an allocation to hold
"refs/heads/frotz" somewhere to call the format-check function, options
like allow-onelevel is sometimes useful, but at the conceptual level it is
not strictly necessary.

> Currently, I believe that code usually handles the short branch/tag
> names via one of two mechanisms:
>
> * Explicitly prepend "refs/heads/" or "refs/tags/" to the short name
> to make the corresponding full name, then work exclusively with the
> full name.

Conceptually everybody should be doing that.

> * Use the ref_rev_parse_rules based functions like dwim_ref() to guess
> which prefix can be applied to a short refname to make it into a full
> refname, then work exclusively with the full name.

That is merely _matching_ with existing name, no?  Once you see an
existing name, you shouldn't be _rejecting_ anything.  If you see a
questionable name, you may warn.

I am not sure if these "two mechanisms" is a proper characterization of
the problem space.  Shouldn't you be separating between reading and
creating instead?

> * Refnames either have to start with "refs/" or have to be ALL_CAPS.

Sensible.

> * Refnames cannot be directly under "refs/" but should be in a
> namespace like "refs/heads/".  (This is implied by the old
> check-ref-format documentation:

Almost, but beware of stashes.