'HEAD' is not a commit (according to git-checkout)

'HEAD' is not a commit (according to git-checkout)

[Dana Dahlstrom]

What did you do before the bug happened? (Steps to reproduce your issue)

  $ git clone https://github.com/githubtraining/hellogitworld.git
  $ cd hellogitworld
  $ git checkout -b work -t master HEAD
  fatal: 'HEAD' is not a commit and a branch 'work' cannot be created from it
  $ git show -s --oneline
  ef7bebf (HEAD -> master, origin/master, origin/HEAD) Fix groupId […]
  $ git checkout -b work -t master ef7bebf
  fatal: 'ef7bebf' is not a commit and a branch 'work' cannot be created from it

What did you expect to happen? (Expected behavior)

  I expected a new branch named 'work' to be created and checked out,
  pointing to commit ef7bebf and with upstream branch set to 'master'.

What happened instead? (Actual behavior)

  I saw these erroneous messages (copied from above):

  fatal: 'HEAD' is not a commit and a branch 'work' cannot be created from it
  fatal: 'ef7bebf' is not a commit and a branch 'work' cannot be created from it

What's different between what you expected and what actually happened?

  I expected a new branch but instead saw erroneous messages.

Anything else you want to add:

  This question seems to show the same problem:
  stackoverflow.com/questions/48671851


[System Info]
git version:
git version 2.27.0.rc0.183.gde8f92d652-goog
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
uname: Linux 5.2.17-1rodete3-amd64 #1 SMP Debian 5.2.17-1rodete3
(2019-10-21 > 2018) x86_64
compiler info: gnuc: 8.3
libc info: glibc: 2.29

Re: 'HEAD' is not a commit (according to git-checkout)

[Jeff King]

On Thu, May 21, 2020 at 12:00:00PM -0700, Dana Dahlstrom wrote:

> What did you do before the bug happened? (Steps to reproduce your issue)
> 
>   $ git clone https://github.com/githubtraining/hellogitworld.git
>   $ cd hellogitworld
>   $ git checkout -b work -t master HEAD
>   fatal: 'HEAD' is not a commit and a branch 'work' cannot be created from it
>   $ git show -s --oneline
>   ef7bebf (HEAD -> master, origin/master, origin/HEAD) Fix groupId […]
>   $ git checkout -b work -t master ef7bebf
>   fatal: 'ef7bebf' is not a commit and a branch 'work' cannot be created from it

Thanks for a complete reproduction. There are a few things going on
here.

The "-t" option doesn't take an argument; it's a boolean that says
"track the branch we started from". So "master" is taken as the
start-point, and "HEAD" is tacked onto the end. I.e., equivalent to:

  git checkout -b work master HEAD

That error message is wrong and misleading. It looks like what happens
is that we parse "master" as the start-point. And then we try to treat
remaining options (i.e., "HEAD") as a pathspec. That fails, because
there's no such path. But then instead of saying "hey, HEAD isn't a
pathspec" we try to be clever:

                  /*
                   * Try to give more helpful suggestion.
                   * new_branch && argc > 1 will be caught later.
                   */
                  if (opts->new_branch && argc == 1)
                          die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
                                  argv[0], opts->new_branch);

We know we're making a new branch and there's one argument, so we assume
that it didn't get parsed earlier as a start-point. But that misses the
fact that if we _did_ parse a start-point, it would have been removed
from argv, and our "single argument" is actually the former
second-argument.

Something like this works:

diff --git a/builtin/checkout.c b/builtin/checkout.c
index e9d111bb83..6559ac666b 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1553,6 +1553,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 {
 	struct branch_info new_branch_info;
 	int parseopt_flags = 0;
+	int got_start_point = 0;
 
 	memset(&new_branch_info, 0, sizeof(new_branch_info));
 	opts->overwrite_ignore = 1;
@@ -1661,6 +1662,8 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 			!opts->new_branch;
 		int n = parse_branchname_arg(argc, argv, dwim_ok,
 					     &new_branch_info, opts, &rev);
+		if (n)
+			got_start_point = 1;
 		argv += n;
 		argc -= n;
 	} else if (!opts->accept_ref && opts->from_treeish) {
@@ -1689,7 +1692,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 		 * Try to give more helpful suggestion.
 		 * new_branch && argc > 1 will be caught later.
 		 */
-		if (opts->new_branch && argc == 1)
+		if (opts->new_branch && !got_start_point && argc == 1)
 			die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
 				argv[0], opts->new_branch);
 

to produce:

  $ git checkout -b work -t master HEAD
  fatal: '--track' cannot be used with updating paths

  $ git checkout -b work master HEAD
  fatal: Cannot update paths and switch to branch 'work' at the same time.

which are both correct. I wonder if there's a more elegant way to do it,
though (probably not, as there's definitely some heuristic parsing going
on to determine which checkout mode we're in; the new switch/restore
alternatives don't suffer as much from this).

> What did you expect to happen? (Expected behavior)
> 
>   I expected a new branch named 'work' to be created and checked out,
>   pointing to commit ef7bebf and with upstream branch set to 'master'.

So getting back to your actual goal: you can't do what you want with a
single checkout command. I think:

  git checkout -b work HEAD
  git branch --set-upstream-to=master

-Peff

Re: 'HEAD' is not a commit (according to git-checkout)

[René Scharfe]

Am 21.05.20 um 21:00 schrieb Dana Dahlstrom:
> What did you do before the bug happened? (Steps to reproduce your issue)
>
>   $ git clone https://github.com/githubtraining/hellogitworld.git
>   $ cd hellogitworld
>   $ git checkout -b work -t master HEAD
>   fatal: 'HEAD' is not a commit and a branch 'work' cannot be created from it
>   $ git show -s --oneline
>   ef7bebf (HEAD -> master, origin/master, origin/HEAD) Fix groupId […]
>   $ git checkout -b work -t master ef7bebf
>   fatal: 'ef7bebf' is not a commit and a branch 'work' cannot be created from it
>
> What did you expect to happen? (Expected behavior)
>
>   I expected a new branch named 'work' to be created and checked out,
>   pointing to commit ef7bebf and with upstream branch set to 'master'.

The option -t/--track doesn't take an argument.  Try this:

	git checkout -b work -t master

... or perhaps this if you want to track the upstream branch instead of
the local one:

	git checkout -b work -t origin/master

These days I'd use this, though:

	git switch --create=work --track origin/master

> What happened instead? (Actual behavior)
>
>   I saw these erroneous messages (copied from above):
>
>   fatal: 'HEAD' is not a commit and a branch 'work' cannot be created from it
>   fatal: 'ef7bebf' is not a commit and a branch 'work' cannot be created from it

This message is puzzling, but what it means is that the extra "HEAD" and
"ef7bebf" you gave are not being interpreted as a commit to use as the
start point of the new branch and git checkout just doesn't know what to
do with them.

The message used to read, which was a bit worse:

  fatal: Cannot update paths and switch to branch 'work' at the same time.
  Did you intend to checkout 'HEAD' which can not be resolved as commit?

If you pass extra arguments to git switch it reports, which is better:

  fatal: only one reference expected

René

Re: 'HEAD' is not a commit (according to git-checkout)

[René Scharfe]

Am 21.05.20 um 21:16 schrieb Jeff King:
> Something like this works:
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index e9d111bb83..6559ac666b 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1553,6 +1553,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
>  {
>  	struct branch_info new_branch_info;
>  	int parseopt_flags = 0;
> +	int got_start_point = 0;
>
>  	memset(&new_branch_info, 0, sizeof(new_branch_info));
>  	opts->overwrite_ignore = 1;
> @@ -1661,6 +1662,8 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
>  			!opts->new_branch;
>  		int n = parse_branchname_arg(argc, argv, dwim_ok,
>  					     &new_branch_info, opts, &rev);
> +		if (n)
> +			got_start_point = 1;
>  		argv += n;
>  		argc -= n;
>  	} else if (!opts->accept_ref && opts->from_treeish) {
> @@ -1689,7 +1692,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
>  		 * Try to give more helpful suggestion.
>  		 * new_branch && argc > 1 will be caught later.
>  		 */
> -		if (opts->new_branch && argc == 1)
> +		if (opts->new_branch && !got_start_point && argc == 1)
>  			die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
>  				argv[0], opts->new_branch);
>
>
> to produce:
>
>   $ git checkout -b work -t master HEAD
>   fatal: '--track' cannot be used with updating paths
>
>   $ git checkout -b work master HEAD
>   fatal: Cannot update paths and switch to branch 'work' at the same time.
>
> which are both correct. I wonder if there's a more elegant way to do it,
> though (probably not, as there's definitely some heuristic parsing going
> on to determine which checkout mode we're in; the new switch/restore
> alternatives don't suffer as much from this).

Perhaps:

diff --git a/builtin/checkout.c b/builtin/checkout.c
index e9d111bb83..24336e1017 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1689,7 +1689,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 		 * Try to give more helpful suggestion.
 		 * new_branch && argc > 1 will be caught later.
 		 */
-		if (opts->new_branch && argc == 1)
+		if (opts->new_branch && argc == 1 && !new_branch_info.commit)
 			die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
 				argv[0], opts->new_branch);

Re: 'HEAD' is not a commit (according to git-checkout)

[Jeff King]

On Sat, May 23, 2020 at 09:07:50AM +0200, René Scharfe wrote:

> > which are both correct. I wonder if there's a more elegant way to do it,
> > though (probably not, as there's definitely some heuristic parsing going
> > on to determine which checkout mode we're in; the new switch/restore
> > alternatives don't suffer as much from this).
> 
> Perhaps:
> 
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index e9d111bb83..24336e1017 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1689,7 +1689,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
>  		 * Try to give more helpful suggestion.
>  		 * new_branch && argc > 1 will be caught later.
>  		 */
> -		if (opts->new_branch && argc == 1)
> +		if (opts->new_branch && argc == 1 && !new_branch_info.commit)
>  			die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
>  				argv[0], opts->new_branch);
> 

Oh, indeed, that's way better. Want to wrap it up as a patch?

-Peff

[PATCH 1/2] checkout: add tests for -b and --track

[René Scharfe]

Test git checkout -b with and without --track and demonstrate unexpected
error messages when it's given an extra (i.e. unsupported) path
argument.  In both cases it reports:

   $ git checkout -b foo origin/master bar
   fatal: 'bar' is not a commit and a branch 'foo' cannot be created from it

The problem is that the start point we gave for the new branch is
"origin/master" and "bar" is just some extra argument -- it could even
be a valid commit, which would make the message even more confusing.  We
have more fitting error messages in git commit, but get confused; use
the text of the rights ones in the tests.

Reported-by: Dana Dahlstrom <dahlstrom@google.com>
Original-test-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
 t/t2018-checkout-branch.sh | 10 ++++++++++
 t/t2027-checkout-track.sh  | 24 ++++++++++++++++++++++++
 2 files changed, 34 insertions(+)
 create mode 100755 t/t2027-checkout-track.sh

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 21583154d8..d99b699396 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -260,4 +260,14 @@ test_expect_success 'checkout -b to a new branch preserves mergeable changes des
 	test_cmp expect actual
 '

+test_expect_success 'checkout -b rejects an invalid start point' '
+	test_must_fail git checkout -b branch4 file1 2>err &&
+	test_i18ngrep "is not a commit" err
+'
+
+test_expect_failure 'checkout -b rejects an extra path argument' '
+	test_must_fail git checkout -b branch5 branch1 file1 2>err &&
+	test_i18ngrep "Cannot update paths and switch to branch" err
+'
+
 test_done
diff --git a/t/t2027-checkout-track.sh b/t/t2027-checkout-track.sh
new file mode 100755
index 0000000000..d0b41d7cd0
--- /dev/null
+++ b/t/t2027-checkout-track.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+test_description='tests for git branch --track'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit one &&
+	test_commit two
+'
+
+test_expect_success 'checkout --track -b creates a new tracking branch' '
+	git checkout --track -b branch1 master &&
+	test $(git rev-parse --abbrev-ref HEAD) = branch1 &&
+	test $(git config --get branch.branch1.remote) = . &&
+	test $(git config --get branch.branch1.merge) = refs/heads/master
+'
+
+test_expect_failure 'checkout --track -b rejects an extra path argument' '
+	test_must_fail git checkout --track -b branch2 master one.t 2>err &&
+	test_i18ngrep "cannot be used with updating paths" err
+'
+
+test_done
--
2.26.2

[PATCH 2/2] checkout: improve error messages for -b with extra argument

[René Scharfe]

When we try to create a branch "foo" based on "origin/master" and give
git commit -b an extra unsupported argument "bar", it confusingly
reports:

   $ git checkout -b foo origin/master bar
   fatal: 'bar' is not a commit and a branch 'foo' cannot be created from it

   $ git checkout --track -b foo origin/master bar
   fatal: 'bar' is not a commit and a branch 'foo' cannot be created from it

That's wrong, because it very well understands that "origin/master" is
supposed to be the start point for the new branch and not "bar".  Check
if we got a commit and show more fitting messages in that case instead:

   $ git checkout -b foo origin/master bar
   fatal: Cannot update paths and switch to branch 'foo' at the same time.

   $ git checkout --track -b foo origin/master bar
   fatal: '--track' cannot be used with updating paths

Original-patch-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/checkout.c         | 2 +-
 t/t2018-checkout-branch.sh | 2 +-
 t/t2027-checkout-track.sh  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index e9d111bb83..24336e1017 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1689,7 +1689,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 		 * Try to give more helpful suggestion.
 		 * new_branch && argc > 1 will be caught later.
 		 */
-		if (opts->new_branch && argc == 1)
+		if (opts->new_branch && argc == 1 && !new_branch_info.commit)
 			die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
 				argv[0], opts->new_branch);

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index d99b699396..5b8c042997 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -265,7 +265,7 @@ test_expect_success 'checkout -b rejects an invalid start point' '
 	test_i18ngrep "is not a commit" err
 '

-test_expect_failure 'checkout -b rejects an extra path argument' '
+test_expect_success 'checkout -b rejects an extra path argument' '
 	test_must_fail git checkout -b branch4 branch1 file1 2>err &&
 	test_i18ngrep "Cannot update paths and switch to branch" err
 '
diff --git a/t/t2027-checkout-track.sh b/t/t2027-checkout-track.sh
index d0b41d7cd0..bcba1bf90c 100755
--- a/t/t2027-checkout-track.sh
+++ b/t/t2027-checkout-track.sh
@@ -16,7 +16,7 @@ test_expect_success 'checkout --track -b creates a new tracking branch' '
 	test $(git config --get branch.branch1.merge) = refs/heads/master
 '

-test_expect_failure 'checkout --track -b rejects an extra path argument' '
+test_expect_success 'checkout --track -b rejects an extra path argument' '
 	test_must_fail git checkout --track -b branch2 master one.t 2>err &&
 	test_i18ngrep "cannot be used with updating paths" err
 '
--
2.26.2

Re: 'HEAD' is not a commit (according to git-checkout)

[René Scharfe]

Am 23.05.20 um 18:29 schrieb Jeff King:
> On Sat, May 23, 2020 at 09:07:50AM +0200, René Scharfe wrote:
>
>>> which are both correct. I wonder if there's a more elegant way to do it,
>>> though (probably not, as there's definitely some heuristic parsing going
>>> on to determine which checkout mode we're in; the new switch/restore
>>> alternatives don't suffer as much from this).
>>
>> Perhaps:
>>
>> diff --git a/builtin/checkout.c b/builtin/checkout.c
>> index e9d111bb83..24336e1017 100644
>> --- a/builtin/checkout.c
>> +++ b/builtin/checkout.c
>> @@ -1689,7 +1689,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
>>  		 * Try to give more helpful suggestion.
>>  		 * new_branch && argc > 1 will be caught later.
>>  		 */
>> -		if (opts->new_branch && argc == 1)
>> +		if (opts->new_branch && argc == 1 && !new_branch_info.commit)
>>  			die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
>>  				argv[0], opts->new_branch);
>>
>
> Oh, indeed, that's way better. Want to wrap it up as a patch?

OK, but stepping back a bit and trying to forget what I know about the
option --track and pretending to see it for the first time, I have to
ask: Why doesn't it take an argument?  If I check out a raw commit, it
cannot guess the upstream branch anyway.  So I'd assume this to work:

   git checkout -b new-branch --track=upstream start-point

René

Re: 'HEAD' is not a commit (according to git-checkout)

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> OK, but stepping back a bit and trying to forget what I know about the
> option --track and pretending to see it for the first time, I have to
> ask: Why doesn't it take an argument?  If I check out a raw commit, it
> cannot guess the upstream branch anyway.  So I'd assume this to work:
>
>    git checkout -b new-branch --track=upstream start-point

Assuming that --track option is marked with PARSE_OPT_OPTARG and
when the option is given, we internally do a rev-parse of both
upstream and start-point and make sure the tip of the "track" is an
ancestor of the "start-point", I think it makes sense.  That would
catch cases like this:

	git checkout --detach origin/master
	... work work work ...
	git checkout -b new-branch --track=origin/master HEAD

On the other hand, some use case might want to go the other way, e.g.

	git checkout --detach origin/master~12
	... work to fix an older bug ...
	git checkout -b new-branch --track=origin/master HEAD

in which case the start-point and the current tip of the tracking
branch has no relation other than they share a common ancestor.

So, should we allow a random upstream & start-point combination?  It
appears to me that as long as they share _some_ common ancestory, it
may make sense.

Re: [PATCH 1/2] checkout: add tests for -b and --track

[Jeff King]

On Sun, May 24, 2020 at 09:22:51AM +0200, René Scharfe wrote:

> Test git checkout -b with and without --track and demonstrate unexpected
> error messages when it's given an extra (i.e. unsupported) path
> argument.  In both cases it reports:
> 
>    $ git checkout -b foo origin/master bar
>    fatal: 'bar' is not a commit and a branch 'foo' cannot be created from it
> 
> The problem is that the start point we gave for the new branch is
> "origin/master" and "bar" is just some extra argument -- it could even
> be a valid commit, which would make the message even more confusing.  We
> have more fitting error messages in git commit, but get confused; use
> the text of the rights ones in the tests.

Did you mean "more fitting error message in git checkout"?

> Original-test-by: Jeff King <peff@peff.net>

I didn't think I really contributed much, but OK. :)

> diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
> index 21583154d8..d99b699396 100755
> --- a/t/t2018-checkout-branch.sh
> +++ b/t/t2018-checkout-branch.sh
> @@ -260,4 +260,14 @@ test_expect_success 'checkout -b to a new branch preserves mergeable changes des
>  	test_cmp expect actual
>  '
> 
> +test_expect_success 'checkout -b rejects an invalid start point' '
> +	test_must_fail git checkout -b branch4 file1 2>err &&
> +	test_i18ngrep "is not a commit" err
> +'
> +
> +test_expect_failure 'checkout -b rejects an extra path argument' '
> +	test_must_fail git checkout -b branch5 branch1 file1 2>err &&
> +	test_i18ngrep "Cannot update paths and switch to branch" err
> +'

OK, covering the normal case without --track, both with and without the
extra arg. Makes sense.

> +test_expect_success 'checkout --track -b creates a new tracking branch' '
> +	git checkout --track -b branch1 master &&
> +	test $(git rev-parse --abbrev-ref HEAD) = branch1 &&
> +	test $(git config --get branch.branch1.remote) = . &&
> +	test $(git config --get branch.branch1.merge) = refs/heads/master
> +'
> +
> +test_expect_failure 'checkout --track -b rejects an extra path argument' '
> +	test_must_fail git checkout --track -b branch2 master one.t 2>err &&
> +	test_i18ngrep "cannot be used with updating paths" err
> +'

And these ones with --track, which produces a different error message.
Makes sense.

-Peff

Re: [PATCH 2/2] checkout: improve error messages for -b with extra argument

[Jeff King]

On Sun, May 24, 2020 at 09:23:00AM +0200, René Scharfe wrote:

> When we try to create a branch "foo" based on "origin/master" and give
> git commit -b an extra unsupported argument "bar", it confusingly
> reports:
> 
>    $ git checkout -b foo origin/master bar
>    fatal: 'bar' is not a commit and a branch 'foo' cannot be created from it
> 
>    $ git checkout --track -b foo origin/master bar
>    fatal: 'bar' is not a commit and a branch 'foo' cannot be created from it
> 
> That's wrong, because it very well understands that "origin/master" is
> supposed to be the start point for the new branch and not "bar".  Check
> if we got a commit and show more fitting messages in that case instead:
> 
>    $ git checkout -b foo origin/master bar
>    fatal: Cannot update paths and switch to branch 'foo' at the same time.
> 
>    $ git checkout --track -b foo origin/master bar
>    fatal: '--track' cannot be used with updating paths

Well explained.

> Original-patch-by: Jeff King <peff@peff.net>

If you want to count my hackery as a patch, sure...:)

> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index e9d111bb83..24336e1017 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1689,7 +1689,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
>  		 * Try to give more helpful suggestion.
>  		 * new_branch && argc > 1 will be caught later.
>  		 */
> -		if (opts->new_branch && argc == 1)
> +		if (opts->new_branch && argc == 1 && !new_branch_info.commit)
>  			die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
>  				argv[0], opts->new_branch);

And the fix itself looks obviously correct. We fall through to the other
error clauses, which you can't really see in the context, but your tests
verify it.

Thanks.

-Peff

Re: 'HEAD' is not a commit (according to git-checkout)

[Jeff King]

On Sun, May 24, 2020 at 09:15:33AM -0700, Junio C Hamano wrote:

> René Scharfe <l.s.r@web.de> writes:
> 
> > OK, but stepping back a bit and trying to forget what I know about the
> > option --track and pretending to see it for the first time, I have to
> > ask: Why doesn't it take an argument?  If I check out a raw commit, it
> > cannot guess the upstream branch anyway.  So I'd assume this to work:
> >
> >    git checkout -b new-branch --track=upstream start-point
> 
> Assuming that --track option is marked with PARSE_OPT_OPTARG and
> when the option is given, we internally do a rev-parse of both
> upstream and start-point and make sure the tip of the "track" is an
> ancestor of the "start-point", I think it makes sense.  That would
> catch cases like this:
> 
> 	git checkout --detach origin/master
> 	... work work work ...
> 	git checkout -b new-branch --track=origin/master HEAD
> 
> On the other hand, some use case might want to go the other way, e.g.
> 
> 	git checkout --detach origin/master~12
> 	... work to fix an older bug ...
> 	git checkout -b new-branch --track=origin/master HEAD
> 
> in which case the start-point and the current tip of the tracking
> branch has no relation other than they share a common ancestor.

Trying to think back on times I might have used a feature like this,
most of them are like the first case (though I admit I was always
content with "git branch --set-upstream-to" after the fact).

I do hit the other case, too, with something like:

  git checkout next
  ... hack hack hack ...
  # oops, I meant to do this on top of master
  git checkout -b new-branch --track=origin/master HEAD
  git rebase --onto=origin/master HEAD~2

though given the rebase trickery, I tend to instead just do now:

  git checkout -b new-branch origin
  git cherry-pick -2 HEAD@{1}

I don't know if that really goes to show anything. I'm mostly just
thinking out loud.

> So, should we allow a random upstream & start-point combination?  It
> appears to me that as long as they share _some_ common ancestory, it
> may make sense.

But wouldn't just about any two tips in a repository share some common
ancestry? There are obviously funny exceptions like rewriting history,
or storing unrelated branches, but for the most part you'd find _some_
merge base, even if it's a root commit. So it seems like that check is
unlikely to help prevent accidents, would possibly be an impediment to
something clever the user is doing (albeit quite rarely), and makes the
feature slightly harder to describe. That doesn't seem worth it.

-Peff

Re: 'HEAD' is not a commit (according to git-checkout)

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Sun, May 24, 2020 at 09:15:33AM -0700, Junio C Hamano wrote:
>
>> So, should we allow a random upstream & start-point combination?  It
>> appears to me that as long as they share _some_ common ancestory, it
>> may make sense.
>
> But wouldn't just about any two tips in a repository share some common
> ancestry?

Yes, we are on the same page; the above was my round-about way to
say that it does not look useful to restrict the allowed combination
in order to give us some safety.

Thanks.

RE: 'HEAD' is not a commit (according to git-checkout)

[Randall S. Becker]

On May 27, 2020 11:44 AM, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
> > On Sun, May 24, 2020 at 09:15:33AM -0700, Junio C Hamano wrote:
> >
> >> So, should we allow a random upstream & start-point combination?  It
> >> appears to me that as long as they share _some_ common ancestory, it
> >> may make sense.
> >
> > But wouldn't just about any two tips in a repository share some common
> > ancestry?
> 
> Yes, we are on the same page; the above was my round-about way to say
> that it does not look useful to restrict the allowed combination in order to
> give us some safety.

I have seen some strange ones, as part of migrating from other SCM solutions to git, where there were two completely unrelated histories - at least temporarily until stitched together towards the end of the migration. I don't think the assumption about common ancestry holds generally. I might have misunderstood, though.

Randall

Re: 'HEAD' is not a commit (according to git-checkout)

[Jeff King]

On Wed, May 27, 2020 at 11:52:28AM -0400, Randall S. Becker wrote:

> On May 27, 2020 11:44 AM, Junio C Hamano wrote:
> > Jeff King <peff@peff.net> writes:
> > 
> > > On Sun, May 24, 2020 at 09:15:33AM -0700, Junio C Hamano wrote:
> > >
> > >> So, should we allow a random upstream & start-point combination?  It
> > >> appears to me that as long as they share _some_ common ancestory, it
> > >> may make sense.
> > >
> > > But wouldn't just about any two tips in a repository share some common
> > > ancestry?
> > 
> > Yes, we are on the same page; the above was my round-about way to say
> > that it does not look useful to restrict the allowed combination in order to
> > give us some safety.
> 
> I have seen some strange ones, as part of migrating from other SCM
> solutions to git, where there were two completely unrelated histories
> - at least temporarily until stitched together towards the end of the
> migration. I don't think the assumption about common ancestry holds
> generally. I might have misunderstood, though.

No, I don't think you've misunderstood. It does happen, and there's even
an example in git.git. Doing:

  git checkout -b new-branch --track=origin/todo origin/master

would be nonsense. But it's a rare enough case that I don't think it's
worth worrying too much about. Plus it's pretty easy to undo, or at
least no harder than lots of other mistakes (e.g., trying to rebase on
the wrong branch).

-Peff

Re: [PATCH 1/2] checkout: add tests for -b and --track

[René Scharfe]

Am 27.05.20 um 08:40 schrieb Jeff King:
> On Sun, May 24, 2020 at 09:22:51AM +0200, René Scharfe wrote:
>
>> Test git checkout -b with and without --track and demonstrate unexpected
>> error messages when it's given an extra (i.e. unsupported) path
>> argument.  In both cases it reports:
>>
>>    $ git checkout -b foo origin/master bar
>>    fatal: 'bar' is not a commit and a branch 'foo' cannot be created from it
>>
>> The problem is that the start point we gave for the new branch is
>> "origin/master" and "bar" is just some extra argument -- it could even
>> be a valid commit, which would make the message even more confusing.  We
>> have more fitting error messages in git commit, but get confused; use
>> the text of the rights ones in the tests.
>
> Did you mean "more fitting error message in git checkout"?

Dang, yes, a typo -- it's all about checkout.

René