diff mbox series

[2/1] t6300: format missing tagger

Message ID 20190822135528.GB28725@shiar.net (mailing list archive)
State New, archived
Headers show
Series ref-filter: initialize empty name or email fields | expand

Commit Message

Mischa POSLAWSKY Aug. 22, 2019, 1:55 p.m. UTC
Junio wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Mischa POSLAWSKY <git@shiar.nl> writes:
> >
> >> If I understand correctly, such tags cannot be produced normally anymore.
> >> Therefore I'm unsure how to make tests, and if that is even warranted.
> >
> > Thanks for spotting.
> 
> A quick trial to recreate a tag object seems to succeed:
> 
>     $ git cat-file tag v0.99 |
>     > sed -e '/-----BEGIN/,$d' |
>     > git hash-object --stdin -w -t tag
>     667d141b478eee5e53d2ee05acd61bb1f640249a
>     $ git cat-file tag 667d141b47
>     object a3eb250f996bf5e12376ec88622c4ccaabf20ea8
>     type commit
>     tag v0.99
> 
>     Test-release for wider distribution.
> 
>     I'll make the first public RPM's etc, thus the tag.
> 
> So we should be able to do something along the above line.  Here is
> my quick-n-dirty one.
> 
>  t/t6300-for-each-ref.sh | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index ab69aa176d..b3a6b336fa 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -869,4 +869,16 @@ test_expect_success 'for-each-ref --ignore-case ignores case' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'show a taggerless tag' '
> +	test_commit tagged &&
> +	git tag -a -m "a normal tag" to-be-shown-0 HEAD &&
> +	another=$(git cat-file tag to-be-shown-0 |
> +		sed -e "/^tagger /d" \
> +		    -e "/^tag to-be-shown/s/0/1/" \
> +		    -e "s/a normal tag/a broken tag/" |
> +		git hash-object --stdin -w -t tag) &&
> +	git tag to-be-shown-1 $another &&
> +	git for-each-ref --format="%(refname:short) %(taggername)" refs/tags/to-be-shown\*
> +'
> +
>  test_done
> 

Alright, thanks for the pointer.
Here's a batch of tests on all pertaining atoms.

-- >8 --

Strip an annotated tag of its tagger header and verify it's ignored
correctly in all cases, as fixed in commit e2a81276e8 (ref-filter:
initialize empty name or email fields, 2019-08-19).

Signed-off-by: Mischa POSLAWSKY <git@shiar.nl>
---
 t/t6300-for-each-ref.sh | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Junio C Hamano Aug. 22, 2019, 4:15 p.m. UTC | #1
Mischa POSLAWSKY <git@shiar.nl> writes:

> Alright, thanks for the pointer.
> Here's a batch of tests on all pertaining atoms.

Good to see that you made it much more thorough than my q-n-d
illustration patch ;-)

> -- >8 --
>
> Strip an annotated tag of its tagger header and verify it's ignored
> correctly in all cases, as fixed in commit e2a81276e8 (ref-filter:
> initialize empty name or email fields, 2019-08-19).

I am inclined to squash this test part of the update into the said
commit; you'd lose one commit count, but hopefully you do not mind?

My motivation for doing so is that it would allow us to lose the "as
fixed in commit X" comment in a log message, which in turn would
mean that the code-fix patch can later be rebased safely without
having to remember that this one needs to be adjusted ("git rebase"
does not do such a rewrite for us, and I personally do not think
"git rebase" should do such a rewrite silently, as I cannot quantify
the risk of false positives).

>
> Signed-off-by: Mischa POSLAWSKY <git@shiar.nl>
> ---
>  t/t6300-for-each-ref.sh | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index ab69aa176d..9c910ce746 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -526,6 +526,25 @@ test_expect_success 'Check ambiguous head and tag refs II (loose)' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'create tag without tagger' '
> +	git tag -a -m "Broken tag" taggerless &&
> +	git tag -f taggerless $(git cat-file tag taggerless |
> +		sed -e "/^tagger /d" |
> +		git hash-object --stdin -w -t tag)
> +'
> +
> +test_atom refs/tags/taggerless type 'commit'
> +test_atom refs/tags/taggerless tag 'taggerless'
> +test_atom refs/tags/taggerless tagger ''
> +test_atom refs/tags/taggerless taggername ''
> +test_atom refs/tags/taggerless taggeremail ''
> +test_atom refs/tags/taggerless taggerdate ''
> +test_atom refs/tags/taggerless committer ''
> +test_atom refs/tags/taggerless committername ''
> +test_atom refs/tags/taggerless committeremail ''
> +test_atom refs/tags/taggerless committerdate ''
> +test_atom refs/tags/taggerless subject 'Broken tag'
> +
>  test_expect_success 'an unusual tag with an incomplete line' '
>  
>  	git tag -m "bogo" bogo &&
Mischa POSLAWSKY Aug. 22, 2019, 4:27 p.m. UTC | #2
Junio wrote:
> 
> Mischa POSLAWSKY <git@shiar.nl> writes:
> > Strip an annotated tag of its tagger header and verify it's ignored
> > correctly in all cases, as fixed in commit e2a81276e8 (ref-filter:
> > initialize empty name or email fields, 2019-08-19).
> 
> I am inclined to squash this test part of the update into the said
> commit; you'd lose one commit count, but hopefully you do not mind?
> 
> My motivation for doing so is that it would allow us to lose the "as
> fixed in commit X" comment in a log message, which in turn would
> mean that the code-fix patch can later be rebased safely without
> having to remember that this one needs to be adjusted ("git rebase"
> does not do such a rewrite for us, and I personally do not think
> "git rebase" should do such a rewrite silently, as I cannot quantify
> the risk of false positives).

Of course.  Might get one commit back if you pick it into maint :)
Junio C Hamano Aug. 22, 2019, 6:05 p.m. UTC | #3
Mischa POSLAWSKY <git@shiar.nl> writes:

> Junio wrote:
>> 
>> Mischa POSLAWSKY <git@shiar.nl> writes:
>> > Strip an annotated tag of its tagger header and verify it's ignored
>> > correctly in all cases, as fixed in commit e2a81276e8 (ref-filter:
>> > initialize empty name or email fields, 2019-08-19).
>> 
>> I am inclined to squash this test part of the update into the said
>> commit; you'd lose one commit count, but hopefully you do not mind?
>> 
>> My motivation for doing so is that it would allow us to lose the "as
>> fixed in commit X" comment in a log message, which in turn would
>> mean that the code-fix patch can later be rebased safely without
>> having to remember that this one needs to be adjusted ("git rebase"
>> does not do such a rewrite for us, and I personally do not think
>> "git rebase" should do such a rewrite silently, as I cannot quantify
>> the risk of false positives).
>
> Of course.  Might get one commit back if you pick it into maint :)

Actually you won't; I generally do not cherry-pick, even though I
merge down relevant fixes to older maintenance tracks.
diff mbox series

Patch

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index ab69aa176d..9c910ce746 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -526,6 +526,25 @@  test_expect_success 'Check ambiguous head and tag refs II (loose)' '
 	test_cmp expected actual
 '
 
+test_expect_success 'create tag without tagger' '
+	git tag -a -m "Broken tag" taggerless &&
+	git tag -f taggerless $(git cat-file tag taggerless |
+		sed -e "/^tagger /d" |
+		git hash-object --stdin -w -t tag)
+'
+
+test_atom refs/tags/taggerless type 'commit'
+test_atom refs/tags/taggerless tag 'taggerless'
+test_atom refs/tags/taggerless tagger ''
+test_atom refs/tags/taggerless taggername ''
+test_atom refs/tags/taggerless taggeremail ''
+test_atom refs/tags/taggerless taggerdate ''
+test_atom refs/tags/taggerless committer ''
+test_atom refs/tags/taggerless committername ''
+test_atom refs/tags/taggerless committeremail ''
+test_atom refs/tags/taggerless committerdate ''
+test_atom refs/tags/taggerless subject 'Broken tag'
+
 test_expect_success 'an unusual tag with an incomplete line' '
 
 	git tag -m "bogo" bogo &&