[PATCH 2/2] rebase -i: fix SIGSEGV when 'merge <branch>' fails
- From
- Phillip Wood <phillip.wood@talktalk.net>
- Date
- Aug 15, 2018, 09:39 UTC
- Message-ID
- <20180815093935.10222-3-phillip.wood@talktalk.net>
- In-Reply-To
- <20180815093935.10222-1-phillip.wood@talktalk.net>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If a merge command in the todo list specifies just a branch to merge with no -C/-c argument then item->commit is NULL. This means that if there are merge conflicts error_with_patch() is passed a NULL commit which causes a segmentation fault when make_patch() tries to look it up.
This commit implements a minimal fix which fixes the crash and allows the user to successfully commit a conflict resolution with 'git rebase --continue'. It does not write .git/rebase-merge/patch, .git/rebase-merge/stopped-sha or update REBASE_HEAD. To sensibly get the hashes of the merge parents would require refactoring do_merge() to extract the code that parses the merge parents into a separate function which error_with_patch() could then use to write the parents into the stopped-sha file. To create meaningful output make_patch() and 'git rebase --show-current-patch' would also need to be modified to diff the merge parent and merge base in this case.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- sequencer.c | 24 +++++++++++++++++++----- t/t3430-rebase-merges.sh | 15 ++++++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/sequencer.c b/sequencer.c index 4034c0461b..df49199175 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2590,8 +2590,12 @@ static int error_with_patch(struct commit *commit, const char *subject, int subject_len, struct replay_opts *opts, int exit_code, int to_amend) { - if (make_patch(commit, opts)) - return -1; + if (commit) { + if (make_patch(commit, opts)) + return -1; + } else if (copy_file(rebase_path_message(), git_path_merge_msg(), 0666)) + return error(_("unable to copy '%s' to '%s'"), + git_path_merge_msg(), rebase_path_message()); if (to_amend) { if (intend_to_amend()) @@ -2604,9 +2608,19 @@ static int error_with_patch(struct commit *commit, "Once you are satisfied with your changes, run\n" "\n" " git rebase --continue\n", gpg_sign_opt_quoted(opts)); - } else if (exit_code) - fprintf(stderr, "Could not apply %s... %.*s\n", - short_commit_name(commit), subject_len, subject); + } else if (exit_code) { + if (commit) + fprintf(stderr, "Could not apply %s... %.*s\n", + short_commit_name(commit), + subject_len, subject); + else + /* + * We don't have the hash of the parent so + * just print the line from the todo file. + */ + fprintf(stderr, "Could not merge %.*s\n", + subject_len, subject); + } return exit_code; } diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 31fe4268d5..2e767d4f1e 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -129,7 +129,7 @@ test_expect_success '`reset` refuses to overwrite untracked files' ' git rebase --abort ' -test_expect_success 'failed `merge` writes patch (may be rescheduled, too)' ' +test_expect_success 'failed `merge -C` writes patch (may be rescheduled, too)' ' test_when_finished "test_might_fail git rebase --abort" && git checkout -b conflicting-merge A && @@ -151,6 +151,19 @@ test_expect_success 'failed `merge` writes patch (may be rescheduled, too)' ' test_path_is_file .git/rebase-merge/patch ' +SQ="'" +test_expect_success 'failed `merge <branch>` does not crash' ' + test_when_finished "test_might_fail git rebase --abort" && + git checkout conflicting-G && + + echo "merge G" >script-from-scratch && + test_config sequence.editor \""$PWD"/replace-editor.sh\" && + test_tick && + test_must_fail git rebase -ir HEAD && + ! grep "^merge G$" .git/rebase-merge/git-rebase-todo && + grep "^Merge branch ${SQ}G${SQ}$" .git/rebase-merge/message +' + test_expect_success 'with a branch tip that was cherry-picked already' ' git checkout -b already-upstream master && base="$(git rev-parse --verify HEAD)" &&
-- 2.18.0