{"thread":{"id":"48499","subject":"[PATCHv2 0/3] Reroll of sb/submodule-merge-in-merge-recursive","startedAt":"2018-05-15T20:00:39Z","lastAt":"2018-06-11T19:07:50Z","messageCount":19,"participants":["Stefan Beller","Leif Middelschulte","Junio C Hamano","Elijah Newren"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"347710","messageId":"20180515200030.88731-1-sbeller@google.com","threadId":"48499","inReplyTo":null,"subject":"[PATCHv2 0/3] Reroll of sb/submodule-merge-in-merge-recursive","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-15T20:00:25Z","receivedAt":"2018-05-15T20:00:39Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This rerolls the two commits found at [1] with the feedback of Eliah\nand puts Leifs patch[2] on top, that I edited according to Eliahs feedback,\nbut kept Leifs ownership. \n\nThis has addressed all of Eliahs feedback AFAICT.\nYou'll find a branch-diff below[3], which lacks\nthe new patch of Leif in that series, but is part of the reroll?\n\nLeif, what do you think?\n\nThanks,\nStefan\n\n[1] https://public-inbox.org/git/20180510211917.138518-1-sbeller@google.com/\n[2] https://public-inbox.org/git/20180514205737.21313-2-leif.middelschulte@gmail.com/\n[3] git branch-diff origin/master..origin/sb/submodule-merge-in-merge-recursive origin/master..HEAD >>0000-cover-letter.patch\n\nLeif Middelschulte (1):\n Inform about fast-forwarding of submodules during merge\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 | 185 +++++++++++++++++++++++++++++++++++++++++++++-\n submodule.c | 168 +----------------------------------------\n submodule.h | 6 +-\n 3 files changed, 186 insertions(+), 173 deletions(-)\n\n-- \n2.17.0.582.gccdcbd54c44.dirty\n\n\n\n1: e022c7976ae ! 1: 3b638ccac64 submodule.c: move submodule merging to merge-recursive.c\n @@ -20,7 +20,6 @@\n This commit is best viewed with --color-moved.\n \n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n diff --git a/merge-recursive.c b/merge-recursive.c\n --- a/merge-recursive.c\n2: 2c02ece7e01 ! 2: eb43110df9d merge-recursive: i18n submodule merge output and respect verbosity\n @@ -7,7 +7,6 @@\n internationalisation as well as the verbosity setting.\n \n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n diff --git a/merge-recursive.c b/merge-recursive.c\n --- a/merge-recursive.c\n @@ -73,10 +72,10 @@\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\toutput(o, 2, _(\"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\toutput(o, 2, _(\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-: ----------- > 3: 4a3bc435023 Inform about fast-forwarding of submodules during merge\n"},{"id":"347711","messageId":"20180515200030.88731-2-sbeller@google.com","threadId":"48499","inReplyTo":"20180515200030.88731-1-sbeller@google.com","subject":"[PATCH] git-submodule.sh: try harder to fetch a submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-15T20:00:26Z","receivedAt":"2018-05-15T20:00:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This is the logical continuum of fb43e31f2b4 (submodule: try harder to\nfetch needed sha1 by direct fetching sha1, 2016-02-23) and fixes it as\nsome assumptions were not correct.\n\nThe commit states:\n> If $sha1 was not part of the default fetch ... fail ourselves here\n> assumes that the fetch_in_submodule only fails when the serverside does\n> not support fetching by sha1.\n\nThere are other failures, why such a fetch may fail, such as\n fatal: Couldn't find remote ref HEAD\nwhich can happen if the remote side doesn't advertise HEAD and we do not\nhave a local fetch refspec.\n\nNot advertising HEAD is allowed by the protocol spec and would happen,\nif HEAD points at an unborn branch for example.\n\nNot having a local fetch refspec can happen when submodules are fetched\nshallowly, as then git-clone doesn't setup a fetch refspec.\n\nSo do try even harder for a submodule by ignoring the exit code of the\nfirst fetch and rather relying on the following is_tip_reachable to\nsee if we try fetching again.\n\nSigned-off-by: Stefan Beller \n---\n git-submodule.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 24914963ca2..00fcd69138f 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -614,7 +614,7 @@ cmd_update()\n \t\t\t\t# is not reachable from a ref.\n \t\t\t\tis_tip_reachable \"$sm_path\" \"$sha1\" ||\n \t\t\t\tfetch_in_submodule \"$sm_path\" $depth ||\n-\t\t\t\tdie \"$(eval_gettext \"Unable to fetch in submodule path '\\$displaypath'\")\"\n+\t\t\t\tsay \"$(eval_gettext \"Unable to fetch in submodule path '\\$displaypath'\")\"\n \n \t\t\t\t# Now we tried the usual fetch, but $sha1 may\n \t\t\t\t# not be reachable from any of the refs\n-- \n2.17.0.582.gccdcbd54c44.dirty\n\n"},{"id":"347712","messageId":"20180515200030.88731-5-sbeller@google.com","threadId":"48499","inReplyTo":"20180515200030.88731-1-sbeller@google.com","subject":"[PATCH 2/3] merge-recursive: i18n submodule merge output and respect verbosity","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-15T20:00:29Z","receivedAt":"2018-05-15T20:00:49Z","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..0571919ee0a 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, 2, _(\"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, 2, _(\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.582.gccdcbd54c44.dirty\n\n"},{"id":"347713","messageId":"20180515200030.88731-6-sbeller@google.com","threadId":"48499","inReplyTo":"20180515200030.88731-1-sbeller@google.com","subject":"[PATCH 3/3] Inform about fast-forwarding of submodules during merge","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-15T20:00:30Z","receivedAt":"2018-05-15T20:00:52Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Leif Middelschulte \n\nInform the user about an automatically fast-forwarded submodule. The\nsilent merge behavior was introduced by commit 68d03e4a6e44 (\"Implement\nautomatic fast-forward merge for submodules\", 2010-07-07)).\n\nSigned-off-by: Leif Middelschulte \nSigned-off-by: Stefan Beller \n---\n merge-recursive.c | 16 ++++++++++++++++\n 1 file changed, 16 insertions(+)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 0571919ee0a..29a430c418a 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1093,10 +1093,26 @@ 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\tif (show(o, 3)) {\n+\t\t\toutput(o, 1, _(\"Fast-forwarding submodule %s to the following commit:\"), path);\n+\t\t\toutput_commit_title(o, commit_b);\n+\t\t} else if (show(o, 2))\n+\t\t\toutput(o, 2, _(\"Fast-forwarding submodule %s to %s\"), path, oid_to_hex(b));\n+\t\telse\n+\t\t\t; /* no output */\n+\n \t\treturn 1;\n \t}\n \tif (in_merge_bases(commit_b, commit_a)) {\n \t\toidcpy(result, a);\n+\t\tif (show(o, 3)) {\n+\t\t\toutput(o, 1, _(\"Fast-forwarding submodule %s to the following commit:\"), path);\n+\t\t\toutput_commit_title(o, commit_a);\n+\t\t} else if (show(o, 2))\n+\t\t\toutput(o, 2, _(\"Fast-forwarding submodule %s to %s\"), path, oid_to_hex(a));\n+\t\telse\n+\t\t\t; /* no output */\n+\n \t\treturn 1;\n \t}\n \n-- \n2.17.0.582.gccdcbd54c44.dirty\n\n"},{"id":"347714","messageId":"20180515200030.88731-3-sbeller@google.com","threadId":"48499","inReplyTo":"20180515200030.88731-1-sbeller@google.com","subject":"[PATCH] grep: handle corrupt index files early","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-15T20:00:27Z","receivedAt":"2018-05-15T20:00:55Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Any other caller of 'repo_read_index' dies upon a negative return of\nit, so grep should, too.\n\nSigned-off-by: Stefan Beller \n---\n\nFound while reviewing the series\nhttps://public-inbox.org/git/20180514105823.8378-1-ao2@ao2.it/\n\n builtin/grep.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 6e7bc76785a..69f0743619f 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -488,7 +488,8 @@ static int grep_cache(struct grep_opt *opt, struct repository *repo,\n \t\tstrbuf_addstr(&name, repo->submodule_prefix);\n \t}\n \n-\trepo_read_index(repo);\n+\tif (repo_read_index(repo) < 0)\n+\t\tdie(\"index file corrupt\");\n \n \tfor (nr = 0; nr < repo->index->cache_nr; nr++) {\n \t\tconst struct cache_entry *ce = repo->index->cache[nr];\n-- \n2.17.0.582.gccdcbd54c44.dirty\n\n"},{"id":"347715","messageId":"20180515200030.88731-4-sbeller@google.com","threadId":"48499","inReplyTo":"20180515200030.88731-1-sbeller@google.com","subject":"[PATCH 1/3] submodule.c: move submodule merging to merge-recursive.c","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-15T20:00:28Z","receivedAt":"2018-05-15T20:00:59Z","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.582.gccdcbd54c44.dirty\n\n"},{"id":"347716","messageId":"CAGZ79kYp89yLTca3SzrUnHbD-V8n3B7QAs1G9DkJKZQAd=exCQ@mail.gmail.com","threadId":"48499","inReplyTo":"20180515200030.88731-1-sbeller@google.com","subject":"Re: [PATCHv2 0/3] Reroll of sb/submodule-merge-in-merge-recursive","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-15T20:02:25Z","receivedAt":"2018-05-15T20:02:30Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"And I resent two of my earlier patches, please ignore those\n(0001-grep-handle-corrupt-index-files-early.patch and\n0001-git-submodule.sh-try-harder-to-fetch-a-submodule.patch)\n\nStefan\n"},{"id":"347718","messageId":"CANw0+A-FK-zkuhX-jU8q7LfqgaNM6E4Xy92UOs2gkwkBf1TYOA@mail.gmail.com","threadId":"48499","inReplyTo":"20180515200030.88731-1-sbeller@google.com","subject":"Re: [PATCHv2 0/3] Reroll of sb/submodule-merge-in-merge-recursive","fromName":"Leif Middelschulte","fromEmail":"leif.middelschulte@gmail.com","sentAt":"2018-05-15T20:15:21Z","receivedAt":"2018-05-15T20:15:25Z","isPatch":false,"sender":{"key":"leif.middelschulte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1136427?v=4"},"body":"Hello Stefan,\n\nthank you once again for your effort.\n\nAm 15. Mai 2018 um 22:00:34, Stefan Beller\n(sbeller@google.com(mailto:sbeller@google.com)) schrieb:\n\n> This rerolls the two commits found at [1] with the feedback of Eliah\n> and puts Leifs patch[2] on top, that I edited according to Eliahs feedback,\n> but kept Leifs ownership.\n>\n> This has addressed all of Eliahs feedback AFAICT.\n> You'll find a branch-diff below[3], which lacks\n> the new patch of Leif in that series, but is part of the reroll?\n>\n> Leif, what do you think?\n\nSeems great to me. Thank you for picking up and improving my changes :)\nOne Question though: Shouldn’t an enum (like\nNOTES_MERGE_VERBOSITY_DEFAULT) be used instead of numbers?\n\n\nCheers,\n\n\nLeif\n\n>\n> Thanks,\n> Stefan\n>\n> [1] https://public-inbox.org/git/20180510211917.138518-1-sbeller@google.com/\n> [2] https://public-inbox.org/git/20180514205737.21313-2-leif.middelschulte@gmail.com/\n> [3] git branch-diff origin/master..origin/sb/submodule-merge-in-merge-recursive origin/master..HEAD >>0000-cover-letter.patch\n>\n> Leif Middelschulte (1):\n> Inform about fast-forwarding of submodules during merge\n>\n> Stefan 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 | 185 +++++++++++++++++++++++++++++++++++++++++++++-\n> submodule.c | 168 +----------------------------------------\n> submodule.h | 6 +-\n> 3 files changed, 186 insertions(+), 173 deletions(-)\n>\n> --\n> 2.17.0.582.gccdcbd54c44.dirty\n>\n>\n>\n> 1: e022c7976ae ! 1: 3b638ccac64 submodule.c: move submodule merging to merge-recursive.c\n> @@ -20,7 +20,6 @@\n> This commit is best viewed with --color-moved.\n>\n> Signed-off-by: Stefan Beller\n> - Signed-off-by: Junio C Hamano\n>\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> --- a/merge-recursive.c\n> 2: 2c02ece7e01 ! 2: eb43110df9d merge-recursive: i18n submodule merge output and respect verbosity\n> @@ -7,7 +7,6 @@\n> internationalisation as well as the verbosity setting.\n>\n> Signed-off-by: Stefan Beller\n> - Signed-off-by: Junio C Hamano\n>\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> --- a/merge-recursive.c\n> @@ -73,10 +72,10 @@\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> -+ output(o, 1, _(\"Found a possible merge resolution for the submodule:\\n\"));\n> ++ output(o, 2, _(\"Found a possible merge resolution for the submodule:\\n\"));\n> print_commit((struct commit *) merges.objects[0].item);\n> - fprintf(stderr,\n> -+ output(o, 1, _(\n> ++ output(o, 2, _(\n> \"If this is correct simply add it to the index \"\n> \"for example\\n\"\n> \"by using:\\n\\n\"\n> -: ----------- > 3: 4a3bc435023 Inform about fast-forwarding of submodules during merge\n"},{"id":"347719","messageId":"CAGZ79kanoU5W7phXRQQmAJrQDF9fhd57ck0geeAf+4Q9QAn+QA@mail.gmail.com","threadId":"48499","inReplyTo":"CANw0+A-FK-zkuhX-jU8q7LfqgaNM6E4Xy92UOs2gkwkBf1TYOA@mail.gmail.com","subject":"Re: [PATCHv2 0/3] Reroll of sb/submodule-merge-in-merge-recursive","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-15T20:49:05Z","receivedAt":"2018-05-15T20:49:10Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, May 15, 2018 at 1:15 PM, Leif Middelschulte\n wrote:\n> Hello Stefan,\n>\n> thank you once again for your effort.\n>\n> Am 15. Mai 2018 um 22:00:34, Stefan Beller\n> (sbeller@google.com(mailto:sbeller@google.com)) schrieb:\n>\n>> This rerolls the two commits found at [1] with the feedback of Eliah\n>> and puts Leifs patch[2] on top, that I edited according to Eliahs feedback,\n>> but kept Leifs ownership.\n>>\n>> This has addressed all of Eliahs feedback AFAICT.\n>> You'll find a branch-diff below[3], which lacks\n>> the new patch of Leif in that series, but is part of the reroll?\n>>\n>> Leif, what do you think?\n>\n> Seems great to me. Thank you for picking up and improving my changes :)\n> One Question though: Shouldn’t an enum (like\n> NOTES_MERGE_VERBOSITY_DEFAULT) be used instead of numbers?\n\nHah! I did not know that existed.\n\n$ git grep NOTES_MERGE_VERBOSITY_DEFAULT\nbuiltin/notes.c:810: o.verbosity = verbosity + NOTES_MERGE_VERBOSITY_DEFAULT;\nnotes-merge.c:22: o->verbosity = NOTES_MERGE_VERBOSITY_DEFAULT;\nnotes-merge.h:9: NOTES_MERGE_VERBOSITY_DEFAULT = 2,\n\nIt doesn't seem to be used much, as opposed to numbers:\n\n$ git grep show -- merge-recursive.c\nmerge-recursive.c:201:static int show(struct merge_options *o, int v)\nmerge-recursive.c:211: if (!show(o, v))\nmerge-recursive.c:570: opts.show_rename_progress = o->show_rename_progress;\nmerge-recursive.c:1096: if (show(o, 3)) {\nmerge-recursive.c:1099: } else if (show(o, 2))\nmerge-recursive.c:1108: if (show(o, 3)) {\nmerge-recursive.c:1111: } else if (show(o, 2))\nmerge-recursive.c:2178: if (show(o, 4) || o->call_depth)\nmerge-recursive.c:2275: if (show(o, 4)) {\nmerge-recursive.c:2286: if (show(o, 5)) {\nmerge-recursive.c:2351: if (show(o, 2))\n\n(The first two are the implementation of show/output, third is\nsomewhat unrelated to show() and all the rest is numbers).\n\nIf we'd want to use NOTES_MERGE_VERBOSITY_DEFAULT,\nI would suggest to send a followup series on top of this?\n\nI would think numbers are fine for now.\n\nThanks,\nStefan\n"},{"id":"347745","messageId":"xmqqtvr87622.fsf@gitster-ct.c.googlers.com","threadId":"48499","inReplyTo":"20180515200030.88731-5-sbeller@google.com","subject":"Re: [PATCH 2/3] merge-recursive: i18n submodule merge output and respect verbosity","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-16T01:17:09Z","receivedAt":"2018-05-16T01:17:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\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\nI kind of like this \"while at it\" change in this patch ;-)\n"},{"id":"347746","messageId":"CABPp-BH3mi5L+9QKXQAK0sp8z9zzfpwYY9R-itkfOo3CYTUo6g@mail.gmail.com","threadId":"48499","inReplyTo":"20180515200030.88731-6-sbeller@google.com","subject":"Re: [PATCH 3/3] Inform about fast-forwarding of submodules during merge","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-05-16T01:36:04Z","receivedAt":"2018-05-16T01:36:09Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, May 15, 2018 at 1:00 PM, Stefan Beller wrote:\n> From: Leif Middelschulte \n>\n> Inform the user about an automatically fast-forwarded submodule. The\n> silent merge behavior was introduced by commit 68d03e4a6e44 (\"Implement\n> automatic fast-forward merge for submodules\", 2010-07-07)).\n>\n> Signed-off-by: Leif Middelschulte \n> Signed-off-by: Stefan Beller \n> ---\n> merge-recursive.c | 16 ++++++++++++++++\n> 1 file changed, 16 insertions(+)\n>\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 0571919ee0a..29a430c418a 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -1093,10 +1093,26 @@ 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> + if (show(o, 3)) {\n> + output(o, 1, _(\"Fast-forwarding submodule %s to the following commit:\"), path);\n\nSeems slightly odd to mix 3 and 1 here; although it'll work just fine,\nI would have expected use of 3 in both places (much like you did with\nthe 2 and 2 below).\n\n> + output_commit_title(o, commit_b);\n> + } else if (show(o, 2))\n> + output(o, 2, _(\"Fast-forwarding submodule %s to %s\"), path, oid_to_hex(b));\n> + else\n> + ; /* no output */\n> +\n> return 1;\n> }\n> if (in_merge_bases(commit_b, commit_a)) {\n> oidcpy(result, a);\n> + if (show(o, 3)) {\n> + output(o, 1, _(\"Fast-forwarding submodule %s to the following commit:\"), path);\n\nSame.\n\n> + output_commit_title(o, commit_a);\n> + } else if (show(o, 2))\n> + output(o, 2, _(\"Fast-forwarding submodule %s to %s\"), path, oid_to_hex(a));\n> + else\n> + ; /* no output */\n> +\n> return 1;\n> }\n>\n> --\n> 2.17.0.582.gccdcbd54c44.dirty\n\nOther than that nit-pick, looks good to me.\n"},{"id":"347747","messageId":"xmqqo9hg7554.fsf@gitster-ct.c.googlers.com","threadId":"48499","inReplyTo":"20180515200030.88731-6-sbeller@google.com","subject":"Re: [PATCH 3/3] Inform about fast-forwarding of submodules during merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-16T01:36:55Z","receivedAt":"2018-05-16T01:37:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> From: Leif Middelschulte \n>\n\nSubject: merge-recursive: give notice when submodule commit gets fast-forwarded\n\nperhaps?\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\tif (show(o, 3)) {\n> +\t\t\toutput(o, 1, _(\"Fast-forwarding submodule %s to the following commit:\"), path);\n> +\t\t\toutput_commit_title(o, commit_b);\n> +\t\t} else if (show(o, 2))\n> +\t\t\toutput(o, 2, _(\"Fast-forwarding submodule %s to %s\"), path, oid_to_hex(b));\n> +\t\telse\n> +\t\t\t; /* no output */\n> +\n\nmerge.verbosity::\n\tControls the amount of output shown by the recursive merge\n\tstrategy. Level 0 outputs nothing except a final error\n\tmessage if conflicts were detected. Level 1 outputs only\n\tconflicts, 2 outputs conflicts and file changes. Level 5 and\n\tabove outputs debugging information. The default is level 2.\n\tCan be overridden by the `GIT_MERGE_VERBOSITY` environment variable.\n\nSo, by default, we report the fact that we update submodule to a\nparticular commit, which is quite similar to how we report auto\nmerged paths using the content level 3-way merge; when you squint\nyour eyes, the \"fast-forward\" of submodules look somewhat like a\ncontent-level 3-way merge anyway ;-)\n\nAnd at level 3, which currently is used to report a non-event that\ndoes not change the result of the merge from what was naturally\nexpected, we give a bit more detail by citing the commit the\nsubmodule gets fast-forwarded to [*1*].\n\nSort of makes sense.\n\n\n[Footnote]\n\n*1* I wonder if that is really necessary, though---we do not give\n\"here is a diff\" or \"this is the new contents\" after a path gets\nmerged for normal files. And if it is needed perhaps because\nsubmodules are so special, I wonder if we also need to give the\ncommit the submodule gets fast-forwarded from, i.e. the original\none, the same way.\n"},{"id":"347748","messageId":"xmqqk1s474vx.fsf@gitster-ct.c.googlers.com","threadId":"48499","inReplyTo":"20180515200030.88731-6-sbeller@google.com","subject":"Re: [PATCH 3/3] Inform about fast-forwarding of submodules during merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-16T01:42:26Z","receivedAt":"2018-05-16T01:42:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> From: Leif Middelschulte \n>\n> Inform the user about an automatically fast-forwarded submodule. The\n> silent merge behavior was introduced by commit 68d03e4a6e44 (\"Implement\n> automatic fast-forward merge for submodules\", 2010-07-07)).\n\nOh, another thing I forgot to mention.\n\nThese three lines do not convey much useful information. The first\nsentence can be read from the patch text, and the rest can be read\nfrom \"git blame\" and \"git log\" output.\n\nIt is correct that the silent behaviour was introduced long time\nago. The proposed log message does not even say if that silent\nbehaviour is bad in any way, let alone why it is bad and need to be\nchanged.\n\nPerhaps Leif can elaborate why this change is a good idea in the\nfirst place?\n\nThanks.\n"},{"id":"349291","messageId":"20180604184805.36332-1-leif.middelschulte@gmail.com","threadId":"48499","inReplyTo":"xmqqk1s474vx.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v4 0/1] merge-recursive: give notice when submodule commit gets fast-forwarded","fromName":"Leif Middelschulte","fromEmail":"leif.middelschulte@gmail.com","sentAt":"2018-06-04T18:48:04Z","receivedAt":"2018-06-04T18:48:31Z","isPatch":true,"sender":{"key":"leif.middelschulte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1136427?v=4"},"body":"From: Leif Middelschulte \n\nThe provided patch is in response to Elijah Newren's [0] and Junio Hamano's [1]\ncomments on my prior patch regarding the reasoning and implementation of a user\nnotification during (clean) merges of submodules.\n\n[0] https://public-inbox.org/git/xmqqo9hg7554.fsf@gitster-ct.c.googlers.com/#t\n[1] https://public-inbox.org/git/xmqqzi0t1waf.fsf@gitster-ct.c.googlers.com/\n\nLeif Middelschulte (1):\n Inform about Auto-merging of submodules during merge\n\n merge-recursive.c | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"349292","messageId":"20180604184805.36332-2-leif.middelschulte@gmail.com","threadId":"48499","inReplyTo":"20180604184805.36332-1-leif.middelschulte@gmail.com","subject":"[PATCH 1/1] merge-recursive: give notice when submodule commit gets fast-forwarded","fromName":"Leif Middelschulte","fromEmail":"leif.middelschulte@gmail.com","sentAt":"2018-06-04T18:48:05Z","receivedAt":"2018-06-04T18:48:35Z","isPatch":true,"sender":{"key":"leif.middelschulte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1136427?v=4"},"body":"From: Leif Middelschulte \n\nSince submodules are treated similarly to ordinary files (i.e. not as 'dumb'\npointers), an automatic merge should be mentioned if the user asks for it.\nJust as it is mentioned for oridnary files.\n\nSigned-off-by: Leif Middelschulte \n---\n merge-recursive.c | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex a4b91d17f..0990a135b 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1093,10 +1093,20 @@ 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\tif (show(o, 2))\n+\t\t\toutput(o, 2, _(\"Auto-merging %s\"), path);\n+\t\telse\n+\t\t\t; /* no output */\n+\n \t\treturn 1;\n \t}\n \tif (in_merge_bases(commit_b, commit_a)) {\n \t\toidcpy(result, a);\n+\t\tif (show(o, 2))\n+\t\t\toutput(o, 2, _(\"Auto-merging %s\"), path);\n+\t\telse\n+\t\t\t; /* no output */\n+\n \t\treturn 1;\n \t}\n \n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"349597","messageId":"CABPp-BEdaSVG9GGwxLwr=neWGq-vO39njC+Ri4tDmN5PPtAntg@mail.gmail.com","threadId":"48499","inReplyTo":"20180604184805.36332-2-leif.middelschulte@gmail.com","subject":"Re: [PATCH 1/1] merge-recursive: give notice when submodule commit gets fast-forwarded","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-06-07T05:22:27Z","receivedAt":"2018-06-07T05:22:33Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Leif,\n\nOn Mon, Jun 4, 2018 at 11:48 AM, Leif Middelschulte\n wrote:\n> From: Leif Middelschulte \n>\n> Since submodules are treated similarly to ordinary files (i.e. not as 'dumb'\n> pointers), an automatic merge should be mentioned if the user asks for it.\n> Just as it is mentioned for oridnary files.\n\nThanks for following up; sorry it took me a few days to respond.\nHowever, it looks like Junio merged the\nsb/submodule-merge-in-merge-recursive topic, including your patch, to\nmaster back on May 30. As such, instead of re-rolling your patch,\nwe'd need a patch on top of the other existing change.\n\nAlso, take a look at the preliminary release announcement -- you show\nup as a new contributor to git! See it at\n https://public-inbox.org/git/xmqqwove4pzo.fsf@gitster-ct.c.googlers.com/\n\n\n> + output(o, 2, _(\"Auto-merging %s\"), path);\n...\n> + output(o, 2, _(\"Auto-merging %s\"), path);\n\nI preferred your old initial wording here, \"Fast-forwarding submodule\n%s\" (I just wanted the \"to %s\" part at the end removed). I'm afraid\nthat users who saw \"Auto-merging $submodule\" would assume that we\ndescended into the submodule and ran a full merge there.\n\nCould you submit a patch that just removed that \"to %s\" part?\n"},{"id":"349917","messageId":"20180611173128.9917-1-leif.middelschulte@gmail.com","threadId":"48499","inReplyTo":"CABPp-BEdaSVG9GGwxLwr=neWGq-vO39njC+Ri4tDmN5PPtAntg@mail.gmail.com","subject":"[PATCH] merge-submodule: reduce output verbosity","fromName":"Leif Middelschulte","fromEmail":"leif.middelschulte@gmail.com","sentAt":"2018-06-11T17:31:28Z","receivedAt":"2018-06-11T17:32:01Z","isPatch":true,"sender":{"key":"leif.middelschulte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1136427?v=4"},"body":"From: Leif Middelschulte \n\nThe output shall behave more similar to ordinary file merges' output to provide\na more consistent user experience.\n\nSigned-off-by: Leif Middelschulte \n---\n merge-recursive.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex ac27abbd4..5eb907f46 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1208,7 +1208,7 @@ static int merge_submodule(struct merge_options *o,\n \t\t\toutput(o, 3, _(\"Fast-forwarding submodule %s to the following commit:\"), path);\n \t\t\toutput_commit_title(o, commit_b);\n \t\t} else if (show(o, 2))\n-\t\t\toutput(o, 2, _(\"Fast-forwarding submodule %s to %s\"), path, oid_to_hex(b));\n+\t\t\toutput(o, 2, _(\"Fast-forwarding submodule %s\"), path);\n \t\telse\n \t\t\t; /* no output */\n \n@@ -1220,7 +1220,7 @@ static int merge_submodule(struct merge_options *o,\n \t\t\toutput(o, 3, _(\"Fast-forwarding submodule %s to the following commit:\"), path);\n \t\t\toutput_commit_title(o, commit_a);\n \t\t} else if (show(o, 2))\n-\t\t\toutput(o, 2, _(\"Fast-forwarding submodule %s to %s\"), path, oid_to_hex(a));\n+\t\t\toutput(o, 2, _(\"Fast-forwarding submodule %s\"), path);\n \t\telse\n \t\t\t; /* no output */\n \n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"349924","messageId":"xmqqr2ldb3oe.fsf@gitster-ct.c.googlers.com","threadId":"48499","inReplyTo":"20180611173128.9917-1-leif.middelschulte@gmail.com","subject":"Re: [PATCH] merge-submodule: reduce output verbosity","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-11T18:04:17Z","receivedAt":"2018-06-11T18:04:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Leif Middelschulte writes:\n\n> From: Leif Middelschulte \n>\n> The output shall behave more similar to ordinary file merges' output to provide\n> a more consistent user experience.\n>\n> Signed-off-by: Leif Middelschulte \n> ---\n> merge-recursive.c | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n\nThanks, both. Very much appreciated.\n\n>\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index ac27abbd4..5eb907f46 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -1208,7 +1208,7 @@ static int merge_submodule(struct merge_options *o,\n> \t\t\toutput(o, 3, _(\"Fast-forwarding submodule %s to the following commit:\"), path);\n> \t\t\toutput_commit_title(o, commit_b);\n> \t\t} else if (show(o, 2))\n> -\t\t\toutput(o, 2, _(\"Fast-forwarding submodule %s to %s\"), path, oid_to_hex(b));\n> +\t\t\toutput(o, 2, _(\"Fast-forwarding submodule %s\"), path);\n> \t\telse\n> \t\t\t; /* no output */\n> \n> @@ -1220,7 +1220,7 @@ static int merge_submodule(struct merge_options *o,\n> \t\t\toutput(o, 3, _(\"Fast-forwarding submodule %s to the following commit:\"), path);\n> \t\t\toutput_commit_title(o, commit_a);\n> \t\t} else if (show(o, 2))\n> -\t\t\toutput(o, 2, _(\"Fast-forwarding submodule %s to %s\"), path, oid_to_hex(a));\n> +\t\t\toutput(o, 2, _(\"Fast-forwarding submodule %s\"), path);\n> \t\telse\n> \t\t\t; /* no output */\n"},{"id":"349931","messageId":"CANw0+A9TAD5q6ux4sRd=L65vkqWzzapE=4nE7uSM=BH1imipsw@mail.gmail.com","threadId":"48499","inReplyTo":"xmqqr2ldb3oe.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] merge-submodule: reduce output verbosity","fromName":"Leif Middelschulte","fromEmail":"leif.middelschulte@gmail.com","sentAt":"2018-06-11T19:07:46Z","receivedAt":"2018-06-11T19:07:50Z","isPatch":true,"sender":{"key":"leif.middelschulte@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1136427?v=4"},"body":"Hello,\n\nAm 11. Juni 2018 um 20:04:19, Junio C Hamano\n(gitster@pobox.com(mailto:gitster@pobox.com)) schrieb:\n\n> Leif Middelschulte writes:\n>\n> > From: Leif Middelschulte\n> >\n> > The output shall behave more similar to ordinary file merges' output to provide\n> > a more consistent user experience.\n> >\n> > Signed-off-by: Leif Middelschulte\n> > ---\n> > merge-recursive.c | 4 ++--\n> > 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> Thanks, both. Very much appreciated.\nYou are welcome. Thank all of you, who participated in the discussions\nabout this topic, for your patience, advice, and guidance. I am sorry\nit took me so long to reply and provide the patches.\n\nCheers,\n\nLeif\n>\n> >\n> > diff --git a/merge-recursive.c b/merge-recursive.c\n> > index ac27abbd4..5eb907f46 100644\n> > --- a/merge-recursive.c\n> > +++ b/merge-recursive.c\n> > @@ -1208,7 +1208,7 @@ static int merge_submodule(struct merge_options *o,\n> > output(o, 3, _(\"Fast-forwarding submodule %s to the following commit:\"), path);\n> > output_commit_title(o, commit_b);\n> > } else if (show(o, 2))\n> > - output(o, 2, _(\"Fast-forwarding submodule %s to %s\"), path, oid_to_hex(b));\n> > + output(o, 2, _(\"Fast-forwarding submodule %s\"), path);\n> > else\n> > ; /* no output */\n> >\n> > @@ -1220,7 +1220,7 @@ static int merge_submodule(struct merge_options *o,\n> > output(o, 3, _(\"Fast-forwarding submodule %s to the following commit:\"), path);\n> > output_commit_title(o, commit_a);\n> > } else if (show(o, 2))\n> > - output(o, 2, _(\"Fast-forwarding submodule %s to %s\"), path, oid_to_hex(a));\n> > + output(o, 2, _(\"Fast-forwarding submodule %s\"), path);\n> > else\n> > ; /* no output */\n"}]}