From d54e18986291474f077283c9d4cb5e970e1373a4 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Wed, 15 Aug 2018 10:39:34 +0100 Subject: [PATCH 1/2] t3430: add conflicting commit Move the creation of conflicting-G from a test to the setup so that it can be used in subsequent tests without creating the kind of implicit dependencies that plague t3404. While we're at it simplify the arguments to the test_commit() call the creates the conflicting commit. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- t/t3430-rebase-merges.sh | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 78f7c99580..31fe4268d5 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -13,8 +13,10 @@ Initial setup: -- B -- (first) / \ A - C - D - E - H (master) - \ / - F - G (second) + \ \ / + \ F - G (second) + \ + Conflicting-G ' . ./test-lib.sh . "$TEST_DIRECTORY"/lib-rebase.sh @@ -49,7 +51,9 @@ test_expect_success 'setup' ' git merge --no-commit G && test_tick && git commit -m H && - git tag -m H H + git tag -m H H && + git checkout A && + test_commit conflicting-G G.t ' test_expect_success 'create completely different structure' ' @@ -72,7 +76,7 @@ test_expect_success 'create completely different structure' ' EOF test_config sequence.editor \""$PWD"/replace-editor.sh\" && test_tick && - git rebase -i -r A && + git rebase -i -r A master && test_cmp_graph <<-\EOF * Merge the topic branch '\''onebranch'\'' |\ @@ -141,8 +145,7 @@ test_expect_success 'failed `merge` writes patch (may be rescheduled, too)' ' : fail because of merge conflict && rm G.t .git/rebase-merge/patch && - git reset --hard && - test_commit conflicting-G G.t not-G conflicting-G && + git reset --hard conflicting-G && test_must_fail git rebase --continue && ! grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo && test_path_is_file .git/rebase-merge/patch From bc9238bb0910f6d21e021e5c3061bf2124c3a176 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Wed, 15 Aug 2018 10:39:35 +0100 Subject: [PATCH 2/2] rebase -i: fix SIGSEGV when 'merge ' 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 Acked-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- 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 ` 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)" &&