{"thread":{"id":"33163","subject":"[PATCH] tag: --force is quiet about new tags","startedAt":"2013-03-12T23:13:41Z","lastAt":"2013-03-13T09:16:52Z","messageCount":4,"participants":["Phil Hord","Junio C Hamano","Michael J Gruber"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"211177","messageId":"1363130021-28624-1-git-send-email-hordp@cisco.com","threadId":"33163","inReplyTo":null,"subject":"[PATCH] tag: --force is quiet about new tags","fromName":"Phil Hord","fromEmail":"hordp@cisco.com","sentAt":"2013-03-12T23:13:41Z","receivedAt":"2013-03-12T23:13:41Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"git tag --force is used to replace an existing tag with\na new reference. Git helpfully tells the user the old\nref when this happens. But if the tag name is new and does\nnot exist, git tells the user the old ref anyway (000000).\n\nTeach git to ignore --force if the tag is new. Add a test\nfor this and also to ensure --force can replace tags at all.\n\nSigned-off-by: Phil Hord \n---\n builtin/tag.c | 2 +-\n t/t7004-tag.sh | 12 ++++++++++++\n 2 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex f826688..af3af3f 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -582,7 +582,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"%s: cannot lock the ref\"), ref.buf);\n \tif (write_ref_sha1(lock, object, NULL) < 0)\n \t\tdie(_(\"%s: cannot update the ref\"), ref.buf);\n-\tif (force && hashcmp(prev, object))\n+\tif (force && !is_null_sha1(prev) && hashcmp(prev, object))\n \t\tprintf(_(\"Updated tag '%s' (was %s)\\n\"), tag, find_unique_abbrev(prev, DEFAULT_ABBREV));\n \n \tstrbuf_release(&buf);\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex f5a79b1..c8d6e9f 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -104,6 +104,18 @@ test_expect_success 'creating a tag using HEAD directly should succeed' '\n \ttag_exists myhead\n '\n \n+test_expect_success '--force can create a tag with the name of one existing' '\n+\ttag_exists mytag &&\n+\tgit tag --force mytag &&\n+\ttag_exists mytag'\n+\n+test_expect_success '--force is moot with a non-existing tag name' '\n+\tgit tag newtag >expect &&\n+\tgit tag --force forcetag >actual &&\n+\ttest_cmp expect actual\n+'\n+git tag -d newtag forcetag\n+\n # deleting tags:\n \n test_expect_success 'trying to delete an unknown tag should fail' '\n-- \n1.8.2.rc3.394.g2617418.dirty\n"},{"id":"211194","messageId":"7vy5ds3re0.fsf@alter.siamese.dyndns.org","threadId":"33163","inReplyTo":"1363130021-28624-1-git-send-email-hordp@cisco.com","subject":"Re: [PATCH] tag: --force is quiet about new tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-13T03:33:43Z","receivedAt":"2013-03-13T03:33:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phil Hord writes:\n\n> git tag --force is used to replace an existing tag with\n> a new reference. Git helpfully tells the user the old\n> ref when this happens. But if the tag name is new and does\n> not exist, git tells the user the old ref anyway (000000).\n>\n> Teach git to ignore --force if the tag is new. Add a test\n> for this and also to ensure --force can replace tags at all.\n>\n> Signed-off-by: Phil Hord \n> ---\n\nI think we would still want to allow the operation to go through,\neven when the --force option is given, to create a new tag. I agree\nthat the message should not say \"Updated\". So teaching Git not to\nissue the \"Updated\" message makes perfect sense. It is somewhat\nmisleading to say we are teaching Git to ignore the option, though.\n\nThanks.\n"},{"id":"211198","messageId":"CABURp0oT8ZN5fKtKh67Gm1P5rf_a3diqcYbw8mUga-gv5MPu6Q@mail.gmail.com","threadId":"33163","inReplyTo":"7vy5ds3re0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] tag: --force is quiet about new tags","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-03-13T04:21:00Z","receivedAt":"2013-03-13T04:21:00Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Tue, Mar 12, 2013 at 11:33 PM, Junio C Hamano wrote:\n> Phil Hord writes:\n>\n>> git tag --force is used to replace an existing tag with\n>> a new reference. Git helpfully tells the user the old\n>> ref when this happens. But if the tag name is new and does\n>> not exist, git tells the user the old ref anyway (000000).\n>>\n>> Teach git to ignore --force if the tag is new. Add a test\n>> for this and also to ensure --force can replace tags at all.\n>>\n>> Signed-off-by: Phil Hord \n>> ---\n>\n> I think we would still want to allow the operation to go through,\n> even when the --force option is given, to create a new tag. I agree\n> that the message should not say \"Updated\". So teaching Git not to\n> issue the \"Updated\" message makes perfect sense. It is somewhat\n> misleading to say we are teaching Git to ignore the option, though.\n>\n> Thanks.\n\nMy phrasing was too ambiguous. What you described is exactly what the\npatch does. --force is superfluous when the tag does not already\nexist. It is only checked in two places, and one of those is to\ndecide whether to print the \"Updated\" message. How's this?\n\n Teach 'git tag --force' to suppress the update message if\n the tag is new. Add a test for this and also to ensure\n --force can replace tags at all.\n\nPhil\n"},{"id":"211205","messageId":"51404404.4030005@drmicha.warpmail.net","threadId":"33163","inReplyTo":"CABURp0oT8ZN5fKtKh67Gm1P5rf_a3diqcYbw8mUga-gv5MPu6Q@mail.gmail.com","subject":"Re: [PATCH] tag: --force is quiet about new tags","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2013-03-13T09:16:52Z","receivedAt":"2013-03-13T09:16:52Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Phil Hord venit, vidit, dixit 13.03.2013 05:21:\n> On Tue, Mar 12, 2013 at 11:33 PM, Junio C Hamano wrote:\n>> Phil Hord writes:\n>>\n>>> git tag --force is used to replace an existing tag with\n>>> a new reference. Git helpfully tells the user the old\n>>> ref when this happens. But if the tag name is new and does\n>>> not exist, git tells the user the old ref anyway (000000).\n>>>\n>>> Teach git to ignore --force if the tag is new. Add a test\n>>> for this and also to ensure --force can replace tags at all.\n>>>\n>>> Signed-off-by: Phil Hord \n>>> ---\n>>\n>> I think we would still want to allow the operation to go through,\n>> even when the --force option is given, to create a new tag. I agree\n>> that the message should not say \"Updated\". So teaching Git not to\n>> issue the \"Updated\" message makes perfect sense. It is somewhat\n>> misleading to say we are teaching Git to ignore the option, though.\n>>\n>> Thanks.\n> \n> My phrasing was too ambiguous. What you described is exactly what the\n> patch does. --force is superfluous when the tag does not already\n> exist. It is only checked in two places, and one of those is to\n> decide whether to print the \"Updated\" message. How's this?\n> \n> Teach 'git tag --force' to suppress the update message if\n> the tag is new. Add a test for this and also to ensure\n> --force can replace tags at all.\n> \n> Phil\n\nLooks good to me, both the patch and the (updated) commit message.\n\nMichael\n"}]}