Junio C Hamano <gitster@xxxxxxxxx> writes:
> I think the right fix is to make sure that "refs/heads/<name>" does not
> exist by checking exactly that.
>
> Perhaps something like this. I am not sure if we want to use the "yield
> non zero to signal not an error but an early return" trick like I did in
> this patch, though.
Let's do this instead. I don't know what I was thinking when I wrote that
inefficient "loop refs to see if there is that one" patch.
-- >8 --
Subject: [PATCH] checkout -b <name>: correctly detect existing branch
When create a new branch, we fed "refs/heads/<proposed name>" as a string
to get_sha1() and expected it to fail when a branch already exists.
The right way to check if a ref exists is to check with resolve_ref().
A naÃve solution that might appear attractive but does not work is to
forbid slashes in get_describe_name() but that will not work. A describe
name is is in the form of "ANYTHING-g<short sha1>", and that ANYTHING part
comes from a original tag name used in the repository the user ran the
describe command. A sick user could have a confusing hierarchical tag
whose name is "refs/heads/foobar" (stored as refs/tags/refs/heads/foobar")
to generate a describe name "refs/heads/foobar-6-g02ac983", and we should
be able to use that name to refer to the object whose name is 02ac983.
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
builtin/checkout.c | 2 +-
refs.c | 6 ++++++
refs.h | 1 +
t/t2018-checkout-branch.sh | 11 +++++++++++
4 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 4ad7427..88708d4 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -874,7 +874,7 @@ no_reference:
if (strbuf_check_branch_ref(&buf, opts.new_branch))
die("git checkout: we do not like '%s' as a branch name.",
opts.new_branch);
- if (!get_sha1(buf.buf, rev)) {
+ if (ref_exists(buf.buf)) {
opts.branch_exists = 1;
if (!opts.new_branch_force)
die("git checkout: branch %s already exists",
diff --git a/refs.c b/refs.c
index 6f486ae..92cd0d1 100644
--- a/refs.c
+++ b/refs.c
@@ -1732,6 +1732,12 @@ int update_ref(const char *action, const char *refname,
return 0;
}
+int ref_exists(char *refname)
+{
+ unsigned char sha1[20];
+ return !!resolve_ref(refname, sha1, 1, NULL);
+}
+
struct ref *find_ref_by_name(const struct ref *list, const char *name)
{
for ( ; list; list = list->next)
diff --git a/refs.h b/refs.h
index 762ce50..070a7d9 100644
--- a/refs.h
+++ b/refs.h
@@ -46,6 +46,7 @@ extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refn
*/
extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags);
extern void clear_extra_refs(void);
+extern int ref_exists(char *);
extern int peel_ref(const char *, unsigned char *);
diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 1caffea..741d842 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -163,4 +163,15 @@ test_expect_success 'checkout -f -B to an existing branch with mergeable changes
test_must_fail test_dirty_mergeable
'
+test_expect_success 'checkout -b <describe>' '
+ git tag -f -m "First commit" initial initial &&
+ git checkout -f change1 &&
+ name=$(git describe) &&
+ git checkout -b $name &&
+ git diff --exit-code change1 &&
+ echo "refs/heads/$name" >expect &&
+ git symbolic-ref HEAD >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.7.6.rc0.106.g68174
--
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