Re: [PATCH 04/16] refactor skip_prefix to return a boolean
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:01:44
Jeff King [off-list ref] writes:
quoted hunk
diff --git a/builtin/clone.c b/builtin/clone.c index b12989d..a5b2d9d 100644 --- a/builtin/clone.c +++ b/builtin/clone.c@@ -703,9 +703,12 @@ static void write_refspec_config(const char* src_ref_prefix, strbuf_addf(&value, "+%s:%s%s", our_head_points_at->name, branch_top->buf, option_branch); } else if (remote_head_points_at) { + const char *head = remote_head_points_at->name; + if (!skip_prefix(head, "refs/heads/", &head)) + die("BUG: remote HEAD points at non-head?"); + strbuf_addf(&value, "+%s:%s%s", remote_head_points_at->name, - branch_top->buf, - skip_prefix(remote_head_points_at->name, "refs/heads/")); + branch_top->buf, head); } /* * otherwise, the next "git fetch" will
I was re-reading this and noticed another possible bug. builtin/clone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index b12989d..df659dd 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c@@ -696,7 +696,7 @@ static void write_refspec_config(const char* src_ref_prefix, if (option_mirror || !option_bare) { if (option_single_branch && !option_mirror) { if (option_branch) { - if (strstr(our_head_points_at->name, "refs/tags/")) + if (starts_with(our_head_points_at->name, "refs/tags/")) strbuf_addf(&value, "+%s:%s", our_head_points_at->name, our_head_points_at->name); else
Because the pattern is not anchored to the left with a slash, it is clear that the original cannot even claim that it was trying to munge "foo/refs/tags/" as well. Which means this is trivially correct, but at the same time I wonder what it means for our-head to point at a ref in refs/tags/ hierarchy.