threads / discuss / 50445

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

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

## tl;dr

8 messages between Feb 8, 2019 and Feb 9, 2019.

replies: 7people: 5as markdown or json

randall.s.becker@rogers.com· Feb 8, 2019, 22:07 UTC · lore
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
Jeff King· Feb 8, 2019, 22:37 UTC · re: randall.s.becker@rogers.com · lore

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

On Fri, Feb 08, 2019 at 05:07:51PM -0500, randall.s.becker@rogers.com wrote:
Show 5 quoted lines
> 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
Junio C Hamano· Feb 8, 2019, 23:38 UTC · re: randall.s.becker@rogers.com · lore

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

randall.s.becker@rogers.com writes:
Show 21 quoted lines
> 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?

Show 10 quoted lines
>  		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
>  '
Randall S. Becker· Feb 9, 2019, 17:00 UTC · re: Junio C Hamano · lore

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

On February 8, 2019 18:39, Junio C Hamano wrote:
Show 23 quoted lines
> 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.
Show 13 quoted lines
> 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

Johannes Sixt· Feb 9, 2019, 08:46 UTC · re: randall.s.becker@rogers.com · lore

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

Am 08.02.19 um 23:07 schrieb randall.s.becker@rogers.com:
Show 31 quoted lines
> 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
Junio C Hamano· Feb 9, 2019, 18:25 UTC · re: Johannes Sixt · lore

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

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
Randall S. Becker· Feb 9, 2019, 19:07 UTC · re: Junio C Hamano · lore

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

On February 9, 2019 13:25, Junio C Hamano wrote:
Show 7 quoted lines
> 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
Show 31 quoted lines
> 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
Show 15 quoted lines
> --- 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

Johannes Sixt· Feb 9, 2019, 21:54 UTC · re: Junio C Hamano · lore

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

Am 09.02.19 um 19:25 schrieb Junio C Hamano:
Show 15 quoted lines
> 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
Show 43 quoted lines
> 
>  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
>

← back to recent threads