Andrew Ardill [off-list ref] writes:
Since git branch has the default behaviour to create a branch 'in the
background' it makes sense to fail when trying to create a new branch
this way from an empty branch. The error message should be improved to
handle this edge case in a nicer way. If we allow for renaming empty
branches (described below) then the message can be even more helpful.
Instead of
fatal: Not a valid object name: 'master'.
perhaps
fatal: Cannot create branch 'foo' from empty branch 'master'. To
rename 'master' use 'git branch -m master foo'.
The first new sentence is a definite improvement, but I do not think
the advice in the second sentence is necessarily a good idea,
because it is dubious that the user is likely to have wanted to
rename 'master' to something else. "git branch foo master" (or its
moral equivalent "git checkout -b foo" while on master) is a wish to
have a history that ends in 'foo' *forked* from history of 'master',
but because you do not even have anything on 'master' yet, you
cannot fork the history, as you explained earlier (snipped). In
that sense, 'empty branch' is a slight misnomer---as far as "git
branch foo master" is concerned, the 'master' branch does not yet
exist (and that is why we often call it an "unborn branch", not
"empty").
fatal: cannot fork master's history that does not exist yet.
would be more accurate description of the situation.
So explicitly, I am proposing the following behaviour changes:
When trying to create a new branch without specifying a start point,
if HEAD points to an empty branch, error with a more useful message
that assumes the user might want to rename the empty branch.
I do not think that is the right assumption in the first place. It
is very likely that the user does not yet know how Git works when
she attempts to fork from a history that does not exist. It is also
very likely that she is expecting, after "git branch foo master"
succeeds when 'master' is yet to be born, have two branches 'foo'
and 'master', so that "git checkout foo" and "git checkout master"
can be done subsequently.
But that expectation is wrong, and it would help the user in the
longer run to correct that expectation. "We assume you wanted to
rename 'master' to 'foo'" is a logical consequence that changing
HEAD that points at an unborn 'master' to point at an unborn 'foo'
is the best (or closest) thing the user can do, *if* the user
understands that the current branch being unborn is a special state
and there can only be one such unborn branch (that is, the current
one). The user who gets this error message, however, clearly does
not understand that, so it is not a logical consequence to her at
all. The advice does not help her, but instead invites "No, I did
not want to rename it, I wanted to have 'foo' without losing
'master'", leading to even more confusion.
When trying to create a new branch whilst specifying an empty branch
as the start point,
if HEAD points to the same empty branch that is listed as the start
point, error with a more useful message that assumes the user might
want to rename the empty branch.
otherwise error due to invalid ref
See above (for all the other cases, too).
On 15 November 2012 08:27, Junio C Hamano [off-list ref] wrote:
... "git branch foo master" (or its
moral equivalent "git checkout -b foo" while on master) is a wish to
have a history that ends in 'foo' *forked* from history of 'master',
but because you do not even have anything on 'master' yet, you
cannot fork the history, as you explained earlier (snipped). In
that sense, 'empty branch' is a slight misnomer---as far as "git
branch foo master" is concerned, the 'master' branch does not yet
exist (and that is why we often call it an "unborn branch", not
"empty").
fatal: cannot fork master's history that does not exist yet.
would be more accurate description of the situation.
So in the situation where a start-point is an unborn branch (ie it is
referenced in HEAD) we should modify the error to show that the unborn
branch is in fact unborn. This is a good improvement I think, as it
catches this edge case and improves understanding of what an unborn
branch is. The term 'fork' is not used at all in branch's man page, so
perhaps a more consistent error message is something like
fatal: 'master' has no history, so we cannot create a new branch from it
Also, currently 'git checkout -b' *will* rename an unborn branch which
is different from the 'git branch' behaviour. Not sure how this
behaviour can be corrected without causing a regression or introducing
potentially confusing error messages for the git branch case.
quoted
So explicitly, I am proposing the following behaviour changes:
When trying to create a new branch without specifying a start point,
if HEAD points to an empty branch, error with a more useful message
that assumes the user might want to rename the empty branch.
I do not think that is the right assumption in the first place. It
is very likely that the user does not yet know how Git works when
she attempts to fork from a history that does not exist. It is also
very likely that she is expecting, after "git branch foo master"
succeeds when 'master' is yet to be born, have two branches 'foo'
and 'master', so that "git checkout foo" and "git checkout master"
can be done subsequently.
But that expectation is wrong, and it would help the user in the
longer run to correct that expectation. "We assume you wanted to
rename 'master' to 'foo'" is a logical consequence that changing
HEAD that points at an unborn 'master' to point at an unborn 'foo'
is the best (or closest) thing the user can do, *if* the user
understands that the current branch being unborn is a special state
and there can only be one such unborn branch (that is, the current
one). The user who gets this error message, however, clearly does
not understand that, so it is not a logical consequence to her at
all. The advice does not help her, but instead invites "No, I did
not want to rename it, I wanted to have 'foo' without losing
'master'", leading to even more confusion.
I can't speak for others, but I think it is more likely that a new
user who doesn't understand git and tries to use the branch command on
a brand new repository is not trying to fork master's history, but is
rather trying to switch to a different branch. A subversion user might
want to switch from 'master' to 'trunk' as a (somewhat?) common use
case. That is the reasoning behind my recommendation to show how to
rename the branch. Furthermore, we can educate the user about unborn
branches and suggest a possible solution at the same time.
With the new error message from above, something like the following
might be nice.
fatal: 'master' has no history, so we cannot create a new branch from it
hint: A repository can only contain one unborn branch at a time.
hint: To rename a branch use 'git branch -m [<old-branch>] <new-branch>'
I'm not sure about the intended syntax of hint messages but this seems
consistent.
quoted
When trying to create a new branch whilst specifying an empty branch
as the start point,
if HEAD points to the same empty branch that is listed as the start
point, error with a more useful message that assumes the user might
want to rename the empty branch.
otherwise error due to invalid ref
See above (for all the other cases, too).
You didn't address the branch rename command 'git branch -m
[<old-branch>] <new-branch>' (or you included it with the others). I
think it is a significantly different action to creating a new branch,
and should handle unborn branches in the expected way, which is to
rename them. What are your thoughts on that?
Regards,
Andrew Ardill
On Wed, Nov 14, 2012 at 01:27:33PM -0800, Junio C Hamano wrote:
quoted
Instead of
fatal: Not a valid object name: 'master'.
perhaps
fatal: Cannot create branch 'foo' from empty branch 'master'. To
rename 'master' use 'git branch -m master foo'.
The first new sentence is a definite improvement, but I do not think
the advice in the second sentence is necessarily a good idea,
because it is dubious that the user is likely to have wanted to
rename 'master' to something else. "git branch foo master" (or its
moral equivalent "git checkout -b foo" while on master) is a wish to
have a history that ends in 'foo' *forked* from history of 'master',
but because you do not even have anything on 'master' yet, you
cannot fork the history, as you explained earlier (snipped). In
that sense, 'empty branch' is a slight misnomer---as far as "git
branch foo master" is concerned, the 'master' branch does not yet
exist (and that is why we often call it an "unborn branch", not
"empty").
fatal: cannot fork master's history that does not exist yet.
would be more accurate description of the situation.
I agree with most of your reasoning. I think simply using Andrew's first
sentence is a little more clear to a new user, even though it may be
less technically precise, but I don't have a strong opinion.
That still leaves "checkout -b". I do not see it as a problem that "git
branch foo" does not work whereas "git checkout -b foo" does; we
special-case the latter because it can do something sensible, whereas
there is nothing sensible for "git branch foo" to do. However, I think
it is missing one piece:
-- >8 --
Subject: checkout: print a message when switching unborn branches
When we switch to a new branch using checkout, we usually output a
message indicating what happened. However, when we switch from an unborn
branch to a new branch, we do not print anything, which may leave the
user wondering what happened.
The reason is that the unborn branch is a special case (see abe1998),
and does not follow the usual switch_branches code path. Let's add a
similar informational message to the special case to match the usual
code path.
Signed-off-by: Jeff King <redacted>
---
Two possible tweaks:
1. The message is the same as "git checkout -b" when we are actually
moving to a new branch. We could optionally mention that the branch
is empty or unborn.
2. We do not check whether the old unborn branch has the same name as
the new one. So you get "Switched to a new branch..." if you try to
checkout the same branch. We'd have to pass more information into
the special case to detect this. I don't know if we care.
builtin/checkout.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 781295b..a9c1b5a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -951,6 +951,9 @@ static int switch_unborn_to_new_branch(const struct checkout_opts *opts)
strbuf_addf(&branch_ref, "refs/heads/%s", opts->new_branch);
status = create_symref("HEAD", branch_ref.buf, "checkout -b");
strbuf_release(&branch_ref);
+ if (!opts->quiet)
+ fprintf(stderr, _("Switched to a new branch '%s'\n"),
+ opts->new_branch);
return status;
}