commit: do not switch branch during a rebase unless -f is given
PATCH 1/1 commit: do not switch branch during a rebase unless -f is given
It does not make much sense to switch branch when you are in a middle of a rebase. Sometimes you might want to switch away for a moment then back with "git checkout - ". But I find myself so many times switching away then forget that I was rebasing something.
Avoid doing that by default. Users can use -f if they really want to.
---
I know there are other commands like rebase ("git am" comes to mind) but I don't use those. Feel free to put some more on top if somebody finds it a good thing to do.
builtin/checkout.c | 7 +++++++ t/t2018-checkout-branch.sh | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 0 deletions(-)
builtin/checkout.c +7 -0
@@ -571,6 +571,13 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)struct branch_info old;unsigned char rev[20 ];int flag; +struct stat st; + +if (!opts->force && + ((!stat (git_path ("rebase-merge" ), &st) &&S_ISDIR (st.st_mode)) || + (!stat (git_path ("rebase-apply" ), &st) &&S_ISDIR (st.st_mode)))) +die ("You should not switch branch during a rebase. Use '-f' if you really want to." ); +memset (&old,0 ,sizeof (old)); old.path =resolve_ref ("HEAD" , rev,0 , &flag); old.commit =lookup_commit_reference_gently (rev,1 );
t/t2018-checkout-branch.sh +22 -0
@@ -169,4 +169,26 @@ test_expect_success 'checkout -f -B to an existing branch with mergeable changes test_must_fail test_dirty_mergeable ' +test_expect_success 'checkout fails during rebase' ' + git reset --hard && + git checkout branch1 && + mkdir .git/rebase-merge && + test_must_fail git checkout branch2 && + git checkout -f branch2 +' + +test_expect_success 'checkout fails during rebase (2)' ' + rmdir .git/rebase-merge && + git reset --hard && + git checkout branch1 && + mkdir .git/rebase-apply && + test_must_fail git checkout branch2 && + git checkout -f branch2 +' + +# this is to be incorporated to the next test +test_expect_success 'cleanup' ' + rmdir .git/rebase-apply +' + test_done
Signature · 1 line
1.7.1.rc1.70.g788ca
PATCH 1/1 checkout: do not switch branch during a rebase unless -f is given
It does not make much sense to switch branch when you are in a middle of a rebase. Sometimes you might want to switch away for a moment then back with "git checkout - ". But I find myself so many times switching away then forget that I was rebasing something.
Avoid doing that by default. Users can use -f if they really want to.
---
Sorry, the previous patch had wrong subject line
I know there are other commands like rebase ("git am" comes to mind) but I don't use those. Feel free to put some more on top if somebody finds it a good thing to do.
builtin/checkout.c | 7 +++++++ t/t2018-checkout-branch.sh | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 0 deletions(-)
builtin/checkout.c +7 -0
@@ -571,6 +571,13 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)struct branch_info old;unsigned char rev[20 ];int flag; +struct stat st; + +if (!opts->force && + ((!stat (git_path ("rebase-merge" ), &st) &&S_ISDIR (st.st_mode)) || + (!stat (git_path ("rebase-apply" ), &st) &&S_ISDIR (st.st_mode)))) +die ("You should not switch branch during a rebase. Use '-f' if you really want to." ); +memset (&old,0 ,sizeof (old)); old.path =resolve_ref ("HEAD" , rev,0 , &flag); old.commit =lookup_commit_reference_gently (rev,1 );
t/t2018-checkout-branch.sh +22 -0
@@ -169,4 +169,26 @@ test_expect_success 'checkout -f -B to an existing branch with mergeable changes test_must_fail test_dirty_mergeable ' +test_expect_success 'checkout fails during rebase' ' + git reset --hard && + git checkout branch1 && + mkdir .git/rebase-merge && + test_must_fail git checkout branch2 && + git checkout -f branch2 +' + +test_expect_success 'checkout fails during rebase (2)' ' + rmdir .git/rebase-merge && + git reset --hard && + git checkout branch1 && + mkdir .git/rebase-apply && + test_must_fail git checkout branch2 && + git checkout -f branch2 +' + +# this is to be incorporated to the next test +test_expect_success 'cleanup' ' + rmdir .git/rebase-apply +' + test_done
Signature · 1 line
1.7.1.rc1.70.g788ca
Re: [PATCH] checkout: do not switch branch during a rebase unless -f is given
(+cc: Frédéric, Dscho, Christian)
Quoted 6 lines · Nguyễn Thái Ngọc Duy
It does not make much sense to switch branch when you are in a middle of a rebase. Sometimes you might want to switch away for a moment then back with "git checkout - ". But I find myself so many times switching away then forget that I was rebasing something.
Avoid doing that by default. Users can use -f if they really want to.
Nice! But I worry is that scripts that use "git checkout" porcelain during a rebase would be broken.
Even rebase --interactive uses checkout from time to time:
- for preserving merges
- to move to the correct branch in response to "git rebase -i
<upstream> <branch>"
- to move to the target in "git rebase -i --onto <new base> <upstream>"
Unfortunately I do not have any good advice. Would it make sense to
- first, change these call sites in git to use checkout -f
- teach checkout to warn (without erroring out) to give people time
to change their scripts
- warn loudly about the upcoming change in the release notes
- later, change checkout to error out when -f is not supplied
or am I being too paranoid?
Quoted 3 lines
I know there are other commands like rebase ("git am" comes to mind) but I don't use those. Feel free to put some more on top if somebody finds it a good thing to do.
Another interesting one is "git bisect".
Re: [PATCH] checkout: do not switch branch during a rebase unless -f is given
2010/9/28 Jonathan Nieder <[email protected]>:
Quoted 9 lines
(+cc: Frédéric, Dscho, Christian)
Quoted 5 lines · Nguyễn Thái Ngọc Duy
It does not make much sense to switch branch when you are in a middle of a rebase. Sometimes you might want to switch away for a moment then back with "git checkout - ". But I find myself so many times switching away then forget that I was rebasing something.
Quoted 4 lines
Quoted 1 line
Avoid doing that by default. Users can use -f if they really want to.
Nice! But I worry is that scripts that use "git checkout" porcelain during a rebase would be broken.
Right. I used git-rebase from my installation, the new "git checkout" from worktree and missed that as a result.
Quoted 9 lines
Even rebase --interactive uses checkout from time to time:
- for preserving merges
- to move to the correct branch in response to "git rebase -i <upstream> <branch>"
- to move to the target in "git rebase -i --onto <new base> <upstream>"Unfortunately I do not have any good advice. Would it make sense to
Quoted 7 lines
- first, change these call sites in git to use checkout -f
- teach checkout to warn (without erroring out) to give people time to change their scripts
- warn loudly about the upcoming change in the release notes
- later, change checkout to error out when -f is not suppliedor am I being too paranoid?
No. But I don't like the idea of making scripts use "checkout -f". My intention was to stop users from doing that, not scripts. Putting "-f" everywhere might have more negative side effects.
Maybe adding "--porcelain" to checkout first, updating scripts to use it, then only check for rebase/bisect/am when --porcelain is missing.
Signature · 1 line
Duy
Re: [PATCH] checkout: do not switch branch during a rebase unless -f is given
2010/9/28 Nguyen Thai Ngoc Duy <[email protected]>:
Quoted 15 lines
Quoted 15 lines
Even rebase --interactive uses checkout from time to time:
- for preserving merges
- to move to the correct branch in response to "git rebase -i <upstream> <branch>"
- to move to the target in "git rebase -i --onto <new base> <upstream>"Unfortunately I do not have any good advice. Would it make sense to
- first, change these call sites in git to use checkout -f
- teach checkout to warn (without erroring out) to give people time to change their scripts
- warn loudly about the upcoming change in the release notes
- later, change checkout to error out when -f is not supplied
Quoted 8 lines
Quoted 1 line
or am I being too paranoid?
No. But I don't like the idea of making scripts use "checkout -f". My intention was to stop users from doing that, not scripts. Putting "-f" everywhere might have more negative side effects.
Maybe adding "--porcelain" to checkout first, updating scripts to use it, then only check for rebase/bisect/am when --porcelain is missing.
Another approach is to let checkout work as usual, but refuse update refs:
- after rebase starts, HEAD can only be updated either by rebase, or
any commands that keep HEAD a headless ref.
- the branch being rebased is locked. No commands but rebase can update it.
I think the second point is good for all interactive commands like rebase. Create a .lock file with a signature inside (e.g. command name). If update_ref() callers do not give correct signature, refuse to update.
Signature · 1 line
Duy
Re: [PATCH] checkout: do not switch branch during a rebase unless -f is given
Quoted 10 lines · Nguyen Thai Ngoc Duy
Another approach is to let checkout work as usual, but refuse update refs:
- after rebase starts, HEAD can only be updated either by rebase, or
any commands that keep HEAD a headless ref.
- the branch being rebased is locked. No commands but rebase can update it.I think the second point is good for all interactive commands like rebase. Create a .lock file with a signature inside (e.g. command name). If
update_ref()callers do not give correct signature, refuse to update.
I like it. Would it be possible to make sure the (widespread?) practice of using
rm -fr .git/rebase-merge
to terminate a rebase without going back to the original branch still works? I think it should be.
Re: [PATCH] checkout: do not switch branch during a rebase unless -f is given
Heya,
2010/11/9 Jonathan Nieder <[email protected]>:
Quoted 4 lines
I like it. Would it be possible to make sure the (widespread?) practice of using
rm -fr .git/rebase-merge
Awr, and here I was, all hopeful reading your message up to here.
Quoted 2 lines
to terminate a rebase without going back to the original branch still works? I think it should be.
And then here you smash my hopes that you were suggesting we stop asking the user to mess around in the .git directory :(. After all, if we let that rm be handled by some 'git rebase' mode, it could also unlock any branches it has locked.
Signature · 3 lines
Cheers, Sverre Rabbelier
Re: [PATCH] checkout: do not switch branch during a rebase unless -f is given
Quoted 4 lines · Sverre Rabbelier
And then here you smash my hopes that you were suggesting we stop asking the user to mess around in the .git directory :(. After all, if we let that rm be handled by some 'git rebase' mode, it could also unlock any branches it has locked.
Well, I think that is worth doing, too. It's just that it's hard to change existing practices and much easier to change the tools to keep supporting them.
I don't think the manual advertises .git/rebase-merge anywhere. I have witnessed people succumbing to the temptation to delete it when another rebase fails.
Maybe "git rebase" should have a --override-rebase-in-progress option? The following is simpler, sort of imitating the "git bisect reset" command. With it applied, if you do
git rebase --abort HEAD
then the rebase will abort without changing the current HEAD (yes, I know this interface is stupid; please feel free to pick it up and do better).
Other practices worth abstracting away:
- Reading .git/rebase-merge/patch and .git/rebase-merge/message when a patch had conflicts
- Checking for the existence of .git/rebase-merge in $PS1 prompt
- Tweaking .git/rebase-merge/todo to reflect a change of plan
---
Documentation/git-rebase.txt +3 -0
@@ -227,6 +227,9 @@ leave out at most one of A and B, in which case it defaults to HEAD. --abort:: Restore the original branch and abort the rebase operation. +--abort HEAD:: + Abort the rebase operation, without changing the current branch. + --skip:: Restart the rebasing process by skipping the current patch.
git-rebase--interactive.sh +8 -0
@@ -772,6 +772,14 @@ first and then run 'git rebase --continue' again." do_rest ;; --abort) + if test "$2" = HEAD + then + shift + is_standalone "$@" || usage + get_saved_options + rm -rf "$DOTEST" + exit + fi is_standalone "$@" || usage get_saved_options comment_for_reflog abort
git-rebase.sh +5 -0
@@ -273,6 +273,11 @@ do test -d "$dotest" -o -d "$GIT_DIR"/rebase-apply || die "No rebase in progress?" + if test "$2" = HEAD + then + rm -r "$dotest" + exit + fi git rerere clear if test -d "$dotest" then
Re: [PATCH] checkout: do not switch branch during a rebase unless -f is given
In reply to Jonathan Nieder2010/11/9 Jonathan Nieder <[email protected]>:
Quoted 13 lines
Quoted 10 lines · Nguyen Thai Ngoc Duy
Another approach is to let checkout work as usual, but refuse update refs:
- after rebase starts, HEAD can only be updated either by rebase, or any commands that keep HEAD a headless ref.
- the branch being rebased is locked. No commands but rebase can update it.I think the second point is good for all interactive commands like rebase. Create a .lock file with a signature inside (e.g. command name). If
update_ref()callers do not give correct signature, refuse to update.
Quoted 8 lines
I like it. Would it be possible to make sure the (widespread?) practice of using
rm -fr .git/rebase-merge
to terminate a rebase without going back to the original branch still works? I think it should be.
I don't know. If we can place the branch lock inside .git/rebase-merge, then yes removing that directory will also remove the lock. But how does git find the locks if they are everywhere in .git?
Signature · 1 line
Duy
Re: [PATCH] checkout: do not switch branch during a rebase unless -f is given
Quoted 4 lines · Nguyen Thai Ngoc Duy
I don't know. If we can place the branch lock inside .git/rebase-merge, then yes removing that directory will also remove the lock. But how does git find the locks if they are everywhere in .git?
Could the lock signature say something to the effect that "this lock is only valid against commands that don't declare this signature, and only so long as .git/rebase-merge/ref has such-and-such content"?
Quoted 8 lines · Nguyễn Thái Ngọc Duy
It does not make much sense to switch branch when you are in a middle of a rebase. Sometimes you might want to switch away for a moment then back with "git checkout - ". But I find myself so many times switching away then forget that I was rebasing something.
Avoid doing that by default. Users can use -f if they really want to.
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>
I think you need to retitle the patch.
You should realize that overloading "-f" and conflating its totally different meaning, which is to discard local changes, with this new usage to say "override the 'you are in rebase' switch" closes the door to future improvements we could make to rebase so that it does not to require a totally clean working tree. It would be Ok with today's rebase, as you wouldn't have local changes when it runs, but we might regret this 9 months down the road.
Quoted 3 lines
I know there are other commands like rebase ("git am" comes to mind) but I don't use those. Feel free to put some more on top if somebody finds it a good thing to do.
If you envision something similar to be used during "git am", then you really shouldn't be reusing "-f", as "git am" is designed to be usable in a dirty working tree.
Heya,
2010/9/28 Nguyễn Thái Ngọc Duy <[email protected]>:
Quoted 1 line
+ die("You should not switch branch during a rebase. Use '-f' if you really want to.");
On top of Junio's comments, I think it makes sense here to add "or use 'git rebase --abort' to abort the rebase that is in progress".
Signature · 3 lines
Cheers, Sverre Rabbelier