Skip to content

Commit

Permalink
rebase: apply and cleanup autostash when rebase fails to start
Browse files Browse the repository at this point in the history
If "git rebase" fails to start after stashing the user's uncommitted
changes then it fails to restore the stashed changes and remove state
directory.  To make matters worse running "git rebase --abort" to try
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.

Reported-by: Brian Lyles <brianmlyles@gmail.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
  • Loading branch information
phillipwood committed Aug 14, 2024
1 parent 39bf06a commit 4e6edbe
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 10 deletions.
39 changes: 32 additions & 7 deletions builtin/rebase.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

/*
Expand All @@ -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))
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1851,9 +1871,14 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)

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;
}
8 changes: 6 additions & 2 deletions t/t3400-rebase.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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' '
Expand Down Expand Up @@ -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' '
Expand Down
4 changes: 3 additions & 1 deletion t/t3413-rebase-hook.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)' '
Expand Down
40 changes: 40 additions & 0 deletions t/t3420-rebase-autostash.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down

0 comments on commit 4e6edbe

Please sign in to comment.