* [PATCH 0/3] DWIM with "git checkout -f -b" @ 2010-06-20 16:33 Tay Ray Chuan 2010-06-20 16:33 ` [PATCH 1/3] add tests for checkout -b Tay Ray Chuan 0 siblings, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-20 16:33 UTC (permalink / raw) To: Git Mailing List Right now, the -f/--force in git-checkout doesn't carry over to -b operations - ie. you can't do git checkout -b branch on an existing branch. This patch series changes this, so that one can do git checkout -f -b branch ref which is equivalent to git branch -f branch ref && git checkout branch Contents: [PATCH 1/3] add tests for checkout -b [PATCH 2/3] t2018-checkout-branch.sh: show that checkout -f -b doesn't DWIM [PATCH 3/3] builtin/checkout: DWIM with -f -b Documentation/git-checkout.txt | 3 + builtin/checkout.c | 4 +- t/t2018-checkout-branch.sh | 86 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 2 deletions(-) create mode 100755 t/t2018-checkout-branch.sh ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] add tests for checkout -b 2010-06-20 16:33 [PATCH 0/3] DWIM with "git checkout -f -b" Tay Ray Chuan @ 2010-06-20 16:33 ` Tay Ray Chuan 2010-06-20 16:33 ` [PATCH 2/3] t2018-checkout-branch.sh: show that checkout -f -b doesn't DWIM Tay Ray Chuan 0 siblings, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-20 16:33 UTC (permalink / raw) To: Git Mailing List Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- t/t2018-checkout-branch.sh | 66 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 66 insertions(+), 0 deletions(-) create mode 100755 t/t2018-checkout-branch.sh diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh new file mode 100755 index 0000000..678a34f --- /dev/null +++ b/t/t2018-checkout-branch.sh @@ -0,0 +1,66 @@ +#!/bin/sh + +test_description='checkout ' + +. ./test-lib.sh + +# Arguments: <branch> <sha> [<checkout options>] +do_checkout() { + exp_branch=$1 && + exp_ref="refs/heads/$exp_branch" && + + # if <sha> is not specified, use HEAD. + exp_sha=${2:-$(git rev-parse --verify HEAD)} && + + git checkout ${3+"$3"} -b $exp_branch $exp_sha && + + test $exp_ref = $(git rev-parse --symbolic-full-name HEAD) && + test $exp_sha = $(git rev-parse --verify HEAD) +} + +test_dirty() { + ! git diff --exit-code >/dev/null +} + +setup_dirty() { + echo >>file1 change2 +} + +test_expect_success 'setup' ' + test_commit initial file1 && + HEAD1=$(git rev-parse --verify HEAD) && + + test_commit change1 file1 && + HEAD2=$(git rev-parse --verify HEAD) && + + git branch -m branch1 +' + +test_expect_success 'checkout -b to a new branch' ' + do_checkout branch2 +' + +test_expect_success 'checkout -b to a new branch (explicit ref)' ' + git checkout branch1 && + git branch -D branch2 && + + do_checkout branch2 $HEAD1 +' + +test_expect_success 'checkout -b to a new branch (dirty)' ' + git checkout branch1 && + git branch -D branch2 && + + setup_dirty && + test_must_fail do_checkout branch2 $HEAD1 && + test_dirty +' + +test_expect_success 'checkout -b to an existing branch fails' ' + git reset --hard HEAD && + git branch branch2 && + + test_must_fail do_checkout branch2 $HEAD2 +' + +test_done -- 1.7.1.513.g4f18 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 2/3] t2018-checkout-branch.sh: show that checkout -f -b doesn't DWIM 2010-06-20 16:33 ` [PATCH 1/3] add tests for checkout -b Tay Ray Chuan @ 2010-06-20 16:33 ` Tay Ray Chuan 2010-06-20 16:33 ` [PATCH 3/3] builtin/checkout: DWIM with -f -b Tay Ray Chuan 0 siblings, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-20 16:33 UTC (permalink / raw) To: Git Mailing List Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- t/t2018-checkout-branch.sh | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index 678a34f..e6c0f8c 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -63,4 +63,24 @@ test_expect_success 'checkout -b to an existing branch fails' ' test_must_fail do_checkout branch2 $HEAD2 ' +test_expect_failure 'checkout -f -b to an existing branch resets branch' ' + git checkout branch1 && + + do_checkout branch2 "" -f +' + +test_expect_failure 'checkout -f -b to an existing branch resets branch (explicit ref)' ' + git checkout branch1 && + + do_checkout branch2 $HEAD1 -f +' + +test_expect_failure 'checkout -f -b to an existing branch resets branch (dirty) ' ' + git checkout branch1 && + + setup_dirty && + do_checkout branch2 $HEAD1 -f && + test_must_fail test_dirty +' + test_done -- 1.7.1.513.g4f18 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 3/3] builtin/checkout: DWIM with -f -b 2010-06-20 16:33 ` [PATCH 2/3] t2018-checkout-branch.sh: show that checkout -f -b doesn't DWIM Tay Ray Chuan @ 2010-06-20 16:33 ` Tay Ray Chuan 2010-06-20 19:12 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-20 16:33 UTC (permalink / raw) To: Git Mailing List Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- Documentation/git-checkout.txt | 3 +++ builtin/checkout.c | 4 ++-- t/t2018-checkout-branch.sh | 6 +++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 261dd90..d15d224 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -74,6 +74,9 @@ entries; instead, unmerged entries are ignored. -b:: Create a new branch named <new_branch> and start it at <start_point>; see linkgit:git-branch[1] for details. ++ +When used with `-f`, the branch is reset; changes in the index or the +working tree are thrown away too. -t:: --track:: diff --git a/builtin/checkout.c b/builtin/checkout.c index 1994be9..962d938 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -511,7 +511,7 @@ static void update_refs_for_switch(struct checkout_opts *opts, } } else - create_branch(old->name, opts->new_branch, new->name, 0, + create_branch(old->name, opts->new_branch, new->name, opts->force, opts->new_branch_log, opts->track); new->name = opts->new_branch; setup_branch_path(new); @@ -858,7 +858,7 @@ no_reference: if (strbuf_check_branch_ref(&buf, opts.new_branch)) die("git checkout: we do not like '%s' as a branch name.", opts.new_branch); - if (!get_sha1(buf.buf, rev)) + if (!get_sha1(buf.buf, rev) && !opts.force) die("git checkout: branch %s already exists", opts.new_branch); strbuf_release(&buf); } diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index e6c0f8c..9895ff5 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -63,19 +63,19 @@ test_expect_success 'checkout -b to an existing branch fails' ' test_must_fail do_checkout branch2 $HEAD2 ' -test_expect_failure 'checkout -f -b to an existing branch resets branch' ' +test_expect_success 'checkout -f -b to an existing branch resets branch' ' git checkout branch1 && do_checkout branch2 "" -f ' -test_expect_failure 'checkout -f -b to an existing branch resets branch (explicit ref)' ' +test_expect_success 'checkout -f -b to an existing branch resets branch (explicit ref)' ' git checkout branch1 && do_checkout branch2 $HEAD1 -f ' -test_expect_failure 'checkout -f -b to an existing branch resets branch (dirty) ' ' +test_expect_success 'checkout -f -b to an existing branch resets branch (dirty) ' ' git checkout branch1 && setup_dirty && -- 1.7.1.513.g4f18 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] builtin/checkout: DWIM with -f -b 2010-06-20 16:33 ` [PATCH 3/3] builtin/checkout: DWIM with -f -b Tay Ray Chuan @ 2010-06-20 19:12 ` Junio C Hamano 2010-06-20 20:11 ` Jeff King 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2010-06-20 19:12 UTC (permalink / raw) To: Tay Ray Chuan; +Cc: Git Mailing List I think this is not "DWIM" but is "-f does not work with -b; fix it". Will queue; thanks. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] builtin/checkout: DWIM with -f -b 2010-06-20 19:12 ` Junio C Hamano @ 2010-06-20 20:11 ` Jeff King 2010-06-20 21:07 ` Clemens Buchacher 2010-06-20 21:10 ` Junio C Hamano 0 siblings, 2 replies; 29+ messages in thread From: Jeff King @ 2010-06-20 20:11 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tay Ray Chuan, Git Mailing List On Sun, Jun 20, 2010 at 12:12:55PM -0700, Junio C Hamano wrote: > I think this is not "DWIM" but is "-f does not work with -b; fix it". > > Will queue; thanks. I am not sure it is fixing a bug. There are really two possible things to be forced during checkout: 1. throw away local changes 2. overwrite an existing branch Right now, "checkout -f" means just (1). This patch makes it _also_ mean (2). Do we want to conflate those two cases? In some sense, it is reasonable for "-f" to mean "force anything that might be questionable". But I wonder if it pays to be a little more conservative given that this is a safety valve we are talking about. The user can of course still split their compound action into: git branch [-f] new_branch old_branch git checkout [-f] new_branch Less convenient, but there is no ambiguity. I dunno. I don't have a strong feeling on how it should be, but I think it is more than a simple bug fix. -Peff ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] builtin/checkout: DWIM with -f -b 2010-06-20 20:11 ` Jeff King @ 2010-06-20 21:07 ` Clemens Buchacher 2010-06-20 21:10 ` Junio C Hamano 1 sibling, 0 replies; 29+ messages in thread From: Clemens Buchacher @ 2010-06-20 21:07 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Tay Ray Chuan, Jeff King On Sun, Jun 20, 2010 at 04:11:38PM -0400, Jeff King wrote: > On Sun, Jun 20, 2010 at 12:12:55PM -0700, Junio C Hamano wrote: > > > I think this is not "DWIM" but is "-f does not work with -b; fix it". Actually, it currently behaves exactly as the manpage suggests. > I am not sure it is fixing a bug. There are really two possible things > to be forced during checkout: > > 1. throw away local changes > > 2. overwrite an existing branch > > Right now, "checkout -f" means just (1). This patch makes it _also_ mean > (2). Do we want to conflate those two cases? > [...] > I dunno. I don't have a strong feeling on how it should be, but I think > it is more than a simple bug fix. I don't have anything to add to that, except that in my opinion checkout does more than enough conflating already, and I am therefore strongly against this change. Clemens ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] builtin/checkout: DWIM with -f -b 2010-06-20 20:11 ` Jeff King 2010-06-20 21:07 ` Clemens Buchacher @ 2010-06-20 21:10 ` Junio C Hamano 2010-06-20 21:16 ` Jeff King 1 sibling, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2010-06-20 21:10 UTC (permalink / raw) To: Jeff King; +Cc: Tay Ray Chuan, Git Mailing List Jeff King <peff@peff.net> writes: > I am not sure it is fixing a bug. There are really two possible things > to be forced during checkout: > > 1. throw away local changes > > 2. overwrite an existing branch > > Right now, "checkout -f" means just (1). This patch makes it _also_ mean > (2). Do we want to conflate those two cases? > > In some sense, it is reasonable for "-f" to mean "force anything that > might be questionable". But I wonder if it pays to be a little more > conservative given that this is a safety valve we are talking about. The > user can of course still split their compound action into: > > git branch [-f] new_branch old_branch > git checkout [-f] new_branch > > Less convenient, but there is no ambiguity. > > I dunno. I don't have a strong feeling on how it should be, but I think > it is more than a simple bug fix. Ok, fair enough. FWIW, I often end up doing this when rebuilding 'pu' (and 'jch' that is used internally that keeps a version of 'next' with cleaner history). ... a script to regenerate jch on top of master (and pu on jch) $ Meta/Reintegrate master..jch >Meta/redo-jch.sh $ Meta/Reintegrate jch..pu >Meta/redo-pu.sh ... rewind and rebuild jch on top of updated master $ git checkout -f -b jch master ... error ... oops $ git branch -f jch master $ git checkout jch $ Meta/redo-jch.sh $ git diff next ... no output---good. $ git checkout -f -b pu jch ... error ... oops $ git branch -f pu jch $ git checkout pu $ Meta/redo-pu.sh ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] builtin/checkout: DWIM with -f -b 2010-06-20 21:10 ` Junio C Hamano @ 2010-06-20 21:16 ` Jeff King 2010-06-21 0:09 ` Junio C Hamano 2010-06-21 2:58 ` [PATCH 3/3] builtin/checkout: DWIM with -f -b Michel Lespinasse 0 siblings, 2 replies; 29+ messages in thread From: Jeff King @ 2010-06-20 21:16 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tay Ray Chuan, Git Mailing List On Sun, Jun 20, 2010 at 02:10:32PM -0700, Junio C Hamano wrote: > FWIW, I often end up doing this when rebuilding 'pu' (and 'jch' that is > used internally that keeps a version of 'next' with cleaner history). > > ... a script to regenerate jch on top of master (and pu on jch) > $ Meta/Reintegrate master..jch >Meta/redo-jch.sh > $ Meta/Reintegrate jch..pu >Meta/redo-pu.sh > > ... rewind and rebuild jch on top of updated master > $ git checkout -f -b jch master > ... error ... oops > $ git branch -f jch master > $ git checkout jch > $ Meta/redo-jch.sh > $ git diff next > ... no output---good. I'm certainly sympathetic, but as I understand it, with this new patch you are doing the equivalent of: git branch -f jch master git checkout -f jch which is different than what you wrote above. For your workflow, I doubt it matters, but it is potentially destructive. -Peff ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] builtin/checkout: DWIM with -f -b 2010-06-20 21:16 ` Jeff King @ 2010-06-21 0:09 ` Junio C Hamano 2010-06-21 1:08 ` Tay Ray Chuan 2010-06-21 2:58 ` [PATCH 3/3] builtin/checkout: DWIM with -f -b Michel Lespinasse 1 sibling, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2010-06-21 0:09 UTC (permalink / raw) To: Jeff King; +Cc: Tay Ray Chuan, Git Mailing List Jeff King <peff@peff.net> writes: > ... For your workflow, I > doubt it matters, but it is potentially destructive. Yes; I thought the implication of "-f" to be destructive would be a justification enough, but I agree with you that conflating the two may be a bad idea. When a user says "git checkout -f -b jch" after seeing the command without "-f" fail due to existing "jch", it is quite clear that the user wants to clobber the history of existing "jch" branch (why else would he giving "-f"), but it is not a justification to clobber local changes he has in the index and the work tree. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] builtin/checkout: DWIM with -f -b 2010-06-21 0:09 ` Junio C Hamano @ 2010-06-21 1:08 ` Tay Ray Chuan 2010-06-21 5:15 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-21 1:08 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, Git Mailing List Hi, On Mon, Jun 21, 2010 at 8:09 AM, Junio C Hamano <gitster@pobox.com> wrote: > Jeff King <peff@peff.net> writes: > >> ... For your workflow, I >> doubt it matters, but it is potentially destructive. > > Yes; I thought the implication of "-f" to be destructive would be a > justification enough, but I agree with you that conflating the two may be > a bad idea. When a user says "git checkout -f -b jch" after seeing the > command without "-f" fail due to existing "jch", it is quite clear that > the user wants to clobber the history of existing "jch" branch (why else > would he giving "-f"), but it is not a justification to clobber local > changes he has in the index and the work tree. How about doing git checkout -f -f -b <branch> ? By having the user to specify "-f" twice, we're can be really sure that the user wants to 1) throw away local changes, which is what the current "-f" is supposed to do, and 2) reset an existing branch - new behaviour. -- Cheers, Ray Chuan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] builtin/checkout: DWIM with -f -b 2010-06-21 1:08 ` Tay Ray Chuan @ 2010-06-21 5:15 ` Junio C Hamano 2010-06-21 17:19 ` [PATCH 0/3] teach git-checkout -B WAS " Tay Ray Chuan 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2010-06-21 5:15 UTC (permalink / raw) To: Tay Ray Chuan; +Cc: Jeff King, Git Mailing List Tay Ray Chuan <rctay89@gmail.com> writes: >> Yes; I thought the implication of "-f" to be destructive would be a >> justification enough, but I agree with you that conflating the two may be >> a bad idea. When a user says "git checkout -f -b jch" after seeing the >> command without "-f" fail due to existing "jch", it is quite clear that >> the user wants to clobber the history of existing "jch" branch (why else >> would he giving "-f"), but it is not a justification to clobber local >> changes he has in the index and the work tree. > > How about doing > > git checkout -f -f -b <branch> That is not the problem I want you to solve. What your patch does solves only half of my issue as Peff pointed out (it lets me reuse the name of the existing branch for an unrelated history, but it loses the local changes I may have in my working tree). IOW, I want to start <branch> from scratch, based on an half-done work I started from my current branch in my work tree. That is what "git checkout -b <branch>" usually gives us, if the name <branch> is not in use. I think you were suggesting not to nuke local change with a single -f only when -b is also in use, but I think that makes things even more confusing to the users. Sometimes a single -f discards local changes, but some other times you would need double -f. That will lead to insanity. Perhaps we would want another option that is similar to -b but lets us discard the named branch if it exists. Let's call it tentatively -B. * git checkout -b <branch> [<start-point>] - starts a new branch <branch> at <start-point> commit; - attempts three-way merge to keep the local change forward while doing so, but aborts if it needs a real file-level merge; - complains and aborts if <branch> already exists. When -f is used, instead of attempting three-way merge to keep the local changes, it discards them. When -m is used, instead of failing when it needs a real file-level merge, it will attempt CVS/SVN "update" style merge (and can cause conflicts). When -B is used instead of -b, it does the three-way merge and the usual rejection upon a file-level merge unless -m is given. If it can go ahead, but <branch> already exists, then the existing <branch> is discarded and replaced with the <start-point> commit. Something like that would reduce the confusion factor and would help the "recreate an existing branch from scratch" (or "reuse the name of an existing branch") workflow a bit. ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 0/3] teach git-checkout -B WAS builtin/checkout: DWIM with -f -b 2010-06-21 5:15 ` Junio C Hamano @ 2010-06-21 17:19 ` Tay Ray Chuan 2010-06-21 17:19 ` [PATCH 1/3] add tests for checkout -b Tay Ray Chuan ` (2 more replies) 0 siblings, 3 replies; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-21 17:19 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse, Erick Mattos Right now, the -f/--force in git-checkout doesn't carry over to -b operations - ie. you can't do git checkout -b branch on an existing branch. This patch allows you to do this with the -B option, so that one can do git checkout -B branch ref which is equivalent to git branch -f branch ref && git checkout branch Contents: [PATCH 1/3] add tests for checkout -b [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT [PATCH 3/3] builtin/checkout: learn -B Tay Ray Chuan (3): add tests for checkout -b builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT builtin/checkout: learn -B Documentation/git-checkout.txt | 15 +++- builtin/checkout.c | 46 +++++++---- t/t2018-checkout-branch.sh | 166 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 209 insertions(+), 18 deletions(-) create mode 100755 t/t2018-checkout-branch.sh ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] add tests for checkout -b 2010-06-21 17:19 ` [PATCH 0/3] teach git-checkout -B WAS " Tay Ray Chuan @ 2010-06-21 17:19 ` Tay Ray Chuan 2010-06-21 17:19 ` [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT Tay Ray Chuan 2010-06-21 17:24 ` [PATCH 0/3] teach git-checkout -B WAS builtin/checkout: DWIM with -f -b Tay Ray Chuan 2010-06-23 19:28 ` [PATCH v2 " Tay Ray Chuan 2 siblings, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-21 17:19 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse, Erick Mattos Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- t/t2018-checkout-branch.sh | 121 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 121 insertions(+), 0 deletions(-) create mode 100755 t/t2018-checkout-branch.sh diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh new file mode 100755 index 0000000..3c13065 --- /dev/null +++ b/t/t2018-checkout-branch.sh @@ -0,0 +1,121 @@ +#!/bin/sh + +test_description='checkout ' + +. ./test-lib.sh + +# Arguments: <branch> <sha> [<checkout options>] +# +# Runs "git checkout" to switch to <branch>, testing that +# +# 1) we are on the specified branch, <branch>; +# 2) HEAD is <sha>; if <sha> is not specified, the old HEAD is used. +# +# If <checkout options> is not specified, "git checkout" is run with -b. +do_checkout() { + exp_branch=$1 && + exp_ref="refs/heads/$exp_branch" && + + # if <sha> is not specified, use HEAD. + exp_sha=${2:-$(git rev-parse --verify HEAD)} && + + # default options for git checkout: -b + if [ -z "$3" ]; then + opts="-b" + else + opts="$3" + fi + + git checkout $opts $exp_branch $exp_sha && + + test $exp_ref = $(git rev-parse --symbolic-full-name HEAD) && + test $exp_sha = $(git rev-parse --verify HEAD) +} + +test_dirty_unmergeable() { + ! git diff --exit-code >/dev/null +} + +setup_dirty_unmergeable() { + echo >>file1 change2 +} + +test_dirty_mergeable() { + ! git diff --cached --exit-code >/dev/null +} + +setup_dirty_mergeable() { + echo >file2 file2 && + git add file2 +} + +test_expect_success 'setup' ' + test_commit initial file1 && + HEAD1=$(git rev-parse --verify HEAD) && + + test_commit change1 file1 && + HEAD2=$(git rev-parse --verify HEAD) && + + git branch -m branch1 +' + +test_expect_success 'checkout -b to a new branch, set to HEAD' ' + do_checkout branch2 +' + +test_expect_success 'checkout -b to a new branch, set to an explicit ref' ' + git checkout branch1 && + 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' ' + # 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 && + + setup_dirty_mergeable && + do_checkout branch2 $HEAD1 && + test_dirty_mergeable +' + +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 && + + 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_must_fail do_checkout branch2 $HEAD2 +' + +test_done -- 1.7.1.513.g4f18 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT 2010-06-21 17:19 ` [PATCH 1/3] add tests for checkout -b Tay Ray Chuan @ 2010-06-21 17:19 ` Tay Ray Chuan 2010-06-21 17:19 ` [PATCH 3/3] builtin/checkout: learn -B Tay Ray Chuan 2010-06-21 23:04 ` [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT Erick Mattos 0 siblings, 2 replies; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-21 17:19 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse, Erick Mattos This is in preparation for multiple levels of "-b". To check this change's interaction with --track, the following tests (obtained with grep -l "\-\-track" t/*.sh)were run, and they passed. t1507-rev-parse-upstream.sh t3200-branch.sh t5505-remote.sh t5520-pull.sh t6040-tracking-info.sh t7201-co.sh (t9114-git-svn-dcommit-merge.sh was excluded as I don't have svn.) Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- builtin/checkout.c | 41 ++++++++++++++++++++++++++--------------- 1 files changed, 26 insertions(+), 15 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 1994be9..e794e1e 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -32,7 +32,8 @@ struct checkout_opts { int writeout_stage; int writeout_error; - const char *new_branch; + int new_branch; + const char *new_branch_name; const char *new_orphan_branch; int new_branch_log; enum branch_track track; @@ -492,7 +493,7 @@ static void update_refs_for_switch(struct checkout_opts *opts, { struct strbuf msg = STRBUF_INIT; const char *old_desc; - if (opts->new_branch) { + if (opts->new_branch_name) { if (opts->new_orphan_branch) { if (opts->new_branch_log && !log_all_ref_updates) { int temp; @@ -511,9 +512,9 @@ static void update_refs_for_switch(struct checkout_opts *opts, } } else - create_branch(old->name, opts->new_branch, new->name, 0, + create_branch(old->name, opts->new_branch_name, new->name, 0, opts->new_branch_log, opts->track); - new->name = opts->new_branch; + new->name = opts->new_branch_name; setup_branch_path(new); } @@ -531,7 +532,7 @@ static void update_refs_for_switch(struct checkout_opts *opts, new->name); else fprintf(stderr, "Switched to%s branch '%s'\n", - opts->new_branch ? " a new" : "", + opts->new_branch_name ? " a new" : "", new->name); } if (old->path && old->name) { @@ -657,7 +658,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) int dwim_new_local_branch = 1; struct option options[] = { OPT__QUIET(&opts.quiet), - OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"), + OPT_SET_INT('b', NULL, &opts.new_branch, + "create and checkout a new branch", 1), OPT_BOOLEAN('l', NULL, &opts.new_branch_log, "log for new branch"), OPT_SET_INT('t', "track", &opts.track, "track", BRANCH_TRACK_EXPLICIT), @@ -692,8 +694,17 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) || opts.new_branch_log || opts.merge || opts.force)) die ("--patch is incompatible with all other options"); + if (opts.new_branch > 0) { + const char *arg = argv[0]; + if (!argc || !strcmp(arg, "--")) + die ("Missing branch name"); + opts.new_branch_name = arg; + argv++; + argc--; + } + /* --track without -b should DWIM */ - if (0 < opts.track && !opts.new_branch) { + if (0 < opts.track && !opts.new_branch_name) { const char *argv0 = argv[0]; if (!argc || !strcmp(argv0, "--")) die ("--track needs a branch name"); @@ -704,7 +715,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) argv0 = strchr(argv0, '/'); if (!argv0 || !argv0[1]) die ("Missing branch name; try -b"); - opts.new_branch = argv0 + 1; + opts.new_branch_name = argv0 + 1; } if (opts.new_orphan_branch) { @@ -712,7 +723,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) die("--orphan and -b are mutually exclusive"); if (opts.track > 0) die("--orphan cannot be used with -t"); - opts.new_branch = opts.new_orphan_branch; + opts.new_branch_name = opts.new_orphan_branch; } if (conflict_style) { @@ -771,13 +782,13 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) if (!patch_mode && dwim_new_local_branch && opts.track == BRANCH_TRACK_UNSPECIFIED && - !opts.new_branch && + !opts.new_branch_name && !check_filename(NULL, arg) && argc == 1) { const char *remote = unique_tracking_name(arg); if (!remote || get_sha1(remote, rev)) goto no_reference; - opts.new_branch = arg; + opts.new_branch_name = arg; arg = remote; /* DWIMmed to create local branch */ } @@ -853,13 +864,13 @@ no_reference: if (patch_mode) return interactive_checkout(new.name, NULL, &opts); - if (opts.new_branch) { + if (opts.new_branch_name) { struct strbuf buf = STRBUF_INIT; - if (strbuf_check_branch_ref(&buf, opts.new_branch)) + if (strbuf_check_branch_ref(&buf, opts.new_branch_name)) die("git checkout: we do not like '%s' as a branch name.", - opts.new_branch); + opts.new_branch_name); if (!get_sha1(buf.buf, rev)) - die("git checkout: branch %s already exists", opts.new_branch); + die("git checkout: branch %s already exists", opts.new_branch_name); strbuf_release(&buf); } -- 1.7.1.513.g4f18 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 3/3] builtin/checkout: learn -B 2010-06-21 17:19 ` [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT Tay Ray Chuan @ 2010-06-21 17:19 ` Tay Ray Chuan 2010-06-23 18:36 ` Junio C Hamano 2010-06-21 23:04 ` [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT Erick Mattos 1 sibling, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-21 17:19 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse, Erick Mattos Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- Documentation/git-checkout.txt | 15 +++++++++++- builtin/checkout.c | 7 ++++- t/t2018-checkout-branch.sh | 45 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 261dd90..5849e13 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -9,7 +9,7 @@ SYNOPSIS -------- [verse] 'git checkout' [-q] [-f] [-m] [<branch>] -'git checkout' [-q] [-f] [-m] [[-b|--orphan] <new_branch>] [<start_point>] +'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>] 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>... 'git checkout' --patch [<tree-ish>] [--] [<paths>...] @@ -21,7 +21,7 @@ also update `HEAD` to set the specified branch as the current branch. 'git checkout' [<branch>]:: -'git checkout' -b <new branch> [<start point>]:: +'git checkout' -b|-B <branch> [<start point>]:: This form switches branches by updating the index, working tree, and HEAD to reflect the specified branch. @@ -31,6 +31,13 @@ were called and then checked out; in this case you can use the `--track` or `--no-track` options, which will be passed to 'git branch'. As a convenience, `--track` without `-b` implies branch creation; see the description of `--track` below. ++ +If `-B` is given, <branch> is created if it doesn't exist; otherwise, it +is reset. This is equivalent to ++ +------------ +$ git branch -f <branch> [<start point>] && git checkout <branch> +------------ 'git checkout' [--patch] [<tree-ish>] [--] <pathspec>...:: @@ -75,6 +82,10 @@ entries; instead, unmerged entries are ignored. Create a new branch named <new_branch> and start it at <start_point>; see linkgit:git-branch[1] for details. +-B:: + Checks out to the branch named <branch>, creating it if it does + not exist; otherwise, the branch is reset. + -t:: --track:: When creating a new branch, set up "upstream" configuration. See diff --git a/builtin/checkout.c b/builtin/checkout.c index e794e1e..f7c2cdd 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -512,7 +512,8 @@ static void update_refs_for_switch(struct checkout_opts *opts, } } else - create_branch(old->name, opts->new_branch_name, new->name, 0, + create_branch(old->name, opts->new_branch_name, new->name, + opts->new_branch > 1, opts->new_branch_log, opts->track); new->name = opts->new_branch_name; setup_branch_path(new); @@ -660,6 +661,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) OPT__QUIET(&opts.quiet), OPT_SET_INT('b', NULL, &opts.new_branch, "create and checkout a new branch", 1), + OPT_SET_INT('B', NULL, &opts.new_branch, + "create and checkout a branch, resetting it if it exists", 2), OPT_BOOLEAN('l', NULL, &opts.new_branch_log, "log for new branch"), OPT_SET_INT('t', "track", &opts.track, "track", BRANCH_TRACK_EXPLICIT), @@ -869,7 +872,7 @@ no_reference: if (strbuf_check_branch_ref(&buf, opts.new_branch_name)) die("git checkout: we do not like '%s' as a branch name.", opts.new_branch_name); - if (!get_sha1(buf.buf, rev)) + if (!get_sha1(buf.buf, rev) && !(opts.new_branch > 1)) die("git checkout: branch %s already exists", opts.new_branch_name); strbuf_release(&buf); } diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index 3c13065..1caffea 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -118,4 +118,49 @@ test_expect_success 'checkout -b to an existing branch fails' ' test_must_fail do_checkout branch2 $HEAD2 ' +test_expect_success 'checkout -B to an existing branch resets branch to HEAD' ' + git checkout branch1 && + + do_checkout branch2 "" -B +' + +test_expect_success 'checkout -B to an existing branch with an explicit ref resets branch to that ref' ' + git checkout branch1 && + + do_checkout branch2 $HEAD1 -B +' + +test_expect_success 'checkout -B to an existing branch with unmergeable changes fails' ' + git checkout branch1 && + + setup_dirty_unmergeable && + test_must_fail do_checkout branch2 $HEAD1 -B && + test_dirty_unmergeable +' + +test_expect_success 'checkout -f -B to an existing branch with unmergeable changes discards changes' ' + # still dirty and on branch1 + do_checkout branch2 $HEAD1 "-f -B" && + test_must_fail test_dirty_unmergeable +' + +test_expect_success 'checkout -B to an existing branch preserves mergeable changes' ' + git checkout branch1 && + + setup_dirty_mergeable && + do_checkout branch2 $HEAD1 -B && + test_dirty_mergeable +' + +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 && + do_checkout branch2 $HEAD1 "-f -B" && + test_must_fail test_dirty_mergeable +' + test_done -- 1.7.1.513.g4f18 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] builtin/checkout: learn -B 2010-06-21 17:19 ` [PATCH 3/3] builtin/checkout: learn -B Tay Ray Chuan @ 2010-06-23 18:36 ` Junio C Hamano 2010-06-23 19:13 ` Tay Ray Chuan 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2010-06-23 18:36 UTC (permalink / raw) To: Tay Ray Chuan Cc: Git Mailing List, Jeff King, Clemens Buchacher, Michel Lespinasse, Erick Mattos Tay Ray Chuan <rctay89@gmail.com> writes: > diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt > index 261dd90..5849e13 100644 > --- a/Documentation/git-checkout.txt > +++ b/Documentation/git-checkout.txt > @@ -31,6 +31,13 @@ were called and then checked out; in this case you can > use the `--track` or `--no-track` options, which will be passed to > 'git branch'. As a convenience, `--track` without `-b` implies branch > creation; see the description of `--track` below. > ++ > +If `-B` is given, <branch> is created if it doesn't exist; otherwise, it > +is reset. This is equivalent to > ++ > +------------ > +$ git branch -f <branch> [<start point>] && git checkout <branch> > +------------ It is somewhat sad if these are truly equivalent. In the above sequence, "git checkout <branch>" could stop to prevent you from clobbering your local changes, but at that point the branch has already been updated. I was hoping that the check (and stop) can be done first and then the branch head is cloberred after you know you will succeed the checkout. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] builtin/checkout: learn -B 2010-06-23 18:36 ` Junio C Hamano @ 2010-06-23 19:13 ` Tay Ray Chuan 0 siblings, 0 replies; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-23 19:13 UTC (permalink / raw) To: Junio C Hamano Cc: Git Mailing List, Jeff King, Clemens Buchacher, Michel Lespinasse, Erick Mattos On Thu, Jun 24, 2010 at 2:36 AM, Junio C Hamano <gitster@pobox.com> wrote: > Tay Ray Chuan <rctay89@gmail.com> writes: > >> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt >> index 261dd90..5849e13 100644 >> --- a/Documentation/git-checkout.txt >> +++ b/Documentation/git-checkout.txt >> @@ -31,6 +31,13 @@ were called and then checked out; in this case you can >> use the `--track` or `--no-track` options, which will be passed to >> 'git branch'. As a convenience, `--track` without `-b` implies branch >> creation; see the description of `--track` below. >> ++ >> +If `-B` is given, <branch> is created if it doesn't exist; otherwise, it >> +is reset. This is equivalent to >> ++ >> +------------ >> +$ git branch -f <branch> [<start point>] && git checkout <branch> >> +------------ > > It is somewhat sad if these are truly equivalent. In the above sequence, > "git checkout <branch>" could stop to prevent you from clobbering your > local changes, but at that point the branch has already been updated. > > I was hoping that the check (and stop) can be done first and then the > branch head is cloberred after you know you will succeed the checkout. hmm - would git checkout <branch> && git reset --keep <start point> make the cut? -- Cheers, Ray Chuan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT 2010-06-21 17:19 ` [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT Tay Ray Chuan 2010-06-21 17:19 ` [PATCH 3/3] builtin/checkout: learn -B Tay Ray Chuan @ 2010-06-21 23:04 ` Erick Mattos 2010-06-23 19:04 ` Tay Ray Chuan 1 sibling, 1 reply; 29+ messages in thread From: Erick Mattos @ 2010-06-21 23:04 UTC (permalink / raw) To: Tay Ray Chuan Cc: Git Mailing List, Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse Hi, 2010/6/21 Tay Ray Chuan <rctay89@gmail.com>: > diff --git a/builtin/checkout.c b/builtin/checkout.c > index 1994be9..e794e1e 100644 > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -32,7 +32,8 @@ struct checkout_opts { > int writeout_stage; > int writeout_error; > > - const char *new_branch; > + int new_branch; > + const char *new_branch_name; The change of name of the existent variable creates more hassle than helps. As you are adding a new option I suggest you to create a new variable named new_branch_forced or whatever. This way you avoid making a lot of changes as you did and minimize the possibility of adding new bugs by not catching all the problems affected by the name change. I think you have chosen to do that just because of the variable names then I think you should find a variable naming alternative to satisfy you without changing existing ones. My suggestion is to do the same I did with --orphan (const char *new_orphan_branch): * Create a C string variable that receives its data by a new OPT_STRING. * After making all tests needed, point new_branch to your created variable. * You will always know if your new option was used or not by checking nullity of the just created C string. This way any existing implementation remains untouched and thus it is much more easy to avoid bugs. > @@ -692,8 +694,17 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) > || opts.new_branch_log || opts.merge || opts.force)) > die ("--patch is incompatible with all other options"); > > + if (opts.new_branch > 0) { > + const char *arg = argv[0]; > + if (!argc || !strcmp(arg, "--")) > + die ("Missing branch name"); > + opts.new_branch_name = arg; > + argv++; > + argc--; > + } You won't use this if you accept my previous suggestions. To conclude, IMHO, I don't think this patch is a good thing to do. /* I would like to point out that I am criticizing it but also presenting suggestions! So it is a constructive critic! ;-) */ Best regards ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT 2010-06-21 23:04 ` [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT Erick Mattos @ 2010-06-23 19:04 ` Tay Ray Chuan 2010-06-23 21:37 ` Erick Mattos 0 siblings, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-23 19:04 UTC (permalink / raw) To: Erick Mattos Cc: Git Mailing List, Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse Hi, On Tue, Jun 22, 2010 at 7:04 AM, Erick Mattos <erick.mattos@gmail.com> wrote: > Hi, > > 2010/6/21 Tay Ray Chuan <rctay89@gmail.com>: >> diff --git a/builtin/checkout.c b/builtin/checkout.c >> index 1994be9..e794e1e 100644 >> --- a/builtin/checkout.c >> +++ b/builtin/checkout.c >> @@ -32,7 +32,8 @@ struct checkout_opts { >> int writeout_stage; >> int writeout_error; >> >> - const char *new_branch; >> + int new_branch; >> + const char *new_branch_name; > > The change of name of the existent variable creates more hassle than > helps. > > As you are adding a new option I suggest you to create a new > variable named new_branch_forced or whatever. This way you avoid > making a lot of changes as you did and minimize the possibility of > adding new bugs by not catching all the problems affected by the > name change. Erick, thanks for the suggestion - I've a new iteration that takes some of your points. -- Cheers, Ray Chuan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT 2010-06-23 19:04 ` Tay Ray Chuan @ 2010-06-23 21:37 ` Erick Mattos 0 siblings, 0 replies; 29+ messages in thread From: Erick Mattos @ 2010-06-23 21:37 UTC (permalink / raw) To: Tay Ray Chuan Cc: Git Mailing List, Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse Hi, 2010/6/23 Tay Ray Chuan <rctay89@gmail.com>: > Erick, thanks for the suggestion - I've a new iteration that takes > some of your points. > > -- > Cheers, > Ray Chuan You are welcome. Good luck then. As I heard here once: thank you for making Git better!. Regards ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/3] teach git-checkout -B WAS builtin/checkout: DWIM with -f -b 2010-06-21 17:19 ` [PATCH 0/3] teach git-checkout -B WAS " Tay Ray Chuan 2010-06-21 17:19 ` [PATCH 1/3] add tests for checkout -b Tay Ray Chuan @ 2010-06-21 17:24 ` Tay Ray Chuan 2010-06-21 21:30 ` Erick Mattos 2010-06-23 19:28 ` [PATCH v2 " Tay Ray Chuan 2 siblings, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-21 17:24 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse, Erick Mattos On Tue, Jun 22, 2010 at 1:19 AM, Tay Ray Chuan <rctay89@gmail.com> wrote: > Right now, the -f/--force in git-checkout doesn't carry over to -b > operations - ie. you can't do git checkout -b branch on an existing > branch. > > This patch allows you to do this with the -B option, so that one can do > > git checkout -B branch ref > > which is equivalent to > > git branch -f branch ref && git checkout branch oops - forgot to add this to the cover letter: Jeff, I've tried my best to check that the --track DWIM-ery still works; could you second this? Erick, does this series change how --orphan should be used? -- Cheers, Ray Chuan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/3] teach git-checkout -B WAS builtin/checkout: DWIM with -f -b 2010-06-21 17:24 ` [PATCH 0/3] teach git-checkout -B WAS builtin/checkout: DWIM with -f -b Tay Ray Chuan @ 2010-06-21 21:30 ` Erick Mattos 0 siblings, 0 replies; 29+ messages in thread From: Erick Mattos @ 2010-06-21 21:30 UTC (permalink / raw) To: Tay Ray Chuan Cc: Git Mailing List, Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse Hi, 2010/6/21 Tay Ray Chuan <rctay89@gmail.com>: > On Tue, Jun 22, 2010 at 1:19 AM, Tay Ray Chuan <rctay89@gmail.com> wrote: >> Right now, the -f/--force in git-checkout doesn't carry over to -b >> operations - ie. you can't do git checkout -b branch on an existing >> branch. >> >> This patch allows you to do this with the -B option, so that one can do >> >> git checkout -B branch ref >> >> which is equivalent to >> >> git branch -f branch ref && git checkout branch If I need to do that, I would do: git checkout branch && git reset --hard ref. Another way to do it is: git branch -D branch && git checkout -b branch ref. So, we have enough porcelain commands to do it. I would recommend a second thought to see if your refinement is really necessary. I would agree so if those procedures are needed very often. > Erick, does this series change how --orphan should be used? If you be careful It won't make any change. I will comment that on the series if needed. Thanks very much for asking. Kind regards ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 0/3] teach git-checkout -B WAS builtin/checkout: DWIM with -f -b 2010-06-21 17:19 ` [PATCH 0/3] teach git-checkout -B WAS " Tay Ray Chuan 2010-06-21 17:19 ` [PATCH 1/3] add tests for checkout -b Tay Ray Chuan 2010-06-21 17:24 ` [PATCH 0/3] teach git-checkout -B WAS builtin/checkout: DWIM with -f -b Tay Ray Chuan @ 2010-06-23 19:28 ` Tay Ray Chuan 2010-06-23 19:28 ` [PATCH v2 1/3] add tests for checkout -b Tay Ray Chuan 2 siblings, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-23 19:28 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse, Erick Mattos Right now, the -f/--force in git-checkout doesn't carry over to -b operations - ie. you can't do git checkout -b branch on an existing branch. This patch allows you to do this with the -B option, so that one can do git checkout -B branch ref which is equivalent to git branch -f branch ref && git checkout branch Changes from v1: - reworked option handling - don't replace <new_branch> with <branch> in documentation Contents: [PATCH v2 1/3] add tests for checkout -b [PATCH v2 2/3] builtin/checkout: reword hint for -b [PATCH v2 3/3] builtin/checkout: learn -B Documentation/git-checkout.txt | 21 +++++- builtin/checkout.c | 32 ++++++-- t/t2018-checkout-branch.sh | 166 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 211 insertions(+), 8 deletions(-) create mode 100755 t/t2018-checkout-branch.sh ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 1/3] add tests for checkout -b 2010-06-23 19:28 ` [PATCH v2 " Tay Ray Chuan @ 2010-06-23 19:28 ` Tay Ray Chuan 2010-06-23 19:28 ` [PATCH v2 2/3] builtin/checkout: reword hint for -b Tay Ray Chuan 0 siblings, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-23 19:28 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse, Erick Mattos Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- t/t2018-checkout-branch.sh | 121 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 121 insertions(+), 0 deletions(-) create mode 100755 t/t2018-checkout-branch.sh diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh new file mode 100755 index 0000000..3c13065 --- /dev/null +++ b/t/t2018-checkout-branch.sh @@ -0,0 +1,121 @@ +#!/bin/sh + +test_description='checkout ' + +. ./test-lib.sh + +# Arguments: <branch> <sha> [<checkout options>] +# +# Runs "git checkout" to switch to <branch>, testing that +# +# 1) we are on the specified branch, <branch>; +# 2) HEAD is <sha>; if <sha> is not specified, the old HEAD is used. +# +# If <checkout options> is not specified, "git checkout" is run with -b. +do_checkout() { + exp_branch=$1 && + exp_ref="refs/heads/$exp_branch" && + + # if <sha> is not specified, use HEAD. + exp_sha=${2:-$(git rev-parse --verify HEAD)} && + + # default options for git checkout: -b + if [ -z "$3" ]; then + opts="-b" + else + opts="$3" + fi + + git checkout $opts $exp_branch $exp_sha && + + test $exp_ref = $(git rev-parse --symbolic-full-name HEAD) && + test $exp_sha = $(git rev-parse --verify HEAD) +} + +test_dirty_unmergeable() { + ! git diff --exit-code >/dev/null +} + +setup_dirty_unmergeable() { + echo >>file1 change2 +} + +test_dirty_mergeable() { + ! git diff --cached --exit-code >/dev/null +} + +setup_dirty_mergeable() { + echo >file2 file2 && + git add file2 +} + +test_expect_success 'setup' ' + test_commit initial file1 && + HEAD1=$(git rev-parse --verify HEAD) && + + test_commit change1 file1 && + HEAD2=$(git rev-parse --verify HEAD) && + + git branch -m branch1 +' + +test_expect_success 'checkout -b to a new branch, set to HEAD' ' + do_checkout branch2 +' + +test_expect_success 'checkout -b to a new branch, set to an explicit ref' ' + git checkout branch1 && + 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' ' + # 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 && + + setup_dirty_mergeable && + do_checkout branch2 $HEAD1 && + test_dirty_mergeable +' + +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 && + + 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_must_fail do_checkout branch2 $HEAD2 +' + +test_done -- 1.7.1.513.g4f18 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 2/3] builtin/checkout: reword hint for -b 2010-06-23 19:28 ` [PATCH v2 1/3] add tests for checkout -b Tay Ray Chuan @ 2010-06-23 19:28 ` Tay Ray Chuan 2010-06-23 19:29 ` [PATCH v2 3/3] builtin/checkout: learn -B Tay Ray Chuan 0 siblings, 1 reply; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-23 19:28 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse, Erick Mattos Shift the 'new' from the param to the hint, and expand the hint. Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- builtin/checkout.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 1994be9..3969683 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -657,7 +657,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) int dwim_new_local_branch = 1; struct option options[] = { OPT__QUIET(&opts.quiet), - OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"), + OPT_STRING('b', NULL, &opts.new_branch, "branch", + "create and checkout a new branch"), OPT_BOOLEAN('l', NULL, &opts.new_branch_log, "log for new branch"), OPT_SET_INT('t', "track", &opts.track, "track", BRANCH_TRACK_EXPLICIT), -- 1.7.1.513.g4f18 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 3/3] builtin/checkout: learn -B 2010-06-23 19:28 ` [PATCH v2 2/3] builtin/checkout: reword hint for -b Tay Ray Chuan @ 2010-06-23 19:29 ` Tay Ray Chuan 0 siblings, 0 replies; 29+ messages in thread From: Tay Ray Chuan @ 2010-06-23 19:29 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Jeff King, Clemens Buchacher, Michel Lespinasse, Erick Mattos Internally, --track and --orphan still use the 'safe' -b, not -B. Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> --- Junio, I didn't wait for your reply to my suggestion about using "git reset" instead, as I hope to put this out for review before I creep to bed - I'll put it in the next iteration, if need be. Documentation/git-checkout.txt | 21 ++++++++++++++++- builtin/checkout.c | 29 +++++++++++++++++++++---- t/t2018-checkout-branch.sh | 45 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 7 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 261dd90..97c5144 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -9,7 +9,7 @@ SYNOPSIS -------- [verse] 'git checkout' [-q] [-f] [-m] [<branch>] -'git checkout' [-q] [-f] [-m] [[-b|--orphan] <new_branch>] [<start_point>] +'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>] 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>... 'git checkout' --patch [<tree-ish>] [--] [<paths>...] @@ -21,7 +21,7 @@ also update `HEAD` to set the specified branch as the current branch. 'git checkout' [<branch>]:: -'git checkout' -b <new branch> [<start point>]:: +'git checkout' -b|-B <new_branch> [<start point>]:: This form switches branches by updating the index, working tree, and HEAD to reflect the specified branch. @@ -31,6 +31,17 @@ were called and then checked out; in this case you can use the `--track` or `--no-track` options, which will be passed to 'git branch'. As a convenience, `--track` without `-b` implies branch creation; see the description of `--track` below. ++ +If `-B` is given, <new_branch> is created if it doesn't exist; otherwise, it +is reset. This is the transactional equivalent of ++ +------------ +$ git branch -f <branch> [<start point>] +$ git checkout <branch> +------------ ++ +that is to say, the branch is not reset/created unless "git checkout" is +successful. 'git checkout' [--patch] [<tree-ish>] [--] <pathspec>...:: @@ -75,6 +86,12 @@ entries; instead, unmerged entries are ignored. Create a new branch named <new_branch> and start it at <start_point>; see linkgit:git-branch[1] for details. +-B:: + Creates the branch <new_branch> and start it at <start_point>; + if it already exists, then reset it to <start_point>. This is + equivalent to running "git branch" with "-f"; see + linkgit:git-branch[1] for details. + -t:: --track:: When creating a new branch, set up "upstream" configuration. See diff --git a/builtin/checkout.c b/builtin/checkout.c index 3969683..ec2ecfa 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -32,7 +32,11 @@ struct checkout_opts { int writeout_stage; int writeout_error; + /* not set by parse_options */ + int branch_exists; + const char *new_branch; + const char *new_branch_force; const char *new_orphan_branch; int new_branch_log; enum branch_track track; @@ -511,7 +515,8 @@ static void update_refs_for_switch(struct checkout_opts *opts, } } else - create_branch(old->name, opts->new_branch, new->name, 0, + create_branch(old->name, opts->new_branch, new->name, + opts->new_branch_force ? 1 : 0, opts->new_branch_log, opts->track); new->name = opts->new_branch; setup_branch_path(new); @@ -531,7 +536,7 @@ static void update_refs_for_switch(struct checkout_opts *opts, new->name); else fprintf(stderr, "Switched to%s branch '%s'\n", - opts->new_branch ? " a new" : "", + opts->branch_exists ? " and reset" : " a new", new->name); } if (old->path && old->name) { @@ -659,6 +664,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) OPT__QUIET(&opts.quiet), OPT_STRING('b', NULL, &opts.new_branch, "branch", "create and checkout a new branch"), + OPT_STRING('B', NULL, &opts.new_branch_force, "branch", + "create/reset and checkout a branch"), OPT_BOOLEAN('l', NULL, &opts.new_branch_log, "log for new branch"), OPT_SET_INT('t', "track", &opts.track, "track", BRANCH_TRACK_EXPLICIT), @@ -689,6 +696,14 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, checkout_usage, PARSE_OPT_KEEP_DASHDASH); + /* we can assume from now on new_branch = !new_branch_force */ + if (opts.new_branch && opts.new_branch_force) + die("-B cannot be used with -b"); + + /* copy -B over to -b, so that we can just check the latter */ + if (opts.new_branch_force) + opts.new_branch = opts.new_branch_force; + if (patch_mode && (opts.track > 0 || opts.new_branch || opts.new_branch_log || opts.merge || opts.force)) die ("--patch is incompatible with all other options"); @@ -710,7 +725,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) if (opts.new_orphan_branch) { if (opts.new_branch) - die("--orphan and -b are mutually exclusive"); + die("--orphan and -b|-B are mutually exclusive"); if (opts.track > 0) die("--orphan cannot be used with -t"); opts.new_branch = opts.new_orphan_branch; @@ -859,8 +874,12 @@ no_reference: if (strbuf_check_branch_ref(&buf, opts.new_branch)) die("git checkout: we do not like '%s' as a branch name.", opts.new_branch); - if (!get_sha1(buf.buf, rev)) - die("git checkout: branch %s already exists", opts.new_branch); + if (!get_sha1(buf.buf, rev)) { + opts.branch_exists = 1; + if (!opts.new_branch_force) + die("git checkout: branch %s already exists", + opts.new_branch); + } strbuf_release(&buf); } diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index 3c13065..1caffea 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -118,4 +118,49 @@ test_expect_success 'checkout -b to an existing branch fails' ' test_must_fail do_checkout branch2 $HEAD2 ' +test_expect_success 'checkout -B to an existing branch resets branch to HEAD' ' + git checkout branch1 && + + do_checkout branch2 "" -B +' + +test_expect_success 'checkout -B to an existing branch with an explicit ref resets branch to that ref' ' + git checkout branch1 && + + do_checkout branch2 $HEAD1 -B +' + +test_expect_success 'checkout -B to an existing branch with unmergeable changes fails' ' + git checkout branch1 && + + setup_dirty_unmergeable && + test_must_fail do_checkout branch2 $HEAD1 -B && + test_dirty_unmergeable +' + +test_expect_success 'checkout -f -B to an existing branch with unmergeable changes discards changes' ' + # still dirty and on branch1 + do_checkout branch2 $HEAD1 "-f -B" && + test_must_fail test_dirty_unmergeable +' + +test_expect_success 'checkout -B to an existing branch preserves mergeable changes' ' + git checkout branch1 && + + setup_dirty_mergeable && + do_checkout branch2 $HEAD1 -B && + test_dirty_mergeable +' + +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 && + do_checkout branch2 $HEAD1 "-f -B" && + test_must_fail test_dirty_mergeable +' + test_done -- 1.7.1.513.g4f18 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] builtin/checkout: DWIM with -f -b 2010-06-20 21:16 ` Jeff King 2010-06-21 0:09 ` Junio C Hamano @ 2010-06-21 2:58 ` Michel Lespinasse 2010-06-21 4:09 ` Michel Lespinasse 1 sibling, 1 reply; 29+ messages in thread From: Michel Lespinasse @ 2010-06-21 2:58 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Tay Ray Chuan, Git Mailing List On Sun, Jun 20, 2010 at 2:16 PM, Jeff King <peff@peff.net> wrote: > I'm certainly sympathetic, but as I understand it, with this new patch > you are doing the equivalent of: > > git branch -f jch master > git checkout -f jch > > which is different than what you wrote above. For your workflow, I > doubt it matters, but it is potentially destructive. I am frequently trying to do this as well, so I would love the checkout -f -b option. However, I think that the desired behavior for checkout -f -b is that it should be equivalent to: git branch -f jch master git checkout jch (without the -f here) rationale: git checkout -b normally preserves the index & the working tree; adding the -f flag to a command that would otherwise succeed should not change its behavior. -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] builtin/checkout: DWIM with -f -b 2010-06-21 2:58 ` [PATCH 3/3] builtin/checkout: DWIM with -f -b Michel Lespinasse @ 2010-06-21 4:09 ` Michel Lespinasse 0 siblings, 0 replies; 29+ messages in thread From: Michel Lespinasse @ 2010-06-21 4:09 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Tay Ray Chuan, Git Mailing List On Sun, Jun 20, 2010 at 7:58 PM, Michel Lespinasse <walken@google.com> wrote: > On Sun, Jun 20, 2010 at 2:16 PM, Jeff King <peff@peff.net> wrote: >> I'm certainly sympathetic, but as I understand it, with this new patch >> you are doing the equivalent of: >> >> git branch -f jch master >> git checkout -f jch >> >> which is different than what you wrote above. For your workflow, I >> doubt it matters, but it is potentially destructive. > > I am frequently trying to do this as well, so I would love the > checkout -f -b option. > > However, I think that the desired behavior for checkout -f -b is that > it should be equivalent to: > git branch -f jch master > git checkout jch (without the -f here) Rhaaa, that actually wouldn't work as the git checkout command would fail if there are changes in the index & working tree. What I want here is actually for git checkout -f -b jch to be equivalent to: git branch -D jch (ignore error if jch branch does not exist) git checkout -b jch > rationale: git checkout -b normally preserves the index & the working > tree; adding the -f flag to a command that would otherwise succeed > should not change its behavior. At least the rationale still holds :) -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2010-06-23 21:37 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-06-20 16:33 [PATCH 0/3] DWIM with "git checkout -f -b" Tay Ray Chuan 2010-06-20 16:33 ` [PATCH 1/3] add tests for checkout -b Tay Ray Chuan 2010-06-20 16:33 ` [PATCH 2/3] t2018-checkout-branch.sh: show that checkout -f -b doesn't DWIM Tay Ray Chuan 2010-06-20 16:33 ` [PATCH 3/3] builtin/checkout: DWIM with -f -b Tay Ray Chuan 2010-06-20 19:12 ` Junio C Hamano 2010-06-20 20:11 ` Jeff King 2010-06-20 21:07 ` Clemens Buchacher 2010-06-20 21:10 ` Junio C Hamano 2010-06-20 21:16 ` Jeff King 2010-06-21 0:09 ` Junio C Hamano 2010-06-21 1:08 ` Tay Ray Chuan 2010-06-21 5:15 ` Junio C Hamano 2010-06-21 17:19 ` [PATCH 0/3] teach git-checkout -B WAS " Tay Ray Chuan 2010-06-21 17:19 ` [PATCH 1/3] add tests for checkout -b Tay Ray Chuan 2010-06-21 17:19 ` [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT Tay Ray Chuan 2010-06-21 17:19 ` [PATCH 3/3] builtin/checkout: learn -B Tay Ray Chuan 2010-06-23 18:36 ` Junio C Hamano 2010-06-23 19:13 ` Tay Ray Chuan 2010-06-21 23:04 ` [PATCH 2/3] builtin/checkout: change -b from an OPTION_STRING to a OPTION_SET_INT Erick Mattos 2010-06-23 19:04 ` Tay Ray Chuan 2010-06-23 21:37 ` Erick Mattos 2010-06-21 17:24 ` [PATCH 0/3] teach git-checkout -B WAS builtin/checkout: DWIM with -f -b Tay Ray Chuan 2010-06-21 21:30 ` Erick Mattos 2010-06-23 19:28 ` [PATCH v2 " Tay Ray Chuan 2010-06-23 19:28 ` [PATCH v2 1/3] add tests for checkout -b Tay Ray Chuan 2010-06-23 19:28 ` [PATCH v2 2/3] builtin/checkout: reword hint for -b Tay Ray Chuan 2010-06-23 19:29 ` [PATCH v2 3/3] builtin/checkout: learn -B Tay Ray Chuan 2010-06-21 2:58 ` [PATCH 3/3] builtin/checkout: DWIM with -f -b Michel Lespinasse 2010-06-21 4:09 ` Michel Lespinasse
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.