[PATCH] rebase: write script before initializing state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



If rebase.instructionFormat is invalid the repository is left in a
strange state when the interactive rebase fails. `git status` outputs
boths the same as it would in the normal case *and* something related to
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)

By attempting to write the rebase script before initializing the state
this potential scenario is avoided.
---
The diff looks perhaps more messy than required. The only required
change is the filling in of make_script_args and the call to
sequencer_make_script() above the call to init_basic_state(). But then
the `if (ret)` looks out of place, and moving that up means adding `goto
cleanup` which means the code that was previously the else case can be
dedented.

get_commit_format() calls die() in this case, so cleaning up the
sequencer state isn't an option. Maybe it shouldn't call die in the
first place, but that looks to be much larger change.

 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")"
-- 
2.43.0





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux