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 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. Reported-by: Brian Lyles <brianmlyles@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Phillip Wood committed
Sep 2, 2024 at 15:12 UTC
bf6ab087d14b839f8a7b188a7af62e43164873fb
4 files changed
+81
-10
builtin/rebase.c
+32
-7
@@ -526,6 +526,23 @@ static int rebase_write_basic_state(struct rebase_options *opts)
526
return 0;
527
}
528
529
+static int cleanup_autostash(struct rebase_options *opts)
530
+{
531
+ int ret;
532
+ struct strbuf dir = STRBUF_INIT;
533
+ const char *path = state_dir_path("autostash", opts);
534
+
535
+ if (!file_exists(path))
536
+ return 0;
537
+ ret = apply_autostash(path);
538
+ strbuf_addstr(&dir, opts->state_dir);
539
+ if (remove_dir_recursively(&dir, 0))
540
+ ret = error_errno(_("could not remove '%s'"), opts->state_dir);
541
+ strbuf_release(&dir);
542
+
543
+ return ret;
544
+}
545
+
546
static int finish_rebase(struct rebase_options *opts)
547
{
548
struct strbuf dir = STRBUF_INIT;
@@ -1726,7 +1743,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1743
if (require_clean_work_tree(the_repository, "rebase",
1744
_("Please commit or stash them."), 1, 1)) {
1745
ret = -1;
1729
- goto cleanup;
1746
+ goto cleanup_autostash;
1747
}
1748
1749
/*
@@ -1749,7 +1766,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1766
if (options.switch_to) {
1767
ret = checkout_up_to_date(&options);
1768
if (ret)
1752
- goto cleanup;
1769
+ goto cleanup_autostash;
1770
}
1771
1772
if (!(options.flags & REBASE_NO_QUIET))
@@ -1775,8 +1792,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1792
/* If a hook exists, give it a chance to interrupt*/
1793
if (!ok_to_skip_pre_rebase &&
1794
run_hooks_l("pre-rebase", options.upstream_arg,
1778
- argc ? argv[0] : NULL, NULL))
1779
- die(_("The pre-rebase hook refused to rebase."));
1795
+ argc ? argv[0] : NULL, NULL)) {
1796
+ ret = error(_("The pre-rebase hook refused to rebase."));
1797
+ goto cleanup_autostash;
1798
+ }
1799
1800
if (options.flags & REBASE_DIFFSTAT) {
1801
struct diff_options opts;
@@ -1821,9 +1840,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1840
RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
1841
ropts.head_msg = msg.buf;
1842
ropts.default_reflog_action = options.reflog_action;
1824
- if (reset_head(the_repository, &ropts))
1825
- die(_("Could not detach HEAD"));
1826
- strbuf_release(&msg);
1843
+ if (reset_head(the_repository, &ropts)) {
1844
+ ret = error(_("Could not detach HEAD"));
1845
+ goto cleanup_autostash;
1846
+ }
1847
1848
/*
1849
* If the onto is a proper descendant of the tip of the branch, then
@@ -1851,9 +1871,14 @@ run_rebase:
1871
1872
cleanup:
1873
strbuf_release(&buf);
1874
+ strbuf_release(&msg);
1875
strbuf_release(&revisions);
1876
rebase_options_release(&options);
1877
free(squash_onto_name);
1878
free(keep_base_onto_name);
1879
return !!ret;
1880
+
1881
+cleanup_autostash:
1882
+ ret |= !!cleanup_autostash(&options);
1883
+ goto cleanup;
1884
}
t/t3400-rebase.sh
+6
-2
@@ -145,7 +145,9 @@ test_expect_success 'Show verbose error when HEAD could not be detached' '
145
test_when_finished "rm -f B" &&
146
test_must_fail git rebase topic 2>output.err >output.out &&
147
test_grep "The following untracked working tree files would be overwritten by checkout:" output.err &&
148
- test_grep B output.err
148
+ test_grep B output.err &&
149
+ test_must_fail git rebase --quit 2>err &&
150
+ test_grep "no rebase in progress" err
151
'
152
153
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' '
424
git checkout main &&
425
git worktree add wt &&
426
test_must_fail git -C wt rebase main main 2>err &&
425
- test_grep "already used by worktree at" err
427
+ test_grep "already used by worktree at" err &&
428
+ test_must_fail git -C wt rebase --quit 2>err &&
429
+ test_grep "no rebase in progress" err
430
'
431
432
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)' '
110
git reset --hard side &&
111
test_must_fail git rebase main &&
112
test "z$(git symbolic-ref HEAD)" = zrefs/heads/test &&
113
- test 0 = $(git rev-list HEAD...side | wc -l)
113
+ test 0 = $(git rev-list HEAD...side | wc -l) &&
114
+ test_must_fail git rebase --quit 2>err &&
115
+ test_grep "no rebase in progress" err
116
'
117
118
test_expect_success 'pre-rebase hook stops rebase (2)' '
t/t3420-rebase-autostash.sh
+40
@@ -82,6 +82,46 @@ testrebase () {
82
type=$1
83
dotest=$2
84
85
+ test_expect_success "rebase$type: restore autostash when pre-rebase hook fails" '
86
+ git checkout -f feature-branch &&
87
+ test_hook pre-rebase <<-\EOF &&
88
+ exit 1
89
+ EOF
90
+
91
+ echo changed >file0 &&
92
+ test_must_fail git rebase $type --autostash -f HEAD^ &&
93
+ test_must_fail git rebase --quit 2>err &&
94
+ test_grep "no rebase in progress" err &&
95
+ echo changed >expect &&
96
+ test_cmp expect file0
97
+ '
98
+
99
+ test_expect_success "rebase$type: restore autostash when checkout onto fails" '
100
+ git checkout -f --detach feature-branch &&
101
+ echo uncommitted-content >file0 &&
102
+ echo untracked >file4 &&
103
+ test_when_finished "rm file4" &&
104
+ test_must_fail git rebase $type --autostash \
105
+ unrelated-onto-branch &&
106
+ test_must_fail git rebase --quit 2>err &&
107
+ test_grep "no rebase in progress" err &&
108
+ echo uncommitted-content >expect &&
109
+ test_cmp expect file0
110
+ '
111
+
112
+ test_expect_success "rebase$type: restore autostash when branch checkout fails" '
113
+ git checkout -f unrelated-onto-branch^ &&
114
+ echo uncommitted-content >file0 &&
115
+ echo untracked >file4 &&
116
+ test_when_finished "rm file4" &&
117
+ test_must_fail git rebase $type --autostash HEAD \
118
+ unrelated-onto-branch &&
119
+ test_must_fail git rebase --quit 2>err &&
120
+ test_grep "no rebase in progress" err &&
121
+ echo uncommitted-content >expect &&
122
+ test_cmp expect file0
123
+ '
124
+
125
test_expect_success "rebase$type: dirty worktree, --no-autostash" '
126
test_config rebase.autostash true &&
127
git reset --hard &&