* [PATCH] merge: indicate remote tracking branches in merge message
@ 2009-08-09 6:59 Jeff King
2009-08-09 7:14 ` Jeff King
2009-08-09 7:31 ` Junio C Hamano
0 siblings, 2 replies; 12+ messages in thread
From: Jeff King @ 2009-08-09 6:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Previously when merging directly from a local tracking
branch like:
git merge origin/master
The merge message said:
Merge commit 'origin/master'
* commit 'origin/master':
...
Instead, let's be more explicit about what we are merging:
Merge remote branch 'origin/master'
* origin/master:
...
We accomplish this by recognizing remote tracking branches
in git-merge when we build the simulated FETCH_HEAD output
that we feed to fmt-merge-msg.
Signed-off-by: Jeff King <peff@peff.net>
---
This is a repost of
http://article.gmane.org/gmane.comp.version-control.git/119909
which got no response from you. I think it is a good idea, but I am not
deeply committed to it. I mainly want a yes or no so I can clean it out
of my patch queue.
builtin-merge.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index 0b12fb3..c5688e1 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -378,6 +378,17 @@ static void merge_name(const char *remote, struct strbuf *msg)
goto cleanup;
}
+ strbuf_setlen(&buf, 0);
+ strbuf_addstr(&buf, "refs/remotes/");
+ strbuf_addstr(&buf, remote);
+ resolve_ref(buf.buf, branch_head, 0, 0);
+
+ if (!hashcmp(remote_head->sha1, branch_head)) {
+ strbuf_addf(msg, "%s\t\tremote branch '%s' of .\n",
+ sha1_to_hex(branch_head), remote);
+ goto cleanup;
+ }
+
/* See if remote matches <name>^^^.. or <name>~<number> */
for (len = 0, ptr = remote + strlen(remote);
remote < ptr && ptr[-1] == '^';
--
1.6.4.72.g50730
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] merge: indicate remote tracking branches in merge message 2009-08-09 6:59 [PATCH] merge: indicate remote tracking branches in merge message Jeff King @ 2009-08-09 7:14 ` Jeff King 2009-08-09 7:31 ` Junio C Hamano 1 sibling, 0 replies; 12+ messages in thread From: Jeff King @ 2009-08-09 7:14 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Sun, Aug 09, 2009 at 02:59:36AM -0400, Jeff King wrote: > This is a repost of > > http://article.gmane.org/gmane.comp.version-control.git/119909 > > which got no response from you. I think it is a good idea, but I am not > deeply committed to it. I mainly want a yes or no so I can clean it out > of my patch queue. Ah, it may have had to do with the fact that it doesn't actually pass the tests. There is a trivial text update needed in t3409, which calls "git merge origin/master" during its setup phase. Updated, passing-all-tests patch is below. Sorry for the confusion. -- >8 -- Subject: [PATCH] merge: indicate remote tracking branches in merge message Previously when merging directly from a local tracking branch like: git merge origin/master The merge message said: Merge commit 'origin/master' * commit 'origin/master': ... Instead, let's be more explicit about what we are merging: Merge remote branch 'origin/master' * origin/master: ... We accomplish this by recognizing remote tracking branches in git-merge when we build the simulated FETCH_HEAD output that we feed to fmt-merge-msg. Signed-off-by: Jeff King <peff@peff.net> --- builtin-merge.c | 11 +++++++++++ t/t3409-rebase-preserve-merges.sh | 2 +- 2 files changed, 12 insertions(+), 1 deletions(-) diff --git a/builtin-merge.c b/builtin-merge.c index 82b5466..1f59567 100644 --- a/builtin-merge.c +++ b/builtin-merge.c @@ -378,6 +378,17 @@ static void merge_name(const char *remote, struct strbuf *msg) goto cleanup; } + strbuf_setlen(&buf, 0); + strbuf_addstr(&buf, "refs/remotes/"); + strbuf_addstr(&buf, remote); + resolve_ref(buf.buf, branch_head, 0, 0); + + if (!hashcmp(remote_head->sha1, branch_head)) { + strbuf_addf(msg, "%s\t\tremote branch '%s' of .\n", + sha1_to_hex(branch_head), remote); + goto cleanup; + } + /* See if remote matches <name>^^^.. or <name>~<number> */ for (len = 0, ptr = remote + strlen(remote); remote < ptr && ptr[-1] == '^'; diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh index e6c8327..2ce0236 100755 --- a/t/t3409-rebase-preserve-merges.sh +++ b/t/t3409-rebase-preserve-merges.sh @@ -71,7 +71,7 @@ test_expect_success 'rebase -p fakes interactive rebase' ' git fetch && git rebase -p origin/topic && test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) && - test 1 = $(git rev-list --all --pretty=oneline | grep "Merge commit" | wc -l) + test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote branch" | wc -l) ) ' -- 1.6.4.178.g7a987 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] merge: indicate remote tracking branches in merge message 2009-08-09 6:59 [PATCH] merge: indicate remote tracking branches in merge message Jeff King 2009-08-09 7:14 ` Jeff King @ 2009-08-09 7:31 ` Junio C Hamano 2009-08-09 7:40 ` Jeff King 1 sibling, 1 reply; 12+ messages in thread From: Junio C Hamano @ 2009-08-09 7:31 UTC (permalink / raw) To: Jeff King; +Cc: git Jeff King <peff@peff.net> writes: > Previously when merging directly from a local tracking > branch like: > > git merge origin/master > > The merge message said: > > Merge commit 'origin/master' > > * commit 'origin/master': > ... > > Instead, let's be more explicit about what we are merging: > > Merge remote branch 'origin/master' > > * origin/master: > ... > > We accomplish this by recognizing remote tracking branches > in git-merge when we build the simulated FETCH_HEAD output > that we feed to fmt-merge-msg. > > Signed-off-by: Jeff King <peff@peff.net> > --- > This is a repost of > > http://article.gmane.org/gmane.comp.version-control.git/119909 > > which got no response from you. I think it is a good idea, but I am not > deeply committed to it. I mainly want a yes or no so I can clean it out > of my patch queue. I somewhat suspect that the patch was not applied because it also lacked necessary adjustments to tests. With this patch, I think the tests would fail. Nevertheless, I think it is a good thing to do. But I am unsure about the implementation. Shouldn't it instead feed what it got from the end user to the dwim machinery, and make sure it dwims into refs/remotes/ hierarchy? In other words, like this. Note that it would be much clearer to see what's needed, if you want to extend it to refs/tags hierarchy ;-) builtin-merge.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-) diff --git a/builtin-merge.c b/builtin-merge.c index 82b5466..f4de73f 100644 --- a/builtin-merge.c +++ b/builtin-merge.c @@ -358,6 +358,7 @@ static void merge_name(const char *remote, struct strbuf *msg) struct strbuf buf = STRBUF_INIT; struct strbuf bname = STRBUF_INIT; const char *ptr; + char *found_ref; int len, early; strbuf_branchname(&bname, remote); @@ -368,14 +369,17 @@ static void merge_name(const char *remote, struct strbuf *msg) if (!remote_head) die("'%s' does not point to a commit", remote); - strbuf_addstr(&buf, "refs/heads/"); - strbuf_addstr(&buf, remote); - resolve_ref(buf.buf, branch_head, 0, NULL); - - if (!hashcmp(remote_head->sha1, branch_head)) { - strbuf_addf(msg, "%s\t\tbranch '%s' of .\n", - sha1_to_hex(branch_head), remote); - goto cleanup; + if (dwim_ref(remote, strlen(remote), branch_head, &found_ref) > 0) { + if (!prefixcmp(found_ref, "refs/heads/")) { + strbuf_addf(msg, "%s\t\tbranch '%s' of .\n", + sha1_to_hex(branch_head), remote); + goto cleanup; + } + if (!prefixcmp(found_ref, "refs/remotes/")) { + strbuf_addf(msg, "%s\t\tremote branch '%s' of .\n", + sha1_to_hex(branch_head), remote); + goto cleanup; + } } /* See if remote matches <name>^^^.. or <name>~<number> */ ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] merge: indicate remote tracking branches in merge message 2009-08-09 7:31 ` Junio C Hamano @ 2009-08-09 7:40 ` Jeff King 2009-08-09 9:14 ` Jeff King 0 siblings, 1 reply; 12+ messages in thread From: Jeff King @ 2009-08-09 7:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Sun, Aug 09, 2009 at 12:31:04AM -0700, Junio C Hamano wrote: > I somewhat suspect that the patch was not applied because it also lacked > necessary adjustments to tests. With this patch, I think the tests would > fail. Yeah, see my follow-up patch. > Nevertheless, I think it is a good thing to do. But I am unsure about the > implementation. > > Shouldn't it instead feed what it got from the end user to the dwim > machinery, and make sure it dwims into refs/remotes/ hierarchy? I'm not sure that is all that different in practice than what is happening now. Mainly I did it the way I did so that I didn't touch the code path for detecting local branches. But assuming they are functionally identical, I think your patch is much more readable. > In other words, like this. Note that it would be much clearer to see > what's needed, if you want to extend it to refs/tags hierarchy ;-) I'm not sure adding "tag foo" will actually work, as it still has to make it through the bit where we parse FETCH_HEAD. I'm not sure if it would get mutilated to "commit foo" by that code or not. -Peff ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] merge: indicate remote tracking branches in merge message 2009-08-09 7:40 ` Jeff King @ 2009-08-09 9:14 ` Jeff King 2009-08-09 10:00 ` Jeff King 0 siblings, 1 reply; 12+ messages in thread From: Jeff King @ 2009-08-09 9:14 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Sun, Aug 09, 2009 at 03:40:35AM -0400, Jeff King wrote: > > Shouldn't it instead feed what it got from the end user to the dwim > > machinery, and make sure it dwims into refs/remotes/ hierarchy? > > I'm not sure that is all that different in practice than what is > happening now. Mainly I did it the way I did so that I didn't touch the > code path for detecting local branches. > > But assuming they are functionally identical, I think your patch is much > more readable. I tested your patch; the two methods aren't identical. In fact, yours fixes a bug. :) In t4202, we have a branch name and a tag name that are the same (octopus-a), and we "git merge octopus-a". This actually merges the tag, but because the branch name existed, we write "Merge branch 'octopus-a'" in the log, which is not true. With your patch, it does the right thing and says "Merge commit 'octopus-a'". The simple thing is to just update the "expect" text. Though the current behavior does show off the ability to collape the two branches and say Merge branches 'octopus-a' and 'octopus-b' instead of Merge commit 'octopus-a'; commit 'octopus-b' which we could preserve that by renaming the tags, as below. --- diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 48e0088..9d0db2a 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -306,13 +306,13 @@ test_expect_success 'set up more tangled history' ' git checkout master && git merge tangle && git checkout -b reach && - test_commit reach && + test_commit reach-1 && git checkout master && git checkout -b octopus-a && - test_commit octopus-a && + test_commit octopus-a-1 && git checkout master && git checkout -b octopus-b && - test_commit octopus-b && + test_commit octopus-b-1 && git checkout master && test_commit seventh && git merge octopus-a octopus-b @@ -327,12 +327,12 @@ cat > expect <<\EOF *-. \ Merge branches 'octopus-a' and 'octopus-b' |\ \ \ * | | | seventh -| | * | octopus-b +| | * | octopus-b-1 | |/ / |/| | -| * | octopus-a +| * | octopus-a-1 |/ / -| * reach +| * reach-1 |/ * Merge branch 'tangle' |\ ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] merge: indicate remote tracking branches in merge message 2009-08-09 9:14 ` Jeff King @ 2009-08-09 10:00 ` Jeff King 2009-08-09 10:01 ` [PATCH 1/3] add tests for merge message headings Jeff King ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Jeff King @ 2009-08-09 10:00 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Sun, Aug 09, 2009 at 05:14:43AM -0400, Jeff King wrote: > In t4202, we have a branch name and a tag name that are the same > (octopus-a), and we "git merge octopus-a". This actually merges the tag, > but because the branch name existed, we write "Merge branch 'octopus-a'" > in the log, which is not true. With your patch, it does the right thing > and says "Merge commit 'octopus-a'". > > The simple thing is to just update the "expect" text. Though the current > behavior does show off the ability to collape the two branches and say > > Merge branches 'octopus-a' and 'octopus-b' > > instead of > > Merge commit 'octopus-a'; commit 'octopus-b' Thinking about it for a few seconds, it's silly to try to test something that happens to occur in a totally unrelated test. The right thing to do is to write actual tests for this area, fix the bug, and then add the new feature. So how about this series: [1/3] add tests for merge message headings [2/3] merge: fix incorrect merge message for ambiguous tag/branch [3/3] merge: indicate remote tracking branches in merge message -Peff ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] add tests for merge message headings 2009-08-09 10:00 ` Jeff King @ 2009-08-09 10:01 ` Jeff King 2009-08-09 10:02 ` [PATCH 2/3] merge: fix incorrect merge message for ambiguous tag/branch Jeff King ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Jeff King @ 2009-08-09 10:01 UTC (permalink / raw) To: Junio C Hamano; +Cc: git When calling "git merge $X", we automatically generate a commit message containing something like "Merge branch '$X'". This test script checks that those messages say what they should, and exposes a failure when merging a refname that is ambiguous between a tag and a branch. Signed-off-by: Jeff King <peff@peff.net> --- t/t7608-merge-messages.sh | 50 +++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 50 insertions(+), 0 deletions(-) create mode 100755 t/t7608-merge-messages.sh diff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh new file mode 100755 index 0000000..9d10583 --- /dev/null +++ b/t/t7608-merge-messages.sh @@ -0,0 +1,50 @@ +#!/bin/sh + +test_description='test auto-generated merge messages' +. ./test-lib.sh + +check_oneline() { + echo "$1" | sed "s/Q/'/g" >expect && + git log -1 --pretty=tformat:%s >actual && + test_cmp expect actual +} + +test_expect_success 'merge local branch' ' + test_commit master-1 && + git checkout -b local-branch && + test_commit branch-1 && + git checkout master && + test_commit master-2 && + git merge local-branch && + check_oneline "Merge branch Qlocal-branchQ" +' + +test_expect_success 'merge octopus branches' ' + git checkout -b octopus-a master && + test_commit octopus-1 && + git checkout -b octopus-b master && + test_commit octopus-2 && + git checkout master && + git merge octopus-a octopus-b && + check_oneline "Merge branches Qoctopus-aQ and Qoctopus-bQ" +' + +test_expect_success 'merge tag' ' + git checkout -b tag-branch master && + test_commit tag-1 && + git checkout master && + test_commit master-3 && + git merge tag-1 && + check_oneline "Merge commit Qtag-1Q" +' + +test_expect_failure 'ambiguous tag' ' + git checkout -b ambiguous master && + test_commit ambiguous && + git checkout master && + test_commit master-4 && + git merge ambiguous && + check_oneline "Merge commit QambiguousQ" +' + +test_done -- 1.6.4.178.g7a987 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] merge: fix incorrect merge message for ambiguous tag/branch 2009-08-09 10:00 ` Jeff King 2009-08-09 10:01 ` [PATCH 1/3] add tests for merge message headings Jeff King @ 2009-08-09 10:02 ` Jeff King 2009-08-09 10:02 ` [PATCH 3/3] merge: indicate remote tracking branches in merge message Jeff King 2009-08-09 10:07 ` [PATCH] " Jeff King 3 siblings, 0 replies; 12+ messages in thread From: Jeff King @ 2009-08-09 10:02 UTC (permalink / raw) To: Junio C Hamano; +Cc: git If we have both a tag and a branch named "foo", then calling "git merge foo" will warn about the ambiguous ref, but merge the tag. When generating the commit message, though, we simply checked whether "refs/heads/foo" existed, and if it did, assumed it was a branch. This led to the statement "Merge branch 'foo'" in the commit message, which is quite wrong. Instead, we should use dwim_ref to find the actual ref used, and describe it appropriately. In addition to the test in t7608, we must also tweak the expected output of t4202, which was accidentally triggering this bug. Signed-off-by: Jeff King <peff@peff.net> --- builtin-merge.c | 15 +++++++-------- t/t4202-log.sh | 4 ++-- t/t7608-merge-messages.sh | 2 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/builtin-merge.c b/builtin-merge.c index 82b5466..f7db148 100644 --- a/builtin-merge.c +++ b/builtin-merge.c @@ -358,6 +358,7 @@ static void merge_name(const char *remote, struct strbuf *msg) struct strbuf buf = STRBUF_INIT; struct strbuf bname = STRBUF_INIT; const char *ptr; + char *found_ref; int len, early; strbuf_branchname(&bname, remote); @@ -368,14 +369,12 @@ static void merge_name(const char *remote, struct strbuf *msg) if (!remote_head) die("'%s' does not point to a commit", remote); - strbuf_addstr(&buf, "refs/heads/"); - strbuf_addstr(&buf, remote); - resolve_ref(buf.buf, branch_head, 0, NULL); - - if (!hashcmp(remote_head->sha1, branch_head)) { - strbuf_addf(msg, "%s\t\tbranch '%s' of .\n", - sha1_to_hex(branch_head), remote); - goto cleanup; + if (dwim_ref(remote, strlen(remote), branch_head, &found_ref) > 0) { + if (!prefixcmp(found_ref, "refs/heads/")) { + strbuf_addf(msg, "%s\t\tbranch '%s' of .\n", + sha1_to_hex(branch_head), remote); + goto cleanup; + } } /* See if remote matches <name>^^^.. or <name>~<number> */ diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 48e0088..1e952ca 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -320,11 +320,11 @@ test_expect_success 'set up more tangled history' ' ' cat > expect <<\EOF -* Merge branch 'reach' +* Merge commit 'reach' |\ | \ | \ -*-. \ Merge branches 'octopus-a' and 'octopus-b' +*-. \ Merge commit 'octopus-a'; commit 'octopus-b' |\ \ \ * | | | seventh | | * | octopus-b diff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh index 9d10583..81ced8a 100755 --- a/t/t7608-merge-messages.sh +++ b/t/t7608-merge-messages.sh @@ -38,7 +38,7 @@ test_expect_success 'merge tag' ' check_oneline "Merge commit Qtag-1Q" ' -test_expect_failure 'ambiguous tag' ' +test_expect_success 'ambiguous tag' ' git checkout -b ambiguous master && test_commit ambiguous && git checkout master && -- 1.6.4.178.g7a987 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] merge: indicate remote tracking branches in merge message 2009-08-09 10:00 ` Jeff King 2009-08-09 10:01 ` [PATCH 1/3] add tests for merge message headings Jeff King 2009-08-09 10:02 ` [PATCH 2/3] merge: fix incorrect merge message for ambiguous tag/branch Jeff King @ 2009-08-09 10:02 ` Jeff King 2009-08-09 10:07 ` [PATCH] " Jeff King 3 siblings, 0 replies; 12+ messages in thread From: Jeff King @ 2009-08-09 10:02 UTC (permalink / raw) To: Junio C Hamano; +Cc: git Previously when merging directly from a local tracking branch like: git merge origin/master The merge message said: Merge commit 'origin/master' * commit 'origin/master': ... Instead, let's be more explicit about what we are merging: Merge remote branch 'origin/master' * origin/master: ... We accomplish this by recognizing remote tracking branches in git-merge when we build the simulated FETCH_HEAD output that we feed to fmt-merge-msg. In addition to a new test in t7608, we have to tweak the expected output of t3409, which does such a merge. Signed-off-by: Jeff King <peff@peff.net> --- builtin-merge.c | 5 +++++ t/t3409-rebase-preserve-merges.sh | 2 +- t/t7608-merge-messages.sh | 10 ++++++++++ 3 files changed, 16 insertions(+), 1 deletions(-) diff --git a/builtin-merge.c b/builtin-merge.c index f7db148..f4de73f 100644 --- a/builtin-merge.c +++ b/builtin-merge.c @@ -375,6 +375,11 @@ static void merge_name(const char *remote, struct strbuf *msg) sha1_to_hex(branch_head), remote); goto cleanup; } + if (!prefixcmp(found_ref, "refs/remotes/")) { + strbuf_addf(msg, "%s\t\tremote branch '%s' of .\n", + sha1_to_hex(branch_head), remote); + goto cleanup; + } } /* See if remote matches <name>^^^.. or <name>~<number> */ diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh index e6c8327..297d165 100755 --- a/t/t3409-rebase-preserve-merges.sh +++ b/t/t3409-rebase-preserve-merges.sh @@ -71,7 +71,7 @@ test_expect_success 'rebase -p fakes interactive rebase' ' git fetch && git rebase -p origin/topic && test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) && - test 1 = $(git rev-list --all --pretty=oneline | grep "Merge commit" | wc -l) + test 1 = $(git rev-list --all --pretty=oneline | grep "Merge remote branch " | wc -l) ) ' diff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh index 81ced8a..28d5679 100755 --- a/t/t7608-merge-messages.sh +++ b/t/t7608-merge-messages.sh @@ -47,4 +47,14 @@ test_expect_success 'ambiguous tag' ' check_oneline "Merge commit QambiguousQ" ' +test_expect_success 'remote branch' ' + git checkout -b remote master && + test_commit remote-1 && + git update-ref refs/remotes/origin/master remote && + git checkout master && + test_commit master-5 && + git merge origin/master && + check_oneline "Merge remote branch Qorigin/masterQ" +' + test_done -- 1.6.4.178.g7a987 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] merge: indicate remote tracking branches in merge message 2009-08-09 10:00 ` Jeff King ` (2 preceding siblings ...) 2009-08-09 10:02 ` [PATCH 3/3] merge: indicate remote tracking branches in merge message Jeff King @ 2009-08-09 10:07 ` Jeff King 2009-08-09 19:36 ` Junio C Hamano 3 siblings, 1 reply; 12+ messages in thread From: Jeff King @ 2009-08-09 10:07 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Sun, Aug 09, 2009 at 06:00:45AM -0400, Jeff King wrote: > [1/3] add tests for merge message headings > [2/3] merge: fix incorrect merge message for ambiguous tag/branch > [3/3] merge: indicate remote tracking branches in merge message And here is the 4/3 you mentioned earlier: -- >8 -- Subject: [PATCH] merge: describe tags as such in merge message Previously, merging a tag directly via "git merge tag" would get you the message "Merge commit 'tag'". It is a little more descriptive to note that it was actually a tag (i.e., "Merge tag 'tag'"). Signed-off-by: Jeff King <peff@peff.net> --- builtin-merge.c | 5 +++++ t/t7608-merge-messages.sh | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/builtin-merge.c b/builtin-merge.c index f4de73f..db74901 100644 --- a/builtin-merge.c +++ b/builtin-merge.c @@ -380,6 +380,11 @@ static void merge_name(const char *remote, struct strbuf *msg) sha1_to_hex(branch_head), remote); goto cleanup; } + if (!prefixcmp(found_ref, "refs/tags/")) { + strbuf_addf(msg, "%s\t\ttag '%s' of .\n", + sha1_to_hex(branch_head), remote); + goto cleanup; + } } /* See if remote matches <name>^^^.. or <name>~<number> */ diff --git a/t/t7608-merge-messages.sh b/t/t7608-merge-messages.sh index 28d5679..3ee0983 100755 --- a/t/t7608-merge-messages.sh +++ b/t/t7608-merge-messages.sh @@ -35,7 +35,7 @@ test_expect_success 'merge tag' ' git checkout master && test_commit master-3 && git merge tag-1 && - check_oneline "Merge commit Qtag-1Q" + check_oneline "Merge tag Qtag-1Q" ' test_expect_success 'ambiguous tag' ' @@ -44,7 +44,7 @@ test_expect_success 'ambiguous tag' ' git checkout master && test_commit master-4 && git merge ambiguous && - check_oneline "Merge commit QambiguousQ" + check_oneline "Merge tag QambiguousQ" ' test_expect_success 'remote branch' ' -- 1.6.4.178.g7a987 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] merge: indicate remote tracking branches in merge message 2009-08-09 10:07 ` [PATCH] " Jeff King @ 2009-08-09 19:36 ` Junio C Hamano 2009-08-09 21:49 ` Jeff King 0 siblings, 1 reply; 12+ messages in thread From: Junio C Hamano @ 2009-08-09 19:36 UTC (permalink / raw) To: Jeff King; +Cc: git Jeff King <peff@peff.net> writes: > On Sun, Aug 09, 2009 at 06:00:45AM -0400, Jeff King wrote: > >> [1/3] add tests for merge message headings >> [2/3] merge: fix incorrect merge message for ambiguous tag/branch >> [3/3] merge: indicate remote tracking branches in merge message > > And here is the 4/3 you mentioned earlier: > > -- >8 -- > Subject: [PATCH] merge: describe tags as such in merge message > > Previously, merging a tag directly via "git merge tag" would > get you the message "Merge commit 'tag'". It is a little > more descriptive to note that it was actually a tag (i.e., > "Merge tag 'tag'"). Maybe "Merge version 'v1.6.3'" or "Merge commit tagged as 'v1.6.3'"? I dunno. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] merge: indicate remote tracking branches in merge message 2009-08-09 19:36 ` Junio C Hamano @ 2009-08-09 21:49 ` Jeff King 0 siblings, 0 replies; 12+ messages in thread From: Jeff King @ 2009-08-09 21:49 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Sun, Aug 09, 2009 at 12:36:17PM -0700, Junio C Hamano wrote: > > Previously, merging a tag directly via "git merge tag" would > > get you the message "Merge commit 'tag'". It is a little > > more descriptive to note that it was actually a tag (i.e., > > "Merge tag 'tag'"). > > Maybe "Merge version 'v1.6.3'" or "Merge commit tagged as 'v1.6.3'"? > I dunno. Definitely "Merge version" is bad. Doing "git tag feature-x-working" and merging it would lead to "Merge version 'feature-x-working'", which doesn't really make sense. I don't think the developer would refer to such a tag as a "version". Your "Merge commit tagged as..." version is reasonable to me. I'm dubious whether the additional information that it was a tag is actually worth anything. That is, given (1) Merge commit '1234abcd' (2) Merge commit 'v1.6.3' (3) Merge tag 'v1.6.3' where (1) is what we do now and will do in the future for arbitrary commits, (2) is what we do now for tags, and (3) is the proposed change. I am not sure that (3) really tells the reader anything useful over the other two. And your "Merge commit tagged as" is just the same information written slightly differently. So maybe this patch should simply be dropped. I don't feel strongly either way. -Peff ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2009-08-09 21:49 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-08-09 6:59 [PATCH] merge: indicate remote tracking branches in merge message Jeff King 2009-08-09 7:14 ` Jeff King 2009-08-09 7:31 ` Junio C Hamano 2009-08-09 7:40 ` Jeff King 2009-08-09 9:14 ` Jeff King 2009-08-09 10:00 ` Jeff King 2009-08-09 10:01 ` [PATCH 1/3] add tests for merge message headings Jeff King 2009-08-09 10:02 ` [PATCH 2/3] merge: fix incorrect merge message for ambiguous tag/branch Jeff King 2009-08-09 10:02 ` [PATCH 3/3] merge: indicate remote tracking branches in merge message Jeff King 2009-08-09 10:07 ` [PATCH] " Jeff King 2009-08-09 19:36 ` Junio C Hamano 2009-08-09 21:49 ` Jeff King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).