* [PATCH] merge-ort: turn submodule conflict suggestions into an advice
@ 2024-01-29 13:29 Philippe Blain via GitGitGadget
2024-02-26 13:27 ` [PATCH v2] " Philippe Blain via GitGitGadget
0 siblings, 1 reply; 5+ messages in thread
From: Philippe Blain via GitGitGadget @ 2024-01-29 13:29 UTC (permalink / raw)
To: git; +Cc: Calvin Wan, Philippe Blain, Philippe Blain
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.
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
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1661%2Fphil-blain%2Fmerge-submodule-conflict-advice-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1661/phil-blain/merge-submodule-conflict-advice-v1
Pull-Request: https://github.com/git/git/pull/1661
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 25c09175244..32701b96828 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -128,6 +128,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 f6e4c2f302e..eee27b5bebc 100644
--- a/advice.c
+++ b/advice.c
@@ -73,6 +73,7 @@ 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 },
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 77ba7f3020c..59f025db26f 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"
@@ -4555,7 +4556,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: b50a608ba20348cb3dfc16a696816d51780e3f0f
--
gitgitgadget
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2] merge-ort: turn submodule conflict suggestions into an advice 2024-01-29 13:29 [PATCH] merge-ort: turn submodule conflict suggestions into an advice Philippe Blain via GitGitGadget @ 2024-02-26 13:27 ` Philippe Blain via GitGitGadget 2024-02-26 16:22 ` Junio C Hamano 0 siblings, 1 reply; 5+ messages in thread From: Philippe Blain via GitGitGadget @ 2024-02-26 13:27 UTC (permalink / raw) To: git Cc: Calvin Wan, Glen Choo, Elijah Newren, Ævar Arnfjörð Bjarmason, Philippe Blain, Philippe Blain 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] merge-ort: turn submodule conflict suggestions into an advice 2024-02-26 13:27 ` [PATCH v2] " Philippe Blain via GitGitGadget @ 2024-02-26 16:22 ` Junio C Hamano 2024-02-26 18:14 ` Philippe Blain 0 siblings, 1 reply; 5+ messages in thread From: Junio C Hamano @ 2024-02-26 16:22 UTC (permalink / raw) To: Philippe Blain via GitGitGadget Cc: git, Calvin Wan, Glen Choo, Elijah Newren, Ævar Arnfjörð Bjarmason, Philippe Blain "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes: > 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. When specifically called out like this, it makes it sound as if this one is disabled by default, while everybody else is enabled unless it is disabled. > Update the tests as the expected message now appears on stderr instead > of stdout. Makes sense. Thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] merge-ort: turn submodule conflict suggestions into an advice 2024-02-26 16:22 ` Junio C Hamano @ 2024-02-26 18:14 ` Philippe Blain 2024-02-26 18:34 ` Junio C Hamano 0 siblings, 1 reply; 5+ messages in thread From: Philippe Blain @ 2024-02-26 18:14 UTC (permalink / raw) To: Junio C Hamano, Philippe Blain via GitGitGadget Cc: git, Calvin Wan, Glen Choo, Elijah Newren, Ævar Arnfjörð Bjarmason Hi Junio, Le 2024-02-26 à 11:22, Junio C Hamano a écrit : > "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> 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. > > When specifically called out like this, it makes it sound as if this > one is disabled by default, while everybody else is enabled unless it > is disabled. Yes, re-reading myself, this did not come out as I wanted. I'll update the message to read more like "Adjust merge-ort.c::print_submodule_conflict_suggestion to use the advice API" or something like this. Thanks, Philippe. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] merge-ort: turn submodule conflict suggestions into an advice 2024-02-26 18:14 ` Philippe Blain @ 2024-02-26 18:34 ` Junio C Hamano 0 siblings, 0 replies; 5+ messages in thread From: Junio C Hamano @ 2024-02-26 18:34 UTC (permalink / raw) To: Philippe Blain Cc: Philippe Blain via GitGitGadget, git, Calvin Wan, Glen Choo, Elijah Newren, Ævar Arnfjörð Bjarmason Philippe Blain <levraiphilippeblain@gmail.com> writes: > Hi Junio, > > Le 2024-02-26 à 11:22, Junio C Hamano a écrit : >> "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes: >> >>> 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. >> >> When specifically called out like this, it makes it sound as if this >> one is disabled by default, while everybody else is enabled unless it >> is disabled. > > Yes, re-reading myself, this did not come out as I wanted. I'll update > the message to read more like "Adjust merge-ort.c::print_submodule_conflict_suggestion > to use the advice API" or something like this. I've replaced the last sentence with """The message can now be disabled with the advice mechanism.""" ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-02-26 18:34 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-29 13:29 [PATCH] merge-ort: turn submodule conflict suggestions into an advice Philippe Blain via GitGitGadget 2024-02-26 13:27 ` [PATCH v2] " Philippe Blain via GitGitGadget 2024-02-26 16:22 ` Junio C Hamano 2024-02-26 18:14 ` Philippe Blain 2024-02-26 18:34 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).