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

Reply via email to