Re: Should `@` be really a valid git tag name?
- From
Carlo Marcelo Arenas Belón <carenas@gmail.com>
- Date
- Sep 17, 2021, 22:58 UTC
- Message-ID
- <YUUdjNDkL7Uy5erG@carlos-mbp.lan>
- In-Reply-To
- <YUUM5VqTEtjhnfjj@coredump.intra.peff.net>
On Fri, Sep 17, 2021 at 05:47:17PM -0400, Jeff King wrote:
Show 24 quoted lines
> On Fri, Sep 17, 2021 at 01:53:52PM -0700, Junio C Hamano wrote: > > > On Fri, Sep 17, 2021 at 7:12 AM <gitmailinglist.bentolor@xoxy.net> wrote: > > >> > > >> A SO commenter pointed out, that git-check-ref-format forbids @ and > > >> maybe I should report this as a potential bug. Is it? > > > > > > a reference that is named "@" only is invalid, but refs/tags/@ is not. > > > > ;-) > > > > "git check-ref-format master ; echo $?" would show that any single > > level name is "forbidden", so probably the SO commenter (whatever > > that is) was confused---it is not about @ at all. > > > > In any case, a tag whose name is @ may be another source of > > confusion in the modern world, after we added @ as a synonym to > > HEAD. I do not know, for example, offhand which between the HEAD or > > that tag "git show @" would choose. It makes sense to avoid it. > > In the past when we've had confusing names (like refs/heads/HEAD), we > continue to allow them at the plumbing level (to retain backwards > compatibility), but flag them at the porcelain level to prevent users > shooting themselves in the foot. This seems like a good candidate for > that (for both git-branch and git-tag).
I was leaning towards something like that plus a Documentation update, but noticed that the current behaviour was inconsistent, and the confusion pointed out by Junio seems to indicate it is better if fully restricted.
Carlo
----- >8 ----- Subject: [PATCH] refs: mark "@" as an invalid refspec
9ba89f484e (Add new @ shortcut for HEAD, 2013-09-02), adds "@" to the list of invalid refspec, but does it only for full refspec and not as a component.
Move the logic to validate it at the component level to prevent also tags to be created with that name.
Reported-by: Benjamin <gitmailinglist.bentolor@xoxy.net> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> --- refs.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c index 8b9f7c3a80..2ca6995c0f 100644 --- a/refs.c +++ b/refs.c @@ -47,13 +47,14 @@ int ref_storage_backend_exists(const char *name) * 4: A bad character: ASCII control characters, and * ":", "?", "[", "\", "^", "~", SP, or TAB * 5: *, reject unless REFNAME_REFSPEC_PATTERN is set + * 6: @, only valid if used around valid characters */ static unsigned char refname_disposition[256] = { 1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, 0, 0, 0, 2, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 4, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 6, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 4, 0, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4 @@ -137,6 +138,14 @@ static int check_refname_component(const char *refname, int *flags, */ *flags &= ~ REFNAME_REFSPEC_PATTERN; break; + case 6: + if (last == '\0' && *(cp + 1) == '\0') { + if (sanitized) + sanitized->buf[sanitized->len] = '-'; + else + return -1; + } + break; } last = ch; } @@ -167,14 +176,6 @@ static int check_or_sanitize_refname(const char *refname, int flags, { int component_len, component_count = 0; - if (!strcmp(refname, "@")) { - /* Refname is a single character '@'. */ - if (sanitized) - strbuf_addch(sanitized, '-'); - else - return -1; - } - while (1) { if (sanitized && sanitized->len) strbuf_complete(sanitized, '/');
-- 2.33.0.911.gbe391d4e11