rebase: apply and cleanup autostash when rebase fails to start
PATCH 1/1 rebase: apply and cleanup autostash when rebase fails to start
From: Phillip Wood <[email protected]>
If "git rebase" fails to start after stashing the user's uncommitted changes then it forgets to restore the stashed changes and remove state directory. To make matters worse running "git rebase --abort" to apply the stashed changes and cleanup the state directory fails because the state directory only contains the "autostash" file and is missing the "head-name" and "onto" required by read_basic_state().
Fix this by applying the autostash and removing the state directory if the pre-rebase hook or initial checkout fail. New tests are added to check the autostash is applied and the state directory is removed if the rebase fails to start. Checks are also added to some existing tests in order to ensure there is no state directory left behind when a rebase fails to start and no autostash has been created.
---
rebase: apply and cleanup autostash when rebase fails to start
Thanks to Brian for reporting this. This patch is based on maint.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1772%2Fphillipwood%2Frebase-fix-autostash-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1772/phillipwood/rebase-fix-autostash-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1772
builtin/rebase.c | 39 +++++++++++++++++++++++++++++------- t/t3400-rebase.sh | 8 ++++++-- t/t3413-rebase-hook.sh | 4 +++- t/t3420-rebase-autostash.sh | 40 +++++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 10 deletions(-)
builtin/rebase.c +32 -7
@@ -526,6 +526,23 @@ static int rebase_write_basic_state(struct rebase_options *opts)return 0 ; } +static int cleanup_autostash (struct rebase_options *opts) +{ +int ret; +struct strbuf dir =STRBUF_INIT ; +const char *path =state_dir_path ("autostash" , opts); + +if (!file_exists (path)) +return 0 ; + ret =apply_autostash (path); +strbuf_addstr (&dir, opts->state_dir); +if (remove_dir_recursively (&dir,0 )) + ret =error_errno (_ ("could not remove '%s'" ), opts->state_dir); +strbuf_release (&dir); + +return ret; +} +static int finish_rebase (struct rebase_options *opts) {struct strbuf dir =STRBUF_INIT ;
@@ -1726,7 +1743,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)if (require_clean_work_tree (the_repository,"rebase" ,_ ("Please commit or stash them." ),1 ,1 )) { ret =-1 ; - goto cleanup; + goto cleanup_autostash; }/*
@@ -1749,7 +1766,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)if (options.switch_to) { ret =checkout_up_to_date (&options);if (ret) - goto cleanup; + goto cleanup_autostash; }if (!(options.flags &REBASE_NO_QUIET ))
@@ -1775,8 +1792,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)/* If a hook exists, give it a chance to interrupt*/ if (!ok_to_skip_pre_rebase &&run_hooks_l ("pre-rebase" , options.upstream_arg, - argc ? argv[0 ] :NULL ,NULL )) -die (_ ("The pre-rebase hook refused to rebase." )); + argc ? argv[0 ] :NULL ,NULL )) { + ret =error (_ ("The pre-rebase hook refused to rebase." )); + goto cleanup_autostash; + }if (options.flags &REBASE_DIFFSTAT ) {struct diff_options opts;
@@ -1821,9 +1840,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)RESET_HEAD_RUN_POST_CHECKOUT_HOOK ; ropts.head_msg = msg.buf; ropts.default_reflog_action = options.reflog_action; -if (reset_head (the_repository, &ropts)) -die (_ ("Could not detach HEAD" )); -strbuf_release (&msg); +if (reset_head (the_repository, &ropts)) { + ret =error (_ ("Could not detach HEAD" )); + goto cleanup_autostash; + }/* * If the onto is a proper descendant of the tip of the branch, then
@@ -1851,9 +1871,14 @@ run_rebase: cleanup:strbuf_release (&buf); +strbuf_release (&msg);strbuf_release (&revisions);rebase_options_release (&options);free (squash_onto_name);free (keep_base_onto_name);return !!ret; + +cleanup_autostash: + ret |= !!cleanup_autostash (&options); + goto cleanup; }
t/t3400-rebase.sh +6 -2
@@ -145,7 +145,9 @@ test_expect_success 'Show verbose error when HEAD could not be detached' ' test_when_finished "rm -f B" && test_must_fail git rebase topic 2>output.err >output.out && test_grep "The following untracked working tree files would be overwritten by checkout:" output.err && - test_grep B output.err + test_grep B output.err && + test_must_fail git rebase --quit 2>err && + test_grep "no rebase in progress" err ' test_expect_success 'fail when upstream arg is missing and not on branch' '
@@ -422,7 +424,9 @@ test_expect_success 'refuse to switch to branch checked out elsewhere' ' git checkout main && git worktree add wt && test_must_fail git -C wt rebase main main 2>err && - test_grep "already used by worktree at" err + test_grep "already used by worktree at" err && + test_must_fail git -C wt rebase --quit 2>err && + test_grep "no rebase in progress" err ' test_expect_success 'rebase when inside worktree subdirectory' '
t/t3413-rebase-hook.sh +3 -1
@@ -110,7 +110,9 @@ test_expect_success 'pre-rebase hook stops rebase (1)' ' git reset --hard side && test_must_fail git rebase main && test "z$(git symbolic-ref HEAD)" = zrefs/heads/test && - test 0 = $(git rev-list HEAD...side | wc -l) + test 0 = $(git rev-list HEAD...side | wc -l) && + test_must_fail git rebase --quit 2>err && + test_grep "no rebase in progress" err ' test_expect_success 'pre-rebase hook stops rebase (2)' '
t/t3420-rebase-autostash.sh +40 -0
@@ -82,6 +82,46 @@ testrebase () { type=$1 dotest=$2 + test_expect_success "rebase$type: restore autostash when pre-rebase hook fails" ' + git checkout -f feature-branch && + test_hook pre-rebase <<-\EOF && + exit 1 + EOF + + echo changed >file0 && + test_must_fail git rebase $type --autostash -f HEAD^ && + test_must_fail git rebase --quit 2>err && + test_grep "no rebase in progress" err && + echo changed >expect && + test_cmp expect file0 + ' + + test_expect_success "rebase$type: restore autostash when checkout onto fails" ' + git checkout -f --detach feature-branch && + echo uncommitted-content >file0 && + echo untracked >file4 && + test_when_finished "rm file4" && + test_must_fail git rebase $type --autostash \ + unrelated-onto-branch && + test_must_fail git rebase --quit 2>err && + test_grep "no rebase in progress" err && + echo uncommitted-content >expect && + test_cmp expect file0 + ' + + test_expect_success "rebase$type: restore autostash when branch checkout fails" ' + git checkout -f unrelated-onto-branch^ && + echo uncommitted-content >file0 && + echo untracked >file4 && + test_when_finished "rm file4" && + test_must_fail git rebase $type --autostash HEAD \ + unrelated-onto-branch && + test_must_fail git rebase --quit 2>err && + test_grep "no rebase in progress" err && + echo uncommitted-content >expect && + test_cmp expect file0 + ' + test_expect_success "rebase$type: dirty worktree, --no-autostash" ' test_config rebase.autostash true && git reset --hard &&
base-commit: 39bf06adf96da25b87c9aa7d35a32ef3683eb4a4
Signature · 1 line
gitgitgadget
Quoted 4 lines · Phillip Wood via GitGitGadget
From: Phillip Wood <[email protected]>
If "git rebase" fails to start after stashing the user's uncommitted changes then it forgets to restore the stashed changes and remove state
s/remove/& the/
Quoted 1 line
directory. To make matters worse running "git rebase --abort" to apply
s/worse/&,/
Quoted 1 line
the stashed changes and cleanup the state directory fails because the
s/cleanup/& of/
Quoted 10 lines
builtin/rebase.c +2
-0@@ -526,6 +526,23 @@ static int rebase_write_basic_state(struct rebase_options *opts)return 0 ; } +static int cleanup_autostash (struct rebase_options *opts) +{
builtin/rebase.c static int rebase_write_basic_state(struct rebase_options *opts) · line 540
Quoted 10 lines
builtin/rebase.c +10
-0+int ret; +struct strbuf dir =STRBUF_INIT ; +const char *path =state_dir_path ("autostash" , opts); + +if (!file_exists (path)) +return 0 ; + ret =apply_autostash (path); +strbuf_addstr (&dir, opts->state_dir); +if (remove_dir_recursively (&dir,0 )) + ret =error_errno (_ ("could not remove '%s'" ), opts->state_dir);
This seems somewhat dangerous to me though. Should we really delete the autostash directory in case applying the autostash has failed?
Quoted 5 lines
builtin/rebase.c +1
-0@@ -1851,9 +1871,14 @@ run_rebase: cleanup:strbuf_release (&buf); +strbuf_release (&msg);
builtin/rebase.c run_rebase: · line 1884
Quoted 10 lines
builtin/rebase.c +4
-0strbuf_release (&revisions);rebase_options_release (&options);free (squash_onto_name);free (keep_base_onto_name);return !!ret; + +cleanup_autostash: + ret |= !!cleanup_autostash (&options); + goto cleanup; }
It's somewhat curious of a code flow to jump backwards. It might be easier to reason about the flow if we kept track of a variable autostash_needs_cleanup that we set such that all jumps can go to the cleanup label instead.
Patrick
Hi Patrick
Quoted 8 lines · Patrick Steinhardt
Quoted 4 lines · Phillip Wood via GitGitGadget
From: Phillip Wood <[email protected]>
If "git rebase" fails to start after stashing the user's uncommitted changes then it forgets to restore the stashed changes and remove state
s/remove/& the/
Quoted 7 lines
Quoted 1 line
directory. To make matters worse running "git rebase --abort" to apply
s/worse/&,/
Quoted 1 line
the stashed changes and cleanup the state directory fails because the
s/cleanup/& of/
Thanks for catching those typos
Quoted 8 lines
Quoted 8 lines
builtin/rebase.c +0
-0@@ -526,6 +526,23 @@ static int rebase_write_basic_state(struct rebase_options *opts)return 0 ; }
Quoted 15 lines
builtin/rebase.c static int rebase_write_basic_state(struct rebase_options *opts) · line 540
Quoted 12 lines
builtin/rebase.c +12
-0+static int cleanup_autostash (struct rebase_options *opts) +{ +int ret; +struct strbuf dir =STRBUF_INIT ; +const char *path =state_dir_path ("autostash" , opts); + +if (!file_exists (path)) +return 0 ; + ret =apply_autostash (path); +strbuf_addstr (&dir, opts->state_dir); +if (remove_dir_recursively (&dir,0 )) + ret =error_errno (_ ("could not remove '%s'" ), opts->state_dir);This seems somewhat dangerous to me though. Should we really delete the autostash directory in case applying the autostash has failed?
Applying the stash should not fail because the rebase has not started and so HEAD, the index and the worktree are unchanged since the stash was created. If it does fail for some reason then apply_autostash() creates a new entry under refs/stash. We definitely do want to remove the directory otherwise we're left with the inconsistent state we're tying to fix.
Quoted 12 lines
Quoted 12 lines
builtin/rebase.c +3
-0@@ -1851,9 +1871,14 @@ run_rebase: cleanup:strbuf_release (&buf); +strbuf_release (&msg);strbuf_release (&revisions);rebase_options_release (&options);free (squash_onto_name);free (keep_base_onto_name);return !!ret; + +cleanup_autostash:
Quoted 8 lines
builtin/rebase.c run_rebase: · line 1884
Quoted 3 lines
builtin/rebase.c +2
-0+ ret |= !!cleanup_autostash (&options); + goto cleanup; }It's somewhat curious of a code flow to jump backwards. It might be easier to reason about the flow if we kept track of a variable
autostash_needs_cleanupthat we set such that all jumps can go to thecleanuplabel instead.
It is slightly odd, but in the end I decided it was simpler to say "goto cleanup_autostash" than try and keep track of what needs cleaning up when saying "goto cleanup". There are a couple of similar examples in builtin/stash.c:show_stash() and config.c:git_config_set_multivar_in_file_gently()
Thanks for the review
Phillip
Quoted 2 lines
Patrick
Quoted 6 lines · Phillip Wood
Applying the stash should not fail because the rebase has not started and so HEAD, the index and the worktree are unchanged since the stash was created. If it does fail for some reason then
apply_autostash()creates a new entry under refs/stash. We definitely do want to remove the directory otherwise we're left with the inconsistent state we're tying to fix.
If it is not expected to fail 99% of times, it feels more prudent to abort loudly without making further damage to lose information and ask the user to check what happened in the working tree, rather than blindly removing the clue to understand what went wrong. For example, could the reason why applying the stash failed be because the user forgot that the working tree was being used for rebasing and mucked with its contents from say another terminal?
Thanks.
Hi Junio
Quoted 9 lines · Junio C Hamano
Quoted 6 lines · Phillip Wood
Applying the stash should not fail because the rebase has not started and so HEAD, the index and the worktree are unchanged since the stash was created. If it does fail for some reason then
apply_autostash()creates a new entry under refs/stash. We definitely do want to remove the directory otherwise we're left with the inconsistent state we're tying to fix.
Quoted 7 lines
If it is not expected to fail 99% of times, it feels more prudent to abort loudly without making further damage to lose information and ask the user to check what happened in the working tree, rather than blindly removing the clue to understand what went wrong. For example, could the reason why applying the stash failed be because the user forgot that the working tree was being used for rebasing and mucked with its contents from say another terminal?
If the working tree has changed then the stash will still apply but possibly with conflicts - the same thing can happen when the branch has been successfully rebased. If there are conflicts then the stash is saved in refs/stash as well. This code is just doing what we do at the end of a successful rebase so I'm don't really understand what the issue is. Looking at finish_rebase() we don't even check the return value of apply_autostash() when applying the stash at the end of a successful rebase.
Best Wishes
Phillip
Quoted 2 lines
Thanks.
PATCH v2 1/1 rebase: apply and cleanup autostash when rebase fails to start
In reply to Phillip Wood via GitGitGadgetFrom: Phillip Wood <[email protected]>
If "git rebase" fails to start after stashing the user's uncommitted changes then it forgets to restore the stashed changes and remove the state directory. To make matters worse, running "git rebase --abort" to apply the stashed changes and cleanup the state directory fails because the state directory only contains the "autostash" file and is missing the "head-name" and "onto" files required by read_basic_state().
Fix this by applying the autostash and removing the state directory if the pre-rebase hook or initial checkout fail. This matches what finish_rebase() does at the end of a successful rebase. If the user modifies any files after the autostash is created it is possible there will be conflicts when the autostash is applied. In that case apply_autostash() saves the stash in a new entry under refs/stash and so it is safe to remove the state directory containing the autostash file.
New tests are added to check the autostash is applied and the state directory is removed if the rebase fails to start. Checks are also added to some existing tests in order to ensure there is no state directory left behind when a rebase fails to start and no autostash has been created.
---
rebase: apply and cleanup autostash when rebase fails to start
Thanks to Junio and Patrick for their comments on V1. I've updated the commit message to correct the typos found by Patrick and added an explanation of why it is safe to remove the state directory.
In his review Patrick suggested removing the backwards jump after applying the autostash and cleaning up the state directory in favor of a variable that tracks if we need to apply the autostash. I've decided not to do that as I think having to set a variable before jumping to a cleanup label is more complicated and error prone. There are similar backward jumps in builtin/stash.c:show_stash() and config.c:git_config_set_multivar_in_file_gently()
Thanks to Brian for reporting this. This patch is based on maint, when merging with master there is an easily resolved conflict with a change from 'ps/config-wo-the-repository' which modifies an adjacent line.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1772%2Fphillipwood%2Frebase-fix-autostash-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1772/phillipwood/rebase-fix-autostash-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1772
Range-diff vs v1:
1: f5b46082047 ! 1: c76c05092aa rebase: apply and cleanup autostash when rebase fails to start
@@ Commit message
rebase: apply and cleanup autostash when rebase fails to start
If "git rebase" fails to start after stashing the user's uncommitted
- changes then it forgets to restore the stashed changes and remove state
- directory. To make matters worse running "git rebase --abort" to apply
- the stashed changes and cleanup the state directory fails because the
- state directory only contains the "autostash" file and is missing the
- "head-name" and "onto" required by read_basic_state().
+ changes then it forgets to restore the stashed changes and remove the
+ state directory. To make matters worse, running "git rebase --abort" to
+ apply the stashed changes and cleanup the state directory fails because
+ the state directory only contains the "autostash" file and is missing
+ the "head-name" and "onto" files required by read_basic_state().
Fix this by applying the autostash and removing the state directory if
- the pre-rebase hook or initial checkout fail. New tests are added to
- check the autostash is applied and the state directory is removed if the
- rebase fails to start. Checks are also added to some existing tests in
- order to ensure there is no state directory left behind when a rebase
- fails to start and no autostash has been created.
+ the pre-rebase hook or initial checkout fail. This matches what
+ finish_rebase() does at the end of a successful rebase. If the user
+ modifies any files after the autostash is created it is possible there
+ will be conflicts when the autostash is applied. In that case
+ apply_autostash() saves the stash in a new entry under refs/stash and so
+ it is safe to remove the state directory containing the autostash file.
+
+ New tests are added to check the autostash is applied and the state
+ directory is removed if the rebase fails to start. Checks are also added
+ to some existing tests in order to ensure there is no state directory
+ left behind when a rebase fails to start and no autostash has been
+ created.
builtin/rebase.c | 39 +++++++++++++++++++++++++++++------- t/t3400-rebase.sh | 8 ++++++-- t/t3413-rebase-hook.sh | 4 +++- t/t3420-rebase-autostash.sh | 40 +++++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 10 deletions(-)
builtin/rebase.c +32 -7
@@ -526,6 +526,23 @@ static int rebase_write_basic_state(struct rebase_options *opts)return 0 ; } +static int cleanup_autostash (struct rebase_options *opts) +{ +int ret; +struct strbuf dir =STRBUF_INIT ; +const char *path =state_dir_path ("autostash" , opts); + +if (!file_exists (path)) +return 0 ; + ret =apply_autostash (path); +strbuf_addstr (&dir, opts->state_dir); +if (remove_dir_recursively (&dir,0 )) + ret =error_errno (_ ("could not remove '%s'" ), opts->state_dir); +strbuf_release (&dir); + +return ret; +} +static int finish_rebase (struct rebase_options *opts) {struct strbuf dir =STRBUF_INIT ;
@@ -1726,7 +1743,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)if (require_clean_work_tree (the_repository,"rebase" ,_ ("Please commit or stash them." ),1 ,1 )) { ret =-1 ; - goto cleanup; + goto cleanup_autostash; }/*
@@ -1749,7 +1766,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)if (options.switch_to) { ret =checkout_up_to_date (&options);if (ret) - goto cleanup; + goto cleanup_autostash; }if (!(options.flags &REBASE_NO_QUIET ))
@@ -1775,8 +1792,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)/* If a hook exists, give it a chance to interrupt*/ if (!ok_to_skip_pre_rebase &&run_hooks_l ("pre-rebase" , options.upstream_arg, - argc ? argv[0 ] :NULL ,NULL )) -die (_ ("The pre-rebase hook refused to rebase." )); + argc ? argv[0 ] :NULL ,NULL )) { + ret =error (_ ("The pre-rebase hook refused to rebase." )); + goto cleanup_autostash; + }if (options.flags &REBASE_DIFFSTAT ) {struct diff_options opts;
@@ -1821,9 +1840,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)RESET_HEAD_RUN_POST_CHECKOUT_HOOK ; ropts.head_msg = msg.buf; ropts.default_reflog_action = options.reflog_action; -if (reset_head (the_repository, &ropts)) -die (_ ("Could not detach HEAD" )); -strbuf_release (&msg); +if (reset_head (the_repository, &ropts)) { + ret =error (_ ("Could not detach HEAD" )); + goto cleanup_autostash; + }/* * If the onto is a proper descendant of the tip of the branch, then
@@ -1851,9 +1871,14 @@ run_rebase: cleanup:strbuf_release (&buf); +strbuf_release (&msg);strbuf_release (&revisions);rebase_options_release (&options);free (squash_onto_name);free (keep_base_onto_name);return !!ret; + +cleanup_autostash: + ret |= !!cleanup_autostash (&options); + goto cleanup; }
t/t3400-rebase.sh +6 -2
@@ -145,7 +145,9 @@ test_expect_success 'Show verbose error when HEAD could not be detached' ' test_when_finished "rm -f B" && test_must_fail git rebase topic 2>output.err >output.out && test_grep "The following untracked working tree files would be overwritten by checkout:" output.err && - test_grep B output.err + test_grep B output.err && + test_must_fail git rebase --quit 2>err && + test_grep "no rebase in progress" err ' test_expect_success 'fail when upstream arg is missing and not on branch' '
@@ -422,7 +424,9 @@ test_expect_success 'refuse to switch to branch checked out elsewhere' ' git checkout main && git worktree add wt && test_must_fail git -C wt rebase main main 2>err && - test_grep "already used by worktree at" err + test_grep "already used by worktree at" err && + test_must_fail git -C wt rebase --quit 2>err && + test_grep "no rebase in progress" err ' test_expect_success 'rebase when inside worktree subdirectory' '
t/t3413-rebase-hook.sh +3 -1
@@ -110,7 +110,9 @@ test_expect_success 'pre-rebase hook stops rebase (1)' ' git reset --hard side && test_must_fail git rebase main && test "z$(git symbolic-ref HEAD)" = zrefs/heads/test && - test 0 = $(git rev-list HEAD...side | wc -l) + test 0 = $(git rev-list HEAD...side | wc -l) && + test_must_fail git rebase --quit 2>err && + test_grep "no rebase in progress" err ' test_expect_success 'pre-rebase hook stops rebase (2)' '
t/t3420-rebase-autostash.sh +40 -0
@@ -82,6 +82,46 @@ testrebase () { type=$1 dotest=$2 + test_expect_success "rebase$type: restore autostash when pre-rebase hook fails" ' + git checkout -f feature-branch && + test_hook pre-rebase <<-\EOF && + exit 1 + EOF + + echo changed >file0 && + test_must_fail git rebase $type --autostash -f HEAD^ && + test_must_fail git rebase --quit 2>err && + test_grep "no rebase in progress" err && + echo changed >expect && + test_cmp expect file0 + ' + + test_expect_success "rebase$type: restore autostash when checkout onto fails" ' + git checkout -f --detach feature-branch && + echo uncommitted-content >file0 && + echo untracked >file4 && + test_when_finished "rm file4" && + test_must_fail git rebase $type --autostash \ + unrelated-onto-branch && + test_must_fail git rebase --quit 2>err && + test_grep "no rebase in progress" err && + echo uncommitted-content >expect && + test_cmp expect file0 + ' + + test_expect_success "rebase$type: restore autostash when branch checkout fails" ' + git checkout -f unrelated-onto-branch^ && + echo uncommitted-content >file0 && + echo untracked >file4 && + test_when_finished "rm file4" && + test_must_fail git rebase $type --autostash HEAD \ + unrelated-onto-branch && + test_must_fail git rebase --quit 2>err && + test_grep "no rebase in progress" err && + echo uncommitted-content >expect && + test_cmp expect file0 + ' + test_expect_success "rebase$type: dirty worktree, --no-autostash" ' test_config rebase.autostash true && git reset --hard &&
base-commit: 39bf06adf96da25b87c9aa7d35a32ef3683eb4a4
Signature · 1 line
gitgitgadget
Quoted 5 lines · Phillip Wood
... This code is just doing what we do at
the end of a successful rebase so I'm don't really understand what the issue is. Looking atfinish_rebase()we don't even check the return value ofapply_autostash()when applying the stash at the end of a successful rebase.
At that point we give control back the user, so if things are left in conflicted or any other "unexpected" funny state, the user kill keep the both halves. As long as the user clearly understands why the working tree is in such a funny state, we should be OK (and I would imagine that we are giving messages like "applying preexisting changes stashed away before rebasing" or something).
Thanks.
Quoted 5 lines · Phillip Wood
... This code is just doing what we do at
the end of a successful rebase so I'm don't really understand what the issue is. Looking atfinish_rebase()we don't even check the return value ofapply_autostash()when applying the stash at the end of a successful rebase.
At that point we give control back the user, so if things are left in conflicted or any other "unexpected" funny state, the user kill keep the both halves. As long as the user clearly understands why the working tree is in such a funny state, we should be OK (and I would imagine that we are giving messages like "applying preexisting changes stashed away before rebasing" or something).
Thanks.
Quoted 3 lines · Phillip Wood via GitGitGadget
Thanks to Junio and Patrick for their comments on V1. I've updated the commit message to correct the typos found by Patrick and added an explanation of why it is safe to remove the state directory.
Any other comments, or are we all happy with this iteration?
Thanks.
Quoted 7 lines · Junio C Hamano
Quoted 3 lines · Phillip Wood via GitGitGadget
Thanks to Junio and Patrick for their comments on V1. I've updated the commit message to correct the typos found by Patrick and added an explanation of why it is safe to remove the state directory.
Any other comments, or are we all happy with this iteration?
Let me mark the topic for 'next'. Thanks for reporting, fixing and reviewing.