Re: [PATCH v2 4/5] tests: replace remaining packetize() with "test-tool pkt-line"

> On Mon, Jul 12, 2021 at 06:44:19PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> Move the only remaining users of "packetize()" over to "test-tool
>> pkt-line", for this we need a new "pack-raw-stdin" subcommand in the
>> test-tool. The "pack" command takes input on stdin, but splits it by
>> "\n", furthermore we'll format the output using C-strings, so the
>> embedded "\0" being tested for here would cause the string to be
>> truncated.
>> 
>> So we need another mode that just calls packet_write() on the raw
>> input we got on stdin.
>
> Makes sense. Lacking this "raw" version was the sticking point in the
> past for using the helper in more places.
>
>> +static void pack_raw_stdin(void)
>> +{
>> +	struct strbuf sb = STRBUF_INIT;
>> +	strbuf_read(&sb, 0, 0);
>> +	if (strbuf_read(&sb, 0, 0) < 0)
>> +		die_errno("failed to read from stdin");
>
> What's up with the two reads here?
>
> It looks like just a duplication. And it happens to work because each
> one tries to read to eof, making the second one generally a noop.

>> diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
>> index 2dde0348816..b52afb0cdea 100755
>> --- a/t/t5570-git-daemon.sh
>> +++ b/t/t5570-git-daemon.sh
>> @@ -193,10 +193,12 @@ test_expect_success 'hostname cannot break out of directory' '
>>  '
>>  
>>  test_expect_success FAKENC 'hostname interpolation works after LF-stripping' '
>> -	{
>> -		printf "git-upload-pack /interp.git\n\0host=localhost" | packetize
>> -		printf "0000"
>> -	} >input &&
>> +	printf "git-upload-pack /interp.git\n\0host=localhost" >has-null &&
>> +	test-tool pkt-line pack-raw-stdin >input <has-null &&
>> +	test-tool pkt-line pack >>input <<-\EOF &&
>> +	0000
>> +	EOF
>
> This is a minor style nit, but I really prefer redirecting output from
> a block (as in the original) rather than iterative appending (in the
> result). IMHO it makes it more obvious what is going into the file and
> what is not.
>
> I.e.:
>
>   {
>           printf "git-upload-pack /interp.git\n\0host=localhost" |
> 	  test-tool pkt-line pack-raw-stdin &&
> 	  printf "0000" | test-tool pkt-line pack
>   } >input
>
> (again here the packing of "0000" is pointless, but I'm OK with it for
> consistency).