Re: [PATCH v2] rebase: write script before initializing state
From: Junio C Hamano <hidden>
Date: 2025-07-11 21:25:44
Øystein Walle [off-list ref] writes:
If rebase.instructionFormat is invalid the repository is left in a
strange state when the interactive rebase fails. `git status` outputs
both the same as it would have in the normal case *and* something
related to the interactive rebase:
$ git -c rebase.instructionFormat=blah rebase -i
fatal: invalid --pretty format: blah
$ git status
On branch master
Your branch is ahead of 'upstream/master' by 1 commit.
(use "git push" to publish your local commits)
git-rebase-todo is missing.
No commands done.
No commands remaining.
You are currently editing a commit while rebasing branch 'master' on '8db3019401'.
(use "git commit --amend" to amend the current commit)
(use "git rebase --continue" once you are satisfied with your changes)
get_commit_format() calls die() on failure so we cannot handle the error
gracefully. By attempting to write the rebase script before initializing
the state this bad state can be avoided.
Signed-off-by: Øystein Walle <redacted>
---
So sorry for the delay. I saw that the signoff was missing, then saw
Phillip's review, decided to think about it and then life happened in
the mean time...No need to be sorry. Life happens, indeed.
This patch is identical to the first one except it has the missing signoff and a few typos in the commit message corrected. Phillip's suggestions are noted and appreciated but unfortunately I am unable to work on the at the moment. And I do think my patch is at least an improvement albeit perhaps less thorough than it could have been.
Well, as long as we are making a step in the right direction, such a partial improvement gives us a better foundation for somebody else to further build on. It does not look like that this patch would make it harder to later give us a more thorough solution. Thanks for working on the topic.
quoted hunk
builtin/rebase.c | 42 ++++++++++++++++++------------------ t/t3415-rebase-autosquash.sh | 10 +++++++++ 2 files changed, 31 insertions(+), 21 deletions(-)diff --git a/builtin/rebase.c b/builtin/rebase.c index 2e8c4ee678..8139816417 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c@@ -293,15 +293,6 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) &revisions, &shortrevisions)) goto cleanup; - if (init_basic_state(&replay, - opts->head_name ? opts->head_name : "detached HEAD", - opts->onto, &opts->orig_head->object.oid)) - goto cleanup; - - if (!opts->upstream && opts->squash_onto) - write_file(path_squash_onto(), "%s\n", - oid_to_hex(opts->squash_onto)); - strvec_pushl(&make_script_args, "", revisions, NULL); if (opts->restrict_revision) strvec_pushf(&make_script_args, "^%s",@@ -310,21 +301,30 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) ret = sequencer_make_script(the_repository, &todo_list.buf, make_script_args.nr, make_script_args.v, flags); - - if (ret) + if (ret) { error(_("could not generate todo list")); - else { - discard_index(the_repository->index); - if (todo_list_parse_insn_buffer(the_repository, &replay, - todo_list.buf.buf, &todo_list)) - BUG("unusable todo list"); - - ret = complete_action(the_repository, &replay, flags, - shortrevisions, opts->onto_name, opts->onto, - &opts->orig_head->object.oid, &opts->exec, - opts->autosquash, opts->update_refs, &todo_list); + goto cleanup; } + if (init_basic_state(&replay, + opts->head_name ? opts->head_name : "detached HEAD", + opts->onto, &opts->orig_head->object.oid)) + goto cleanup; + + if (!opts->upstream && opts->squash_onto) + write_file(path_squash_onto(), "%s\n", + oid_to_hex(opts->squash_onto)); + + discard_index(the_repository->index); + if (todo_list_parse_insn_buffer(the_repository, &replay, + todo_list.buf.buf, &todo_list)) + BUG("unusable todo list"); + + ret = complete_action(the_repository, &replay, flags, + shortrevisions, opts->onto_name, opts->onto, + &opts->orig_head->object.oid, &opts->exec, + opts->autosquash, opts->update_refs, &todo_list); + cleanup: replay_opts_release(&replay); free(revisions);diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh index 26b42a526a..5d093e3a7a 100755 --- a/t/t3415-rebase-autosquash.sh +++ b/t/t3415-rebase-autosquash.sh@@ -394,6 +394,16 @@ test_expect_success 'autosquash with empty custom instructionFormat' ' ) ' +test_expect_success 'autosquash with invalid custom instructionFormat' ' + git reset --hard base && + test_commit invalid-instructionFormat-test && + ( + test_must_fail git -c rebase.instructionFormat=blah \ + rebase --autosquash --force-rebase -i HEAD^ && + test_path_is_missing .git/rebase-merge + ) +' + set_backup_editor () { write_script backup-editor.sh <<-\EOF cp "$1" .git/backup-"$(basename "$1")"