git checkout -b origin/mybranch origin/mybranch

>   One of my collegues did:
> 
> git checkout origin/somebranch
> 
>   git complained that they need to specify the name with -b.  So they did:
> 
> git checkout -b origin/somebranch origin/somebranch

> Hi,
>
> On Thu, 12 Mar 2009, John Tapsell wrote:
>
>>   One of my collegues did:
>>
>> git checkout origin/somebranch
>>
>>   git complained that they need to specify the name with -b.  So they did:
>>
>> git checkout -b origin/somebranch origin/somebranch
>
> Yeah, a pilot error.  It should have been
>
>        $ git checkout -t origin/somebranch

>>   Git accepts this with no problems, but boy - all hell broke loose.
>> Doing a push or pull gave errors, because "origin/somebranch" is now
>> ambigous (since there is two of them).
>
> I strongly doubt that it gave errors, but rather warnings.  I have a
> repository where I get warnings all the time (it has a cvsimport and an
> origin remote), but it works without problems.
>
>>  They can't even:  "git checkout -b somebranch origin/somebranch"
>> anymore, since "origin/somebranch" is ambigous.  It all got into a mess.
>
> I wonder why you did not just "git branch -m somebranch".

> 2009/3/12 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>
> > On Thu, 12 Mar 2009, John Tapsell wrote:
> >
> >>   One of my collegues did:
> >>
> >> git checkout origin/somebranch
> >>
> >>   git complained that they need to specify the name with -b.  So they did:
> >>
> >> git checkout -b origin/somebranch origin/somebranch
> >
> > Yeah, a pilot error.  It should have been
> >
> >        $ git checkout -t origin/somebranch
> 
> Maybe the error message for "git checkout origin/somebranch"  should
> suggest:  git checkout -t origin/somebranch?
> 
> > I have to wonder, though, why "git checkout origin/somebranch" did not
> > detach your HEAD.
> 
> It did.  But that doesn't affect doing "git checkout -b
> origin/somebranch origin/somebranch"  afterwards.

> Hi,
>
> On Thu, 12 Mar 2009, John Tapsell wrote:
>
>> 2009/3/12 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>>
>> > On Thu, 12 Mar 2009, John Tapsell wrote:
>> >
>> >>   One of my collegues did:
>> >>
>> >> git checkout origin/somebranch
>> >>
>> >>   git complained that they need to specify the name with -b.  So they did:
>> >>
>> >> git checkout -b origin/somebranch origin/somebranch
>> >
>> > Yeah, a pilot error.  It should have been
>> >
>> >        $ git checkout -t origin/somebranch
>>
>> Maybe the error message for "git checkout origin/somebranch"  should
>> suggest:  git checkout -t origin/somebranch?
>>
>> > I have to wonder, though, why "git checkout origin/somebranch" did not
>> > detach your HEAD.
>>
>> It did.  But that doesn't affect doing "git checkout -b
>> origin/somebranch origin/somebranch"  afterwards.
>
> So did the warning read something like this?
>
> -- snip --
> moving to "d36a18dc9cdf1dfce8632e42491b826387aa3230" which isn't a local
> branch
> If you want to create a new branch from this checkout, you may do so
> (now or later) by using -b with the checkout command again. Example:
>  git checkout -b <new_branch_name>
> -- snap --
>
> ?

> Heya,
>
> On Thu, Mar 12, 2009 at 14:18, John Tapsell <johnflux@gmail.com> wrote:
>> Is probably a mistake by the user.  We should warn the user and point
>> them in the right direction.
>
> The point is that we _already_ warned the user (like Dscho pointed
> out), and that(as you pointed out), it didn't work :P.

>
> --
> Cheers,
>
> Sverre Rabbelier
>

> 2009/3/12 Sverre Rabbelier <srabbelier@gmail.com>:
>> Heya,
>>
>> On Thu, Mar 12, 2009 at 14:18, John Tapsell <johnflux@gmail.com> wrote:
>>> Is probably a mistake by the user.  We should warn the user and point
>>> them in the right direction.
>>
>> The point is that we _already_ warned the user (like Dscho pointed
>> out), and that(as you pointed out), it didn't work :P.
> 
> Just doing:
> 
> git branch -b origin/master origin/master
> 
> gives no error or warning at all.

> You can also get this in other interactions, for example:
>
> 	$ git checkout -b origin/test HEAD
> 	$ git checkout -b origin/test master
>
> yes, these might be user errors, but I still think it's not OK to create a 
> new ref 'refs/heads/origin/test' if there's also a 'refs/ 
> remotes/origin/test' (as I've said a few months ago).

> On Thu, Mar 12, 2009 at 03:21:48PM +0000, Pieter de Bie wrote:
>
>> You can also get this in other interactions, for example:
>>
>>       $ git checkout -b origin/test HEAD
>>       $ git checkout -b origin/test master
>>
>> yes, these might be user errors, but I still think it's not OK to create a
>> new ref 'refs/heads/origin/test' if there's also a 'refs/
>> remotes/origin/test' (as I've said a few months ago).
>
> One thing that has been missing from this discussion (and I think you
> are getting to it here) is a concrete rule for "X is harmful, and Y is
> not". That is, how do we know when to warn, and then what do we do?
>
> John's original example was "git checkout -b origin/test origin/test".
> So it's a problem that they're textually the same, but obviously there
> are more problematic cases.
>
> The behavior I think you are implying would be something like:
>
>  When making origin/test, try to resolve_ref("origin/test"); if it
>  fails, we are OK. If it succeeds, then we know we will be creating an
>  ambiguity. Complain and refuse the creation unless "-f" is given.
>
> This would actually be a superset of the "branch already exists" case,
> so it should be pretty simple to code, and it makes for a simple rule:
> it is now "ref already exists".

> I was thinking more along the lines of checking if it begins with
> remotes/, origin/, tags/, stash/, bisect/    and blacklisting these.
> 
> Can anyone suggest a reason that you really might want to create a
> branch called  origin/something ?

>> I think the future-proofing is probably not worth the effort.
>> Catching things that are ambiguous _now_ will cover the "oops, I
>> typed the wrong thing" case, which I think is really the issue.
>
> Exactly, that's the common case where things go wrong. I guess using
> dwim_ref should be DWIM enough? :)

> On Thu, Mar 12, 2009 at 04:40:10PM +0000, Pieter de Bie wrote:
>
>>> I think the future-proofing is probably not worth the effort.
>>> Catching things that are ambiguous _now_ will cover the "oops, I
>>> typed the wrong thing" case, which I think is really the issue.
>>
>> Exactly, that's the common case where things go wrong. I guess using
>> dwim_ref should be DWIM enough? :)
>
> Hmm. Yeah, I mispoke before: I should have said dwim_ref instead of
> resolve_ref (which doesn't dwym :) ).
>
> Here's a sloppy patch that I think does what you want; but it might make
> more sense to just iterate over ref_rev_parse_rules ourselves, as
> dwim_ref does more than we care about (and we should probably
> differentiate between "a branch already exists" and "this would make an
> ambiguous ref").
>
> ---
> diff --git a/branch.c b/branch.c
> index d20fb04..409f445 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -122,6 +122,7 @@ void create_branch(const char *head,
>        unsigned char sha1[20];
>        char *real_ref, msg[PATH_MAX + 20];
>        struct strbuf ref = STRBUF_INIT;
> +       char *junk;
>        int forcing = 0;
>        int len;
>
> @@ -135,7 +136,8 @@ void create_branch(const char *head,
>        if (check_ref_format(ref.buf))
>                die("'%s' is not a valid branch name.", name);
>
> -       if (resolve_ref(ref.buf, sha1, 1, NULL)) {
> +       if (dwim_ref(name, strlen(name), sha1, &junk)) {
> +               free(junk);

> > -       if (resolve_ref(ref.buf, sha1, 1, NULL)) {
> > +       if (dwim_ref(name, strlen(name), sha1, &junk)) {
> > +               free(junk);
> 
> Presumably 'junk' is the resolved name?  I wonder if it's worth
> putting this info in the error message?

> On Thu, Mar 12, 2009 at 04:58:22PM +0000, John Tapsell wrote:
>
>> > -       if (resolve_ref(ref.buf, sha1, 1, NULL)) {
>> > +       if (dwim_ref(name, strlen(name), sha1, &junk)) {
>> > +               free(junk);
>>
>> Presumably 'junk' is the resolved name?  I wonder if it's worth
>> putting this info in the error message?
>
> Hey, I said it was sloppy, right? ;)
>
> Here's your suggestion, plus specifying which situation (existing branch
> or ambiguous ref) would occur. It would still need tests. But I'm
> curious to hear more opinions on this direction before cleaning it up
> much more (at the very least, it needs some tests).

> --- a/branch.c
> +++ b/branch.c
> @@ -133,6 +133,7 @@ void create_branch(const char *head,
>        unsigned char sha1[20];
>        char *real_ref, msg[PATH_MAX + 20];
>        struct strbuf ref = STRBUF_INIT;
> +       char *existing;

>        int forcing = 0;
>        int len;
>
> @@ -146,12 +147,18 @@ void create_branch(const char *head,
>        if (check_ref_format(ref.buf))
>                die("'%s' is not a valid branch name.", name);
>
> -       if (resolve_ref(ref.buf, sha1, 1, NULL)) {
> -               if (!force)
> -                       die("A branch named '%s' already exists.", name);
> +       if (dwim_ref(name, strlen(name), sha1, &existing)) {
> +               if (!force) {
> +                       if (!prefixcmp(existing, "refs/heads/"))
> +                               die("A branch named '%s' already exists.",
> +                                   name);
> +                       die("Creating '%s' would be ambiguous with"
> +                           " the existing %s", name, existing);

> +               }
>                else if (!is_bare_repository() && !strcmp(head, name))
>                        die("Cannot force update the current branch.");
>                forcing = 1;
> +               free(existing);
>        }
>
>        real_ref = NULL;
>

> On Thu, Mar 12, 2009 at 04:16:09PM +0000, John Tapsell wrote:
>
>> I was thinking more along the lines of checking if it begins with
>> remotes/, origin/, tags/, stash/, bisect/    and blacklisting these.
>>
>> Can anyone suggest a reason that you really might want to create a
>> branch called  origin/something ?
>
> The name "origin" is simply convention. So if you are thinking about
> blacklisting "origin/*", then it is certainly possible to have a false
> positive (although as you note, it is unlikely). But what is worse is
> that it is very likely for you to have a false negative if you use a
> different remote name (and people often do if they have multiple
> remotes).
>
> For example, in one of my projects where I do integration, "origin" is
> my own public repo, and I have a remote pointing to the public repo of a
> number of other developers from whom I pull. So I would encounter the
> same error by doing:
>
>  git checkout -b mike/master mike/master
>
> but it would not be caught by your rule.
>
> One area where your rule _is_ nicer than mine is that mine only looks at
> what exists _now_ and doesn't future-proof you at all. So I could say
>
>  git checkout -b origin/newtopic
>
> which might not be ambiguous. But if the remote adds a "newtopic"
> branch, then the next time I fetch it will _become_ ambiguous.
>
> Potentially you could blacklist "X/*" for every remote.X.* that
> exists in your config. Even that, of course, isn't future-proof against
> you creating a new remote. :)
>
> I think the future-proofing is probably not worth the effort. Catching
> things that are ambiguous _now_ will cover the "oops, I typed the wrong
> thing" case, which I think is really the issue.

> I'm saying that:
>
> git checkout -b origin/mybranch  origin/mybranch
>
> Is probably a mistake by the user.  We should warn the user and point
> them in the right direction.