* [PATCH v2 1/3] t2018: cleanup in current test
2019-04-26 19:21 ` [PATCH v2 0/3] allow checkout and branch to create branches " Denton Liu
@ 2019-04-26 19:21 ` Denton Liu
2019-04-26 19:21 ` [PATCH v2 2/3] t2018: demonstrate checkout -b merge base bug Denton Liu
` (3 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Denton Liu @ 2019-04-26 19:21 UTC (permalink / raw)
To: Git Mailing List; +Cc: Eric Sunshine, Junio C Hamano
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 2/3] t2018: demonstrate checkout -b merge base bug
2019-04-26 19:21 ` [PATCH v2 0/3] allow checkout and branch to create branches " Denton Liu
2019-04-26 19:21 ` [PATCH v2 1/3] t2018: cleanup in current test Denton Liu
@ 2019-04-26 19:21 ` Denton Liu
2019-04-26 19:21 ` [PATCH v2 3/3] branch: make create_branch accept a merge base rev Denton Liu
` (2 subsequent siblings)
4 siblings, 0 replies; 21+ messages in thread
From: Denton Liu @ 2019-04-26 19:21 UTC (permalink / raw)
To: Git Mailing List; +Cc: Eric Sunshine, Junio C Hamano
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 3/3] branch: make create_branch accept a merge base rev
2019-04-26 19:21 ` [PATCH v2 0/3] allow checkout and branch to create branches " Denton Liu
2019-04-26 19:21 ` [PATCH v2 1/3] t2018: cleanup in current test Denton Liu
2019-04-26 19:21 ` [PATCH v2 2/3] t2018: demonstrate checkout -b merge base bug Denton Liu
@ 2019-04-26 19:21 ` Denton Liu
2019-04-26 23:07 ` [PATCH v2 0/3] allow checkout and branch to create branches on a merge base Junio C Hamano
2019-04-27 12:02 ` [PATCH v3 0/2] " Denton Liu
4 siblings, 0 replies; 21+ messages in thread
From: Denton Liu @ 2019-04-26 19:21 UTC (permalink / raw)
To: Git Mailing List; +Cc: Eric Sunshine, Junio C Hamano
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/3] allow checkout and branch to create branches on a merge base
2019-04-26 19:21 ` [PATCH v2 0/3] allow checkout and branch to create branches " Denton Liu
` (2 preceding siblings ...)
2019-04-26 19:21 ` [PATCH v2 3/3] branch: make create_branch accept a merge base rev Denton Liu
@ 2019-04-26 23:07 ` Junio C Hamano
2019-04-26 23:40 ` Denton Liu
2019-04-27 12:02 ` [PATCH v3 0/2] " Denton Liu
4 siblings, 1 reply; 21+ messages in thread
From: Junio C Hamano @ 2019-04-26 23:07 UTC (permalink / raw)
To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/3] allow checkout and branch to create branches on a merge base
2019-04-26 23:07 ` [PATCH v2 0/3] allow checkout and branch to create branches on a merge base Junio C Hamano
@ 2019-04-26 23:40 ` Denton Liu
2019-04-26 23:52 ` Denton Liu
0 siblings, 1 reply; 21+ messages in thread
From: Denton Liu @ 2019-04-26 23:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Eric Sunshine
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/3] allow checkout and branch to create branches on a merge base
2019-04-26 23:40 ` Denton Liu
@ 2019-04-26 23:52 ` Denton Liu
2019-04-27 0:01 ` Junio C Hamano
0 siblings, 1 reply; 21+ messages in thread
From: Denton Liu @ 2019-04-26 23:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Eric Sunshine
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/3] allow checkout and branch to create branches on a merge base
2019-04-26 23:52 ` Denton Liu
@ 2019-04-27 0:01 ` Junio C Hamano
0 siblings, 0 replies; 21+ messages in thread
From: Junio C Hamano @ 2019-04-27 0:01 UTC (permalink / raw)
To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 0/2] allow checkout and branch to create branches on a merge base
2019-04-26 19:21 ` [PATCH v2 0/3] allow checkout and branch to create branches " Denton Liu
` (3 preceding siblings ...)
2019-04-26 23:07 ` [PATCH v2 0/3] allow checkout and branch to create branches on a merge base Junio C Hamano
@ 2019-04-27 12:02 ` Denton Liu
2019-04-27 12:02 ` [PATCH v3 1/2] t2018: cleanup in current test Denton Liu
2019-04-27 12:02 ` [PATCH v3 2/2] branch: make create_branch accept a merge base rev Denton Liu
4 siblings, 2 replies; 21+ messages in thread
From: Denton Liu @ 2019-04-27 12:02 UTC (permalink / raw)
To: Git Mailing List; +Cc: Eric Sunshine, Junio C Hamano
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 1/2] t2018: cleanup in current test
2019-04-27 12:02 ` [PATCH v3 0/2] " Denton Liu
@ 2019-04-27 12:02 ` Denton Liu
2019-04-27 12:02 ` [PATCH v3 2/2] branch: make create_branch accept a merge base rev Denton Liu
1 sibling, 0 replies; 21+ messages in thread
From: Denton Liu @ 2019-04-27 12:02 UTC (permalink / raw)
To: Git Mailing List; +Cc: Eric Sunshine, Junio C Hamano
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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 2/2] branch: make create_branch accept a merge base rev
2019-04-27 12:02 ` [PATCH v3 0/2] " Denton Liu
2019-04-27 12:02 ` [PATCH v3 1/2] t2018: cleanup in current test Denton Liu
@ 2019-04-27 12:02 ` Denton Liu
1 sibling, 0 replies; 21+ messages in thread
From: Denton Liu @ 2019-04-27 12:02 UTC (permalink / raw)
To: Git Mailing List; +Cc: Eric Sunshine, Junio C Hamano
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
^ permalink raw reply related [flat|nested] 21+ messages in thread