[PATCH 0/3] checkout: allow -b/-B to work on a merge base

[PATCH 0/3] checkout: allow -b/-B to work on a merge base

[Denton Liu]

I noticed earlier that running

	$ git checkout -b test master...

would result in the following failure message:

	fatal: Not a valid object name: 'master...'.

I believe that this is a bug, so this patchset will allow this to succeed.

Denton Liu (3):
  t2018: cleanup in current test
  t2018: demonstrate checkout -b merge base bug
  checkout: allow -b/-B to work on a merge base

 builtin/checkout.c         |  4 ++-
 t/t2018-checkout-branch.sh | 52 +++++++++++++++++++-------------------
 2 files changed, 29 insertions(+), 27 deletions(-)

-- 
2.21.0.1033.g0e8cc1100c

[PATCH 1/3] t2018: cleanup in current test

[Denton Liu]

Before, in t2018, if do_checkout failed to create `branch2`, the next
test-case would run `git branch -D branch2` but then fail because it was
expecting `branch2` to exist, even though it doesn't. As a result, an
early failure could cause a cascading failure of tests.

Make test-case responsible for cleaning up their own branches so that
future tests can start with a sane environment.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2018-checkout-branch.sh | 39 +++++++++++++-------------------------
 1 file changed, 13 insertions(+), 26 deletions(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index c5014ad9a6..fdb7fd282d 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -60,38 +60,36 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'checkout -b to a new branch, set to HEAD' '
+	test_when_finished test_might_fail git branch -D branch2 &&
+	test_when_finished git checkout branch1 &&
 	do_checkout branch2
 '
 
 test_expect_success 'checkout -b to a new branch, set to an explicit ref' '
-	git checkout branch1 &&
-	git branch -D branch2 &&
-
+	test_when_finished test_might_fail git branch -D branch2 &&
+	test_when_finished git checkout branch1 &&
 	do_checkout branch2 $HEAD1
 '
 
 test_expect_success 'checkout -b to a new branch with unmergeable changes fails' '
-	git checkout branch1 &&
-
-	# clean up from previous test
-	git branch -D branch2 &&
-
 	setup_dirty_unmergeable &&
 	test_must_fail do_checkout branch2 $HEAD1 &&
 	test_dirty_unmergeable
 '
 
 test_expect_success 'checkout -f -b to a new branch with unmergeable changes discards changes' '
+	test_when_finished test_might_fail git branch -D branch2 &&
+	test_when_finished git checkout branch1 &&
+
 	# still dirty and on branch1
 	do_checkout branch2 $HEAD1 "-f -b" &&
 	test_must_fail test_dirty_unmergeable
 '
 
 test_expect_success 'checkout -b to a new branch preserves mergeable changes' '
-	git checkout branch1 &&
-
-	# clean up from previous test
-	git branch -D branch2 &&
+	test_when_finished test_might_fail git branch -D branch2 &&
+	test_when_finished git checkout branch1 &&
+	test_when_finished git reset --hard &&
 
 	setup_dirty_mergeable &&
 	do_checkout branch2 $HEAD1 &&
@@ -99,27 +97,18 @@ test_expect_success 'checkout -b to a new branch preserves mergeable changes' '
 '
 
 test_expect_success 'checkout -f -b to a new branch with mergeable changes discards changes' '
-	# clean up from previous test
-	git reset --hard &&
-
-	git checkout branch1 &&
-
-	# clean up from previous test
-	git branch -D branch2 &&
-
+	test_when_finished git reset --hard HEAD &&
 	setup_dirty_mergeable &&
 	do_checkout branch2 $HEAD1 "-f -b" &&
 	test_must_fail test_dirty_mergeable
 '
 
 test_expect_success 'checkout -b to an existing branch fails' '
-	git reset --hard HEAD &&
-
+	test_when_finished git reset --hard HEAD &&
 	test_must_fail do_checkout branch2 $HEAD2
 '
 
 test_expect_success 'checkout -b to @{-1} fails with the right branch name' '
-	git reset --hard HEAD &&
 	git checkout branch1 &&
 	git checkout branch2 &&
 	echo  >expect "fatal: A branch named '\''branch1'\'' already exists." &&
@@ -160,6 +149,7 @@ test_expect_success 'checkout -f -B to an existing branch with unmergeable chang
 '
 
 test_expect_success 'checkout -B to an existing branch preserves mergeable changes' '
+	test_when_finished git reset --hard &&
 	git checkout branch1 &&
 
 	setup_dirty_mergeable &&
@@ -168,9 +158,6 @@ test_expect_success 'checkout -B to an existing branch preserves mergeable chang
 '
 
 test_expect_success 'checkout -f -B to an existing branch with mergeable changes discards changes' '
-	# clean up from previous test
-	git reset --hard &&
-
 	git checkout branch1 &&
 
 	setup_dirty_mergeable &&
-- 
2.21.0.1033.g0e8cc1100c

Re: [PATCH 1/3] t2018: cleanup in current test

[Eric Sunshine]

On Thu, Apr 25, 2019 at 5:10 PM Denton Liu <liu.denton@gmail.com> wrote:
> Before, in t2018, if do_checkout failed to create `branch2`, the next
> test-case would run `git branch -D branch2` but then fail because it was
> expecting `branch2` to exist, even though it doesn't. As a result, an
> early failure could cause a cascading failure of tests.
>
> Make test-case responsible for cleaning up their own branches so that
> future tests can start with a sane environment.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
> @@ -60,38 +60,36 @@ test_expect_success 'setup' '
>  test_expect_success 'checkout -b to a new branch, set to HEAD' '
> +       test_when_finished test_might_fail git branch -D branch2 &&
> +       test_when_finished git checkout branch1 &&

I'm aware that when-finished actions fire in reverse order but the
inherent subtlety of ordering of these two invocations still caught me
off-guard for a moment since they are reverse the order in which one
logically thinks about the actions which need to be performed. I
wonder if it would be easier to digest if written like this:

    test_when_finished '
        git checkout branch1 &&
        test_might_fail git branch -D branch2
    ' &&

(Probably not worth a re-roll.)

Re: [PATCH 1/3] t2018: cleanup in current test

[Denton Liu]

On Thu, Apr 25, 2019 at 06:34:07PM -0400, Eric Sunshine wrote:
> On Thu, Apr 25, 2019 at 5:10 PM Denton Liu <liu.denton@gmail.com> wrote:
> > Before, in t2018, if do_checkout failed to create `branch2`, the next
> > test-case would run `git branch -D branch2` but then fail because it was
> > expecting `branch2` to exist, even though it doesn't. As a result, an
> > early failure could cause a cascading failure of tests.
> >
> > Make test-case responsible for cleaning up their own branches so that
> > future tests can start with a sane environment.
> >
> > Signed-off-by: Denton Liu <liu.denton@gmail.com>
> > ---
> > diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
> > @@ -60,38 +60,36 @@ test_expect_success 'setup' '
> >  test_expect_success 'checkout -b to a new branch, set to HEAD' '
> > +       test_when_finished test_might_fail git branch -D branch2 &&
> > +       test_when_finished git checkout branch1 &&
> 
> I'm aware that when-finished actions fire in reverse order but the
> inherent subtlety of ordering of these two invocations still caught me
> off-guard for a moment since they are reverse the order in which one
> logically thinks about the actions which need to be performed. I
> wonder if it would be easier to digest if written like this:
> 
>     test_when_finished '
>         git checkout branch1 &&
>         test_might_fail git branch -D branch2
>     ' &&
> 
> (Probably not worth a re-roll.)

This sounds like a good idea. If Junio doesn't get around to it, I'll
send a v1.5 of this patch later. Thanks for the suggestion!

Re: [PATCH 1/3] t2018: cleanup in current test

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>>  test_expect_success 'checkout -b to a new branch, set to HEAD' '
>> +       test_when_finished test_might_fail git branch -D branch2 &&
>> +       test_when_finished git checkout branch1 &&
>
> I'm aware that when-finished actions fire in reverse order but the
> inherent subtlety of ordering of these two invocations still caught me
> off-guard for a moment since they are reverse the order in which one
> logically thinks about the actions which need to be performed. I
> wonder if it would be easier to digest if written like this:
>
>     test_when_finished '
>         git checkout branch1 &&
>         test_might_fail git branch -D branch2
>     ' &&

Perhaps.  Be careful in choosing the quotes between sq and dq,
though.

Re: [PATCH 1/3] t2018: cleanup in current test

[Eric Sunshine]

On Thu, Apr 25, 2019 at 10:50 PM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> >     test_when_finished '
> >         git checkout branch1 &&
> >         test_might_fail git branch -D branch2
> >     ' &&
>
> Perhaps.  Be careful in choosing the quotes between sq and dq,
> though.

Yep, thanks for catching my mistake. Definitely want double-quote, not
single-quote.

[PATCH 2/3] t2018: demonstrate checkout -b merge base bug

[Denton Liu]

In git-checkout.txt, it states

	As a special case, you may use `"A...B"` as a shortcut for the
	merge base of `A` and `B` if there is exactly one merge base. You can
	leave out at most one of `A` and `B`, in which case it defaults to `HEAD`.

However, there exists a bug where performing

	$ git checkout -b test master...

fails with the message

	fatal: Not a valid object name: 'master...'.

Demonstrate this failure so that it can be corrected later.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2018-checkout-branch.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index fdb7fd282d..a3fa520d2e 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -65,6 +65,12 @@ test_expect_success 'checkout -b to a new branch, set to HEAD' '
 	do_checkout branch2
 '
 
+test_expect_failure 'checkout -b to a merge base' '
+	test_when_finished test_might_fail git branch -D branch2 &&
+	test_when_finished git checkout branch1 &&
+	git checkout -b branch2 branch1...
+'
+
 test_expect_success 'checkout -b to a new branch, set to an explicit ref' '
 	test_when_finished test_might_fail git branch -D branch2 &&
 	test_when_finished git checkout branch1 &&
@@ -122,6 +128,13 @@ test_expect_success 'checkout -B to an existing branch resets branch to HEAD' '
 	do_checkout branch2 "" -B
 '
 
+test_expect_failure 'checkout -B to a merge base' '
+	git checkout branch1 &&
+	git branch -D branch2 &&
+
+	git checkout -B branch2 branch1...
+'
+
 test_expect_success 'checkout -B to an existing branch from detached HEAD resets branch to HEAD' '
 	git checkout $(git rev-parse --verify HEAD) &&
 
-- 
2.21.0.1033.g0e8cc1100c

[PATCH 3/3] checkout: allow -b/-B to work on a merge base

[Denton Liu]

When we ran something like

    $ git checkout -b test master...

it would fail with the message

    fatal: Not a valid object name: 'master...'.

This was caused by the call to `create_branch` where `start_name` is
expected to be a valid rev. However, git-checkout allows the branch to
not be a valid rev in the case where we have "..." to specify getting
the merge base.

In the case where a branch with "..." is specified, use the oid instead
so that `start_name` is a valid rev.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 builtin/checkout.c         | 4 +++-
 t/t2018-checkout-branch.sh | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index ffa776c6e1..d99b3f3925 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1229,7 +1229,9 @@ static int parse_branchname_arg(int argc, const char **argv,
 	argv++;
 	argc--;
 
-	new_branch_info->name = arg;
+	new_branch_info->name = strstr(arg, "...") ?
+		xstrdup(oid_to_hex(rev)) :
+		arg;
 	setup_branch_path(new_branch_info);
 
 	if (!check_refname_format(new_branch_info->path, 0) &&
diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index a3fa520d2e..d6ea556d84 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -65,7 +65,7 @@ test_expect_success 'checkout -b to a new branch, set to HEAD' '
 	do_checkout branch2
 '
 
-test_expect_failure 'checkout -b to a merge base' '
+test_expect_success 'checkout -b to a merge base' '
 	test_when_finished test_might_fail git branch -D branch2 &&
 	test_when_finished git checkout branch1 &&
 	git checkout -b branch2 branch1...
@@ -128,7 +128,7 @@ test_expect_success 'checkout -B to an existing branch resets branch to HEAD' '
 	do_checkout branch2 "" -B
 '
 
-test_expect_failure 'checkout -B to a merge base' '
+test_expect_success 'checkout -B to a merge base' '
 	git checkout branch1 &&
 	git branch -D branch2 &&
 
-- 
2.21.0.1033.g0e8cc1100c

Re: [PATCH 3/3] checkout: allow -b/-B to work on a merge base

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> -	new_branch_info->name = arg;
> +	new_branch_info->name = strstr(arg, "...") ?
> +		xstrdup(oid_to_hex(rev)) :
> +		arg;

Can we do better?

I am not sure why we want to hardcode the knowledge of "..." syntax
like this here.  "git checkout A...B" introduced in 2009 needed only
a single-liner change from get_sha1() to get_sha1_mb() without making
the ugly implementation detail seep into this layer.

Re: [PATCH 3/3] checkout: allow -b/-B to work on a merge base

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Denton Liu <liu.denton@gmail.com> writes:
>
>> -	new_branch_info->name = arg;
>> +	new_branch_info->name = strstr(arg, "...") ?
>> +		xstrdup(oid_to_hex(rev)) :
>> +		arg;
>
> Can we do better?
>
> I am not sure why we want to hardcode the knowledge of "..." syntax
> like this here.  "git checkout A...B" introduced in 2009 needed only
> a single-liner change from get_sha1() to get_sha1_mb() without making
> the ugly implementation detail seep into this layer.

I _think_ what you are trying to work around is that a syntax that
is not understood as a reference to a single revision by get_oid()
but is understood as such by get_oid_mb() can be in the .name field,
and that eventually gets passed to branch.c::create_branch() as
"start_name" in the codepath.  The function ought to know that
start_name wants to name a single revision and never a revision
range, but fails to use get_oid_mb() but just a plain get_oid(),
and fails.

If that is the case, wouldn't a better fix be more like the
attached?  This hasn't even been compile tested, but I suspect that
without taking this approach, you would introduce a new "bug",
namely, that

	$ git checkout -b newbranch master...

ought to behave exactly like

	$ git branch newbranch master...
	$ git checkout newbranch

But the first step would hit the same branch.c::create_branch()
and would not work, no?

The reason I care about hiding syntax details like "..." from users
of get_*_mb() is because I anticipate that we'll want to extend it
to things other than just "merge base between two" with syntax
different from "..." (while probably renaming _mb suffix to
something else).

The codebase is not ready to replace get_oid() with get_oid_mb()
blindly and wholesale, I think, as the former is used as an
implementation detail of parsing range syntax like A..B but places
that are end user facing *and* expect to take only a single revision
(e.g. "rebase --onto <commit>", "checkout <commit>", etc.) and never
a range that currently use get_oid() should be able to get replaced
with get_oid_mb() to learn "A...B means their merge base, not a
symmetric range" semantics for free.

 branch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/branch.c b/branch.c
index 28b81a7e02..a84c8aaca2 100644
--- a/branch.c
+++ b/branch.c
@@ -268,7 +268,7 @@ void create_branch(struct repository *r,
 	}
 
 	real_ref = NULL;
-	if (get_oid(start_name, &oid)) {
+	if (get_oid_mb(start_name, &oid)) {
 		if (explicit_tracking) {
 			if (advice_set_upstream_failure) {
 				error(_(upstream_missing), start_name);

[PATCH v2 0/3] allow checkout and branch to create branches on a merge base

[Denton Liu]

Thanks for your comments, Eric and Junio.

Eric, I've combined the `test_when_finished` calls together so that the
statements within appear in a more "logical" order.

Junio, I've taken your suggestion and moved the change into
`create_branch`. Initially, I didn't want to do this because I didn't
want to change the semantics of git-branch but introducing the merge
base syntax seems to be a positive change so let's do it.

Thanks,

Denton

---

Changes since v1:

* Moved multiple `test_when_finished` calls that appeared in "reverse
  order" into one call that appears in the logical order
* Made create_branch handle merge base revs instead of putting a hack
  into checkout

Denton Liu (3):
  t2018: cleanup in current test
  t2018: demonstrate checkout -b merge base bug
  branch: make create_branch accept a merge base rev

 Documentation/git-branch.txt |  6 +++-
 branch.c                     |  2 +-
 t/t2018-checkout-branch.sh   | 56 +++++++++++++++++++-----------------
 t/t3200-branch.sh            | 14 ++++++---
 4 files changed, 46 insertions(+), 32 deletions(-)

Range-diff against v1:
1:  c0c7171e3d ! 1:  9d04faf29d t2018: cleanup in current test
    @@ -19,8 +19,9 @@
      '
      
      test_expect_success 'checkout -b to a new branch, set to HEAD' '
    -+	test_when_finished test_might_fail git branch -D branch2 &&
    -+	test_when_finished git checkout branch1 &&
    ++	test_when_finished "
    ++		git checkout branch1 &&
    ++		test_might_fail git branch -D branch2" &&
      	do_checkout branch2
      '
      
    @@ -28,8 +29,9 @@
     -	git checkout branch1 &&
     -	git branch -D branch2 &&
     -
    -+	test_when_finished test_might_fail git branch -D branch2 &&
    -+	test_when_finished git checkout branch1 &&
    ++	test_when_finished "
    ++		git checkout branch1 &&
    ++		test_might_fail git branch -D branch2" &&
      	do_checkout branch2 $HEAD1
      '
      
    @@ -45,8 +47,9 @@
      '
      
      test_expect_success 'checkout -f -b to a new branch with unmergeable changes discards changes' '
    -+	test_when_finished test_might_fail git branch -D branch2 &&
    -+	test_when_finished git checkout branch1 &&
    ++	test_when_finished "
    ++		git checkout branch1 &&
    ++		test_might_fail git branch -D branch2" &&
     +
      	# still dirty and on branch1
      	do_checkout branch2 $HEAD1 "-f -b" &&
    @@ -58,9 +61,10 @@
     -
     -	# clean up from previous test
     -	git branch -D branch2 &&
    -+	test_when_finished test_might_fail git branch -D branch2 &&
    -+	test_when_finished git checkout branch1 &&
    -+	test_when_finished git reset --hard &&
    ++	test_when_finished "
    ++		git reset --hard &&
    ++		git checkout branch1 &&
    ++		test_might_fail git branch -D branch2" &&
      
      	setup_dirty_mergeable &&
      	do_checkout branch2 $HEAD1 &&
2:  ff38bdb564 ! 2:  5e8320cd80 t2018: demonstrate checkout -b merge base bug
    @@ -28,21 +28,21 @@
      '
      
     +test_expect_failure 'checkout -b to a merge base' '
    -+	test_when_finished test_might_fail git branch -D branch2 &&
    -+	test_when_finished git checkout branch1 &&
    ++	test_when_finished "
    ++		git checkout branch1 &&
    ++		test_might_fail git branch -D branch2" &&
     +	git checkout -b branch2 branch1...
     +'
     +
      test_expect_success 'checkout -b to a new branch, set to an explicit ref' '
    - 	test_when_finished test_might_fail git branch -D branch2 &&
    - 	test_when_finished git checkout branch1 &&
    + 	test_when_finished "
    + 		git checkout branch1 &&
     @@
      	do_checkout branch2 "" -B
      '
      
     +test_expect_failure 'checkout -B to a merge base' '
     +	git checkout branch1 &&
    -+	git branch -D branch2 &&
     +
     +	git checkout -B branch2 branch1...
     +'
3:  031780431d < -:  ---------- checkout: allow -b/-B to work on a merge base
-:  ---------- > 3:  c91c7535a7 branch: make create_branch accept a merge base rev
-- 
2.21.0.1033.g0e8cc1100c

[PATCH v2 1/3] t2018: cleanup in current test

[Denton Liu]

Before, in t2018, if do_checkout failed to create `branch2`, the next
test-case would run `git branch -D branch2` but then fail because it was
expecting `branch2` to exist, even though it doesn't. As a result, an
early failure could cause a cascading failure of tests.

Make test-case responsible for cleaning up their own branches so that
future tests can start with a sane environment.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2018-checkout-branch.sh | 43 +++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index c5014ad9a6..f1c7023e1a 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -60,38 +60,40 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'checkout -b to a new branch, set to HEAD' '
+	test_when_finished "
+		git checkout branch1 &&
+		test_might_fail git branch -D branch2" &&
 	do_checkout branch2
 '
 
 test_expect_success 'checkout -b to a new branch, set to an explicit ref' '
-	git checkout branch1 &&
-	git branch -D branch2 &&
-
+	test_when_finished "
+		git checkout branch1 &&
+		test_might_fail git branch -D branch2" &&
 	do_checkout branch2 $HEAD1
 '
 
 test_expect_success 'checkout -b to a new branch with unmergeable changes fails' '
-	git checkout branch1 &&
-
-	# clean up from previous test
-	git branch -D branch2 &&
-
 	setup_dirty_unmergeable &&
 	test_must_fail do_checkout branch2 $HEAD1 &&
 	test_dirty_unmergeable
 '
 
 test_expect_success 'checkout -f -b to a new branch with unmergeable changes discards changes' '
+	test_when_finished "
+		git checkout branch1 &&
+		test_might_fail git branch -D branch2" &&
+
 	# still dirty and on branch1
 	do_checkout branch2 $HEAD1 "-f -b" &&
 	test_must_fail test_dirty_unmergeable
 '
 
 test_expect_success 'checkout -b to a new branch preserves mergeable changes' '
-	git checkout branch1 &&
-
-	# clean up from previous test
-	git branch -D branch2 &&
+	test_when_finished "
+		git reset --hard &&
+		git checkout branch1 &&
+		test_might_fail git branch -D branch2" &&
 
 	setup_dirty_mergeable &&
 	do_checkout branch2 $HEAD1 &&
@@ -99,27 +101,18 @@ test_expect_success 'checkout -b to a new branch preserves mergeable changes' '
 '
 
 test_expect_success 'checkout -f -b to a new branch with mergeable changes discards changes' '
-	# clean up from previous test
-	git reset --hard &&
-
-	git checkout branch1 &&
-
-	# clean up from previous test
-	git branch -D branch2 &&
-
+	test_when_finished git reset --hard HEAD &&
 	setup_dirty_mergeable &&
 	do_checkout branch2 $HEAD1 "-f -b" &&
 	test_must_fail test_dirty_mergeable
 '
 
 test_expect_success 'checkout -b to an existing branch fails' '
-	git reset --hard HEAD &&
-
+	test_when_finished git reset --hard HEAD &&
 	test_must_fail do_checkout branch2 $HEAD2
 '
 
 test_expect_success 'checkout -b to @{-1} fails with the right branch name' '
-	git reset --hard HEAD &&
 	git checkout branch1 &&
 	git checkout branch2 &&
 	echo  >expect "fatal: A branch named '\''branch1'\'' already exists." &&
@@ -160,6 +153,7 @@ test_expect_success 'checkout -f -B to an existing branch with unmergeable chang
 '
 
 test_expect_success 'checkout -B to an existing branch preserves mergeable changes' '
+	test_when_finished git reset --hard &&
 	git checkout branch1 &&
 
 	setup_dirty_mergeable &&
@@ -168,9 +162,6 @@ test_expect_success 'checkout -B to an existing branch preserves mergeable chang
 '
 
 test_expect_success 'checkout -f -B to an existing branch with mergeable changes discards changes' '
-	# clean up from previous test
-	git reset --hard &&
-
 	git checkout branch1 &&
 
 	setup_dirty_mergeable &&
-- 
2.21.0.1033.g0e8cc1100c

[PATCH v2 2/3] t2018: demonstrate checkout -b merge base bug

[Denton Liu]

In git-checkout.txt, it states

	As a special case, you may use `"A...B"` as a shortcut for the
	merge base of `A` and `B` if there is exactly one merge base. You can
	leave out at most one of `A` and `B`, in which case it defaults to `HEAD`.

However, there exists a bug where performing

	$ git checkout -b test master...

fails with the message

	fatal: Not a valid object name: 'master...'.

Demonstrate this failure so that it can be corrected later.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2018-checkout-branch.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index f1c7023e1a..3cd142657a 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -66,6 +66,13 @@ test_expect_success 'checkout -b to a new branch, set to HEAD' '
 	do_checkout branch2
 '
 
+test_expect_failure 'checkout -b to a merge base' '
+	test_when_finished "
+		git checkout branch1 &&
+		test_might_fail git branch -D branch2" &&
+	git checkout -b branch2 branch1...
+'
+
 test_expect_success 'checkout -b to a new branch, set to an explicit ref' '
 	test_when_finished "
 		git checkout branch1 &&
@@ -126,6 +133,12 @@ test_expect_success 'checkout -B to an existing branch resets branch to HEAD' '
 	do_checkout branch2 "" -B
 '
 
+test_expect_failure 'checkout -B to a merge base' '
+	git checkout branch1 &&
+
+	git checkout -B branch2 branch1...
+'
+
 test_expect_success 'checkout -B to an existing branch from detached HEAD resets branch to HEAD' '
 	git checkout $(git rev-parse --verify HEAD) &&
 
-- 
2.21.0.1033.g0e8cc1100c

[PATCH v2 3/3] branch: make create_branch accept a merge base rev

[Denton Liu]

When we ran something like

    $ git checkout -b test master...

it would fail with the message

    fatal: Not a valid object name: 'master...'.

This was caused by the call to `create_branch` where `start_name` is
expected to be a valid rev. However, git-checkout allows the branch to
be a valid _merge base_ rev (i.e. with a "...") so it was possible for
an invalid rev to be passed in.

Make `create_branch` accept a merge base rev so that this case does not
error out.

As a side-effect, teach git-branch how to handle merge base revs as
well.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-branch.txt |  6 +++++-
 branch.c                     |  2 +-
 t/t2018-checkout-branch.sh   |  4 ++--
 t/t3200-branch.sh            | 14 ++++++++++----
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 0cd87ddeff..6ebd512b4f 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -45,7 +45,11 @@ argument is missing it defaults to `HEAD` (i.e. the tip of the current
 branch).
 
 The command's second form creates a new branch head named <branchname>
-which points to the current `HEAD`, or <start-point> if given.
+which points to the current `HEAD`, or <start-point> if given. As a
+special case, for <start-point>, you may use `"A...B"` as a shortcut for
+the merge base of `A` and `B` if there is exactly one merge base. You
+can leave out at most one of `A` and `B`, in which case it defaults to
+`HEAD`.
 
 Note that this will create the new branch, but it will not switch the
 working tree to it; use "git checkout <newbranch>" to switch to the
diff --git a/branch.c b/branch.c
index 28b81a7e02..a84c8aaca2 100644
--- a/branch.c
+++ b/branch.c
@@ -268,7 +268,7 @@ void create_branch(struct repository *r,
 	}
 
 	real_ref = NULL;
-	if (get_oid(start_name, &oid)) {
+	if (get_oid_mb(start_name, &oid)) {
 		if (explicit_tracking) {
 			if (advice_set_upstream_failure) {
 				error(_(upstream_missing), start_name);
diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 3cd142657a..822381dd9d 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -66,7 +66,7 @@ test_expect_success 'checkout -b to a new branch, set to HEAD' '
 	do_checkout branch2
 '
 
-test_expect_failure 'checkout -b to a merge base' '
+test_expect_success 'checkout -b to a merge base' '
 	test_when_finished "
 		git checkout branch1 &&
 		test_might_fail git branch -D branch2" &&
@@ -133,7 +133,7 @@ test_expect_success 'checkout -B to an existing branch resets branch to HEAD' '
 	do_checkout branch2 "" -B
 '
 
-test_expect_failure 'checkout -B to a merge base' '
+test_expect_success 'checkout -B to a merge base' '
 	git checkout branch1 &&
 
 	git checkout -B branch2 branch1...
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 478b82cf9b..acb16b62dd 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -42,6 +42,10 @@ test_expect_success 'git branch a/b/c should create a branch' '
 	git branch a/b/c && test_path_is_file .git/refs/heads/a/b/c
 '
 
+test_expect_success 'git branch mb master... should create a branch' '
+	git branch mb master... && test_path_is_file .git/refs/heads/mb
+'
+
 test_expect_success 'git branch HEAD should fail' '
 	test_must_fail git branch HEAD
 '
@@ -292,8 +296,8 @@ test_expect_success 'git branch --list -v with --abbrev' '
 test_expect_success 'git branch --column' '
 	COLUMNS=81 git branch --column=column >actual &&
 	cat >expected <<\EOF &&
-  a/b/c     bam       foo       l       * master    n         o/p       r
-  abc       bar       j/k       m/m       master2   o/o       q
+  a/b/c     bam       foo       l       * master    mb        o/o       q
+  abc       bar       j/k       m/m       master2   n         o/p       r
 EOF
 	test_cmp expected actual
 '
@@ -315,6 +319,7 @@ test_expect_success 'git branch --column with an extremely long branch name' '
   m/m
 * master
   master2
+  mb
   n
   o/o
   o/p
@@ -332,8 +337,8 @@ test_expect_success 'git branch with column.*' '
 	git config --unset column.branch &&
 	git config --unset column.ui &&
 	cat >expected <<\EOF &&
-  a/b/c   bam   foo   l   * master    n     o/p   r
-  abc     bar   j/k   m/m   master2   o/o   q
+  a/b/c   bam   foo   l   * master    mb   o/o   q
+  abc     bar   j/k   m/m   master2   n    o/p   r
 EOF
 	test_cmp expected actual
 '
@@ -357,6 +362,7 @@ test_expect_success 'git branch -v with column.ui ignored' '
   m/m
 * master
   master2
+  mb
   n
   o/o
   o/p
-- 
2.21.0.1033.g0e8cc1100c

Re: [PATCH v2 0/3] allow checkout and branch to create branches on a merge base

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> Thanks for your comments, Eric and Junio.
>
> Eric, I've combined the `test_when_finished` calls together so that the
> statements within appear in a more "logical" order.
>
> Junio, I've taken your suggestion and moved the change into
> `create_branch`. Initially, I didn't want to do this because I didn't
> want to change the semantics of git-branch but introducing the merge
> base syntax seems to be a positive change so let's do it.
> ...
> Denton Liu (3):
>   t2018: cleanup in current test
>   t2018: demonstrate checkout -b merge base bug
>   branch: make create_branch accept a merge base rev

Because "checkout -b new" is supposed to be merely a short-hand for
a "branch new" followed by "checkout new", the lack of "branch new
A...B" is the same "bug" as the lack of "checkout -b new A...B".

The second patch that does not talk about the former but singles out
only the latter is being inconsistent.

One person's lack of feature is a bug to another person, and indeed,
when we did "checkout A...B" in 2009, we weren't interested in doing
the same for "checkout -b new", and nobody thought about adding that
until now, and/or considered the lack of feature as a bug.

We do not "demonstrate" the lack of a new feature in a patch with
expect-failure, followed by another patch that adds the feature that
flips expect-failure to expect-success.  A patch that teaches
"checkout -b" about A...B, that is adding a missing feature, should
not have to do so.  As it is shades of gray between a change being a
bugfix and adding a new feature, switching the style of testing
based on the distinction between them does not make much sense.  Be
consistent and stick to just one style.  And having the test and the
code change (be it adding a missing feature or fixing a bug) in a
single patch makes patch management a lot simpler by making it
harder to lose only one half.

Having a preliminary clean-up as a separate step is a good idea, but
for this topic, I think the latter two should be combined into a
single patch that changes the code and adds tests at the same time.

Thanks.

Re: [PATCH v2 0/3] allow checkout and branch to create branches on a merge base

[Denton Liu]

On Sat, Apr 27, 2019 at 08:07:34AM +0900, Junio C Hamano wrote:
> Denton Liu <liu.denton@gmail.com> writes:
> 
> > Thanks for your comments, Eric and Junio.
> >
> > Eric, I've combined the `test_when_finished` calls together so that the
> > statements within appear in a more "logical" order.
> >
> > Junio, I've taken your suggestion and moved the change into
> > `create_branch`. Initially, I didn't want to do this because I didn't
> > want to change the semantics of git-branch but introducing the merge
> > base syntax seems to be a positive change so let's do it.
> > ...
> > Denton Liu (3):
> >   t2018: cleanup in current test
> >   t2018: demonstrate checkout -b merge base bug
> >   branch: make create_branch accept a merge base rev
> 
> Because "checkout -b new" is supposed to be merely a short-hand for
> a "branch new" followed by "checkout new", the lack of "branch new
> A...B" is the same "bug" as the lack of "checkout -b new A...B".

I didn't consider it to be the same "bug" because git-checkout.txt
mentions the "..." syntax, whereas it isn't mentioned in git-branch.txt.
That being said, now that I look at git-checkout.txt again, when doing a
"regular" checkout, the parameter is called <branch> whereas when we're
doing a checkout -b, it's called <start_point>, which doesn't mention
"...".

> 
> The second patch that does not talk about the former but singles out
> only the latter is being inconsistent.
> 
> One person's lack of feature is a bug to another person, and indeed,
> when we did "checkout A...B" in 2009, we weren't interested in doing
> the same for "checkout -b new", and nobody thought about adding that
> until now, and/or considered the lack of feature as a bug.

I agree, based on the above, I now see that it's a lack of feature and
not a bug.

> 
> We do not "demonstrate" the lack of a new feature in a patch with
> expect-failure, followed by another patch that adds the feature that
> flips expect-failure to expect-success.  A patch that teaches
> "checkout -b" about A...B, that is adding a missing feature, should
> not have to do so.  As it is shades of gray between a change being a
> bugfix and adding a new feature, switching the style of testing
> based on the distinction between them does not make much sense.  Be
> consistent and stick to just one style.  And having the test and the
> code change (be it adding a missing feature or fixing a bug) in a
> single patch makes patch management a lot simpler by making it
> harder to lose only one half.
> 
> Having a preliminary clean-up as a separate step is a good idea, but
> for this topic, I think the latter two should be combined into a
> single patch that changes the code and adds tests at the same time.

I'm going send a reroll to update the documentation to mention "..." in
<start_point> and, while I'm at it, I'll do the squash.

Thanks for your comments,

Denton

> 
> Thanks.

Re: [PATCH v2 0/3] allow checkout and branch to create branches on a merge base

[Denton Liu]

On Fri, Apr 26, 2019 at 04:40:24PM -0700, Denton Liu wrote:

[snip]

> I'm going send a reroll to update the documentation to mention "..." in
> <start_point> and, while I'm at it, I'll do the squash.

Actually, I have a quick question for you:

Out of respect for your time as the maintainer (since you have a lot of
topics to deal with), how do you prefer small fixups be submitted?

* A complete reroll of the whole series
* A replacement for one patch in the series
* A follow-up email with an "oops, please change x to y"

Thanks,

Denton

> 
> Thanks for your comments,
> 
> Denton
> 
> > 
> > Thanks.

Re: [PATCH v2 0/3] allow checkout and branch to create branches on a merge base

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> Out of respect for your time as the maintainer (since you have a lot of
> topics to deal with), how do you prefer small fixups be submitted?
>
> * A complete reroll of the whole series
> * A replacement for one patch in the series
> * A follow-up email with an "oops, please change x to y"

Depends on how small your "small" is, but in general, the above is a
good approximation of the preference order.

Wholesale replacement without having to think is the easiest.  When
the latest round is not from a distant past (i.e. less than two
weeks old), replacement patches for only selected steps is also
fine, as I can pretend it is a wholesale replacement by completing
the missing patches by picking them from the mailbox while using the
updated ones.

"Please change x to y" is the least welcome, with a big exception
that it is the easiest when it is a change to a few words in a
single place, either in the log message or a file contents, of a
single patch topic---in other words, something I can deal with
"commit --amend" and needs less than 2 minutes to handle.  Anything
larger than that size and requires me to fire up "rebase -i" starts
to make me wonder why _I_ am doing that, not the topic owner.

Thanks.

[PATCH v3 0/2] allow checkout and branch to create branches on a merge base

[Denton Liu]

Thanks again for the review, Junio.

I've squashed 2/3 and 3/3 together and made that documentation change I
was talking about earlier.

---

Changes since v2:

* Squashed 2/3 with 3/3
* Document merge base syntax for <start_point> in git-checkout.txt

Changes since v1:

* Moved multiple `test_when_finished` calls that appeared in "reverse
  order" into one call that appears in the logical order
* Made create_branch handle merge base revs instead of putting a hack
  into checkout

Denton Liu (2):
  t2018: cleanup in current test
  branch: make create_branch accept a merge base rev

 Documentation/git-branch.txt   |  6 +++-
 Documentation/git-checkout.txt |  4 +++
 branch.c                       |  2 +-
 t/t2018-checkout-branch.sh     | 56 ++++++++++++++++++----------------
 t/t3200-branch.sh              | 14 ++++++---
 5 files changed, 50 insertions(+), 32 deletions(-)

Range-diff against v2:
1:  9d04faf29d = 1:  9d04faf29d t2018: cleanup in current test
2:  5e8320cd80 < -:  ---------- t2018: demonstrate checkout -b merge base bug
3:  c91c7535a7 ! 2:  bb25852740 branch: make create_branch accept a merge base rev
    @@ -41,6 +41,21 @@
      Note that this will create the new branch, but it will not switch the
      working tree to it; use "git checkout <newbranch>" to switch to the
     
    + diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
    + --- a/Documentation/git-checkout.txt
    + +++ b/Documentation/git-checkout.txt
    +@@
    + <start_point>::
    + 	The name of a commit at which to start the new branch; see
    + 	linkgit:git-branch[1] for details. Defaults to HEAD.
    +++
    ++As a special case, you may use `"A...B"` as a shortcut for the
    ++merge base of `A` and `B` if there is exactly one merge base. You can
    ++leave out at most one of `A` and `B`, in which case it defaults to `HEAD`.
    + 
    + <tree-ish>::
    + 	Tree to checkout from (when paths are given). If not specified,
    +
      diff --git a/branch.c b/branch.c
      --- a/branch.c
      +++ b/branch.c
    @@ -61,20 +76,29 @@
      	do_checkout branch2
      '
      
    --test_expect_failure 'checkout -b to a merge base' '
     +test_expect_success 'checkout -b to a merge base' '
    ++	test_when_finished "
    ++		git checkout branch1 &&
    ++		test_might_fail git branch -D branch2" &&
    ++	git checkout -b branch2 branch1...
    ++'
    ++
    + test_expect_success 'checkout -b to a new branch, set to an explicit ref' '
      	test_when_finished "
      		git checkout branch1 &&
    - 		test_might_fail git branch -D branch2" &&
     @@
      	do_checkout branch2 "" -B
      '
      
    --test_expect_failure 'checkout -B to a merge base' '
     +test_expect_success 'checkout -B to a merge base' '
    - 	git checkout branch1 &&
    ++	git checkout branch1 &&
    ++
    ++	git checkout -B branch2 branch1...
    ++'
    ++
    + test_expect_success 'checkout -B to an existing branch from detached HEAD resets branch to HEAD' '
    + 	git checkout $(git rev-parse --verify HEAD) &&
      
    - 	git checkout -B branch2 branch1...
     
      diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
      --- a/t/t3200-branch.sh
-- 
2.21.0.1000.g11cd861522

[PATCH v3 1/2] t2018: cleanup in current test

[Denton Liu]

Before, in t2018, if do_checkout failed to create `branch2`, the next
test-case would run `git branch -D branch2` but then fail because it was
expecting `branch2` to exist, even though it doesn't. As a result, an
early failure could cause a cascading failure of tests.

Make test-case responsible for cleaning up their own branches so that
future tests can start with a sane environment.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2018-checkout-branch.sh | 43 +++++++++++++++-----------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index c5014ad9a6..f1c7023e1a 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -60,38 +60,40 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'checkout -b to a new branch, set to HEAD' '
+	test_when_finished "
+		git checkout branch1 &&
+		test_might_fail git branch -D branch2" &&
 	do_checkout branch2
 '
 
 test_expect_success 'checkout -b to a new branch, set to an explicit ref' '
-	git checkout branch1 &&
-	git branch -D branch2 &&
-
+	test_when_finished "
+		git checkout branch1 &&
+		test_might_fail git branch -D branch2" &&
 	do_checkout branch2 $HEAD1
 '
 
 test_expect_success 'checkout -b to a new branch with unmergeable changes fails' '
-	git checkout branch1 &&
-
-	# clean up from previous test
-	git branch -D branch2 &&
-
 	setup_dirty_unmergeable &&
 	test_must_fail do_checkout branch2 $HEAD1 &&
 	test_dirty_unmergeable
 '
 
 test_expect_success 'checkout -f -b to a new branch with unmergeable changes discards changes' '
+	test_when_finished "
+		git checkout branch1 &&
+		test_might_fail git branch -D branch2" &&
+
 	# still dirty and on branch1
 	do_checkout branch2 $HEAD1 "-f -b" &&
 	test_must_fail test_dirty_unmergeable
 '
 
 test_expect_success 'checkout -b to a new branch preserves mergeable changes' '
-	git checkout branch1 &&
-
-	# clean up from previous test
-	git branch -D branch2 &&
+	test_when_finished "
+		git reset --hard &&
+		git checkout branch1 &&
+		test_might_fail git branch -D branch2" &&
 
 	setup_dirty_mergeable &&
 	do_checkout branch2 $HEAD1 &&
@@ -99,27 +101,18 @@ test_expect_success 'checkout -b to a new branch preserves mergeable changes' '
 '
 
 test_expect_success 'checkout -f -b to a new branch with mergeable changes discards changes' '
-	# clean up from previous test
-	git reset --hard &&
-
-	git checkout branch1 &&
-
-	# clean up from previous test
-	git branch -D branch2 &&
-
+	test_when_finished git reset --hard HEAD &&
 	setup_dirty_mergeable &&
 	do_checkout branch2 $HEAD1 "-f -b" &&
 	test_must_fail test_dirty_mergeable
 '
 
 test_expect_success 'checkout -b to an existing branch fails' '
-	git reset --hard HEAD &&
-
+	test_when_finished git reset --hard HEAD &&
 	test_must_fail do_checkout branch2 $HEAD2
 '
 
 test_expect_success 'checkout -b to @{-1} fails with the right branch name' '
-	git reset --hard HEAD &&
 	git checkout branch1 &&
 	git checkout branch2 &&
 	echo  >expect "fatal: A branch named '\''branch1'\'' already exists." &&
@@ -160,6 +153,7 @@ test_expect_success 'checkout -f -B to an existing branch with unmergeable chang
 '
 
 test_expect_success 'checkout -B to an existing branch preserves mergeable changes' '
+	test_when_finished git reset --hard &&
 	git checkout branch1 &&
 
 	setup_dirty_mergeable &&
@@ -168,9 +162,6 @@ test_expect_success 'checkout -B to an existing branch preserves mergeable chang
 '
 
 test_expect_success 'checkout -f -B to an existing branch with mergeable changes discards changes' '
-	# clean up from previous test
-	git reset --hard &&
-
 	git checkout branch1 &&
 
 	setup_dirty_mergeable &&
-- 
2.21.0.1000.g11cd861522

[PATCH v3 2/2] branch: make create_branch accept a merge base rev

[Denton Liu]

When we ran something like

    $ git checkout -b test master...

it would fail with the message

    fatal: Not a valid object name: 'master...'.

This was caused by the call to `create_branch` where `start_name` is
expected to be a valid rev. However, git-checkout allows the branch to
be a valid _merge base_ rev (i.e. with a "...") so it was possible for
an invalid rev to be passed in.

Make `create_branch` accept a merge base rev so that this case does not
error out.

As a side-effect, teach git-branch how to handle merge base revs as
well.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-branch.txt   |  6 +++++-
 Documentation/git-checkout.txt |  4 ++++
 branch.c                       |  2 +-
 t/t2018-checkout-branch.sh     | 13 +++++++++++++
 t/t3200-branch.sh              | 14 ++++++++++----
 5 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 0cd87ddeff..6ebd512b4f 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -45,7 +45,11 @@ argument is missing it defaults to `HEAD` (i.e. the tip of the current
 branch).
 
 The command's second form creates a new branch head named <branchname>
-which points to the current `HEAD`, or <start-point> if given.
+which points to the current `HEAD`, or <start-point> if given. As a
+special case, for <start-point>, you may use `"A...B"` as a shortcut for
+the merge base of `A` and `B` if there is exactly one merge base. You
+can leave out at most one of `A` and `B`, in which case it defaults to
+`HEAD`.
 
 Note that this will create the new branch, but it will not switch the
 working tree to it; use "git checkout <newbranch>" to switch to the
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 877e5f503a..964f912d29 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -313,6 +313,10 @@ leave out at most one of `A` and `B`, in which case it defaults to `HEAD`.
 <start_point>::
 	The name of a commit at which to start the new branch; see
 	linkgit:git-branch[1] for details. Defaults to HEAD.
++
+As a special case, you may use `"A...B"` as a shortcut for the
+merge base of `A` and `B` if there is exactly one merge base. You can
+leave out at most one of `A` and `B`, in which case it defaults to `HEAD`.
 
 <tree-ish>::
 	Tree to checkout from (when paths are given). If not specified,
diff --git a/branch.c b/branch.c
index 28b81a7e02..a84c8aaca2 100644
--- a/branch.c
+++ b/branch.c
@@ -268,7 +268,7 @@ void create_branch(struct repository *r,
 	}
 
 	real_ref = NULL;
-	if (get_oid(start_name, &oid)) {
+	if (get_oid_mb(start_name, &oid)) {
 		if (explicit_tracking) {
 			if (advice_set_upstream_failure) {
 				error(_(upstream_missing), start_name);
diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index f1c7023e1a..822381dd9d 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -66,6 +66,13 @@ test_expect_success 'checkout -b to a new branch, set to HEAD' '
 	do_checkout branch2
 '
 
+test_expect_success 'checkout -b to a merge base' '
+	test_when_finished "
+		git checkout branch1 &&
+		test_might_fail git branch -D branch2" &&
+	git checkout -b branch2 branch1...
+'
+
 test_expect_success 'checkout -b to a new branch, set to an explicit ref' '
 	test_when_finished "
 		git checkout branch1 &&
@@ -126,6 +133,12 @@ test_expect_success 'checkout -B to an existing branch resets branch to HEAD' '
 	do_checkout branch2 "" -B
 '
 
+test_expect_success 'checkout -B to a merge base' '
+	git checkout branch1 &&
+
+	git checkout -B branch2 branch1...
+'
+
 test_expect_success 'checkout -B to an existing branch from detached HEAD resets branch to HEAD' '
 	git checkout $(git rev-parse --verify HEAD) &&
 
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 478b82cf9b..acb16b62dd 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -42,6 +42,10 @@ test_expect_success 'git branch a/b/c should create a branch' '
 	git branch a/b/c && test_path_is_file .git/refs/heads/a/b/c
 '
 
+test_expect_success 'git branch mb master... should create a branch' '
+	git branch mb master... && test_path_is_file .git/refs/heads/mb
+'
+
 test_expect_success 'git branch HEAD should fail' '
 	test_must_fail git branch HEAD
 '
@@ -292,8 +296,8 @@ test_expect_success 'git branch --list -v with --abbrev' '
 test_expect_success 'git branch --column' '
 	COLUMNS=81 git branch --column=column >actual &&
 	cat >expected <<\EOF &&
-  a/b/c     bam       foo       l       * master    n         o/p       r
-  abc       bar       j/k       m/m       master2   o/o       q
+  a/b/c     bam       foo       l       * master    mb        o/o       q
+  abc       bar       j/k       m/m       master2   n         o/p       r
 EOF
 	test_cmp expected actual
 '
@@ -315,6 +319,7 @@ test_expect_success 'git branch --column with an extremely long branch name' '
   m/m
 * master
   master2
+  mb
   n
   o/o
   o/p
@@ -332,8 +337,8 @@ test_expect_success 'git branch with column.*' '
 	git config --unset column.branch &&
 	git config --unset column.ui &&
 	cat >expected <<\EOF &&
-  a/b/c   bam   foo   l   * master    n     o/p   r
-  abc     bar   j/k   m/m   master2   o/o   q
+  a/b/c   bam   foo   l   * master    mb   o/o   q
+  abc     bar   j/k   m/m   master2   n    o/p   r
 EOF
 	test_cmp expected actual
 '
@@ -357,6 +362,7 @@ test_expect_success 'git branch -v with column.ui ignored' '
   m/m
 * master
   master2
+  mb
   n
   o/o
   o/p
-- 
2.21.0.1000.g11cd861522