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

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

[Phil Hord]

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(-)

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

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

[Junio C Hamano]

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.

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

[Phil Hord]

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

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

[Michael J Gruber]

Phil Hord venit, vidit, dixit 13.03.2013 05:21:
> 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