{"thread":{"id":"50445","subject":"[Fix v2] t5562: remove dependency on /dev/zero","startedAt":"2019-02-08T22:08:17Z","lastAt":"2019-02-09T21:54:54Z","messageCount":8,"participants":["randall.s.becker@rogers.com","Jeff King","Junio C Hamano","Johannes Sixt","Randall S. Becker"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"368891","messageId":"20190208220751.9936-1-randall.s.becker@rogers.com","threadId":"50445","inReplyTo":null,"subject":"[Fix v2] t5562: remove dependency on /dev/zero","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2019-02-08T22:07:51Z","receivedAt":"2019-02-08T22:08:17Z","isPatch":false,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n\nReplaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero\nwith yes and a translation of its result to a stream of NULL. This is\na more portable solution.\n\nSigned-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n---\n t/t5562-http-backend-content-length.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh\nindex 90d890d02..b8d1913e5 100755\n--- a/t/t5562-http-backend-content-length.sh\n+++ b/t/t5562-http-backend-content-length.sh\n@@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' '\n \n test_expect_success 'CONTENT_LENGTH overflow ssite_t' '\n \tNOT_FIT_IN_SSIZE=$(ssize_b100dots) &&\n-\tenv \\\n+\tyes | tr \"y\" \"\\\\0\" | env \\\n \t\tCONTENT_TYPE=application/x-git-upload-pack-request \\\n \t\tQUERY_STRING=/repo.git/git-upload-pack \\\n \t\tPATH_TRANSLATED=\"$PWD\"/.git/git-upload-pack \\\n \t\tGIT_HTTP_EXPORT_ALL=TRUE \\\n \t\tREQUEST_METHOD=POST \\\n \t\tCONTENT_LENGTH=\"$NOT_FIT_IN_SSIZE\" \\\n-\t\tgit http-backend </dev/zero >/dev/null 2>err &&\n+\t\tgit http-backend >/dev/null 2>err &&\n \tgrep \"fatal:.*CONTENT_LENGTH\" err\n '\n \n-- \n2.12.3\n\n"},{"id":"368896","messageId":"20190208223720.GB2135@sigill.intra.peff.net","threadId":"50445","inReplyTo":"20190208220751.9936-1-randall.s.becker@rogers.com","subject":"Re: [Fix v2] t5562: remove dependency on /dev/zero","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-08T22:37:21Z","receivedAt":"2019-02-08T22:37:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 08, 2019 at 05:07:51PM -0500, randall.s.becker@rogers.com wrote:\n\n> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> \n> Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero\n> with yes and a translation of its result to a stream of NULL. This is\n> a more portable solution.\n\nThey're NULs, not NULLs. :)\n\nI think in this case, though, we don't actually care about the bytes,\nand you can drop the \"tr\" invocation entirely.\n\n-Peff\n"},{"id":"368900","messageId":"xmqq8sypkfzk.fsf@gitster-ct.c.googlers.com","threadId":"50445","inReplyTo":"20190208220751.9936-1-randall.s.becker@rogers.com","subject":"Re: [Fix v2] t5562: remove dependency on /dev/zero","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-08T23:38:55Z","receivedAt":"2019-02-08T23:39:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"randall.s.becker@rogers.com writes:\n\n> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n>\n> Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero\n> with yes and a translation of its result to a stream of NULL. This is\n> a more portable solution.\n>\n> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> ---\n>  t/t5562-http-backend-content-length.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh\n> index 90d890d02..b8d1913e5 100755\n> --- a/t/t5562-http-backend-content-length.sh\n> +++ b/t/t5562-http-backend-content-length.sh\n> @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' '\n>  \n>  test_expect_success 'CONTENT_LENGTH overflow ssite_t' '\n>  \tNOT_FIT_IN_SSIZE=$(ssize_b100dots) &&\n> -\tenv \\\n> +\tyes | tr \"y\" \"\\\\0\" | env \\\n\nI do not quite get this use of tr.  The original feeds a stream of\nNULs out of /dev/zero to the command; the yes-to-tr pipe instead\nfeeds a stream of alternating NUL and LF.\n\nDoes the actual bytes fed to the consumer make any difference?  If\nnot, perhaps we can use 'yes' as-is?\n\n>  \t\tCONTENT_TYPE=application/x-git-upload-pack-request \\\n>  \t\tQUERY_STRING=/repo.git/git-upload-pack \\\n>  \t\tPATH_TRANSLATED=\"$PWD\"/.git/git-upload-pack \\\n>  \t\tGIT_HTTP_EXPORT_ALL=TRUE \\\n>  \t\tREQUEST_METHOD=POST \\\n>  \t\tCONTENT_LENGTH=\"$NOT_FIT_IN_SSIZE\" \\\n> -\t\tgit http-backend </dev/zero >/dev/null 2>err &&\n> +\t\tgit http-backend >/dev/null 2>err &&\n>  \tgrep \"fatal:.*CONTENT_LENGTH\" err\n>  '\n"},{"id":"368915","messageId":"f01141d4-e77e-24ba-2354-e7aebc2c3c57@kdbg.org","threadId":"50445","inReplyTo":"20190208220751.9936-1-randall.s.becker@rogers.com","subject":"Re: [Fix v2] t5562: remove dependency on /dev/zero","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-02-09T08:46:12Z","receivedAt":"2019-02-09T08:46:15Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 08.02.19 um 23:07 schrieb randall.s.becker@rogers.com:\n> From: \"Randall S. Becker\" <rsbecker@nexbridge.com>\n> \n> Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero\n> with yes and a translation of its result to a stream of NULL. This is\n> a more portable solution.\n> \n> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> ---\n>  t/t5562-http-backend-content-length.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh\n> index 90d890d02..b8d1913e5 100755\n> --- a/t/t5562-http-backend-content-length.sh\n> +++ b/t/t5562-http-backend-content-length.sh\n> @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' '\n>  \n>  test_expect_success 'CONTENT_LENGTH overflow ssite_t' '\n>  \tNOT_FIT_IN_SSIZE=$(ssize_b100dots) &&\n> -\tenv \\\n> +\tyes | tr \"y\" \"\\\\0\" | env \\\n>  \t\tCONTENT_TYPE=application/x-git-upload-pack-request \\\n>  \t\tQUERY_STRING=/repo.git/git-upload-pack \\\n>  \t\tPATH_TRANSLATED=\"$PWD\"/.git/git-upload-pack \\\n>  \t\tGIT_HTTP_EXPORT_ALL=TRUE \\\n>  \t\tREQUEST_METHOD=POST \\\n>  \t\tCONTENT_LENGTH=\"$NOT_FIT_IN_SSIZE\" \\\n> -\t\tgit http-backend </dev/zero >/dev/null 2>err &&\n> +\t\tgit http-backend >/dev/null 2>err &&\n>  \tgrep \"fatal:.*CONTENT_LENGTH\" err\n>  '\n\nHow many bytes are needed here? yes() in test-lib.sh generates only 99\n'y', if I am not mistaken.\n\n-- Hannes\n"},{"id":"368926","messageId":"000401d4c099$063ae7c0$12b0b740$@nexbridge.com","threadId":"50445","inReplyTo":"xmqq8sypkfzk.fsf@gitster-ct.c.googlers.com","subject":"RE: [Fix v2] t5562: remove dependency on /dev/zero","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2019-02-09T17:00:51Z","receivedAt":"2019-02-09T17:01:03Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On February 8, 2019 18:39, Junio C Hamano wrote:\n> randall.s.becker@rogers.com writes:\n> > Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero\n> > with yes and a translation of its result to a stream of NULL. This is\n> > a more portable solution.\n> >\n> > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>\n> > ---\n> >  t/t5562-http-backend-content-length.sh | 4 ++--\n> >  1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/t/t5562-http-backend-content-length.sh\n> > b/t/t5562-http-backend-content-length.sh\n> > index 90d890d02..b8d1913e5 100755\n> > --- a/t/t5562-http-backend-content-length.sh\n> > +++ b/t/t5562-http-backend-content-length.sh\n> > @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' '\n> >\n> >  test_expect_success 'CONTENT_LENGTH overflow ssite_t' '\n> >  \tNOT_FIT_IN_SSIZE=$(ssize_b100dots) &&\n> > -\tenv \\\n> > +\tyes | tr \"y\" \"\\\\0\" | env \\\n> \n> I do not quite get this use of tr.  The original feeds a stream of NULs\nout of\n> /dev/zero to the command; the yes-to-tr pipe instead feeds a stream of\n> alternating NUL and LF.\n\nThat's why we're going to go with a generate_zero_bytes function per Peff.\nI'm working on a more comprehensive patch covering t5562, t5318, and\ntest-lib-functions.sh that will (hopefully) be satisfactory and remove the\ndependency on /dev/zero and fixes the related new breakages in 2.21.0-rc0.\n\nThe test case in t5318 is specific about wanting zero bytes although the\ntest is just intending to generate a corrupt block that generates a\ndifferent hash, so yes 'yes' might be sufficient, but I don't like\nrandomness myself if we're taking two different tests being involved.\n\nMy current intent is to add to test-lib-functions.sh, a method of\ngeneralizing blocks of zeros to a pipe:\n\n+# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes).\n+# If $1 is < 0, output forever or until the receiving pipe stops reading,\nwhichever comes first.\n+ generate_zero_bytes () {\n+ \tperl -e ' if ($ARGV[0] < 0) { while (-1) { print \"\\0\" } } else {\nprint \"\\0\" x $ARGV[0] }' \"$@\"\n+ }\n\nAnd then fit that into the two tests, then submit as a patch.\n\n> Does the actual bytes fed to the consumer make any difference?  If not,\n> perhaps we can use 'yes' as-is?\n> \n> >  \t\tCONTENT_TYPE=application/x-git-upload-pack-request \\\n> >  \t\tQUERY_STRING=/repo.git/git-upload-pack \\\n> >  \t\tPATH_TRANSLATED=\"$PWD\"/.git/git-upload-pack \\\n> >  \t\tGIT_HTTP_EXPORT_ALL=TRUE \\\n> >  \t\tREQUEST_METHOD=POST \\\n> >  \t\tCONTENT_LENGTH=\"$NOT_FIT_IN_SSIZE\" \\\n> > -\t\tgit http-backend </dev/zero >/dev/null 2>err &&\n> > +\t\tgit http-backend >/dev/null 2>err &&\n> >  \tgrep \"fatal:.*CONTENT_LENGTH\" err\n> >  '\n\nRegards,\nRandall\n\n"},{"id":"368932","messageId":"xmqqwom8izu1.fsf@gitster-ct.c.googlers.com","threadId":"50445","inReplyTo":"f01141d4-e77e-24ba-2354-e7aebc2c3c57@kdbg.org","subject":"Re: [Fix v2] t5562: remove dependency on /dev/zero","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-09T18:25:26Z","receivedAt":"2019-02-09T18:25:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> How many bytes are needed here? yes() in test-lib.sh generates only 99\n> 'y', if I am not mistaken.\n\nI think we will not use \"yes\" in the end for this topic, which makes\nthis comment totally irrelevant to the thread, but I wonder why we\nhave the limit of 99 there?  It cannot be \"we do not want to worry\nabout sigpipe\" affecting the end result of the test (after all the\nreader may stop reading from after reading just one, and the status\nof the upstream process that would die with sigpipe is lost anyway).\n\nIt turns out it is about sigpipe ;-) but in somewhat a different\nway.  To prevent others from wasting their time wondering about\nthis, probably we want to have something like the attached?\n\n t/README      | 9 +++++++++\n t/test-lib.sh | 6 +++++-\n 2 files changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git a/t/README b/t/README\nindex 1326fd7505..f4e1a82657 100644\n--- a/t/README\n+++ b/t/README\n@@ -927,6 +927,15 @@ library for your script to use.\n    test_oid_init or test_oid_cache.  Providing an unknown key is an\n    error.\n \n+ - yes [<string>]\n+\n+   This is often seen in modern UNIX but some platforms lack it, so\n+   the test harness overrides the platform implementation with a\n+   more limited one.  Use this only when feeding a handful lines of\n+   output to the downstream---unlike the real version, it generates\n+   only up to 99 lines.\n+\n+\n Prerequisites\n -------------\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 42b1a0aa7f..541a37f4c0 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1313,7 +1313,11 @@ then\n \tfi\n fi\n \n-# Provide an implementation of the 'yes' utility\n+# Provide an implementation of the 'yes' utility; the upper bound\n+# limit is there to help Windows that cannot stop this loop from\n+# wasting cycles when the downstream stops reading, so do not be\n+# tempted to turn it into an infinite loop. cf. 6129c930 (\"test-lib:\n+# limit the output of the yes utility\", 2016-02-02)\n yes () {\n \tif test $# = 0\n \tthen\n"},{"id":"368939","messageId":"000801d4c0aa$b5f829a0$21e87ce0$@nexbridge.com","threadId":"50445","inReplyTo":"xmqqwom8izu1.fsf@gitster-ct.c.googlers.com","subject":"RE: [Fix v2] t5562: remove dependency on /dev/zero","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2019-02-09T19:07:27Z","receivedAt":"2019-02-09T19:07:54Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On February 9, 2019 13:25, Junio C Hamano wrote:\n> Johannes Sixt <j6t@kdbg.org> writes:\n> \n> > How many bytes are needed here? yes() in test-lib.sh generates only 99\n> > 'y', if I am not mistaken.\n> \n> I think we will not use \"yes\" in the end for this topic, which makes this\n> comment totally irrelevant to the thread, but I wonder why we have the\nlimit\n> of 99 there?  It cannot be \"we do not want to worry about sigpipe\"\naffecting\n> the end result of the test (after all the reader may stop reading from\nafter\n> reading just one, and the status of the upstream process that would die\nwith\n> sigpipe is lost anyway).\n> \n> It turns out it is about sigpipe ;-) but in somewhat a different way.  To\n> prevent others from wasting their time wondering about this, probably we\n> want to have something like the attached?\n> \n>  t/README      | 9 +++++++++\n>  t/test-lib.sh | 6 +++++-\n>  2 files changed, 14 insertions(+), 1 deletion(-)\n> \n> diff --git a/t/README b/t/README\n> index 1326fd7505..f4e1a82657 100644\n> --- a/t/README\n> +++ b/t/README\n> @@ -927,6 +927,15 @@ library for your script to use.\n>     test_oid_init or test_oid_cache.  Providing an unknown key is an\n>     error.\n> \n> + - yes [<string>]\n> +\n> +   This is often seen in modern UNIX but some platforms lack it, so\n> +   the test harness overrides the platform implementation with a\n> +   more limited one.  Use this only when feeding a handful lines of\n> +   output to the downstream---unlike the real version, it generates\n> +   only up to 99 lines.\n> +\n> +\n>  Prerequisites\n>  -------------\n> \n> diff --git a/t/test-lib.sh b/t/test-lib.sh index 42b1a0aa7f..541a37f4c0\n100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -1313,7 +1313,11 @@ then\n>  \tfi\n>  fi\n> \n> -# Provide an implementation of the 'yes' utility\n> +# Provide an implementation of the 'yes' utility; the upper bound #\n> +limit is there to help Windows that cannot stop this loop from #\n> +wasting cycles when the downstream stops reading, so do not be #\n> +tempted to turn it into an infinite loop. cf. 6129c930 (\"test-lib:\n> +# limit the output of the yes utility\", 2016-02-02)\n>  yes () {\n>  \tif test $# = 0\n>  \tthen\n\nSadly, I already the other path ready, but did not have a chance to send it.\nI'm ok either way as long as I can get the tests running.\n\nRegards,\nRandall\n\n"},{"id":"368946","messageId":"8da682c1-aad2-ab83-9e46-6b7d4a6c68bd@kdbg.org","threadId":"50445","inReplyTo":"xmqqwom8izu1.fsf@gitster-ct.c.googlers.com","subject":"Re: [Fix v2] t5562: remove dependency on /dev/zero","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-02-09T21:54:48Z","receivedAt":"2019-02-09T21:54:54Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 09.02.19 um 19:25 schrieb Junio C Hamano:\n> Johannes Sixt <j6t@kdbg.org> writes:\n> \n>> How many bytes are needed here? yes() in test-lib.sh generates only 99\n>> 'y', if I am not mistaken.\n> \n> I think we will not use \"yes\" in the end for this topic, which makes\n> this comment totally irrelevant to the thread, but I wonder why we\n> have the limit of 99 there?  It cannot be \"we do not want to worry\n> about sigpipe\" affecting the end result of the test (after all the\n> reader may stop reading from after reading just one, and the status\n> of the upstream process that would die with sigpipe is lost anyway).\n> \n> It turns out it is about sigpipe ;-) but in somewhat a different\n> way.  To prevent others from wasting their time wondering about\n> this, probably we want to have something like the attached?\n\nThat would certainly be helpful!\n\n-- Hannes\n\n> \n>  t/README      | 9 +++++++++\n>  t/test-lib.sh | 6 +++++-\n>  2 files changed, 14 insertions(+), 1 deletion(-)\n> \n> diff --git a/t/README b/t/README\n> index 1326fd7505..f4e1a82657 100644\n> --- a/t/README\n> +++ b/t/README\n> @@ -927,6 +927,15 @@ library for your script to use.\n>     test_oid_init or test_oid_cache.  Providing an unknown key is an\n>     error.\n>  \n> + - yes [<string>]\n> +\n> +   This is often seen in modern UNIX but some platforms lack it, so\n> +   the test harness overrides the platform implementation with a\n> +   more limited one.  Use this only when feeding a handful lines of\n> +   output to the downstream---unlike the real version, it generates\n> +   only up to 99 lines.\n> +\n> +\n>  Prerequisites\n>  -------------\n>  \n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 42b1a0aa7f..541a37f4c0 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -1313,7 +1313,11 @@ then\n>  \tfi\n>  fi\n>  \n> -# Provide an implementation of the 'yes' utility\n> +# Provide an implementation of the 'yes' utility; the upper bound\n> +# limit is there to help Windows that cannot stop this loop from\n> +# wasting cycles when the downstream stops reading, so do not be\n> +# tempted to turn it into an infinite loop. cf. 6129c930 (\"test-lib:\n> +# limit the output of the yes utility\", 2016-02-02)\n>  yes () {\n>  \tif test $# = 0\n>  \tthen\n> \n\n"}]}