{"thread":{"id":"48458","subject":"[PATCH 0/1] warn about auto fast-forwarded submodules during merges","startedAt":"2018-05-10T18:27:05Z","lastAt":"2018-05-15T07:34:35Z","messageCount":15,"participants":["Leif Middelschulte","Stefan Beller","Elijah Newren","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"347282","messageId":"20180510182657.65095-1-leif.middelschulte@gmail.com","threadId":"48458","inReplyTo":null,"subject":"[PATCH 0/1] warn about auto fast-forwarded submodules during merges","fromName":"Leif Middelschulte","fromEmail":"leif.middelschulte@gmail.com","sentAt":"2018-05-10T18:26:56Z","receivedAt":"2018-05-10T18:27:05Z","isPatch":true,"sender":{"key":"leif.middelschulte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1136427?v=4"},"body":"From: Leif Middelschulte \n\nWarn the user during merges about automatically fast-forwarded submodules.\nThis is just informational and does *not* change behavior otherwise.\n\nIt is a follow up to Elijah Newren's suggestion[0] to provide the attached patch.\n\n[0] https://marc.info/?l=git&m=152544498723355&w=2\n\nLeif Middelschulte (1):\n Warn about fast-forwarding of submodules during merge\n\n submodule.c | 2 ++\n 1 file changed, 2 insertions(+)\n\n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"347283","messageId":"20180510182657.65095-2-leif.middelschulte@gmail.com","threadId":"48458","inReplyTo":"20180510182657.65095-1-leif.middelschulte@gmail.com","subject":"[PATCH 1/1] Warn about fast-forwarding of submodules during merge","fromName":"Leif Middelschulte","fromEmail":"leif.middelschulte@gmail.com","sentAt":"2018-05-10T18:26:57Z","receivedAt":"2018-05-10T18:27:17Z","isPatch":true,"sender":{"key":"leif.middelschulte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1136427?v=4"},"body":"From: Leif Middelschulte \n\nWarn the user about an automatically fast-forwarded submodule. The silent merge\nbehavior was introduced by commit 68d03e4a6e44 (\"Implement automatic fast-forward\nmerge for submodules\", 2010-07-07)).\n\nSigned-off-by: Leif Middelschulte \n---\n submodule.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/submodule.c b/submodule.c\nindex 74d35b257..0198a72e6 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1817,10 +1817,12 @@ int merge_submodule(struct object_id *result, const char *path,\n \t/* Case #1: a is contained in b or vice versa */\n \tif (in_merge_bases(commit_a, commit_b)) {\n \t\toidcpy(result, b);\n+\t\twarning(\"Fast-forwarding submodule %s\", path);\n \t\treturn 1;\n \t}\n \tif (in_merge_bases(commit_b, commit_a)) {\n \t\toidcpy(result, a);\n+\t\twarning(\"Fast-forwarding submodule %s\", path);\n \t\treturn 1;\n \t}\n \n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"347288","messageId":"CAGZ79ka3kVHSZ9oG=NOvr0=KCHODngxJQLbKApDsFY=xNPhU=A@mail.gmail.com","threadId":"48458","inReplyTo":"20180510182657.65095-2-leif.middelschulte@gmail.com","subject":"Re: [PATCH 1/1] Warn about fast-forwarding of submodules during merge","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-10T18:49:37Z","receivedAt":"2018-05-10T18:49:43Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, May 10, 2018 at 11:26 AM, Leif Middelschulte\n wrote:\n> From: Leif Middelschulte \n\nHi Leif!\n\nthanks for following up with a patch!\n\n> Warn the user about an automatically fast-forwarded submodule. The silent merge\n> behavior was introduced by commit 68d03e4a6e44 (\"Implement automatic fast-forward\n> merge for submodules\", 2010-07-07)).\n>\n> Signed-off-by: Leif Middelschulte \n> ---\n> submodule.c | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/submodule.c b/submodule.c\n> index 74d35b257..0198a72e6 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -1817,10 +1817,12 @@ int merge_submodule(struct object_id *result, const char *path,\n> /* Case #1: a is contained in b or vice versa */\n> if (in_merge_bases(commit_a, commit_b)) {\n> oidcpy(result, b);\n> + warning(\"Fast-forwarding submodule %s\", path);\n> return 1;\n> }\n> if (in_merge_bases(commit_b, commit_a)) {\n> oidcpy(result, a);\n> + warning(\"Fast-forwarding submodule %s\", path);\n> return 1;\n> }\n\nThe code looks correct, however I think we can improve it.\n(Originally I was just wondering if stderr is the right output,\nwhich lead me to the thoughts below:)\n\nLooking through the code of merge-recursive.c,\nall the other merge outputs are done via 'output()'\nthat is able to buffer up the output as well as handles\nthe output for different verbosity settings.\n\nSo I would think we should make the output() function available\noutside of merge-recursive.c. (and rename it to a be more concise\nand descriptive in the global namespace) and make use of it.\n\nFunnily we already have MERGE_WARNING in submodule.c\nwhich outputs information for all the other cases. I would think\nwe ought to convert those to the output(), too.\n\nThanks,\nStefan\n"},{"id":"347309","messageId":"CANw0+A_T5zDUUWznYBe0m9fkSODPnfQaK1yJKPPawHTxi9+9BQ@mail.gmail.com","threadId":"48458","inReplyTo":"CAGZ79ka3kVHSZ9oG=NOvr0=KCHODngxJQLbKApDsFY=xNPhU=A@mail.gmail.com","subject":"Re: [PATCH 1/1] Warn about fast-forwarding of submodules during merge","fromName":"Leif Middelschulte","fromEmail":"leif.middelschulte@gmail.com","sentAt":"2018-05-10T20:30:34Z","receivedAt":"2018-05-10T20:30:38Z","isPatch":true,"sender":{"key":"leif.middelschulte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1136427?v=4"},"body":"Hi Stefan,\n\n\nAm 10. Mai 2018 um 20:49:39, Stefan Beller\n(sbeller@google.com(mailto:sbeller@google.com)) schrieb:\n\n> On Thu, May 10, 2018 at 11:26 AM, Leif Middelschulte\n> wrote:\n> > From: Leif Middelschulte\n>\n> Hi Leif!\n>\n> thanks for following up with a patch!\nsure, thanks for the quick review.\n>\n> > Warn the user about an automatically fast-forwarded submodule. The silent merge\n> > behavior was introduced by commit 68d03e4a6e44 (\"Implement automatic fast-forward\n> > merge for submodules\", 2010-07-07)).\n> >\n> > Signed-off-by: Leif Middelschulte\n> > ---\n> > submodule.c | 2 ++\n> > 1 file changed, 2 insertions(+)\n> >\n> > diff --git a/submodule.c b/submodule.c\n> > index 74d35b257..0198a72e6 100644\n> > --- a/submodule.c\n> > +++ b/submodule.c\n> > @@ -1817,10 +1817,12 @@ int merge_submodule(struct object_id *result, const char *path,\n> > /* Case #1: a is contained in b or vice versa */\n> > if (in_merge_bases(commit_a, commit_b)) {\n> > oidcpy(result, b);\n> > + warning(\"Fast-forwarding submodule %s\", path);\n> > return 1;\n> > }\n> > if (in_merge_bases(commit_b, commit_a)) {\n> > oidcpy(result, a);\n> > + warning(\"Fast-forwarding submodule %s\", path);\n> > return 1;\n> > }\n>\n> The code looks correct, however I think we can improve it.\n> (Originally I was just wondering if stderr is the right output,\n> which lead me to the thoughts below:)\nI’ve had the same thoughts about stderr. However, I thought that using a\nlog function named `warning` to warn the user would be the right choice.\nIf anything, I thought, the warning function might need refactoring.\n\n> Looking through the code of merge-recursive.c,\n> all the other merge outputs are done via 'output()'\n> that is able to buffer up the output as well as handles\n> the output for different verbosity settings.\n>\n> So I would think we should make the output() function available\n> outside of merge-recursive.c. (and rename it to a be more concise\n> and descriptive in the global namespace) and make use of it.\nSure, let me know what to use instead and I’ll update and resubmit the patch.\n\n>\n> Funnily we already have MERGE_WARNING in submodule.c\n> which outputs information for all the other cases. I would think\n> we ought to convert those to the output(), too.\nSure, but `MERGE_WARNING` prefixes all the messages with \"Failed to\nmerge submodule“.\n>\n> Thanks,\n> Stefan\n\nThank you,\nLeif\n"},{"id":"347312","messageId":"20180510211917.138518-1-sbeller@google.com","threadId":"48458","inReplyTo":"CANw0+A_T5zDUUWznYBe0m9fkSODPnfQaK1yJKPPawHTxi9+9BQ@mail.gmail.com","subject":"[PATCH 0/2] Submodule merging: i18n, verbosity","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-10T21:19:15Z","receivedAt":"2018-05-10T21:19:25Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Leif wrote:\n> Sure, let me know what to use instead and I’ll update and resubmit the patch.\n> Sure, but `MERGE_WARNING` prefixes all the messages with \"Failed to\n> merge submodule“.\n\nI thought about replying and coming up with good reasons, but I wrote some\npatches instead.\n\nThey can also be found at https://github.com/stefanbeller/git/tree/submodule_i18n_verbose\n\nI think these would be a good foundation for your patch as well, as you can use the\noutput() function for the desired cases.\n\nFeel free to take these patches as part of your series or adapt\n(or be inspired by) as needed.\n\nThanks,\nStefan\n\n\nStefan Beller (2):\n submodule.c: move submodule merging to merge-recursive.c\n merge-recursive: i18n submodule merge output and respect verbosity\n\n merge-recursive.c | 169 +++++++++++++++++++++++++++++++++++++++++++++-\n submodule.c | 168 +--------------------------------------------\n submodule.h | 6 +-\n 3 files changed, 170 insertions(+), 173 deletions(-)\n\n-- \n2.17.0.255.g8bfb7c0704\n\n"},{"id":"347313","messageId":"20180510211917.138518-2-sbeller@google.com","threadId":"48458","inReplyTo":"20180510211917.138518-1-sbeller@google.com","subject":"[PATCH 1/2] submodule.c: move submodule merging to merge-recursive.c","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-10T21:19:16Z","receivedAt":"2018-05-10T21:19:28Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In a later patch we want to improve submodule merging by using the output()\nfunction in merge-recursive.c for submodule merges to deliver a consistent\nUI to users.\n\nTo do so we could either make the output() function globally available\nso we can use it in submodule.c#merge_submodule(), or we could integrate\nthe submodule merging into the merging code. Choose the later as we\ngenerally want to move submodules closer into the core.\n\nTherefore we move any function related to merging submodules\n(merge_submodule(), find_first_merges() and print_commit) to\nmerge-recursive.c. We'll keep add_submodule_odb() in submodule.c as it\nis used by other submodule functions. While at it, add a TODO note that\nwe do not really like the function add_submodule_odb().\n\nThis commit is best viewed with --color-moved.\n\nSigned-off-by: Stefan Beller \n---\n merge-recursive.c | 166 +++++++++++++++++++++++++++++++++++++++++++++\n submodule.c | 168 +---------------------------------------------\n submodule.h | 6 +-\n 3 files changed, 170 insertions(+), 170 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 0c0d48624da..700ba15bf88 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -23,6 +23,7 @@\n #include \"merge-recursive.h\"\n #include \"dir.h\"\n #include \"submodule.h\"\n+#include \"revision.h\"\n \n struct path_hashmap_entry {\n \tstruct hashmap_entry e;\n@@ -977,6 +978,171 @@ static int merge_3way(struct merge_options *o,\n \treturn merge_status;\n }\n \n+static int find_first_merges(struct object_array *result, const char *path,\n+\t\tstruct commit *a, struct commit *b)\n+{\n+\tint i, j;\n+\tstruct object_array merges = OBJECT_ARRAY_INIT;\n+\tstruct commit *commit;\n+\tint contains_another;\n+\n+\tchar merged_revision[42];\n+\tconst char *rev_args[] = { \"rev-list\", \"--merges\", \"--ancestry-path\",\n+\t\t\t\t \"--all\", merged_revision, NULL };\n+\tstruct rev_info revs;\n+\tstruct setup_revision_opt rev_opts;\n+\n+\tmemset(result, 0, sizeof(struct object_array));\n+\tmemset(&rev_opts, 0, sizeof(rev_opts));\n+\n+\t/* get all revisions that merge commit a */\n+\txsnprintf(merged_revision, sizeof(merged_revision), \"^%s\",\n+\t\t\toid_to_hex(&a->object.oid));\n+\tinit_revisions(&revs, NULL);\n+\trev_opts.submodule = path;\n+\t/* FIXME: can't handle linked worktrees in submodules yet */\n+\trevs.single_worktree = path != NULL;\n+\tsetup_revisions(ARRAY_SIZE(rev_args)-1, rev_args, &revs, &rev_opts);\n+\n+\t/* save all revisions from the above list that contain b */\n+\tif (prepare_revision_walk(&revs))\n+\t\tdie(\"revision walk setup failed\");\n+\twhile ((commit = get_revision(&revs)) != NULL) {\n+\t\tstruct object *o = &(commit->object);\n+\t\tif (in_merge_bases(b, commit))\n+\t\t\tadd_object_array(o, NULL, &merges);\n+\t}\n+\treset_revision_walk();\n+\n+\t/* Now we've got all merges that contain a and b. Prune all\n+\t * merges that contain another found merge and save them in\n+\t * result.\n+\t */\n+\tfor (i = 0; i < merges.nr; i++) {\n+\t\tstruct commit *m1 = (struct commit *) merges.objects[i].item;\n+\n+\t\tcontains_another = 0;\n+\t\tfor (j = 0; j < merges.nr; j++) {\n+\t\t\tstruct commit *m2 = (struct commit *) merges.objects[j].item;\n+\t\t\tif (i != j && in_merge_bases(m2, m1)) {\n+\t\t\t\tcontains_another = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (!contains_another)\n+\t\t\tadd_object_array(merges.objects[i].item, NULL, result);\n+\t}\n+\n+\tobject_array_clear(&merges);\n+\treturn result->nr;\n+}\n+\n+static void print_commit(struct commit *commit)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct pretty_print_context ctx = {0};\n+\tctx.date_mode.type = DATE_NORMAL;\n+\tformat_commit_message(commit, \" %h: %m %s\", &sb, &ctx);\n+\tfprintf(stderr, \"%s\\n\", sb.buf);\n+\tstrbuf_release(&sb);\n+}\n+\n+#define MERGE_WARNING(path, msg) \\\n+\twarning(\"Failed to merge submodule %s (%s)\", path, msg);\n+\n+static int merge_submodule(struct object_id *result, const char *path,\n+\t\t\t const struct object_id *base, const struct object_id *a,\n+\t\t\t const struct object_id *b, int search)\n+{\n+\tstruct commit *commit_base, *commit_a, *commit_b;\n+\tint parent_count;\n+\tstruct object_array merges;\n+\n+\tint i;\n+\n+\t/* store a in result in case we fail */\n+\toidcpy(result, a);\n+\n+\t/* we can not handle deletion conflicts */\n+\tif (is_null_oid(base))\n+\t\treturn 0;\n+\tif (is_null_oid(a))\n+\t\treturn 0;\n+\tif (is_null_oid(b))\n+\t\treturn 0;\n+\n+\tif (add_submodule_odb(path)) {\n+\t\tMERGE_WARNING(path, \"not checked out\");\n+\t\treturn 0;\n+\t}\n+\n+\tif (!(commit_base = lookup_commit_reference(base)) ||\n+\t !(commit_a = lookup_commit_reference(a)) ||\n+\t !(commit_b = lookup_commit_reference(b))) {\n+\t\tMERGE_WARNING(path, \"commits not present\");\n+\t\treturn 0;\n+\t}\n+\n+\t/* check whether both changes are forward */\n+\tif (!in_merge_bases(commit_base, commit_a) ||\n+\t !in_merge_bases(commit_base, commit_b)) {\n+\t\tMERGE_WARNING(path, \"commits don't follow merge-base\");\n+\t\treturn 0;\n+\t}\n+\n+\t/* Case #1: a is contained in b or vice versa */\n+\tif (in_merge_bases(commit_a, commit_b)) {\n+\t\toidcpy(result, b);\n+\t\treturn 1;\n+\t}\n+\tif (in_merge_bases(commit_b, commit_a)) {\n+\t\toidcpy(result, a);\n+\t\treturn 1;\n+\t}\n+\n+\t/*\n+\t * Case #2: There are one or more merges that contain a and b in\n+\t * the submodule. If there is only one, then present it as a\n+\t * suggestion to the user, but leave it marked unmerged so the\n+\t * user needs to confirm the resolution.\n+\t */\n+\n+\t/* Skip the search if makes no sense to the calling context. */\n+\tif (!search)\n+\t\treturn 0;\n+\n+\t/* find commit which merges them */\n+\tparent_count = find_first_merges(&merges, path, commit_a, commit_b);\n+\tswitch (parent_count) {\n+\tcase 0:\n+\t\tMERGE_WARNING(path, \"merge following commits not found\");\n+\t\tbreak;\n+\n+\tcase 1:\n+\t\tMERGE_WARNING(path, \"not fast-forward\");\n+\t\tfprintf(stderr, \"Found a possible merge resolution \"\n+\t\t\t\t\"for the submodule:\\n\");\n+\t\tprint_commit((struct commit *) merges.objects[0].item);\n+\t\tfprintf(stderr,\n+\t\t\t\"If this is correct simply add it to the index \"\n+\t\t\t\"for example\\n\"\n+\t\t\t\"by using:\\n\\n\"\n+\t\t\t\" git update-index --cacheinfo 160000 %s \\\"%s\\\"\\n\\n\"\n+\t\t\t\"which will accept this suggestion.\\n\",\n+\t\t\toid_to_hex(&merges.objects[0].item->oid), path);\n+\t\tbreak;\n+\n+\tdefault:\n+\t\tMERGE_WARNING(path, \"multiple merges found\");\n+\t\tfor (i = 0; i < merges.nr; i++)\n+\t\t\tprint_commit((struct commit *) merges.objects[i].item);\n+\t}\n+\n+\tobject_array_clear(&merges);\n+\treturn 0;\n+}\n+\n static int merge_file_1(struct merge_options *o,\n \t\t\t\t\t const struct diff_filespec *one,\n \t\t\t\t\t const struct diff_filespec *a,\ndiff --git a/submodule.c b/submodule.c\nindex 74d35b25779..654089b3647 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -153,7 +153,8 @@ void stage_updated_gitmodules(struct index_state *istate)\n \t\tdie(_(\"staging updated .gitmodules failed\"));\n }\n \n-static int add_submodule_odb(const char *path)\n+/* TODO: remove this function, use repo_submodule_init instead. */\n+int add_submodule_odb(const char *path)\n {\n \tstruct strbuf objects_directory = STRBUF_INIT;\n \tint ret = 0;\n@@ -1701,171 +1702,6 @@ int submodule_move_head(const char *path,\n \treturn ret;\n }\n \n-static int find_first_merges(struct object_array *result, const char *path,\n-\t\tstruct commit *a, struct commit *b)\n-{\n-\tint i, j;\n-\tstruct object_array merges = OBJECT_ARRAY_INIT;\n-\tstruct commit *commit;\n-\tint contains_another;\n-\n-\tchar merged_revision[42];\n-\tconst char *rev_args[] = { \"rev-list\", \"--merges\", \"--ancestry-path\",\n-\t\t\t\t \"--all\", merged_revision, NULL };\n-\tstruct rev_info revs;\n-\tstruct setup_revision_opt rev_opts;\n-\n-\tmemset(result, 0, sizeof(struct object_array));\n-\tmemset(&rev_opts, 0, sizeof(rev_opts));\n-\n-\t/* get all revisions that merge commit a */\n-\txsnprintf(merged_revision, sizeof(merged_revision), \"^%s\",\n-\t\t\toid_to_hex(&a->object.oid));\n-\tinit_revisions(&revs, NULL);\n-\trev_opts.submodule = path;\n-\t/* FIXME: can't handle linked worktrees in submodules yet */\n-\trevs.single_worktree = path != NULL;\n-\tsetup_revisions(ARRAY_SIZE(rev_args)-1, rev_args, &revs, &rev_opts);\n-\n-\t/* save all revisions from the above list that contain b */\n-\tif (prepare_revision_walk(&revs))\n-\t\tdie(\"revision walk setup failed\");\n-\twhile ((commit = get_revision(&revs)) != NULL) {\n-\t\tstruct object *o = &(commit->object);\n-\t\tif (in_merge_bases(b, commit))\n-\t\t\tadd_object_array(o, NULL, &merges);\n-\t}\n-\treset_revision_walk();\n-\n-\t/* Now we've got all merges that contain a and b. Prune all\n-\t * merges that contain another found merge and save them in\n-\t * result.\n-\t */\n-\tfor (i = 0; i < merges.nr; i++) {\n-\t\tstruct commit *m1 = (struct commit *) merges.objects[i].item;\n-\n-\t\tcontains_another = 0;\n-\t\tfor (j = 0; j < merges.nr; j++) {\n-\t\t\tstruct commit *m2 = (struct commit *) merges.objects[j].item;\n-\t\t\tif (i != j && in_merge_bases(m2, m1)) {\n-\t\t\t\tcontains_another = 1;\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t}\n-\n-\t\tif (!contains_another)\n-\t\t\tadd_object_array(merges.objects[i].item, NULL, result);\n-\t}\n-\n-\tobject_array_clear(&merges);\n-\treturn result->nr;\n-}\n-\n-static void print_commit(struct commit *commit)\n-{\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tstruct pretty_print_context ctx = {0};\n-\tctx.date_mode.type = DATE_NORMAL;\n-\tformat_commit_message(commit, \" %h: %m %s\", &sb, &ctx);\n-\tfprintf(stderr, \"%s\\n\", sb.buf);\n-\tstrbuf_release(&sb);\n-}\n-\n-#define MERGE_WARNING(path, msg) \\\n-\twarning(\"Failed to merge submodule %s (%s)\", path, msg);\n-\n-int merge_submodule(struct object_id *result, const char *path,\n-\t\t const struct object_id *base, const struct object_id *a,\n-\t\t const struct object_id *b, int search)\n-{\n-\tstruct commit *commit_base, *commit_a, *commit_b;\n-\tint parent_count;\n-\tstruct object_array merges;\n-\n-\tint i;\n-\n-\t/* store a in result in case we fail */\n-\toidcpy(result, a);\n-\n-\t/* we can not handle deletion conflicts */\n-\tif (is_null_oid(base))\n-\t\treturn 0;\n-\tif (is_null_oid(a))\n-\t\treturn 0;\n-\tif (is_null_oid(b))\n-\t\treturn 0;\n-\n-\tif (add_submodule_odb(path)) {\n-\t\tMERGE_WARNING(path, \"not checked out\");\n-\t\treturn 0;\n-\t}\n-\n-\tif (!(commit_base = lookup_commit_reference(base)) ||\n-\t !(commit_a = lookup_commit_reference(a)) ||\n-\t !(commit_b = lookup_commit_reference(b))) {\n-\t\tMERGE_WARNING(path, \"commits not present\");\n-\t\treturn 0;\n-\t}\n-\n-\t/* check whether both changes are forward */\n-\tif (!in_merge_bases(commit_base, commit_a) ||\n-\t !in_merge_bases(commit_base, commit_b)) {\n-\t\tMERGE_WARNING(path, \"commits don't follow merge-base\");\n-\t\treturn 0;\n-\t}\n-\n-\t/* Case #1: a is contained in b or vice versa */\n-\tif (in_merge_bases(commit_a, commit_b)) {\n-\t\toidcpy(result, b);\n-\t\treturn 1;\n-\t}\n-\tif (in_merge_bases(commit_b, commit_a)) {\n-\t\toidcpy(result, a);\n-\t\treturn 1;\n-\t}\n-\n-\t/*\n-\t * Case #2: There are one or more merges that contain a and b in\n-\t * the submodule. If there is only one, then present it as a\n-\t * suggestion to the user, but leave it marked unmerged so the\n-\t * user needs to confirm the resolution.\n-\t */\n-\n-\t/* Skip the search if makes no sense to the calling context. */\n-\tif (!search)\n-\t\treturn 0;\n-\n-\t/* find commit which merges them */\n-\tparent_count = find_first_merges(&merges, path, commit_a, commit_b);\n-\tswitch (parent_count) {\n-\tcase 0:\n-\t\tMERGE_WARNING(path, \"merge following commits not found\");\n-\t\tbreak;\n-\n-\tcase 1:\n-\t\tMERGE_WARNING(path, \"not fast-forward\");\n-\t\tfprintf(stderr, \"Found a possible merge resolution \"\n-\t\t\t\t\"for the submodule:\\n\");\n-\t\tprint_commit((struct commit *) merges.objects[0].item);\n-\t\tfprintf(stderr,\n-\t\t\t\"If this is correct simply add it to the index \"\n-\t\t\t\"for example\\n\"\n-\t\t\t\"by using:\\n\\n\"\n-\t\t\t\" git update-index --cacheinfo 160000 %s \\\"%s\\\"\\n\\n\"\n-\t\t\t\"which will accept this suggestion.\\n\",\n-\t\t\toid_to_hex(&merges.objects[0].item->oid), path);\n-\t\tbreak;\n-\n-\tdefault:\n-\t\tMERGE_WARNING(path, \"multiple merges found\");\n-\t\tfor (i = 0; i < merges.nr; i++)\n-\t\t\tprint_commit((struct commit *) merges.objects[i].item);\n-\t}\n-\n-\tobject_array_clear(&merges);\n-\treturn 0;\n-}\n-\n /*\n * Embeds a single submodules git directory into the superprojects git dir,\n * non recursively.\ndiff --git a/submodule.h b/submodule.h\nindex e5526f6aaab..b96689ac0db 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -89,10 +89,8 @@ extern int submodule_uses_gitfile(const char *path);\n #define SUBMODULE_REMOVAL_IGNORE_UNTRACKED (1<<1)\n #define SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED (1<<2)\n extern int bad_to_remove_submodule(const char *path, unsigned flags);\n-extern int merge_submodule(struct object_id *result, const char *path,\n-\t\t\t const struct object_id *base,\n-\t\t\t const struct object_id *a,\n-\t\t\t const struct object_id *b, int search);\n+\n+int add_submodule_odb(const char *path);\n \n /* Checks if there are submodule changes in a..b. */\n extern int submodule_touches_in_range(struct object_id *a,\n-- \n2.17.0.255.g8bfb7c0704\n\n"},{"id":"347314","messageId":"20180510211917.138518-3-sbeller@google.com","threadId":"48458","inReplyTo":"20180510211917.138518-1-sbeller@google.com","subject":"[PATCH 2/2] merge-recursive: i18n submodule merge output and respect verbosity","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-10T21:19:17Z","receivedAt":"2018-05-10T21:19:31Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The submodule merge code now uses the output() function that is used by\nall the rest of the merge-recursive-code. This allows for respecting\ninternationalisation as well as the verbosity setting.\n\nSigned-off-by: Stefan Beller \n---\n merge-recursive.c | 33 +++++++++++++++------------------\n 1 file changed, 15 insertions(+), 18 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 700ba15bf88..a4b91d17f87 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1048,18 +1048,17 @@ static void print_commit(struct commit *commit)\n \tstrbuf_release(&sb);\n }\n \n-#define MERGE_WARNING(path, msg) \\\n-\twarning(\"Failed to merge submodule %s (%s)\", path, msg);\n-\n-static int merge_submodule(struct object_id *result, const char *path,\n+static int merge_submodule(struct merge_options *o,\n+\t\t\t struct object_id *result, const char *path,\n \t\t\t const struct object_id *base, const struct object_id *a,\n-\t\t\t const struct object_id *b, int search)\n+\t\t\t const struct object_id *b)\n {\n \tstruct commit *commit_base, *commit_a, *commit_b;\n \tint parent_count;\n \tstruct object_array merges;\n \n \tint i;\n+\tint search = !o->call_depth;\n \n \t/* store a in result in case we fail */\n \toidcpy(result, a);\n@@ -1073,21 +1072,21 @@ static int merge_submodule(struct object_id *result, const char *path,\n \t\treturn 0;\n \n \tif (add_submodule_odb(path)) {\n-\t\tMERGE_WARNING(path, \"not checked out\");\n+\t\toutput(o, 1, _(\"Failed to merge submodule %s (not checked out)\"), path);\n \t\treturn 0;\n \t}\n \n \tif (!(commit_base = lookup_commit_reference(base)) ||\n \t !(commit_a = lookup_commit_reference(a)) ||\n \t !(commit_b = lookup_commit_reference(b))) {\n-\t\tMERGE_WARNING(path, \"commits not present\");\n+\t\toutput(o, 1, _(\"Failed to merge submodule %s (commits not present)\"), path);\n \t\treturn 0;\n \t}\n \n \t/* check whether both changes are forward */\n \tif (!in_merge_bases(commit_base, commit_a) ||\n \t !in_merge_bases(commit_base, commit_b)) {\n-\t\tMERGE_WARNING(path, \"commits don't follow merge-base\");\n+\t\toutput(o, 1, _(\"Failed to merge submodule %s (commits don't follow merge-base)\"), path);\n \t\treturn 0;\n \t}\n \n@@ -1116,25 +1115,24 @@ static int merge_submodule(struct object_id *result, const char *path,\n \tparent_count = find_first_merges(&merges, path, commit_a, commit_b);\n \tswitch (parent_count) {\n \tcase 0:\n-\t\tMERGE_WARNING(path, \"merge following commits not found\");\n+\t\toutput(o, 1, _(\"Failed to merge submodule %s (merge following commits not found)\"), path);\n \t\tbreak;\n \n \tcase 1:\n-\t\tMERGE_WARNING(path, \"not fast-forward\");\n-\t\tfprintf(stderr, \"Found a possible merge resolution \"\n-\t\t\t\t\"for the submodule:\\n\");\n+\t\toutput(o, 1, _(\"Failed to merge submodule %s (not fast-forward)\"), path);\n+\t\toutput(o, 1, _(\"Found a possible merge resolution for the submodule:\\n\"));\n \t\tprint_commit((struct commit *) merges.objects[0].item);\n-\t\tfprintf(stderr,\n+\t\toutput(o, 1, _(\n \t\t\t\"If this is correct simply add it to the index \"\n \t\t\t\"for example\\n\"\n \t\t\t\"by using:\\n\\n\"\n \t\t\t\" git update-index --cacheinfo 160000 %s \\\"%s\\\"\\n\\n\"\n-\t\t\t\"which will accept this suggestion.\\n\",\n+\t\t\t\"which will accept this suggestion.\\n\"),\n \t\t\toid_to_hex(&merges.objects[0].item->oid), path);\n \t\tbreak;\n \n \tdefault:\n-\t\tMERGE_WARNING(path, \"multiple merges found\");\n+\t\toutput(o, 1, _(\"Failed to merge submodule %s (multiple merges found)\"), path);\n \t\tfor (i = 0; i < merges.nr; i++)\n \t\t\tprint_commit((struct commit *) merges.objects[i].item);\n \t}\n@@ -1205,12 +1203,11 @@ static int merge_file_1(struct merge_options *o,\n \t\t\t\treturn ret;\n \t\t\tresult->clean = (merge_status == 0);\n \t\t} else if (S_ISGITLINK(a->mode)) {\n-\t\t\tresult->clean = merge_submodule(&result->oid,\n+\t\t\tresult->clean = merge_submodule(o, &result->oid,\n \t\t\t\t\t\t one->path,\n \t\t\t\t\t\t &one->oid,\n \t\t\t\t\t\t &a->oid,\n-\t\t\t\t\t\t &b->oid,\n-\t\t\t\t\t\t !o->call_depth);\n+\t\t\t\t\t\t &b->oid);\n \t\t} else if (S_ISLNK(a->mode)) {\n \t\t\tswitch (o->recursive_variant) {\n \t\t\tcase MERGE_RECURSIVE_NORMAL:\n-- \n2.17.0.255.g8bfb7c0704\n\n"},{"id":"347320","messageId":"CABPp-BFPaOxokRoiVnAB+KRMt6=NihmjRH+exS_NbGMbdj+k4Q@mail.gmail.com","threadId":"48458","inReplyTo":"20180510211917.138518-1-sbeller@google.com","subject":"Re: [PATCH 0/2] Submodule merging: i18n, verbosity","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-11T00:04:21Z","receivedAt":"2018-05-11T00:04:26Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, May 10, 2018 at 2:19 PM, Stefan Beller wrote:\n> Leif wrote:\n>> Sure, let me know what to use instead and I’ll update and resubmit the patch.\n>> Sure, but `MERGE_WARNING` prefixes all the messages with \"Failed to\n>> merge submodule“.\n>\n> I thought about replying and coming up with good reasons, but I wrote some\n> patches instead.\n>\n> They can also be found at https://github.com/stefanbeller/git/tree/submodule_i18n_verbose\n>\n> I think these would be a good foundation for your patch as well, as you can use the\n> output() function for the desired cases.\n>\n> Feel free to take these patches as part of your series or adapt\n> (or be inspired by) as needed.\n\nThis is awesome. In addition to the good reasons you gave, switching\nmerge_submodule() to use output() was one of several things on my todo\nlist since I think it'd be needed for remerge-diffs\n(https://bugs.chromium.org/p/git/issues/detail?id=12) and might be\nuseful for merges in bare repos; thanks for tackling it.\n\nPatches look good to me. Having Leif's patch on top of these two\nwould be great.\n\nElijah\n"},{"id":"347321","messageId":"CAGZ79kaiFkq20Com7gOLin371D2KhTPG7cqn1mQ6OaFU12kKPQ@mail.gmail.com","threadId":"48458","inReplyTo":"CABPp-BFPaOxokRoiVnAB+KRMt6=NihmjRH+exS_NbGMbdj+k4Q@mail.gmail.com","subject":"Re: [PATCH 0/2] Submodule merging: i18n, verbosity","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-11T01:00:13Z","receivedAt":"2018-05-11T01:00:18Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi Elijah,\n\nOn Thu, May 10, 2018 at 5:04 PM, Elijah Newren wrote:\n> On Thu, May 10, 2018 at 2:19 PM, Stefan Beller wrote:\n>> Leif wrote:\n>>> Sure, let me know what to use instead and I’ll update and resubmit the patch.\n>>> Sure, but `MERGE_WARNING` prefixes all the messages with \"Failed to\n>>> merge submodule“.\n>>\n>> I thought about replying and coming up with good reasons, but I wrote some\n>> patches instead.\n>>\n>> They can also be found at https://github.com/stefanbeller/git/tree/submodule_i18n_verbose\n>>\n>> I think these would be a good foundation for your patch as well, as you can use the\n>> output() function for the desired cases.\n>>\n>> Feel free to take these patches as part of your series or adapt\n>> (or be inspired by) as needed.\n>\n> This is awesome. In addition to the good reasons you gave, switching\n> merge_submodule() to use output() was one of several things on my todo\n> list since I think it'd be needed for remerge-diffs\n> (https://bugs.chromium.org/p/git/issues/detail?id=12) and might be\n> useful for merges in bare repos; thanks for tackling it.\n\nThanks for the encouraging words!\nThe one nit I find on that series is that we need to rely on and export\nthe add_submodule_odb function as I want to get rid of that function\nonce the object store series has progressed far enough.\n\n>\n> Patches look good to me. Having Leif's patch on top of these two\n> would be great.\n\nok, Let's go with that.\n\nStefan\n"},{"id":"347624","messageId":"20180514205737.21313-1-leif.middelschulte@gmail.com","threadId":"48458","inReplyTo":"CAGZ79kaiFkq20Com7gOLin371D2KhTPG7cqn1mQ6OaFU12kKPQ@mail.gmail.com","subject":"[PATCH 0/1] rebased: inform about auto submodule ff during merge","fromName":"Leif Middelschulte","fromEmail":"leif.middelschulte@gmail.com","sentAt":"2018-05-14T20:57:36Z","receivedAt":"2018-05-14T20:58:01Z","isPatch":true,"sender":{"key":"leif.middelschulte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1136427?v=4"},"body":"From: Leif Middelschulte \n\nThis patch is in response to Stefan Beller's Commit 0357af480\n(\"merge-recursive: i18n submodule merge output and respect verbosity\",\n2018-05-10) and is based on the changes it provided.\n\nLeif Middelschulte (1):\n Inform about fast-forwarding of submodules during merge\n\n merge-recursive.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"347625","messageId":"20180514205737.21313-2-leif.middelschulte@gmail.com","threadId":"48458","inReplyTo":"20180514205737.21313-1-leif.middelschulte@gmail.com","subject":"[PATCH 1/1] Inform about fast-forwarding of submodules during merge","fromName":"Leif Middelschulte","fromEmail":"leif.middelschulte@gmail.com","sentAt":"2018-05-14T20:57:37Z","receivedAt":"2018-05-14T20:58:06Z","isPatch":true,"sender":{"key":"leif.middelschulte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1136427?v=4"},"body":"From: Leif Middelschulte \n\nInform the user about an automatically fast-forwarded submodule. The silent merge\nbehavior was introduced by commit 68d03e4a6e44 (\"Implement automatic fast-forward\nmerge for submodules\", 2010-07-07)).\n\nSigned-off-by: Leif Middelschulte \n---\n merge-recursive.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex a4b91d17f..4a03044d1 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1093,10 +1093,14 @@ static int merge_submodule(struct merge_options *o,\n \t/* Case #1: a is contained in b or vice versa */\n \tif (in_merge_bases(commit_a, commit_b)) {\n \t\toidcpy(result, b);\n+\t\toutput(o, 1, _(\"Note: Fast-forwarding submodule %s to the following commit\"), path);\n+\t\toutput_commit_title(o, commit_b);\n \t\treturn 1;\n \t}\n \tif (in_merge_bases(commit_b, commit_a)) {\n \t\toidcpy(result, a);\n+\t\toutput(o, 1, _(\"Note: Fast-forwarding submodule %s to the following commit:\"), path);\n+\t\toutput_commit_title(o, commit_a);\n \t\treturn 1;\n \t}\n \n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"347666","messageId":"CAGZ79kaKzahJ2oJ7qwerCS6m1c0MBiYiysf+HOU=3uRwfPqOkg@mail.gmail.com","threadId":"48458","inReplyTo":"20180514205737.21313-2-leif.middelschulte@gmail.com","subject":"Re: [PATCH 1/1] Inform about fast-forwarding of submodules during merge","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-15T00:41:37Z","receivedAt":"2018-05-15T00:41:42Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, May 14, 2018 at 1:57 PM, Leif Middelschulte\n wrote:\n> From: Leif Middelschulte \n>\n> Inform the user about an automatically fast-forwarded submodule. The silent merge\n> behavior was introduced by commit 68d03e4a6e44 (\"Implement automatic fast-forward\n> merge for submodules\", 2010-07-07)).\n>\n> Signed-off-by: Leif Middelschulte \n\nThanks for following up with a patch.\nThis looks good to me!\n\nThanks,\nStefan\n\n> ---\n> merge-recursive.c | 4 ++++\n> 1 file changed, 4 insertions(+)\n>\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index a4b91d17f..4a03044d1 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -1093,10 +1093,14 @@ static int merge_submodule(struct merge_options *o,\n> /* Case #1: a is contained in b or vice versa */\n> if (in_merge_bases(commit_a, commit_b)) {\n> oidcpy(result, b);\n> + output(o, 1, _(\"Note: Fast-forwarding submodule %s to the following commit\"), path);\n> + output_commit_title(o, commit_b);\n> return 1;\n> }\n> if (in_merge_bases(commit_b, commit_a)) {\n> oidcpy(result, a);\n> + output(o, 1, _(\"Note: Fast-forwarding submodule %s to the following commit:\"), path);\n> + output_commit_title(o, commit_a);\n> return 1;\n> }\n>\n> --\n> 2.15.1 (Apple Git-101)\n>\n"},{"id":"347670","messageId":"CABPp-BGp_zP8Z2S8FskiNvhNeQH3f=HdnQ39vX6xQz=oSyVfMQ@mail.gmail.com","threadId":"48458","inReplyTo":"20180514205737.21313-2-leif.middelschulte@gmail.com","subject":"Re: [PATCH 1/1] Inform about fast-forwarding of submodules during merge","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-15T01:17:09Z","receivedAt":"2018-05-15T01:17:13Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Leif,\n\nOn Mon, May 14, 2018 at 1:57 PM, Leif Middelschulte\n wrote:\n\nThanks for updating the patch on top of Stefan's series. :-)\n\n> /* Case #1: a is contained in b or vice versa */\n> if (in_merge_bases(commit_a, commit_b)) {\n> oidcpy(result, b);\n> + output(o, 1, _(\"Note: Fast-forwarding submodule %s to the following commit\"), path);\n> + output_commit_title(o, commit_b);\n\nLevel 1 is for conflicts; I don't think this message should have\nhigher priority than \"Auto-merging $PATH\" for normal files, so it\nneeds to be 2 (or maybe 3, see below) rather than 1. (The default\noutput level is 2, so it'd still be shown, but we do allow people to\nremove informational message and just get conflicts by setting\nGIT_MERGE_VERBOSITY to 1, or request extra information by setting it\nhigher)\n\nAlso, this two-line message seems somewhat verbose compared to the\nother messages in merge_submdoule(), and when compared to the simple\n\"Auto-merging $PATH\" we do for normal files. The multi-line nature of\nit particularly strikes me; the merge-recursive code has generally\navoided multi-line messages even for conflicts.\n\nIn comparison, your original patch just had (\"Fast-forwarding\nsubmodule %s\", path).\n\nMaybe you could \"if (show(o, 3)) { output your current message } else\n{ output the simpler message }\" ? Or is this verbosity warranted for\nsubmodules at the default print level?\n\nI'm not a heavy user of submodules, so I may need to get others to\nweigh in on the verbosity and multi-line aspects, but I wanted to at\nleast flag this as somewhat surprising to me.\n\n\nElijah\n"},{"id":"347673","messageId":"CABPp-BFw0g=3i8AoiCDgZR82ScOmiozDQqTggZ4U5kmiurFMdg@mail.gmail.com","threadId":"48458","inReplyTo":"20180510211917.138518-3-sbeller@google.com","subject":"Re: [PATCH 2/2] merge-recursive: i18n submodule merge output and respect verbosity","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-15T01:25:33Z","receivedAt":"2018-05-15T01:25:37Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"I know I said the patches looked okay earlier, but I just noticed something...\n\nOn Thu, May 10, 2018 at 2:19 PM, Stefan Beller wrote:\n\n> case 1:\n> - MERGE_WARNING(path, \"not fast-forward\");\n> - fprintf(stderr, \"Found a possible merge resolution \"\n> - \"for the submodule:\\n\");\n> + output(o, 1, _(\"Failed to merge submodule %s (not fast-forward)\"), path);\n\nWe allow folks to set GIT_MERGE_VERBOSITY to change how much output\nthey get. A setting of 1 should only show conflicts or major\nwarnings. 2 is the default and adds a few more messages (e.g.\n\"Auto-merging $PATH\", \"Adding $PATH\" for one-sided adds, etc.), higher\nlevels show even more.\n\nAnyway this output message is correct to use level 1 since this is a\nconflict, but...\n\n> + output(o, 1, _(\"Found a possible merge resolution for the submodule:\\n\"));\n\nI think this should use level 2.\n\n> print_commit((struct commit *) merges.objects[0].item);\n> - fprintf(stderr,\n> + output(o, 1, _(\n> \"If this is correct simply add it to the index \"\n> \"for example\\n\"\n> \"by using:\\n\\n\"\n> \" git update-index --cacheinfo 160000 %s \\\"%s\\\"\\n\\n\"\n>- \"which will accept this suggestion.\\n\",\n>+ \"which will accept this suggestion.\\n\"),\n> oid_to_hex(&merges.objects[0].item->oid), path);\n\nand so should this one (in fact, I'm tempted to say these last two\nshould use level 3, but since it looks like a command users may have\ndifficulty finding on their own, I'm okay with going with 2).\n"},{"id":"347681","messageId":"xmqqzi118j98.fsf@gitster-ct.c.googlers.com","threadId":"48458","inReplyTo":"CABPp-BGp_zP8Z2S8FskiNvhNeQH3f=HdnQ39vX6xQz=oSyVfMQ@mail.gmail.com","subject":"Re: [PATCH 1/1] Inform about fast-forwarding of submodules during merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-15T07:34:27Z","receivedAt":"2018-05-15T07:34:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> Hi Leif,\n>\n> On Mon, May 14, 2018 at 1:57 PM, Leif Middelschulte\n> wrote:\n>\n> Thanks for updating the patch on top of Stefan's series. :-)\n>\n>> /* Case #1: a is contained in b or vice versa */\n>> if (in_merge_bases(commit_a, commit_b)) {\n>> oidcpy(result, b);\n>> + output(o, 1, _(\"Note: Fast-forwarding submodule %s to the following commit\"), path);\n>> + output_commit_title(o, commit_b);\n>\n> Level 1 is for conflicts; I don't think this message should have\n> higher priority than \"Auto-merging $PATH\" for normal files, so it\n> needs to be 2 (or maybe 3, see below) rather than 1. (The default\n> output level is 2, so it'd still be shown, but we do allow people to\n> remove informational message and just get conflicts by setting\n> GIT_MERGE_VERBOSITY to 1, or request extra information by setting it\n> higher)\n>\n> Also, this two-line message seems somewhat verbose compared to the\n> other messages in merge_submdoule(), and when compared to the simple\n> \"Auto-merging $PATH\" we do for normal files. The multi-line nature of\n> it particularly strikes me; the merge-recursive code has generally\n> avoided multi-line messages even for conflicts.\n>\n> In comparison, your original patch just had (\"Fast-forwarding\n> submodule %s\", path).\n\nFWIW, I share both of your surprises. Between level 2 and 3, after\nskimming merge-recursive.c for existing use of output levels, I\nthink the situation for non-submodule merges that is closest to\nthese two cases the patch covers for submodules is probably the\nmessage given when content merge happened to end up with what we\nalready had. It is part of a normal merge operation that is not a\nsingificant event in the larger picture, yet it is rather rare and\ninteresting when you are curious on events that occur infrequently.\n\nSo a one-liner message as everybody else emitted at level 3 or more\nverbose would probably be a good balance with the remainder of the\nsystem, I would think.\n\nThanks for a review.\n"}]}