Re: [PATCH v2 2/2] ref-filter: add support for %(contents:size)

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 6, 2020, 21:44 UTC
Message-ID: <xmqq5zb09v4e.fsf@gitster.c.googlers.com>
In-Reply-To: <20200702140845.24945-3-chriscool@tuxfamily.org>

Christian Couder <christian.couder@gmail.com> writes:

Show 10 quoted lines

> It's useful and efficient to be able to get the size of the
> contents directly without having to pipe through `wc -c`.
>
> Also the result of the following:
>
> `git for-each-ref --format='%(contents)' | wc -c`
>
> is off by one as `git for-each-ref` appends a newline character
> after the contents, which can be seen by comparing its ouput
> with the output from `git cat-file`.

So that's off by number of refs that are shown?

> +contents:size::
> +	The size in bytes of the complete message.
> +

Complete as opposed to what?

What happens when the object referred to by the ref is not a commit or a tag?

I am fine if it just is silently ignored (which is consistent with already existing behaviour of other requests that do not make sense for the given type) if the thing is a blob or a tree, but we'd need to cover the case with a test or two. It seems you only expect this with a tag object and do not have any test that checks for other types of objects?

Thanks.

Show 37 quoted lines

> +# We cannot use test_atom to check contents:size of signed tags due to sanitize_pgp
> +test_tag_contents_size_pgp () {
> +	ref="$1"
> +	test_expect_success $PREREQ "basic atom: $ref contents:size" "
> +		git cat-file tag $ref | tail -n +6 | wc -c >expected &&
> +		git for-each-ref --format='%(contents:size)' $ref >actual &&
> +		test_cmp expected actual
> +	"
> +}
> +
>  PREREQ=GPG
>  test_atom refs/tags/signed-empty subject ''
>  test_atom refs/tags/signed-empty contents:subject ''
> @@ -629,6 +643,7 @@ test_atom refs/tags/signed-empty body "$sig"
>  test_atom refs/tags/signed-empty contents:body ''
>  test_atom refs/tags/signed-empty contents:signature "$sig"
>  test_atom refs/tags/signed-empty contents "$sig"
> +test_tag_contents_size_pgp refs/tags/signed-empty
>  
>  test_atom refs/tags/signed-short subject 'subject line'
>  test_atom refs/tags/signed-short contents:subject 'subject line'
> @@ -637,6 +652,7 @@ test_atom refs/tags/signed-short contents:body ''
>  test_atom refs/tags/signed-short contents:signature "$sig"
>  test_atom refs/tags/signed-short contents "subject line
>  $sig"
> +test_tag_contents_size_pgp refs/tags/signed-short
>  
>  test_atom refs/tags/signed-long subject 'subject line'
>  test_atom refs/tags/signed-long contents:subject 'subject line'
> @@ -649,6 +665,7 @@ test_atom refs/tags/signed-long contents "subject line
>  
>  body contents
>  $sig"
> +test_tag_contents_size_pgp refs/tags/signed-long
>  
>  test_expect_success 'set up multiple-sort tags' '
>  	for when in 100000 200000