On Mon, Jun 23, 2014 at 11:50:06AM -0700, Junio C Hamano wrote: > 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.
Yeah, the strstr seems very wrong there. Even with the "/", why would you want to match "refs/heads/refs/tags/"? > 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. I think it is for "git clone --branch=v1.0". We create a refspec pulling v1.0 to its local tag in that case (as opposed to to something in "refs/remotes/origin/"). So I really think this does want to be starts_with. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html