From b9e55be7401a5413722299279fc867eb931f4c3b Mon Sep 17 00:00:00 2001 From: Philippe Blain Date: Mon, 26 Feb 2024 13:27:28 +0000 Subject: [PATCH] merge-ort: turn submodule conflict suggestions into an advice Add a new advice type 'submoduleMergeConflict' for the error message shown when a non-trivial submodule conflict is encountered, which was added in 4057523a40 (submodule merge: update conflict error message, 2022-08-04). That commit mentions making this message an advice as possible future work. The message can now be disabled with the advice mechanism. Update the tests as the expected message now appears on stderr instead of stdout. Signed-off-by: Philippe Blain Signed-off-by: Junio C Hamano --- Documentation/config/advice.txt | 3 +++ advice.c | 1 + advice.h | 1 + merge-ort.c | 3 ++- t/t6437-submodule-merge.sh | 14 +++++++------- t/t7402-submodule-rebase.sh | 2 +- 6 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt index c7ea70f2e2..dde8e7840e 100644 --- a/Documentation/config/advice.txt +++ b/Documentation/config/advice.txt @@ -129,6 +129,9 @@ advice.*:: submoduleAlternateErrorStrategyDie:: Advice shown when a submodule.alternateErrorStrategy option configured to "die" causes a fatal error. + submoduleMergeConflict:: + Advice shown when a non-trivial submodule merge conflict is + encountered. submodulesNotUpdated:: Advice shown when a user runs a submodule command that fails because `git submodule update --init` was not run. diff --git a/advice.c b/advice.c index 6e9098ff08..4ba64ee5b2 100644 --- a/advice.c +++ b/advice.c @@ -79,6 +79,7 @@ static struct { [ADVICE_STATUS_U_OPTION] = { "statusUoption" }, [ADVICE_SUBMODULES_NOT_UPDATED] = { "submodulesNotUpdated" }, [ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE] = { "submoduleAlternateErrorStrategyDie" }, + [ADVICE_SUBMODULE_MERGE_CONFLICT] = { "submoduleMergeConflict" }, [ADVICE_SUGGEST_DETACHING_HEAD] = { "suggestDetachingHead" }, [ADVICE_UPDATE_SPARSE_PATH] = { "updateSparsePath" }, [ADVICE_WAITING_FOR_EDITOR] = { "waitingForEditor" }, diff --git a/advice.h b/advice.h index 9d4f49ae38..7d0a821f5c 100644 --- a/advice.h +++ b/advice.h @@ -47,6 +47,7 @@ enum advice_type { ADVICE_STATUS_U_OPTION, ADVICE_SUBMODULES_NOT_UPDATED, ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE, + ADVICE_SUBMODULE_MERGE_CONFLICT, ADVICE_SUGGEST_DETACHING_HEAD, ADVICE_UPDATE_SPARSE_PATH, ADVICE_WAITING_FOR_EDITOR, diff --git a/merge-ort.c b/merge-ort.c index 8617babee4..6a48aea227 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -18,6 +18,7 @@ #include "merge-ort.h" #include "alloc.h" +#include "advice.h" #include "attr.h" #include "cache-tree.h" #include "commit.h" @@ -4556,7 +4557,7 @@ static void print_submodule_conflict_suggestion(struct string_list *csub) { " - commit the resulting index in the superproject\n"), tmp.buf, subs.buf); - printf("%s", msg.buf); + advise_if_enabled(ADVICE_SUBMODULE_MERGE_CONFLICT, "%s", msg.buf); strbuf_release(&subs); strbuf_release(&tmp); diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh index 70650521b0..7a3f1cb27c 100755 --- a/t/t6437-submodule-merge.sh +++ b/t/t6437-submodule-merge.sh @@ -113,7 +113,7 @@ test_expect_success 'merging should conflict for non fast-forward' ' git checkout -b test-nonforward-a b && if test "$GIT_TEST_MERGE_ALGORITHM" = ort then - test_must_fail git merge c >actual && + test_must_fail git merge c 2>actual && sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" && grep "$sub_expect" actual else @@ -154,9 +154,9 @@ test_expect_success 'merging should conflict for non fast-forward (resolution ex git rev-parse --short sub-d > ../expect) && if test "$GIT_TEST_MERGE_ALGORITHM" = ort then - test_must_fail git merge c >actual && + test_must_fail git merge c >actual 2>sub-actual && sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" && - grep "$sub_expect" actual + grep "$sub_expect" sub-actual else test_must_fail git merge c 2> actual fi && @@ -181,9 +181,9 @@ test_expect_success 'merging should fail for ambiguous common parent' ' ) && if test "$GIT_TEST_MERGE_ALGORITHM" = ort then - test_must_fail git merge c >actual && + test_must_fail git merge c >actual 2>sub-actual && sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-c)" && - grep "$sub_expect" actual + grep "$sub_expect" sub-actual else test_must_fail git merge c 2> actual fi && @@ -227,7 +227,7 @@ test_expect_success 'merging should fail for changes that are backwards' ' git commit -a -m "f" && git checkout -b test-backward e && - test_must_fail git merge f >actual && + test_must_fail git merge f 2>actual && if test "$GIT_TEST_MERGE_ALGORITHM" = ort then sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short sub-d)" && @@ -535,7 +535,7 @@ test_expect_success 'merging should fail with no merge base' ' git checkout -b b init && git add sub && git commit -m "b" && - test_must_fail git merge a >actual && + test_must_fail git merge a 2>actual && if test "$GIT_TEST_MERGE_ALGORITHM" = ort then sub_expect="go to submodule (sub), and either merge commit $(git -C sub rev-parse --short HEAD^1)" && diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh index 2b3c363078..aa2fdc31d1 100755 --- a/t/t7402-submodule-rebase.sh +++ b/t/t7402-submodule-rebase.sh @@ -116,7 +116,7 @@ test_expect_success 'rebasing submodule that should conflict' ' test_tick && git commit -m fourth && - test_must_fail git rebase --onto HEAD^^ HEAD^ HEAD^0 >actual_output && + test_must_fail git rebase --onto HEAD^^ HEAD^ HEAD^0 2>actual_output && git ls-files -s submodule >actual && ( cd submodule &&