* Tags named '-'
@ 2011-05-09 15:21 Alex Vandiver
2011-05-09 18:01 ` Junio C Hamano
2011-05-09 22:04 ` Tags named '-' Sverre Rabbelier
0 siblings, 2 replies; 7+ messages in thread
From: Alex Vandiver @ 2011-05-09 15:21 UTC (permalink / raw)
To: git
Heya,
The functionality of `git checkout -` is very useful, and I am hopeful
that `git merge -` will eventually land, as it matches my workflow well
as well. However, the porcelain is not entirely consistent about
forbidding '-' as a ref name:
~/tmp $ git init
Initialized empty Git repository in /home/chmrr/tmp/.git/
~/tmp (master #) $ git commit --allow-empty -m 'First commit'
[master (root-commit) b7402ef] First commit
~/tmp (master) $ git checkout -b other
Switched to a new branch 'other'
~/tmp (other) $ git checkout -
Switched to branch 'master'
~/tmp (master) $ git checkout -b -
fatal: git checkout: we do not like '-' as a branch name.
~/tmp (master) $ git tag -
~/tmp (master) $ git tag -l
-
~/tmp (master) $ git checkout -
Switched to branch 'other'
The likely best fix is to disallow '-' as a tag name, as well, which
would be handy because the typo of leaving off the 'l' on '-l', for
example, which is not an uncommon mistake that I have seen.
- Alex
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: Tags named '-' 2011-05-09 15:21 Tags named '-' Alex Vandiver @ 2011-05-09 18:01 ` Junio C Hamano 2011-05-09 22:56 ` [RFC/PATCH] tag: disallow '-' as tag name Michael Schubert 2011-05-09 22:04 ` Tags named '-' Sverre Rabbelier 1 sibling, 1 reply; 7+ messages in thread From: Junio C Hamano @ 2011-05-09 18:01 UTC (permalink / raw) To: Alex Vandiver; +Cc: git Alex Vandiver <alex@chmrr.net> writes: > The likely best fix is to disallow '-' as a tag name, as well. Sounds sane. Patches welcome. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC/PATCH] tag: disallow '-' as tag name 2011-05-09 18:01 ` Junio C Hamano @ 2011-05-09 22:56 ` Michael Schubert 2011-05-09 23:08 ` Junio C Hamano 0 siblings, 1 reply; 7+ messages in thread From: Michael Schubert @ 2011-05-09 22:56 UTC (permalink / raw) To: Junio C Hamano, Alex Vandiver, git Add strbuf_check_tag_ref() as helper to check a refname for a tag. Signed-off-by: Michael Schubert <mschub@elegosoft.com> --- builtin/tag.c | 30 ++++++++++++++++++++++-------- 1 files changed, 22 insertions(+), 8 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index b66b34a..f087a7f 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -352,11 +352,26 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset) return 0; } +static int strbuf_check_tag_ref(struct strbuf *sb, const char *name) +{ + if (name[0] == '-') + return CHECK_REF_FORMAT_ERROR; + + strbuf_reset(sb); + strbuf_add(sb, "refs/tags/", 10); + strbuf_add(sb, name, strlen(name)); + + if (sb->len > PATH_MAX) + die(_("tag name too long: %.*s..."), 50, name); + + return check_ref_format(sb->buf); +} + int cmd_tag(int argc, const char **argv, const char *prefix) { struct strbuf buf = STRBUF_INIT; + struct strbuf ref = STRBUF_INIT; unsigned char object[20], prev[20]; - char ref[PATH_MAX]; const char *object_ref, *tag; struct ref_lock *lock; @@ -452,12 +467,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix) if (get_sha1(object_ref, object)) die(_("Failed to resolve '%s' as a valid ref."), object_ref); - if (snprintf(ref, sizeof(ref), "refs/tags/%s", tag) > sizeof(ref) - 1) - die(_("tag name too long: %.*s..."), 50, tag); - if (check_ref_format(ref)) + if (strbuf_check_tag_ref(&ref, tag)) die(_("'%s' is not a valid tag name."), tag); - if (!resolve_ref(ref, prev, 1, NULL)) + if (!resolve_ref(ref.buf, prev, 1, NULL)) hashclr(prev); else if (!force) die(_("tag '%s' already exists"), tag); @@ -466,14 +479,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix) create_tag(object, tag, &buf, msg.given || msgfile, sign, prev, object); - lock = lock_any_ref_for_update(ref, prev, 0); + lock = lock_any_ref_for_update(ref.buf, prev, 0); if (!lock) - die(_("%s: cannot lock the ref"), ref); + die(_("%s: cannot lock the ref"), ref.buf); if (write_ref_sha1(lock, object, NULL) < 0) - die(_("%s: cannot update the ref"), ref); + die(_("%s: cannot update the ref"), ref.buf); if (force && hashcmp(prev, object)) printf(_("Updated tag '%s' (was %s)\n"), tag, find_unique_abbrev(prev, DEFAULT_ABBREV)); strbuf_release(&buf); + strbuf_release(&ref); return 0; } -- 1.7.5.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC/PATCH] tag: disallow '-' as tag name 2011-05-09 22:56 ` [RFC/PATCH] tag: disallow '-' as tag name Michael Schubert @ 2011-05-09 23:08 ` Junio C Hamano 2011-05-09 23:36 ` [RFC/PATCH v2] " Michael Schubert 0 siblings, 1 reply; 7+ messages in thread From: Junio C Hamano @ 2011-05-09 23:08 UTC (permalink / raw) To: Michael Schubert; +Cc: Alex Vandiver, git Michael Schubert <mschub@elegosoft.com> writes: > Add strbuf_check_tag_ref() as helper to check a refname for a tag. > > Signed-off-by: Michael Schubert <mschub@elegosoft.com> > --- That was quick ;-). > builtin/tag.c | 30 ++++++++++++++++++++++-------- > 1 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/builtin/tag.c b/builtin/tag.c > index b66b34a..f087a7f 100644 > --- a/builtin/tag.c > +++ b/builtin/tag.c > @@ -352,11 +352,26 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset) > return 0; > } > > +static int strbuf_check_tag_ref(struct strbuf *sb, const char *name) > +{ > + if (name[0] == '-') > + return CHECK_REF_FORMAT_ERROR; So contrary to what the title claims, it forbids a tag that begins with '-', e.g. '-foo', not just a single dash. That is fine by me (we do the same in strbuf-check-branch-ref) but it needs to be explained better. > + strbuf_reset(sb); > + strbuf_add(sb, "refs/tags/", 10); > + strbuf_add(sb, name, strlen(name)); strbuf_addf(sb, "refs/tags/%s", name)? > + if (sb->len > PATH_MAX) > + die(_("tag name too long: %.*s..."), 50, name); I think that should be if (PATH_MAX <= sb->len) but I do not see the point of checking against PATH_MAX if you are already using a strbuf... ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC/PATCH v2] tag: disallow '-' as tag name 2011-05-09 23:08 ` Junio C Hamano @ 2011-05-09 23:36 ` Michael Schubert [not found] ` <BANLkTik7PYjGMMfxaNPubYR7M1OgBrF_qw@mail.gmail.com> 0 siblings, 1 reply; 7+ messages in thread From: Michael Schubert @ 2011-05-09 23:36 UTC (permalink / raw) To: Junio C Hamano, Alex Vandiver, git Disallow '-' as tag name just as any tag name starting with '-' to be consistent with "git checkout". Add strbuf_check_tag_ref() as helper to check a refname for a tag. Signed-off-by: Michael Schubert <mschub@elegosoft.com> --- builtin/tag.c | 26 ++++++++++++++++++-------- 1 files changed, 18 insertions(+), 8 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index b66b34a..ec926fc 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -352,11 +352,22 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset) return 0; } +static int strbuf_check_tag_ref(struct strbuf *sb, const char *name) +{ + if (name[0] == '-') + return CHECK_REF_FORMAT_ERROR; + + strbuf_reset(sb); + strbuf_addf(sb, "refs/tags/%s", name); + + return check_ref_format(sb->buf); +} + int cmd_tag(int argc, const char **argv, const char *prefix) { struct strbuf buf = STRBUF_INIT; + struct strbuf ref = STRBUF_INIT; unsigned char object[20], prev[20]; - char ref[PATH_MAX]; const char *object_ref, *tag; struct ref_lock *lock; @@ -452,12 +463,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix) if (get_sha1(object_ref, object)) die(_("Failed to resolve '%s' as a valid ref."), object_ref); - if (snprintf(ref, sizeof(ref), "refs/tags/%s", tag) > sizeof(ref) - 1) - die(_("tag name too long: %.*s..."), 50, tag); - if (check_ref_format(ref)) + if (strbuf_check_tag_ref(&ref, tag)) die(_("'%s' is not a valid tag name."), tag); - if (!resolve_ref(ref, prev, 1, NULL)) + if (!resolve_ref(ref.buf, prev, 1, NULL)) hashclr(prev); else if (!force) die(_("tag '%s' already exists"), tag); @@ -466,14 +475,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix) create_tag(object, tag, &buf, msg.given || msgfile, sign, prev, object); - lock = lock_any_ref_for_update(ref, prev, 0); + lock = lock_any_ref_for_update(ref.buf, prev, 0); if (!lock) - die(_("%s: cannot lock the ref"), ref); + die(_("%s: cannot lock the ref"), ref.buf); if (write_ref_sha1(lock, object, NULL) < 0) - die(_("%s: cannot update the ref"), ref); + die(_("%s: cannot update the ref"), ref.buf); if (force && hashcmp(prev, object)) printf(_("Updated tag '%s' (was %s)\n"), tag, find_unique_abbrev(prev, DEFAULT_ABBREV)); strbuf_release(&buf); + strbuf_release(&ref); return 0; } -- 1.7.5.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
[parent not found: <BANLkTik7PYjGMMfxaNPubYR7M1OgBrF_qw@mail.gmail.com>]
* Re: [RFC/PATCH v2] tag: disallow '-' as tag name [not found] ` <BANLkTik7PYjGMMfxaNPubYR7M1OgBrF_qw@mail.gmail.com> @ 2011-05-10 9:47 ` Michael Schubert 0 siblings, 0 replies; 7+ messages in thread From: Michael Schubert @ 2011-05-10 9:47 UTC (permalink / raw) To: Sverre Rabbelier; +Cc: Junio C Hamano, Alex Vandiver, git On 05/10/2011 09:07 AM, Sverre Rabbelier wrote: >> Disallow '-' as tag name just as any tag name starting with '-' to be >> consistent with "git checkout". > > This was hard for me to parse, how about:: > > Disallow '-' as tag name, as well as tag names starting with '-', to > be consistent with "git checkout". Thanks. -- >8 -- Subject: [PATCH] tag: disallow '-' as tag name Disallow '-' as tag name, as well as tag names starting with '-', to be consistent with "git checkout". Add strbuf_check_tag_ref() as helper to check a refname for a tag. Signed-off-by: Michael Schubert <mschub@elegosoft.com> --- builtin/tag.c | 26 ++++++++++++++++++-------- 1 files changed, 18 insertions(+), 8 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index b66b34a..ec926fc 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -352,11 +352,22 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset) return 0; } +static int strbuf_check_tag_ref(struct strbuf *sb, const char *name) +{ + if (name[0] == '-') + return CHECK_REF_FORMAT_ERROR; + + strbuf_reset(sb); + strbuf_addf(sb, "refs/tags/%s", name); + + return check_ref_format(sb->buf); +} + int cmd_tag(int argc, const char **argv, const char *prefix) { struct strbuf buf = STRBUF_INIT; + struct strbuf ref = STRBUF_INIT; unsigned char object[20], prev[20]; - char ref[PATH_MAX]; const char *object_ref, *tag; struct ref_lock *lock; @@ -452,12 +463,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix) if (get_sha1(object_ref, object)) die(_("Failed to resolve '%s' as a valid ref."), object_ref); - if (snprintf(ref, sizeof(ref), "refs/tags/%s", tag) > sizeof(ref) - 1) - die(_("tag name too long: %.*s..."), 50, tag); - if (check_ref_format(ref)) + if (strbuf_check_tag_ref(&ref, tag)) die(_("'%s' is not a valid tag name."), tag); - if (!resolve_ref(ref, prev, 1, NULL)) + if (!resolve_ref(ref.buf, prev, 1, NULL)) hashclr(prev); else if (!force) die(_("tag '%s' already exists"), tag); @@ -466,14 +475,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix) create_tag(object, tag, &buf, msg.given || msgfile, sign, prev, object); - lock = lock_any_ref_for_update(ref, prev, 0); + lock = lock_any_ref_for_update(ref.buf, prev, 0); if (!lock) - die(_("%s: cannot lock the ref"), ref); + die(_("%s: cannot lock the ref"), ref.buf); if (write_ref_sha1(lock, object, NULL) < 0) - die(_("%s: cannot update the ref"), ref); + die(_("%s: cannot update the ref"), ref.buf); if (force && hashcmp(prev, object)) printf(_("Updated tag '%s' (was %s)\n"), tag, find_unique_abbrev(prev, DEFAULT_ABBREV)); strbuf_release(&buf); + strbuf_release(&ref); return 0; } -- 1.7.5.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: Tags named '-' 2011-05-09 15:21 Tags named '-' Alex Vandiver 2011-05-09 18:01 ` Junio C Hamano @ 2011-05-09 22:04 ` Sverre Rabbelier 1 sibling, 0 replies; 7+ messages in thread From: Sverre Rabbelier @ 2011-05-09 22:04 UTC (permalink / raw) To: Alex Vandiver, Junio C Hamano; +Cc: git Heya, On Mon, May 9, 2011 at 17:21, Alex Vandiver <alex@chmrr.net> wrote: > The functionality of `git checkout -` is very useful, and I am hopeful > that `git merge -` will eventually land, as it matches my workflow well > as well. Speaking of which, my cursory check of 'man git-checkout' did not give me any information on 'git checkout -'. ISTR it lets you check out the branch you were previously on, but I'm not sure. Is it undocumented, or am I missing it (entirely possible, searching for an option named '-' is somewhat difficult). -- Cheers, Sverre Rabbelier ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-05-10 9:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-09 15:21 Tags named '-' Alex Vandiver
2011-05-09 18:01 ` Junio C Hamano
2011-05-09 22:56 ` [RFC/PATCH] tag: disallow '-' as tag name Michael Schubert
2011-05-09 23:08 ` Junio C Hamano
2011-05-09 23:36 ` [RFC/PATCH v2] " Michael Schubert
[not found] ` <BANLkTik7PYjGMMfxaNPubYR7M1OgBrF_qw@mail.gmail.com>
2011-05-10 9:47 ` Michael Schubert
2011-05-09 22:04 ` Tags named '-' Sverre Rabbelier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).