Re: [PATCH v3 3/7] merge: do not abort early if one strategy fails to handle the merge
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 21, 2022, 16:09 UTC
- Message-ID
- <xmqqlesmla6b.fsf@gitster.g>
- In-Reply-To
- <b41853e3f9908ab458bcb28684d817677e32367b.1658391391.git.gitgitgadget@gmail.com>
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 10 quoted lines
> @@ -754,8 +754,10 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
> else
> clean = merge_recursive(&o, head, remoteheads->item,
> reversed, &result);
> - if (clean < 0)
> - exit(128);
> + if (clean < 0) {
> + rollback_lock_file(&lock);
> + return 2;
> + }Very good find. I however wonder if negative returns are signaling a situation where they cannot cleanly recover from (but even if it is the case, if we are willing to do the save-restore dance, then it is probably OK).
Show 13 quoted lines
> diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh > index 3a32b1a45cf..772238e582c 100755 > --- a/t/t6402-merge-rename.sh > +++ b/t/t6402-merge-rename.sh > @@ -210,7 +210,7 @@ test_expect_success 'updated working tree file should prevent the merge' ' > echo >>M one line addition && > cat M >M.saved && > git update-index M && > - test_expect_code 128 git pull --no-rebase . yellow && > + test_expect_code 2 git pull --no-rebase . yellow && > test_cmp M M.saved && > rm -f M.saved > '
Understandable.
Show 23 quoted lines
> diff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh > index f35d3182b86..8b749e19083 100755 > --- a/t/t6424-merge-unrelated-index-changes.sh > +++ b/t/t6424-merge-unrelated-index-changes.sh > @@ -268,4 +268,20 @@ test_expect_success 'subtree' ' > test_path_is_missing .git/MERGE_HEAD > ' > > +test_expect_success 'resolve && recursive && ort' ' > + git reset --hard && > + git checkout B^0 && > + > + test_seq 0 10 >a && > + git add a && > + > + sane_unset GIT_TEST_MERGE_ALGORITHM && > + test_must_fail git merge -s resolve -s recursive -s ort C^0 >output 2>&1 && > + > + grep "Trying merge strategy resolve..." output && > + grep "Trying merge strategy recursive..." output && > + grep "Trying merge strategy ort..." output && > + grep "No merge strategy handled the merge." output > +'
Makes sense.
Show 13 quoted lines
> test_done
> diff --git a/t/t6439-merge-co-error-msgs.sh b/t/t6439-merge-co-error-msgs.sh
> index 5bfb027099a..52cf0c87690 100755
> --- a/t/t6439-merge-co-error-msgs.sh
> +++ b/t/t6439-merge-co-error-msgs.sh
> @@ -47,6 +47,7 @@ test_expect_success 'untracked files overwritten by merge (fast and non-fast for
> export GIT_MERGE_VERBOSITY &&
> test_must_fail git merge branch 2>out2
> ) &&
> + echo "Merge with strategy ${GIT_TEST_MERGE_ALGORITHM:-ort} failed." >>expect &&
> test_cmp out2 expect &&
> git reset --hard HEAD^
> '