[Fix v2] t5562: remove dependency on /dev/zero

[Fix v2] t5562: remove dependency on /dev/zero

[randall.s.becker]

From: "Randall S. Becker" <rsbecker@nexbridge.com>

Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero
with yes and a translation of its result to a stream of NULL. This is
a more portable solution.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 t/t5562-http-backend-content-length.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh
index 90d890d02..b8d1913e5 100755
--- a/t/t5562-http-backend-content-length.sh
+++ b/t/t5562-http-backend-content-length.sh
@@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' '
 
 test_expect_success 'CONTENT_LENGTH overflow ssite_t' '
 	NOT_FIT_IN_SSIZE=$(ssize_b100dots) &&
-	env \
+	yes | tr "y" "\\0" | env \
 		CONTENT_TYPE=application/x-git-upload-pack-request \
 		QUERY_STRING=/repo.git/git-upload-pack \
 		PATH_TRANSLATED="$PWD"/.git/git-upload-pack \
 		GIT_HTTP_EXPORT_ALL=TRUE \
 		REQUEST_METHOD=POST \
 		CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \
-		git http-backend </dev/zero >/dev/null 2>err &&
+		git http-backend >/dev/null 2>err &&
 	grep "fatal:.*CONTENT_LENGTH" err
 '
 
-- 
2.12.3

Re: [Fix v2] t5562: remove dependency on /dev/zero

[Jeff King]

On Fri, Feb 08, 2019 at 05:07:51PM -0500, randall.s.becker@rogers.com wrote:

> From: "Randall S. Becker" <rsbecker@nexbridge.com>
> 
> Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero
> with yes and a translation of its result to a stream of NULL. This is
> a more portable solution.

They're NULs, not NULLs. :)

I think in this case, though, we don't actually care about the bytes,
and you can drop the "tr" invocation entirely.

-Peff

Re: [Fix v2] t5562: remove dependency on /dev/zero

[Junio C Hamano]

randall.s.becker@rogers.com writes:

> From: "Randall S. Becker" <rsbecker@nexbridge.com>
>
> Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero
> with yes and a translation of its result to a stream of NULL. This is
> a more portable solution.
>
> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> ---
>  t/t5562-http-backend-content-length.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh
> index 90d890d02..b8d1913e5 100755
> --- a/t/t5562-http-backend-content-length.sh
> +++ b/t/t5562-http-backend-content-length.sh
> @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' '
>  
>  test_expect_success 'CONTENT_LENGTH overflow ssite_t' '
>  	NOT_FIT_IN_SSIZE=$(ssize_b100dots) &&
> -	env \
> +	yes | tr "y" "\\0" | env \

I do not quite get this use of tr.  The original feeds a stream of
NULs out of /dev/zero to the command; the yes-to-tr pipe instead
feeds a stream of alternating NUL and LF.

Does the actual bytes fed to the consumer make any difference?  If
not, perhaps we can use 'yes' as-is?

>  		CONTENT_TYPE=application/x-git-upload-pack-request \
>  		QUERY_STRING=/repo.git/git-upload-pack \
>  		PATH_TRANSLATED="$PWD"/.git/git-upload-pack \
>  		GIT_HTTP_EXPORT_ALL=TRUE \
>  		REQUEST_METHOD=POST \
>  		CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \
> -		git http-backend </dev/zero >/dev/null 2>err &&
> +		git http-backend >/dev/null 2>err &&
>  	grep "fatal:.*CONTENT_LENGTH" err
>  '

Re: [Fix v2] t5562: remove dependency on /dev/zero

[Johannes Sixt]

Am 08.02.19 um 23:07 schrieb randall.s.becker@rogers.com:
> From: "Randall S. Becker" <rsbecker@nexbridge.com>
> 
> Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero
> with yes and a translation of its result to a stream of NULL. This is
> a more portable solution.
> 
> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> ---
>  t/t5562-http-backend-content-length.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh
> index 90d890d02..b8d1913e5 100755
> --- a/t/t5562-http-backend-content-length.sh
> +++ b/t/t5562-http-backend-content-length.sh
> @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' '
>  
>  test_expect_success 'CONTENT_LENGTH overflow ssite_t' '
>  	NOT_FIT_IN_SSIZE=$(ssize_b100dots) &&
> -	env \
> +	yes | tr "y" "\\0" | env \
>  		CONTENT_TYPE=application/x-git-upload-pack-request \
>  		QUERY_STRING=/repo.git/git-upload-pack \
>  		PATH_TRANSLATED="$PWD"/.git/git-upload-pack \
>  		GIT_HTTP_EXPORT_ALL=TRUE \
>  		REQUEST_METHOD=POST \
>  		CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \
> -		git http-backend </dev/zero >/dev/null 2>err &&
> +		git http-backend >/dev/null 2>err &&
>  	grep "fatal:.*CONTENT_LENGTH" err
>  '

How many bytes are needed here? yes() in test-lib.sh generates only 99
'y', if I am not mistaken.

-- Hannes

RE: [Fix v2] t5562: remove dependency on /dev/zero

[Randall S. Becker]

On February 8, 2019 18:39, Junio C Hamano wrote:
> randall.s.becker@rogers.com writes:
> > Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero
> > with yes and a translation of its result to a stream of NULL. This is
> > a more portable solution.
> >
> > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> > ---
> >  t/t5562-http-backend-content-length.sh | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/t/t5562-http-backend-content-length.sh
> > b/t/t5562-http-backend-content-length.sh
> > index 90d890d02..b8d1913e5 100755
> > --- a/t/t5562-http-backend-content-length.sh
> > +++ b/t/t5562-http-backend-content-length.sh
> > @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' '
> >
> >  test_expect_success 'CONTENT_LENGTH overflow ssite_t' '
> >  	NOT_FIT_IN_SSIZE=$(ssize_b100dots) &&
> > -	env \
> > +	yes | tr "y" "\\0" | env \
> 
> I do not quite get this use of tr.  The original feeds a stream of NULs
out of
> /dev/zero to the command; the yes-to-tr pipe instead feeds a stream of
> alternating NUL and LF.

That's why we're going to go with a generate_zero_bytes function per Peff.
I'm working on a more comprehensive patch covering t5562, t5318, and
test-lib-functions.sh that will (hopefully) be satisfactory and remove the
dependency on /dev/zero and fixes the related new breakages in 2.21.0-rc0.

The test case in t5318 is specific about wanting zero bytes although the
test is just intending to generate a corrupt block that generates a
different hash, so yes 'yes' might be sufficient, but I don't like
randomness myself if we're taking two different tests being involved.

My current intent is to add to test-lib-functions.sh, a method of
generalizing blocks of zeros to a pipe:

+# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes).
+# If $1 is < 0, output forever or until the receiving pipe stops reading,
whichever comes first.
+ generate_zero_bytes () {
+ 	perl -e ' if ($ARGV[0] < 0) { while (-1) { print "\0" } } else {
print "\0" x $ARGV[0] }' "$@"
+ }

And then fit that into the two tests, then submit as a patch.

> Does the actual bytes fed to the consumer make any difference?  If not,
> perhaps we can use 'yes' as-is?
> 
> >  		CONTENT_TYPE=application/x-git-upload-pack-request \
> >  		QUERY_STRING=/repo.git/git-upload-pack \
> >  		PATH_TRANSLATED="$PWD"/.git/git-upload-pack \
> >  		GIT_HTTP_EXPORT_ALL=TRUE \
> >  		REQUEST_METHOD=POST \
> >  		CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \
> > -		git http-backend </dev/zero >/dev/null 2>err &&
> > +		git http-backend >/dev/null 2>err &&
> >  	grep "fatal:.*CONTENT_LENGTH" err
> >  '

Regards,
Randall

Re: [Fix v2] t5562: remove dependency on /dev/zero

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> How many bytes are needed here? yes() in test-lib.sh generates only 99
> 'y', if I am not mistaken.

I think we will not use "yes" in the end for this topic, which makes
this comment totally irrelevant to the thread, but I wonder why we
have the limit of 99 there?  It cannot be "we do not want to worry
about sigpipe" affecting the end result of the test (after all the
reader may stop reading from after reading just one, and the status
of the upstream process that would die with sigpipe is lost anyway).

It turns out it is about sigpipe ;-) but in somewhat a different
way.  To prevent others from wasting their time wondering about
this, probably we want to have something like the attached?

 t/README      | 9 +++++++++
 t/test-lib.sh | 6 +++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/t/README b/t/README
index 1326fd7505..f4e1a82657 100644
--- a/t/README
+++ b/t/README
@@ -927,6 +927,15 @@ library for your script to use.
    test_oid_init or test_oid_cache.  Providing an unknown key is an
    error.
 
+ - yes [<string>]
+
+   This is often seen in modern UNIX but some platforms lack it, so
+   the test harness overrides the platform implementation with a
+   more limited one.  Use this only when feeding a handful lines of
+   output to the downstream---unlike the real version, it generates
+   only up to 99 lines.
+
+
 Prerequisites
 -------------
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 42b1a0aa7f..541a37f4c0 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1313,7 +1313,11 @@ then
 	fi
 fi
 
-# Provide an implementation of the 'yes' utility
+# Provide an implementation of the 'yes' utility; the upper bound
+# limit is there to help Windows that cannot stop this loop from
+# wasting cycles when the downstream stops reading, so do not be
+# tempted to turn it into an infinite loop. cf. 6129c930 ("test-lib:
+# limit the output of the yes utility", 2016-02-02)
 yes () {
 	if test $# = 0
 	then

RE: [Fix v2] t5562: remove dependency on /dev/zero

[Randall S. Becker]

On February 9, 2019 13:25, Junio C Hamano wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
> > How many bytes are needed here? yes() in test-lib.sh generates only 99
> > 'y', if I am not mistaken.
> 
> I think we will not use "yes" in the end for this topic, which makes this
> comment totally irrelevant to the thread, but I wonder why we have the
limit
> of 99 there?  It cannot be "we do not want to worry about sigpipe"
affecting
> the end result of the test (after all the reader may stop reading from
after
> reading just one, and the status of the upstream process that would die
with
> sigpipe is lost anyway).
> 
> It turns out it is about sigpipe ;-) but in somewhat a different way.  To
> prevent others from wasting their time wondering about this, probably we
> want to have something like the attached?
> 
>  t/README      | 9 +++++++++
>  t/test-lib.sh | 6 +++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/t/README b/t/README
> index 1326fd7505..f4e1a82657 100644
> --- a/t/README
> +++ b/t/README
> @@ -927,6 +927,15 @@ library for your script to use.
>     test_oid_init or test_oid_cache.  Providing an unknown key is an
>     error.
> 
> + - yes [<string>]
> +
> +   This is often seen in modern UNIX but some platforms lack it, so
> +   the test harness overrides the platform implementation with a
> +   more limited one.  Use this only when feeding a handful lines of
> +   output to the downstream---unlike the real version, it generates
> +   only up to 99 lines.
> +
> +
>  Prerequisites
>  -------------
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh index 42b1a0aa7f..541a37f4c0
100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1313,7 +1313,11 @@ then
>  	fi
>  fi
> 
> -# Provide an implementation of the 'yes' utility
> +# Provide an implementation of the 'yes' utility; the upper bound #
> +limit is there to help Windows that cannot stop this loop from #
> +wasting cycles when the downstream stops reading, so do not be #
> +tempted to turn it into an infinite loop. cf. 6129c930 ("test-lib:
> +# limit the output of the yes utility", 2016-02-02)
>  yes () {
>  	if test $# = 0
>  	then

Sadly, I already the other path ready, but did not have a chance to send it.
I'm ok either way as long as I can get the tests running.

Regards,
Randall

Re: [Fix v2] t5562: remove dependency on /dev/zero

[Johannes Sixt]

Am 09.02.19 um 19:25 schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> How many bytes are needed here? yes() in test-lib.sh generates only 99
>> 'y', if I am not mistaken.
> 
> I think we will not use "yes" in the end for this topic, which makes
> this comment totally irrelevant to the thread, but I wonder why we
> have the limit of 99 there?  It cannot be "we do not want to worry
> about sigpipe" affecting the end result of the test (after all the
> reader may stop reading from after reading just one, and the status
> of the upstream process that would die with sigpipe is lost anyway).
> 
> It turns out it is about sigpipe ;-) but in somewhat a different
> way.  To prevent others from wasting their time wondering about
> this, probably we want to have something like the attached?

That would certainly be helpful!

-- Hannes

> 
>  t/README      | 9 +++++++++
>  t/test-lib.sh | 6 +++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/t/README b/t/README
> index 1326fd7505..f4e1a82657 100644
> --- a/t/README
> +++ b/t/README
> @@ -927,6 +927,15 @@ library for your script to use.
>     test_oid_init or test_oid_cache.  Providing an unknown key is an
>     error.
>  
> + - yes [<string>]
> +
> +   This is often seen in modern UNIX but some platforms lack it, so
> +   the test harness overrides the platform implementation with a
> +   more limited one.  Use this only when feeding a handful lines of
> +   output to the downstream---unlike the real version, it generates
> +   only up to 99 lines.
> +
> +
>  Prerequisites
>  -------------
>  
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 42b1a0aa7f..541a37f4c0 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1313,7 +1313,11 @@ then
>  	fi
>  fi
>  
> -# Provide an implementation of the 'yes' utility
> +# Provide an implementation of the 'yes' utility; the upper bound
> +# limit is there to help Windows that cannot stop this loop from
> +# wasting cycles when the downstream stops reading, so do not be
> +# tempted to turn it into an infinite loop. cf. 6129c930 ("test-lib:
> +# limit the output of the yes utility", 2016-02-02)
>  yes () {
>  	if test $# = 0
>  	then
>