{"thread":{"id":"20493","subject":"[PATCH] merge: indicate remote tracking branches in merge message","startedAt":"2009-08-09T06:59:36Z","lastAt":"2009-08-09T21:49:12Z","messageCount":12,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"120031","messageId":"20090809065936.GA24112@coredump.intra.peff.net","threadId":"20493","inReplyTo":null,"subject":"[PATCH] merge: indicate remote tracking branches in merge message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-09T06:59:36Z","receivedAt":"2009-08-09T06:59:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Previously when merging directly from a local tracking\nbranch like:\n\n git merge origin/master\n\nThe merge message said:\n\n Merge commit 'origin/master'\n\n * commit 'origin/master':\n ...\n\nInstead, let's be more explicit about what we are merging:\n\n Merge remote branch 'origin/master'\n\n * origin/master:\n ...\n\nWe accomplish this by recognizing remote tracking branches\nin git-merge when we build the simulated FETCH_HEAD output\nthat we feed to fmt-merge-msg.\n\nSigned-off-by: Jeff King \n---\nThis is a repost of\n\n http://article.gmane.org/gmane.comp.version-control.git/119909\n\nwhich got no response from you. I think it is a good idea, but I am not\ndeeply committed to it. I mainly want a yes or no so I can clean it out\nof my patch queue.\n\n builtin-merge.c | 11 +++++++++++\n 1 files changed, 11 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex 0b12fb3..c5688e1 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -378,6 +378,17 @@ static void merge_name(const char *remote, struct strbuf *msg)\n \t\tgoto cleanup;\n \t}\n \n+\tstrbuf_setlen(&buf, 0);\n+\tstrbuf_addstr(&buf, \"refs/remotes/\");\n+\tstrbuf_addstr(&buf, remote);\n+\tresolve_ref(buf.buf, branch_head, 0, 0);\n+\n+\tif (!hashcmp(remote_head->sha1, branch_head)) {\n+\t\tstrbuf_addf(msg, \"%s\\t\\tremote branch '%s' of .\\n\",\n+\t\t\t\tsha1_to_hex(branch_head), remote);\n+\t\tgoto cleanup;\n+\t}\n+\n \t/* See if remote matches ^^^.. or ~ */\n \tfor (len = 0, ptr = remote + strlen(remote);\n \t remote < ptr && ptr[-1] == '^';\n-- \n1.6.4.72.g50730\n"},{"id":"120032","messageId":"20090809071446.GA24221@coredump.intra.peff.net","threadId":"20493","inReplyTo":"20090809065936.GA24112@coredump.intra.peff.net","subject":"Re: [PATCH] merge: indicate remote tracking branches in merge message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-09T07:14:46Z","receivedAt":"2009-08-09T07:14:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 09, 2009 at 02:59:36AM -0400, Jeff King wrote:\n\n> This is a repost of\n> \n> http://article.gmane.org/gmane.comp.version-control.git/119909\n> \n> which got no response from you. I think it is a good idea, but I am not\n> deeply committed to it. I mainly want a yes or no so I can clean it out\n> of my patch queue.\n\nAh, it may have had to do with the fact that it doesn't actually pass\nthe tests. There is a trivial text update needed in t3409, which calls\n\"git merge origin/master\" during its setup phase.\n\nUpdated, passing-all-tests patch is below. Sorry for the confusion.\n\n-- >8 --\nSubject: [PATCH] merge: indicate remote tracking branches in merge message\n\nPreviously when merging directly from a local tracking\nbranch like:\n\n git merge origin/master\n\nThe merge message said:\n\n Merge commit 'origin/master'\n\n * commit 'origin/master':\n ...\n\nInstead, let's be more explicit about what we are merging:\n\n Merge remote branch 'origin/master'\n\n * origin/master:\n ...\n\nWe accomplish this by recognizing remote tracking branches\nin git-merge when we build the simulated FETCH_HEAD output\nthat we feed to fmt-merge-msg.\n\nSigned-off-by: Jeff King \n---\n builtin-merge.c | 11 +++++++++++\n t/t3409-rebase-preserve-merges.sh | 2 +-\n 2 files changed, 12 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex 82b5466..1f59567 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -378,6 +378,17 @@ static void merge_name(const char *remote, struct strbuf *msg)\n \t\tgoto cleanup;\n \t}\n \n+\tstrbuf_setlen(&buf, 0);\n+\tstrbuf_addstr(&buf, \"refs/remotes/\");\n+\tstrbuf_addstr(&buf, remote);\n+\tresolve_ref(buf.buf, branch_head, 0, 0);\n+\n+\tif (!hashcmp(remote_head->sha1, branch_head)) {\n+\t\tstrbuf_addf(msg, \"%s\\t\\tremote branch '%s' of .\\n\",\n+\t\t\t\tsha1_to_hex(branch_head), remote);\n+\t\tgoto cleanup;\n+\t}\n+\n \t/* See if remote matches ^^^.. or ~ */\n \tfor (len = 0, ptr = remote + strlen(remote);\n \t remote < ptr && ptr[-1] == '^';\ndiff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh\nindex e6c8327..2ce0236 100755\n--- a/t/t3409-rebase-preserve-merges.sh\n+++ b/t/t3409-rebase-preserve-merges.sh\n@@ -71,7 +71,7 @@ test_expect_success 'rebase -p fakes interactive rebase' '\n \tgit fetch &&\n \tgit rebase -p origin/topic &&\n \ttest 1 = $(git rev-list --all --pretty=oneline | grep \"Modify A\" | wc -l) &&\n-\ttest 1 = $(git rev-list --all --pretty=oneline | grep \"Merge commit\" | wc -l)\n+\ttest 1 = $(git rev-list --all --pretty=oneline | grep \"Merge remote branch\" | wc -l)\n \t)\n '\n \n-- \n1.6.4.178.g7a987\n"},{"id":"120033","messageId":"7vab29a1fr.fsf@alter.siamese.dyndns.org","threadId":"20493","inReplyTo":"20090809065936.GA24112@coredump.intra.peff.net","subject":"Re: [PATCH] merge: indicate remote tracking branches in merge message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-09T07:31:04Z","receivedAt":"2009-08-09T07:31:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Previously when merging directly from a local tracking\n> branch like:\n>\n> git merge origin/master\n>\n> The merge message said:\n>\n> Merge commit 'origin/master'\n>\n> * commit 'origin/master':\n> ...\n>\n> Instead, let's be more explicit about what we are merging:\n>\n> Merge remote branch 'origin/master'\n>\n> * origin/master:\n> ...\n>\n> We accomplish this by recognizing remote tracking branches\n> in git-merge when we build the simulated FETCH_HEAD output\n> that we feed to fmt-merge-msg.\n>\n> Signed-off-by: Jeff King \n> ---\n> This is a repost of\n>\n> http://article.gmane.org/gmane.comp.version-control.git/119909\n>\n> which got no response from you. I think it is a good idea, but I am not\n> deeply committed to it. I mainly want a yes or no so I can clean it out\n> of my patch queue.\n\nI somewhat suspect that the patch was not applied because it also lacked\nnecessary adjustments to tests. With this patch, I think the tests would\nfail.\n\nNevertheless, I think it is a good thing to do. But I am unsure about the\nimplementation.\n\nShouldn't it instead feed what it got from the end user to the dwim\nmachinery, and make sure it dwims into refs/remotes/ hierarchy?\n\nIn other words, like this. Note that it would be much clearer to see\nwhat's needed, if you want to extend it to refs/tags hierarchy ;-)\n\n builtin-merge.c | 20 ++++++++++++--------\n 1 files changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex 82b5466..f4de73f 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -358,6 +358,7 @@ static void merge_name(const char *remote, struct strbuf *msg)\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct strbuf bname = STRBUF_INIT;\n \tconst char *ptr;\n+\tchar *found_ref;\n \tint len, early;\n \n \tstrbuf_branchname(&bname, remote);\n@@ -368,14 +369,17 @@ static void merge_name(const char *remote, struct strbuf *msg)\n \tif (!remote_head)\n \t\tdie(\"'%s' does not point to a commit\", remote);\n \n-\tstrbuf_addstr(&buf, \"refs/heads/\");\n-\tstrbuf_addstr(&buf, remote);\n-\tresolve_ref(buf.buf, branch_head, 0, NULL);\n-\n-\tif (!hashcmp(remote_head->sha1, branch_head)) {\n-\t\tstrbuf_addf(msg, \"%s\\t\\tbranch '%s' of .\\n\",\n-\t\t\tsha1_to_hex(branch_head), remote);\n-\t\tgoto cleanup;\n+\tif (dwim_ref(remote, strlen(remote), branch_head, &found_ref) > 0) {\n+\t\tif (!prefixcmp(found_ref, \"refs/heads/\")) {\n+\t\t\tstrbuf_addf(msg, \"%s\\t\\tbranch '%s' of .\\n\",\n+\t\t\t\t sha1_to_hex(branch_head), remote);\n+\t\t\tgoto cleanup;\n+\t\t}\n+\t\tif (!prefixcmp(found_ref, \"refs/remotes/\")) {\n+\t\t\tstrbuf_addf(msg, \"%s\\t\\tremote branch '%s' of .\\n\",\n+\t\t\t\t sha1_to_hex(branch_head), remote);\n+\t\t\tgoto cleanup;\n+\t\t}\n \t}\n \n \t/* See if remote matches ^^^.. or ~ */\n"},{"id":"120035","messageId":"20090809074035.GA4778@coredump.intra.peff.net","threadId":"20493","inReplyTo":"7vab29a1fr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] merge: indicate remote tracking branches in merge message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-09T07:40:35Z","receivedAt":"2009-08-09T07:40:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 09, 2009 at 12:31:04AM -0700, Junio C Hamano wrote:\n\n> I somewhat suspect that the patch was not applied because it also lacked\n> necessary adjustments to tests. With this patch, I think the tests would\n> fail.\n\nYeah, see my follow-up patch.\n\n> Nevertheless, I think it is a good thing to do. But I am unsure about the\n> implementation.\n> \n> Shouldn't it instead feed what it got from the end user to the dwim\n> machinery, and make sure it dwims into refs/remotes/ hierarchy?\n\nI'm not sure that is all that different in practice than what is\nhappening now. Mainly I did it the way I did so that I didn't touch the\ncode path for detecting local branches.\n\nBut assuming they are functionally identical, I think your patch is much\nmore readable.\n\n> In other words, like this. Note that it would be much clearer to see\n> what's needed, if you want to extend it to refs/tags hierarchy ;-)\n\nI'm not sure adding \"tag foo\" will actually work, as it still has to\nmake it through the bit where we parse FETCH_HEAD. I'm not sure if it\nwould get mutilated to \"commit foo\" by that code or not.\n\n-Peff\n"},{"id":"120042","messageId":"20090809091443.GA676@coredump.intra.peff.net","threadId":"20493","inReplyTo":"20090809074035.GA4778@coredump.intra.peff.net","subject":"Re: [PATCH] merge: indicate remote tracking branches in merge message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-09T09:14:43Z","receivedAt":"2009-08-09T09:14:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 09, 2009 at 03:40:35AM -0400, Jeff King wrote:\n\n> > Shouldn't it instead feed what it got from the end user to the dwim\n> > machinery, and make sure it dwims into refs/remotes/ hierarchy?\n> \n> I'm not sure that is all that different in practice than what is\n> happening now. Mainly I did it the way I did so that I didn't touch the\n> code path for detecting local branches.\n> \n> But assuming they are functionally identical, I think your patch is much\n> more readable.\n\nI tested your patch; the two methods aren't identical. In fact, yours\nfixes a bug. :)\n\nIn t4202, we have a branch name and a tag name that are the same\n(octopus-a), and we \"git merge octopus-a\". This actually merges the tag,\nbut because the branch name existed, we write \"Merge branch 'octopus-a'\"\nin the log, which is not true. With your patch, it does the right thing\nand says \"Merge commit 'octopus-a'\".\n\nThe simple thing is to just update the \"expect\" text. Though the current\nbehavior does show off the ability to collape the two branches and say\n\n Merge branches 'octopus-a' and 'octopus-b'\n\ninstead of\n\n Merge commit 'octopus-a'; commit 'octopus-b'\n\nwhich we could preserve that by renaming the tags, as below.\n\n---\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 48e0088..9d0db2a 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -306,13 +306,13 @@ test_expect_success 'set up more tangled history' '\n \tgit checkout master &&\n \tgit merge tangle &&\n \tgit checkout -b reach &&\n-\ttest_commit reach &&\n+\ttest_commit reach-1 &&\n \tgit checkout master &&\n \tgit checkout -b octopus-a &&\n-\ttest_commit octopus-a &&\n+\ttest_commit octopus-a-1 &&\n \tgit checkout master &&\n \tgit checkout -b octopus-b &&\n-\ttest_commit octopus-b &&\n+\ttest_commit octopus-b-1 &&\n \tgit checkout master &&\n \ttest_commit seventh &&\n \tgit merge octopus-a octopus-b\n@@ -327,12 +327,12 @@ cat > expect <<\\EOF\n *-. \\ Merge branches 'octopus-a' and 'octopus-b'\n |\\ \\ \\\n * | | | seventh\n-| | * | octopus-b\n+| | * | octopus-b-1\n | |/ /\n |/| |\n-| * | octopus-a\n+| * | octopus-a-1\n |/ /\n-| * reach\n+| * reach-1\n |/\n * Merge branch 'tangle'\n |\\\n"},{"id":"120045","messageId":"20090809100045.GA25197@coredump.intra.peff.net","threadId":"20493","inReplyTo":"20090809091443.GA676@coredump.intra.peff.net","subject":"Re: [PATCH] merge: indicate remote tracking branches in merge message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-09T10:00:45Z","receivedAt":"2009-08-09T10:00:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 09, 2009 at 05:14:43AM -0400, Jeff King wrote:\n\n> In t4202, we have a branch name and a tag name that are the same\n> (octopus-a), and we \"git merge octopus-a\". This actually merges the tag,\n> but because the branch name existed, we write \"Merge branch 'octopus-a'\"\n> in the log, which is not true. With your patch, it does the right thing\n> and says \"Merge commit 'octopus-a'\".\n> \n> The simple thing is to just update the \"expect\" text. Though the current\n> behavior does show off the ability to collape the two branches and say\n> \n> Merge branches 'octopus-a' and 'octopus-b'\n> \n> instead of\n> \n> Merge commit 'octopus-a'; commit 'octopus-b'\n\nThinking about it for a few seconds, it's silly to try to test something\nthat happens to occur in a totally unrelated test. The right thing to do\nis to write actual tests for this area, fix the bug, and then add the\nnew feature. So how about this series:\n\n [1/3] add tests for merge message headings\n [2/3] merge: fix incorrect merge message for ambiguous tag/branch\n [3/3] merge: indicate remote tracking branches in merge message\n\n-Peff\n"},{"id":"120046","messageId":"20090809100148.GA25231@coredump.intra.peff.net","threadId":"20493","inReplyTo":"20090809100045.GA25197@coredump.intra.peff.net","subject":"[PATCH 1/3] add tests for merge message headings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-09T10:01:48Z","receivedAt":"2009-08-09T10:01:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When calling \"git merge $X\", we automatically generate a\ncommit message containing something like \"Merge branch\n'$X'\". This test script checks that those messages say what\nthey should, and exposes a failure when merging a refname\nthat is ambiguous between a tag and a branch.\n\nSigned-off-by: Jeff King \n---\n t/t7608-merge-messages.sh | 50 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 50 insertions(+), 0 deletions(-)\n create mode 100755 t/t7608-merge-messages.sh\n\ndiff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh\nnew file mode 100755\nindex 0000000..9d10583\n--- /dev/null\n+++ b/t/t7608-merge-messages.sh\n@@ -0,0 +1,50 @@\n+#!/bin/sh\n+\n+test_description='test auto-generated merge messages'\n+. ./test-lib.sh\n+\n+check_oneline() {\n+\techo \"$1\" | sed \"s/Q/'/g\" >expect &&\n+\tgit log -1 --pretty=tformat:%s >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success 'merge local branch' '\n+\ttest_commit master-1 &&\n+\tgit checkout -b local-branch &&\n+\ttest_commit branch-1 &&\n+\tgit checkout master &&\n+\ttest_commit master-2 &&\n+\tgit merge local-branch &&\n+\tcheck_oneline \"Merge branch Qlocal-branchQ\"\n+'\n+\n+test_expect_success 'merge octopus branches' '\n+\tgit checkout -b octopus-a master &&\n+\ttest_commit octopus-1 &&\n+\tgit checkout -b octopus-b master &&\n+\ttest_commit octopus-2 &&\n+\tgit checkout master &&\n+\tgit merge octopus-a octopus-b &&\n+\tcheck_oneline \"Merge branches Qoctopus-aQ and Qoctopus-bQ\"\n+'\n+\n+test_expect_success 'merge tag' '\n+\tgit checkout -b tag-branch master &&\n+\ttest_commit tag-1 &&\n+\tgit checkout master &&\n+\ttest_commit master-3 &&\n+\tgit merge tag-1 &&\n+\tcheck_oneline \"Merge commit Qtag-1Q\"\n+'\n+\n+test_expect_failure 'ambiguous tag' '\n+\tgit checkout -b ambiguous master &&\n+\ttest_commit ambiguous &&\n+\tgit checkout master &&\n+\ttest_commit master-4 &&\n+\tgit merge ambiguous &&\n+\tcheck_oneline \"Merge commit QambiguousQ\"\n+'\n+\n+test_done\n-- \n1.6.4.178.g7a987\n"},{"id":"120047","messageId":"20090809100224.GB25231@coredump.intra.peff.net","threadId":"20493","inReplyTo":"20090809100045.GA25197@coredump.intra.peff.net","subject":"[PATCH 2/3] merge: fix incorrect merge message for ambiguous tag/branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-09T10:02:24Z","receivedAt":"2009-08-09T10:02:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If we have both a tag and a branch named \"foo\", then calling\n\"git merge foo\" will warn about the ambiguous ref, but merge\nthe tag.\n\nWhen generating the commit message, though, we simply\nchecked whether \"refs/heads/foo\" existed, and if it did,\nassumed it was a branch. This led to the statement \"Merge\nbranch 'foo'\" in the commit message, which is quite wrong.\n\nInstead, we should use dwim_ref to find the actual ref used,\nand describe it appropriately.\n\nIn addition to the test in t7608, we must also tweak the\nexpected output of t4202, which was accidentally triggering\nthis bug.\n\nSigned-off-by: Jeff King \n---\n builtin-merge.c | 15 +++++++--------\n t/t4202-log.sh | 4 ++--\n t/t7608-merge-messages.sh | 2 +-\n 3 files changed, 10 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex 82b5466..f7db148 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -358,6 +358,7 @@ static void merge_name(const char *remote, struct strbuf *msg)\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct strbuf bname = STRBUF_INIT;\n \tconst char *ptr;\n+\tchar *found_ref;\n \tint len, early;\n \n \tstrbuf_branchname(&bname, remote);\n@@ -368,14 +369,12 @@ static void merge_name(const char *remote, struct strbuf *msg)\n \tif (!remote_head)\n \t\tdie(\"'%s' does not point to a commit\", remote);\n \n-\tstrbuf_addstr(&buf, \"refs/heads/\");\n-\tstrbuf_addstr(&buf, remote);\n-\tresolve_ref(buf.buf, branch_head, 0, NULL);\n-\n-\tif (!hashcmp(remote_head->sha1, branch_head)) {\n-\t\tstrbuf_addf(msg, \"%s\\t\\tbranch '%s' of .\\n\",\n-\t\t\tsha1_to_hex(branch_head), remote);\n-\t\tgoto cleanup;\n+\tif (dwim_ref(remote, strlen(remote), branch_head, &found_ref) > 0) {\n+\t\tif (!prefixcmp(found_ref, \"refs/heads/\")) {\n+\t\t\tstrbuf_addf(msg, \"%s\\t\\tbranch '%s' of .\\n\",\n+\t\t\t\t sha1_to_hex(branch_head), remote);\n+\t\t\tgoto cleanup;\n+\t\t}\n \t}\n \n \t/* See if remote matches ^^^.. or ~ */\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 48e0088..1e952ca 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -320,11 +320,11 @@ test_expect_success 'set up more tangled history' '\n '\n \n cat > expect <<\\EOF\n-* Merge branch 'reach'\n+* Merge commit 'reach'\n |\\\n | \\\n | \\\n-*-. \\ Merge branches 'octopus-a' and 'octopus-b'\n+*-. \\ Merge commit 'octopus-a'; commit 'octopus-b'\n |\\ \\ \\\n * | | | seventh\n | | * | octopus-b\ndiff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh\nindex 9d10583..81ced8a 100755\n--- a/t/t7608-merge-messages.sh\n+++ b/t/t7608-merge-messages.sh\n@@ -38,7 +38,7 @@ test_expect_success 'merge tag' '\n \tcheck_oneline \"Merge commit Qtag-1Q\"\n '\n \n-test_expect_failure 'ambiguous tag' '\n+test_expect_success 'ambiguous tag' '\n \tgit checkout -b ambiguous master &&\n \ttest_commit ambiguous &&\n \tgit checkout master &&\n-- \n1.6.4.178.g7a987\n"},{"id":"120048","messageId":"20090809100251.GC25231@coredump.intra.peff.net","threadId":"20493","inReplyTo":"20090809100045.GA25197@coredump.intra.peff.net","subject":"[PATCH 3/3] merge: indicate remote tracking branches in merge message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-09T10:02:51Z","receivedAt":"2009-08-09T10:02:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Previously when merging directly from a local tracking\nbranch like:\n\n git merge origin/master\n\nThe merge message said:\n\n Merge commit 'origin/master'\n\n * commit 'origin/master':\n ...\n\nInstead, let's be more explicit about what we are merging:\n\n Merge remote branch 'origin/master'\n\n * origin/master:\n ...\n\nWe accomplish this by recognizing remote tracking branches\nin git-merge when we build the simulated FETCH_HEAD output\nthat we feed to fmt-merge-msg.\n\nIn addition to a new test in t7608, we have to tweak the\nexpected output of t3409, which does such a merge.\n\nSigned-off-by: Jeff King \n---\n builtin-merge.c | 5 +++++\n t/t3409-rebase-preserve-merges.sh | 2 +-\n t/t7608-merge-messages.sh | 10 ++++++++++\n 3 files changed, 16 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex f7db148..f4de73f 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -375,6 +375,11 @@ static void merge_name(const char *remote, struct strbuf *msg)\n \t\t\t\t sha1_to_hex(branch_head), remote);\n \t\t\tgoto cleanup;\n \t\t}\n+\t\tif (!prefixcmp(found_ref, \"refs/remotes/\")) {\n+\t\t\tstrbuf_addf(msg, \"%s\\t\\tremote branch '%s' of .\\n\",\n+\t\t\t\t sha1_to_hex(branch_head), remote);\n+\t\t\tgoto cleanup;\n+\t\t}\n \t}\n \n \t/* See if remote matches ^^^.. or ~ */\ndiff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh\nindex e6c8327..297d165 100755\n--- a/t/t3409-rebase-preserve-merges.sh\n+++ b/t/t3409-rebase-preserve-merges.sh\n@@ -71,7 +71,7 @@ test_expect_success 'rebase -p fakes interactive rebase' '\n \tgit fetch &&\n \tgit rebase -p origin/topic &&\n \ttest 1 = $(git rev-list --all --pretty=oneline | grep \"Modify A\" | wc -l) &&\n-\ttest 1 = $(git rev-list --all --pretty=oneline | grep \"Merge commit\" | wc -l)\n+\ttest 1 = $(git rev-list --all --pretty=oneline | grep \"Merge remote branch \" | wc -l)\n \t)\n '\n \ndiff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh\nindex 81ced8a..28d5679 100755\n--- a/t/t7608-merge-messages.sh\n+++ b/t/t7608-merge-messages.sh\n@@ -47,4 +47,14 @@ test_expect_success 'ambiguous tag' '\n \tcheck_oneline \"Merge commit QambiguousQ\"\n '\n \n+test_expect_success 'remote branch' '\n+\tgit checkout -b remote master &&\n+\ttest_commit remote-1 &&\n+\tgit update-ref refs/remotes/origin/master remote &&\n+\tgit checkout master &&\n+\ttest_commit master-5 &&\n+\tgit merge origin/master &&\n+\tcheck_oneline \"Merge remote branch Qorigin/masterQ\"\n+'\n+\n test_done\n-- \n1.6.4.178.g7a987\n"},{"id":"120049","messageId":"20090809100712.GA26250@coredump.intra.peff.net","threadId":"20493","inReplyTo":"20090809100045.GA25197@coredump.intra.peff.net","subject":"Re: [PATCH] merge: indicate remote tracking branches in merge message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-09T10:07:12Z","receivedAt":"2009-08-09T10:07:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 09, 2009 at 06:00:45AM -0400, Jeff King wrote:\n\n> [1/3] add tests for merge message headings\n> [2/3] merge: fix incorrect merge message for ambiguous tag/branch\n> [3/3] merge: indicate remote tracking branches in merge message\n\nAnd here is the 4/3 you mentioned earlier:\n\n-- >8 --\nSubject: [PATCH] merge: describe tags as such in merge message\n\nPreviously, merging a tag directly via \"git merge tag\" would\nget you the message \"Merge commit 'tag'\". It is a little\nmore descriptive to note that it was actually a tag (i.e.,\n\"Merge tag 'tag'\").\n\nSigned-off-by: Jeff King \n---\n builtin-merge.c | 5 +++++\n t/t7608-merge-messages.sh | 4 ++--\n 2 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex f4de73f..db74901 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -380,6 +380,11 @@ static void merge_name(const char *remote, struct strbuf *msg)\n \t\t\t\t sha1_to_hex(branch_head), remote);\n \t\t\tgoto cleanup;\n \t\t}\n+\t\tif (!prefixcmp(found_ref, \"refs/tags/\")) {\n+\t\t\tstrbuf_addf(msg, \"%s\\t\\ttag '%s' of .\\n\",\n+\t\t\t\t sha1_to_hex(branch_head), remote);\n+\t\t\tgoto cleanup;\n+\t\t}\n \t}\n \n \t/* See if remote matches ^^^.. or ~ */\ndiff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh\nindex 28d5679..3ee0983 100755\n--- a/t/t7608-merge-messages.sh\n+++ b/t/t7608-merge-messages.sh\n@@ -35,7 +35,7 @@ test_expect_success 'merge tag' '\n \tgit checkout master &&\n \ttest_commit master-3 &&\n \tgit merge tag-1 &&\n-\tcheck_oneline \"Merge commit Qtag-1Q\"\n+\tcheck_oneline \"Merge tag Qtag-1Q\"\n '\n \n test_expect_success 'ambiguous tag' '\n@@ -44,7 +44,7 @@ test_expect_success 'ambiguous tag' '\n \tgit checkout master &&\n \ttest_commit master-4 &&\n \tgit merge ambiguous &&\n-\tcheck_oneline \"Merge commit QambiguousQ\"\n+\tcheck_oneline \"Merge tag QambiguousQ\"\n '\n \n test_expect_success 'remote branch' '\n-- \n1.6.4.178.g7a987\n"},{"id":"120083","messageId":"7vprb47pam.fsf@alter.siamese.dyndns.org","threadId":"20493","inReplyTo":"20090809100712.GA26250@coredump.intra.peff.net","subject":"Re: [PATCH] merge: indicate remote tracking branches in merge message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-09T19:36:17Z","receivedAt":"2009-08-09T19:36:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sun, Aug 09, 2009 at 06:00:45AM -0400, Jeff King wrote:\n>\n>> [1/3] add tests for merge message headings\n>> [2/3] merge: fix incorrect merge message for ambiguous tag/branch\n>> [3/3] merge: indicate remote tracking branches in merge message\n>\n> And here is the 4/3 you mentioned earlier:\n>\n> -- >8 --\n> Subject: [PATCH] merge: describe tags as such in merge message\n>\n> Previously, merging a tag directly via \"git merge tag\" would\n> get you the message \"Merge commit 'tag'\". It is a little\n> more descriptive to note that it was actually a tag (i.e.,\n> \"Merge tag 'tag'\").\n\nMaybe \"Merge version 'v1.6.3'\" or \"Merge commit tagged as 'v1.6.3'\"? \nI dunno.\n"},{"id":"120096","messageId":"20090809214911.GA27796@coredump.intra.peff.net","threadId":"20493","inReplyTo":"7vprb47pam.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] merge: indicate remote tracking branches in merge message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-09T21:49:12Z","receivedAt":"2009-08-09T21:49:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 09, 2009 at 12:36:17PM -0700, Junio C Hamano wrote:\n\n> > Previously, merging a tag directly via \"git merge tag\" would\n> > get you the message \"Merge commit 'tag'\". It is a little\n> > more descriptive to note that it was actually a tag (i.e.,\n> > \"Merge tag 'tag'\").\n> \n> Maybe \"Merge version 'v1.6.3'\" or \"Merge commit tagged as 'v1.6.3'\"? \n> I dunno.\n\nDefinitely \"Merge version\" is bad. Doing \"git tag feature-x-working\" and\nmerging it would lead to \"Merge version 'feature-x-working'\", which\ndoesn't really make sense. I don't think the developer would refer to\nsuch a tag as a \"version\".\n\nYour \"Merge commit tagged as...\" version is reasonable to me. I'm\ndubious whether the additional information that it was a tag is actually\nworth anything. That is, given\n\n (1) Merge commit '1234abcd'\n (2) Merge commit 'v1.6.3'\n (3) Merge tag 'v1.6.3'\n\nwhere (1) is what we do now and will do in the future for arbitrary\ncommits, (2) is what we do now for tags, and (3) is the proposed change.\nI am not sure that (3) really tells the reader anything useful over the\nother two. And your \"Merge commit tagged as\" is just the same\ninformation written slightly differently.\n\nSo maybe this patch should simply be dropped. I don't feel strongly\neither way.\n\n-Peff\n"}]}