All of lore.kernel.org
 help / color / mirror / Atom feed
* creation of empty branches
@ 2012-11-14 10:10 Angelo Borsotti
  2012-11-14 12:54 ` Andrew Ardill
  0 siblings, 1 reply; 5+ messages in thread
From: Angelo Borsotti @ 2012-11-14 10:10 UTC (permalink / raw)
  To: git

Hi,

the man page of git checkout does not describe the behavior of
git-checkout when asked to create empty branches. E.g.:

$ git init myrepo
$ cd myrepo
$ git checkout -b newbranch

the last command actually changes only the HEAD. It displays no output
telling the user that no switch to a new branch is done. Moreover, it
can be entered again without receiving any error message (unlike the
creation ot non-empty branches, which is instead rejected).
I would suggest to add to the DESCRIPTION, after the paragraph: "If -b
is given ...":

    "If the repository does not contain any branch, no new branch is
created, but the HEAD is set to refer to it."

Moreover, it is often reported (e.g. in the progit book) that git
checkout -b is equivalent to git branch; git checkout, and this is
true when nonempty branches are created, but it is not when the
repository is empty:

$ git init myrepo
$ cd myrepo
$ git branch master
fatal: Not a valid object name: 'master'.

however:
$ git checkout -b master
.... no error

This seems quite strange and difficult to understand: why should git
branch master issue an error while git checkout does not? I have the
impression that also git branch should not issue an error in this
case.

-Angelo Borsotti

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: creation of empty branches
  2012-11-14 10:10 creation of empty branches Angelo Borsotti
@ 2012-11-14 12:54 ` Andrew Ardill
  2012-11-14 21:27   ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Ardill @ 2012-11-14 12:54 UTC (permalink / raw)
  To: Angelo Borsotti; +Cc: git

On 14 November 2012 21:10, Angelo Borsotti <angelo.borsotti@gmail.com> wrote:
> ... why should git
> branch master issue an error while git checkout does not? I have the
> impression that also git branch should not issue an error in this
> case.
>

Just to help a little, let's first define:
- An empty file in refs/heads is a broken head.
- A non-existent file in refs/heads is an empty branch. Most
references to empty branches are probably mistakes.
- If HEAD points to an empty branch it is in 'root commit' or 'orphan'
mode. A commit made in such a mode first creates the root or orphan
commit object, and then creates the branch head in refs/heads pointing
to that object.

As I understand it, git branch and git checkout without a start point
defined are both intended to create a new branch and point it to the
current HEAD. Checkout will additionally reset HEAD to point to the
new branch, rather than whatever it was pointing to before (which will
normally be either a direct or indirect reference to a commit object).
The problem is that the behaviour when HEAD points to empty branch is
undefined, and this situation is seen to occur when there are no
commit objects at all, in an empty repository. This will also happen
when a branch has been checked out in orphan mode.

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'.

git checkout -b changes the current branch, and so it does make sense
to allow renaming an empty branch, which is the current behaviour.
However, note that currently when HEAD points to an empty branch, 'git
checkout -b foo HEAD' fails because HEAD is an invalid reference.
Obviously some special logic has been added or a very odd bug has
appeared. Perhaps it is most useful to continue to error out on any
reference explicitly listed on the command line as the start point
that points to an empty branch, unless it is pointed to by HEAD. Thus
we would only make HEAD point to a new empty branch when the start
point is omitted or when it matches the current empty branch HEAD
points to.

It would be useful to extend this renaming of empty branches to the
branch commands, and 'git branch -m' is a perfect fit for this from a
user perspective.


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.
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

When checking out a new branch without specifying a start point,
  if HEAD points to an empty branch then HEAD should be pointed to the
new branch, which will also be empty.
When checking out 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, HEAD should be pointed to the new, empty branch
  otherwise error due to invalid ref

When moving to a new branch without specifying an old branch,
  if HEAD points to an empty branch then HEAD should be pointed to the
new branch, which will also be empty.
When moving to a new branch whilst specifying an empty branch as the old branch,
  if HEAD points to the same empty branch that is listed as the old
branch, HEAD should be pointed to the new, empty branch
  otherwise error due to invalid ref

Note that since HEAD points to an empty branch there should be no
conflicts with the working directory or the index, so leave them
unchanged.

Some examples that might help:

~$ git init test
~$ cd test
~/test (master)$ git branch foo
fatal: Cannot create branch 'foo' from empty branch 'master'. To
rename 'master' use 'git branch -m master foo'.
~/test (master)$ git checkout -b foo
~/test (foo)$ git checkout -b bar fo
fatal: Not a valid object name: 'fo'.
~/test (foo)$ git checkout -b bar foo
~/test (bar)$ git branch -m foo
~/test (foo)$ git branch -m fo bar
error: refname refs/heads/fo not found
fatal: Branch rename failed
~/test (foo)$ git branch -m foo bar
~/test (bar)$



I wouldn't mind trying to code this up at the moment, but as I don't
have heaps of time if someone else feels like it go ahead (assuming
it's a good suggestion of course!).

Regards,

Andrew Ardill

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: creation of empty branches
  2012-11-14 12:54 ` Andrew Ardill
@ 2012-11-14 21:27   ` Junio C Hamano
  2012-11-15  0:04     ` Andrew Ardill
  2012-11-15 17:12     ` Jeff King
  0 siblings, 2 replies; 5+ messages in thread
From: Junio C Hamano @ 2012-11-14 21:27 UTC (permalink / raw)
  To: Andrew Ardill; +Cc: Angelo Borsotti, git

Andrew Ardill <andrew.ardill@gmail.com> 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).

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: creation of empty branches
  2012-11-14 21:27   ` Junio C Hamano
@ 2012-11-15  0:04     ` Andrew Ardill
  2012-11-15 17:12     ` Jeff King
  1 sibling, 0 replies; 5+ messages in thread
From: Andrew Ardill @ 2012-11-15  0:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Angelo Borsotti, git

On 15 November 2012 08:27, Junio C Hamano <gitster@pobox.com> 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.

>> 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.

>
>> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: creation of empty branches
  2012-11-14 21:27   ` Junio C Hamano
  2012-11-15  0:04     ` Andrew Ardill
@ 2012-11-15 17:12     ` Jeff King
  1 sibling, 0 replies; 5+ messages in thread
From: Jeff King @ 2012-11-15 17:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andrew Ardill, Angelo Borsotti, git

On Wed, Nov 14, 2012 at 01:27:33PM -0800, Junio C Hamano wrote:

> > 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 <peff@peff.net>
---
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;
 }
 

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-11-15 17:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-14 10:10 creation of empty branches Angelo Borsotti
2012-11-14 12:54 ` Andrew Ardill
2012-11-14 21:27   ` Junio C Hamano
2012-11-15  0:04     ` Andrew Ardill
2012-11-15 17:12     ` Jeff King

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.