On 05/07/2012 06:24 PM, Junio C Hamano wrote:
Michael Haggerty<mhagger@xxxxxxxxxxxx> writes:
This combination "!memcmp(ref->name, "refs/", 5)&&
check_refname_format(ref->name, 0)" is the reason that I suggested
adding a REFNAME_FULL option [1], in which case it could be written
"check_refname_format(ref->name, REFNAME_FULL)".
I personally think that check-refname-format should lose its second
parameter and always check for a concrete full refs, so that we can
tighten the current allow-onelevel case a bit further (i.e. ".git/HEAD" is
OK at the root level, but ".git/refs/heads/HEAD" is asking for trouble,
and ".git/config" is downright confusing. The function does not get a good
enough clue by only ONELEVEL). That would mean REFNAME_FULL will not be
necessary---it should be the only mode of operation, and the callers need
to be adjusted accordingly.
We should also introduce another function check-refspec-half-format to be
used for the checks for left and right halves of refspec ("refs/heads/*"
is OK but "refs/heads*" is not, perhaps).
A single function trying to do both and not doing a good job in either
case is not a pretty picture.
Whether the functionality is presented via one function with multiple
options or multiple functions with no options is of secondary
importance. Let's first worry about the desired functionality, since
even that is not clear.
There's a long weekend coming up in Germany so hopefully I'll have
another increment of time to work on this. I'd like to get feedback on
the requirements. First let me present a summary of what I know about
refnames in git.
There are a few categories of refname that come up in the git code:
* Full refnames like "refs/foo" or "refs/bar/baz" (starting with "refs/").
* Top-level special ALL_CAPS reference names like "MERGE_HEAD".
* Refspec-style patterns with wildcards, like "refs/heads/*" or
"refs/foo/*/bar".
* 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".
* Maybe others...?
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.
* 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.
If code will continue to work this way, then there might be no use for a
function that can check short refnames; one would always convert the
short refname into a full refname and check the latter. On the other
hand, a short refname check might do special things like *reject* a name
that starts with "refs/".
In addition to the strict rules governing refnames, there are other
policies that are enforced at varying levels of strictness or that might
be helpful in the future; for example,
* Refnames either have to start with "refs/" or have to be ALL_CAPS.
* 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:
. They must contain at least one `/`. This enforces the presence of a
category like `heads/`, `tags/` etc. but the actual names are not
restricted.
This sentence also implies that check-ref-format was intended to be used
for refnames shortened by having the "refs/" prefix omitted, which is
curious.)
* Unlikely refnames like "refs/heads/HEAD" could be handled with
suspicion, as you suggest. I would also nominate constructs like
"refs/heads/refs/heads/mybranch", which I've seen cause confusion at $WORK.
In terms of implementation:
* We want to distinguish between what is acceptable for new refnames
versus refnames that already exist in the local repository and/or
refnames received from a remote repo. So there should be some kind of
"REFNAME_RELAXED" option.
* It is important for the "old" check_refname_format() and the "new" one
to coexist during the transition. This is simply because I don't want
to have to analyze and rewrite all ~30 callers of check_refname_format()
in one big bang. I suggest retaining a copy as
check_refname_format_legacy() that is deprecated and removed after all
callers have been rewritten.
* The exact current behavior of "git check-ref-format" will probably not
map well to the new check_refname_format(), and is anyway inadequate and
somewhat inconsistent. We need to decide how strict we need to be about
backwards-compatibility of this command.
* I've been using "git check-ref-format" for testing
check_refname_format(). But I think it would be a bad idea to expose
all of the check_refname_format() options and variants in an installed
plumbing command. Therefore I think that there should be a program that
can be used to call check_refname_format() with all of its option
combinations to be used only for testing purposes.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html