git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] merge-ort: turn submodule conflict suggestions into an advice

From
Philippe Blain via GitGitGadget <gitgitgadget@gmail.com>
Date
Feb 26, 2024, 13:27 UTC
Message-ID
<pull.1661.v2.git.git.1708954048301.gitgitgadget@gmail.com>
In-Reply-To
<pull.1661.git.git.1706534968589.gitgitgadget@gmail.com>
From: Philippe Blain <levraiphilippeblain@gmail.com>

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. Only show the advice if it is enabled.

Update the tests as the expected message now appears on stderr instead of stdout.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
    merge-ort: turn submodule conflict suggestions into an advice
    
    Changes since v1:
    
     * Rebased onto v2.44.0 to solve conflicts
     * CC'ed more people involved in the thread where the message was added
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1661%2Fphil-blain%2Fmerge-submodule-conflict-advice-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1661/phil-blain/merge-submodule-conflict-advice-v2
Pull-Request: https://github.com/git/git/pull/1661
Range-diff vs v1:
 1:  e77f3453fe1 ! 1:  a6d631e0c69 merge-ort: turn submodule conflict suggestions into an advice
     @@ Commit 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.
     +    possible future work. Only show the advice if it is enabled.
      
          Update the tests as the expected message now appears on stderr instead
          of stdout.
     @@ Documentation/config/advice.txt: advice.*::
      
       ## advice.c ##
      @@ advice.c: static struct {
     - 	[ADVICE_STATUS_U_OPTION]			= { "statusUoption", 1 },
     - 	[ADVICE_SUBMODULES_NOT_UPDATED] 		= { "submodulesNotUpdated", 1 },
     - 	[ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE] = { "submoduleAlternateErrorStrategyDie", 1 },
     -+	[ADVICE_SUBMODULE_MERGE_CONFLICT]               = { "submoduleMergeConflict", 1  },
     - 	[ADVICE_SUGGEST_DETACHING_HEAD]			= { "suggestDetachingHead", 1 },
     - 	[ADVICE_UPDATE_SPARSE_PATH]			= { "updateSparsePath", 1 },
     - 	[ADVICE_WAITING_FOR_EDITOR]			= { "waitingForEditor", 1 },
     + 	[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" },
      
       ## advice.h ##
      @@ advice.h: enum advice_type {
 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 c7ea70f2e2e..dde8e7840ef 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 6e9098ff089..4ba64ee5b2d 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 9d4f49ae38b..7d0a821f5cb 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 8617babee41..6a48aea2275 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 70650521b04..7a3f1cb27c1 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 2b3c363078b..aa2fdc31d1a 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 &&

base-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0

-- 
gitgitgadget
Previous: Philippe Blain via GitGitGadgetNext: Junio C Hamano
Message 2 of 5 in “merge-ort: turn submodule conflict suggestions into an advice”
  1. merge-ort: turn submodule conflict suggestions into an advicePhilippe Blain via GitGitGadget, Jan 29, 2024
  2. merge-ort: turn submodule conflict suggestions into an advicePhilippe Blain via GitGitGadget, Feb 26, 2024
  3. Junio C HamanoFeb 26, 2024
  4. Philippe BlainFeb 26, 2024
  5. Junio C HamanoFeb 26, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.