{"thread":{"id":"44533","subject":"[PATCH] git-gui: pass the branch name to git merge","startedAt":"2016-11-22T17:53:18Z","lastAt":"2016-11-23T22:36:44Z","messageCount":6,"participants":["Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"306321","messageId":"6a6dd0b9-436d-327f-c33d-e5cce078b3a0@kdbg.org","threadId":"44533","inReplyTo":null,"subject":"[PATCH] git-gui: pass the branch name to git merge","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-11-22T17:52:52Z","receivedAt":"2016-11-22T17:53:18Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"The recent rewrite of the 'git merge' invocation in b5f325cb\n(git-gui: stop using deprecated merge syntax) replaced the\nsubsidiary call of 'git fmt-merge-msg' to take advantage of\nthe capability of 'git merge' that can construct a merge log\nmessage when the rev being merged is FETCH_HEAD.\n\nA disadvantage of this method is, though, that the conflict\nmarkers are augmented with a raw SHA1 instead of a symbolic\nbranch name. Revert to the former invocation style so that\nwe get both a useful commit message and a symbolic name in the\nconflict markers.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n A minor regression of our effort to get rid of\n deprecated merge syntax. Even though I had done a number of\n merges since then, I noticed this only today because I\n actively looked for the branch name.\n\n lib/merge.tcl | 13 +++----------\n 1 file changed, 3 insertions(+), 10 deletions(-)\n\ndiff --git a/lib/merge.tcl b/lib/merge.tcl\nindex 9f253db..39e3fb4 100644\n--- a/lib/merge.tcl\n+++ b/lib/merge.tcl\n@@ -112,16 +112,9 @@ method _start {} {\n \tclose $fh\n \tset _last_merged_branch $branch\n \n-\tif {[git-version >= \"2.5.0\"]} {\n-\t\tset cmd [list git merge --strategy=recursive FETCH_HEAD]\n-\t} else {\n-\t\tset cmd [list git]\n-\t\tlappend cmd merge\n-\t\tlappend cmd --strategy=recursive\n-\t\tlappend cmd [git fmt-merge-msg <[gitdir FETCH_HEAD]]\n-\t\tlappend cmd HEAD\n-\t\tlappend cmd $name\n-\t}\n+\tset cmd [list git merge --strategy=recursive --no-log -m]\n+\tlappend cmd [git fmt-merge-msg <[gitdir FETCH_HEAD]]\n+\tlappend cmd $name\n \n \tui_status [mc \"Merging %s and %s...\" $current_branch $stitle]\n \tset cons [console::new [mc \"Merge\"] \"merge $stitle\"]\n-- \n2.11.0.rc1.52.g65ffb51\n\n"},{"id":"306338","messageId":"xmqqvavfpbph.fsf@gitster.mtv.corp.google.com","threadId":"44533","inReplyTo":"6a6dd0b9-436d-327f-c33d-e5cce078b3a0@kdbg.org","subject":"Re: [PATCH] git-gui: pass the branch name to git merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-22T19:16:10Z","receivedAt":"2016-11-22T19:16:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> The recent rewrite of the 'git merge' invocation in b5f325cb\n> (git-gui: stop using deprecated merge syntax) replaced the\n> subsidiary call of 'git fmt-merge-msg' to take advantage of\n> the capability of 'git merge' that can construct a merge log\n> message when the rev being merged is FETCH_HEAD.\n>\n> A disadvantage of this method is, though, that the conflict\n> markers are augmented with a raw SHA1 instead of a symbolic\n> branch name.\n\nCan't this be handled on the \"git merge FETCH_HEAD\" codepath\ninstead?  The codepath already does enough \"magic\" things to\nFETCH_HEAD like skipping 'not-for-merge' entries and also knows\nabout the original refnames the commits to be merged came from.\n"},{"id":"306353","messageId":"1dc28731-9000-c3bf-fbed-0cb17c230d8b@kdbg.org","threadId":"44533","inReplyTo":"xmqqvavfpbph.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-gui: pass the branch name to git merge","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-11-22T20:40:03Z","receivedAt":"2016-11-22T20:40:33Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 22.11.2016 um 20:16 schrieb Junio C Hamano:\n> Can't this be handled on the \"git merge FETCH_HEAD\" codepath\n> instead?\n\nAbsolutely. Any takers? ;)\n\n-- Hannes\n\n"},{"id":"306389","messageId":"5baaf25b-6f15-8002-97ea-97c5c6a4b4e4@kdbg.org","threadId":"44533","inReplyTo":"1dc28731-9000-c3bf-fbed-0cb17c230d8b@kdbg.org","subject":"Re: [PATCH] git-gui: pass the branch name to git merge","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-11-23T19:23:53Z","receivedAt":"2016-11-23T19:25:31Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 22.11.2016 um 21:40 schrieb Johannes Sixt:\n> Am 22.11.2016 um 20:16 schrieb Junio C Hamano:\n>> Can't this be handled on the \"git merge FETCH_HEAD\" codepath\n>> instead?\n>\n> Absolutely. Any takers? ;)\n\nI attempted to fix git merge FETCH_HEAD, but I do not see a trivial \nsolution.\n\nBut on second thought, we have an excuse to pick my proposed git-gui \nchange anyway: Without that change and a fix in git-merge only, there is \nstill a regression for all users who use the latest git-gui but some git \nversion between 2.5.0 and the fixed git-merge...\n\n-- Hannes\n\n"},{"id":"306392","messageId":"xmqq4m2ym06p.fsf@gitster.mtv.corp.google.com","threadId":"44533","inReplyTo":"5baaf25b-6f15-8002-97ea-97c5c6a4b4e4@kdbg.org","subject":"Re: [PATCH] git-gui: pass the branch name to git merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-23T20:05:34Z","receivedAt":"2016-11-23T20:05:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Am 22.11.2016 um 21:40 schrieb Johannes Sixt:\n>> Am 22.11.2016 um 20:16 schrieb Junio C Hamano:\n>>> Can't this be handled on the \"git merge FETCH_HEAD\" codepath\n>>> instead?\n>>\n>> Absolutely. Any takers? ;)\n>\n> I attempted to fix git merge FETCH_HEAD, but I do not see a trivial\n> solution.\n>\n> But on second thought, we have an excuse to pick my proposed git-gui\n> change anyway: Without that change and a fix in git-merge only, there\n> is still a regression for all users who use the latest git-gui but\n> some git version between 2.5.0 and the fixed git-merge...\n\nI'll leave it up to Pat, as I do not read tcl very well ;-)\n"},{"id":"306398","messageId":"e61cc267-a59b-3be1-29db-c49d56f521f7@kdbg.org","threadId":"44533","inReplyTo":"xmqq4m2ym06p.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-gui: pass the branch name to git merge","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-11-23T22:36:36Z","receivedAt":"2016-11-23T22:36:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 23.11.2016 um 21:05 schrieb Junio C Hamano:\n> Johannes Sixt <j6t@kdbg.org> writes:\n>> Am 22.11.2016 um 21:40 schrieb Johannes Sixt:\n>>> Am 22.11.2016 um 20:16 schrieb Junio C Hamano:\n>>>> Can't this be handled on the \"git merge FETCH_HEAD\" codepath\n>>>> instead?\n>>>\n>>> Absolutely. Any takers? ;)\n>>\n>> I attempted to fix git merge FETCH_HEAD, but I do not see a trivial\n>> solution.\n>>\n>> But on second thought, we have an excuse to pick my proposed git-gui\n>> change anyway: Without that change and a fix in git-merge only, there\n>> is still a regression for all users who use the latest git-gui but\n>> some git version between 2.5.0 and the fixed git-merge...\n> \n> I'll leave it up to Pat, as I do not read tcl very well ;-)\n\nSure!\n\nIn the mean time, here is my attempt to fix git merge FETCH_HEAD.\nI would feel better if this were just taken as a starter for a\nreal fix by someone who is familiar with the area.\n\n---- 8< ----\n[PATCH] merge FETCH_HEAD: keep track of the branch names\n\n'git merge FETCH_HEAD' is treated differently from 'git merge topic'\nbecause FETCH_HEAD is not just a regular ref name, but contains the\nnames of the branches being merged. However, 'git merge' does not\nstore the names as the \"remote description\" of the merge parent\nstructure that is associated with merged commits, like an ordinary\n'git merge topic' call does for the command line arguments.\nA consequence is that conflict markers are not marked up with the\nbranch name, but with a raw object name. Parse off the branch names\nfrom the FETCH_HEAD file and store them with the merged commits.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n builtin/merge.c | 8 ++++++++\n commit.c        | 1 +\n 2 files changed, 9 insertions(+)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex b65eeaa87d..328517b091 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1051,9 +1051,17 @@ static void handle_fetch_head(struct commit_list **remotes, struct strbuf *merge\n \t\t\tcontinue; /* not-for-merge */\n \t\telse {\n \t\t\tchar saved = merge_names->buf[pos + 40];\n+\t\t\tchar eol = ptr ? *ptr : '\\0';\n \t\t\tmerge_names->buf[pos + 40] = '\\0';\n+\t\t\tif (ptr)\n+\t\t\t\t*ptr = '\\0';\n \t\t\tcommit = get_merge_parent(merge_names->buf + pos);\n+\t\t\tset_merge_remote_desc(commit,\n+\t\t\t\t\tmerge_names->buf + pos + 40 + 2,\n+\t\t\t\t\tmerge_remote_util(commit)->obj);\n \t\t\tmerge_names->buf[pos + 40] = saved;\n+\t\t\tif (ptr)\n+\t\t\t\t*ptr = eol;\n \t\t}\n \t\tif (!commit) {\n \t\t\tif (ptr)\ndiff --git a/commit.c b/commit.c\nindex 856fd4aeef..7ac2ce6518 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1582,6 +1582,7 @@ void set_merge_remote_desc(struct commit *commit,\n \tstruct merge_remote_desc *desc;\n \tFLEX_ALLOC_STR(desc, name, name);\n \tdesc->obj = obj;\n+\tfree(commit->util);\n \tcommit->util = desc;\n }\n \n-- \n2.11.0.rc1.52.g65ffb51\n\n"}]}