threads / patch / 33163

patchtag: --force is quiet about new tags

Subject: [PATCH] tag: --force is quiet about new tags

## tl;dr

4 messages between Mar 12, 2013 and Mar 13, 2013. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Phil Hord· Mar 12, 2013, 23:13 UTC · lore

git tag --force is used to replace an existing tag with a new reference. Git helpfully tells the user the old ref when this happens. But if the tag name is new and does not exist, git tells the user the old ref anyway (000000).

Teach git to ignore --force if the tag is new. Add a test for this and also to ensure --force can replace tags at all.

Signed-off-by: Phil Hord <hordp@cisco.com>
---
 builtin/tag.c  |  2 +-
 t/t7004-tag.sh | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

Show changes to 2 files +13 −1

builtin/tag.c, t/t7004-tag.sh

diff --git a/builtin/tag.c b/builtin/tag.c
index f826688..af3af3f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -582,7 +582,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		die(_("%s: cannot lock the ref"), ref.buf);
 	if (write_ref_sha1(lock, object, NULL) < 0)
 		die(_("%s: cannot update the ref"), ref.buf);
-	if (force && hashcmp(prev, object))
+	if (force && !is_null_sha1(prev) && hashcmp(prev, object))
 		printf(_("Updated tag '%s' (was %s)\n"), tag, find_unique_abbrev(prev, DEFAULT_ABBREV));
 
 	strbuf_release(&buf);
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index f5a79b1..c8d6e9f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -104,6 +104,18 @@ test_expect_success 'creating a tag using HEAD directly should succeed' '
 	tag_exists myhead
 '
 
+test_expect_success '--force can create a tag with the name of one existing' '
+	tag_exists mytag &&
+	git tag --force mytag &&
+	tag_exists mytag'
+
+test_expect_success '--force is moot with a non-existing tag name' '
+	git tag newtag >expect &&
+	git tag --force forcetag >actual &&
+	test_cmp expect actual
+'
+git tag -d newtag forcetag
+
 # deleting tags:
 
 test_expect_success 'trying to delete an unknown tag should fail' '

-- 
1.8.2.rc3.394.g2617418.dirty

Junio C Hamano· Mar 13, 2013, 03:33 UTC · re: Phil Hord · lore

Re: [PATCH] tag: --force is quiet about new tags

Phil Hord <hordp@cisco.com> writes:

Show 10 quoted lines

> git tag --force is used to replace an existing tag with
> a new reference.  Git helpfully tells the user the old
> ref when this happens.  But if the tag name is new and does
> not exist, git tells the user the old ref anyway (000000).
>
> Teach git to ignore --force if the tag is new.  Add a test
> for this and also to ensure --force can replace tags at all.
>
> Signed-off-by: Phil Hord <hordp@cisco.com>
> ---

I think we would still want to allow the operation to go through, even when the --force option is given, to create a new tag. I agree that the message should not say "Updated". So teaching Git not to issue the "Updated" message makes perfect sense. It is somewhat misleading to say we are teaching Git to ignore the option, though.

Thanks.

Phil Hord· Mar 13, 2013, 04:21 UTC · re: Junio C Hamano · lore

Re: [PATCH] tag: --force is quiet about new tags

On Tue, Mar 12, 2013 at 11:33 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 20 quoted lines

> Phil Hord <hordp@cisco.com> writes:
>
>> git tag --force is used to replace an existing tag with
>> a new reference.  Git helpfully tells the user the old
>> ref when this happens.  But if the tag name is new and does
>> not exist, git tells the user the old ref anyway (000000).
>>
>> Teach git to ignore --force if the tag is new.  Add a test
>> for this and also to ensure --force can replace tags at all.
>>
>> Signed-off-by: Phil Hord <hordp@cisco.com>
>> ---
>
> I think we would still want to allow the operation to go through,
> even when the --force option is given, to create a new tag.  I agree
> that the message should not say "Updated".  So teaching Git not to
> issue the "Updated" message makes perfect sense.  It is somewhat
> misleading to say we are teaching Git to ignore the option, though.
>
> Thanks.

My phrasing was too ambiguous. What you described is exactly what the patch does. --force is superfluous when the tag does not already exist. It is only checked in two places, and one of those is to decide whether to print the "Updated" message. How's this?

   Teach 'git tag --force' to suppress the update message if
   the tag is new.  Add a test for this and also to ensure
   --force can replace tags at all.

Phil

Michael J Gruber· Mar 13, 2013, 09:16 UTC · re: Phil Hord · lore

Re: [PATCH] tag: --force is quiet about new tags

Phil Hord venit, vidit, dixit 13.03.2013 05:21:

Show 32 quoted lines

> On Tue, Mar 12, 2013 at 11:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Phil Hord <hordp@cisco.com> writes:
>>
>>> git tag --force is used to replace an existing tag with
>>> a new reference.  Git helpfully tells the user the old
>>> ref when this happens.  But if the tag name is new and does
>>> not exist, git tells the user the old ref anyway (000000).
>>>
>>> Teach git to ignore --force if the tag is new.  Add a test
>>> for this and also to ensure --force can replace tags at all.
>>>
>>> Signed-off-by: Phil Hord <hordp@cisco.com>
>>> ---
>>
>> I think we would still want to allow the operation to go through,
>> even when the --force option is given, to create a new tag.  I agree
>> that the message should not say "Updated".  So teaching Git not to
>> issue the "Updated" message makes perfect sense.  It is somewhat
>> misleading to say we are teaching Git to ignore the option, though.
>>
>> Thanks.
> 
> My phrasing was too ambiguous.  What you described is exactly what the
> patch does.  --force is superfluous when the tag does not already
> exist.  It is only checked in two places, and one of those is to
> decide whether to print the "Updated" message.  How's this?
> 
>    Teach 'git tag --force' to suppress the update message if
>    the tag is new.  Add a test for this and also to ensure
>    --force can replace tags at all.
> 
> Phil

Looks good to me, both the patch and the (updated) commit message.

Michael