@matheusbsbs / matheus-bagliano / commits / bc9238bb09

rebase -i: fix SIGSEGV when 'merge <branch>' fails

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> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed Aug 15, 2018 at 10:39 UTC bc9238bb0910f6d21e021e5c3061bf2124c3a176
2 files changed +33 -6
sequencer.c
+19 -5
@@ -2590,8 +2590,12 @@ static int error_with_patch(struct commit *commit,
2590 const char *subject, int subject_len,
2591 struct replay_opts *opts, int exit_code, int to_amend)
2592 {
2593 - if (make_patch(commit, opts))
2594 - return -1;
2593 + if (commit) {
2594 + if (make_patch(commit, opts))
2595 + return -1;
2596 + } else if (copy_file(rebase_path_message(), git_path_merge_msg(), 0666))
2597 + return error(_("unable to copy '%s' to '%s'"),
2598 + git_path_merge_msg(), rebase_path_message());
2599
2600 if (to_amend) {
2601 if (intend_to_amend())
@@ -2604,9 +2608,19 @@ static int error_with_patch(struct commit *commit,
2608 "Once you are satisfied with your changes, run\n"
2609 "\n"
2610 " git rebase --continue\n", gpg_sign_opt_quoted(opts));
2607 - } else if (exit_code)
2608 - fprintf(stderr, "Could not apply %s... %.*s\n",
2609 - short_commit_name(commit), subject_len, subject);
2611 + } else if (exit_code) {
2612 + if (commit)
2613 + fprintf(stderr, "Could not apply %s... %.*s\n",
2614 + short_commit_name(commit),
2615 + subject_len, subject);
2616 + else
2617 + /*
2618 + * We don't have the hash of the parent so
2619 + * just print the line from the todo file.
2620 + */
2621 + fprintf(stderr, "Could not merge %.*s\n",
2622 + subject_len, subject);
2623 + }
2624
2625 return exit_code;
2626 }
t/t3430-rebase-merges.sh
+14 -1
@@ -129,7 +129,7 @@ test_expect_success '`reset` refuses to overwrite untracked files' '
129 git rebase --abort
130 '
131
132 -test_expect_success 'failed `merge` writes patch (may be rescheduled, too)' '
132 +test_expect_success 'failed `merge -C` writes patch (may be rescheduled, too)' '
133 test_when_finished "test_might_fail git rebase --abort" &&
134 git checkout -b conflicting-merge A &&
135
@@ -151,6 +151,19 @@ test_expect_success 'failed `merge` writes patch (may be rescheduled, too)' '
151 test_path_is_file .git/rebase-merge/patch
152 '
153
154 +SQ="'"
155 +test_expect_success 'failed `merge <branch>` does not crash' '
156 + test_when_finished "test_might_fail git rebase --abort" &&
157 + git checkout conflicting-G &&
158 +
159 + echo "merge G" >script-from-scratch &&
160 + test_config sequence.editor \""$PWD"/replace-editor.sh\" &&
161 + test_tick &&
162 + test_must_fail git rebase -ir HEAD &&
163 + ! grep "^merge G$" .git/rebase-merge/git-rebase-todo &&
164 + grep "^Merge branch ${SQ}G${SQ}$" .git/rebase-merge/message
165 +'
166 +
167 test_expect_success 'with a branch tip that was cherry-picked already' '
168 git checkout -b already-upstream master &&
169 base="$(git rev-parse --verify HEAD)" &&