* 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
* 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
* [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
* 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
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).