Subject: Re: [PATCH] tests: fix and add lint for non-portable head -c N

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 23 Aug 2018 19:24:36 +0200

On Thu, Aug 23 2018, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <[email-masked]> writes:
>
>> Junio: Even though this isn't a 2.19.0-rc0 regression I think it makes
>> sense for 2.19.0. The fix is trivial, and it'll unbreak (at least some
>> of) the tests on stock git on OpenBSD.
>
> I would have been a bit more sympathetic if this were recent
> regression (say, 2.18 or so), even if it is not new in this cycle,
> and the patch applied to the target track cleanly (say, maint or
> maint-2.17),

Yeah it's not a big deal if it's not in 2.19.

> but seeing that some of these are quite old, dating back to 2009, I am
> not sure I am enthused.

Note that those parts are the "while I'm at it" valgrind-specific parts,
i.e. parts of the code that likely never ran on anything except a GNU
toolchain.

> The changes certainly look trivial.  Let's apply.
>
> Thanks.
>
>>
>> OpenBSD guys: If you CC the git mailing list when you find you need to
>> apply patches like these, we're happy to fix this more pro-actively. I
>> just happened to be testing the upcoming 2.19.0 on OpenBSD and spotted
>> this.
>>
>>  t/check-non-portable-shell.pl | 1 +
>>  t/t5310-pack-bitmaps.sh       | 2 +-
>>  t/test-lib.sh                 | 4 ++--
>>  3 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
>> index d5823f71d8..94a7e6165e 100755
>> --- a/t/check-non-portable-shell.pl
>> +++ b/t/check-non-portable-shell.pl
>> @@ -35,6 +35,7 @@ sub err {
>>  		chomp;
>>  	}
>>
>> +	/\bhead\s+-c\b/ and err 'head -c is not portable (use test_copy_bytes BYTES <file >out)';
>>  	/\bsed\s+-i/ and err 'sed -i is not portable';
>>  	/\becho\s+-[neE]/ and err 'echo with option is not portable (use printf)';
>>  	/^\s*declare\s+/ and err 'arrays/declare not portable';
>> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
>> index 557bd0d0c0..7bff7923f2 100755
>> --- a/t/t5310-pack-bitmaps.sh
>> +++ b/t/t5310-pack-bitmaps.sh
>> @@ -335,7 +335,7 @@ test_expect_success 'truncated bitmap fails gracefully' '
>>  	git rev-list --use-bitmap-index --count --all >expect &&
>>  	bitmap=$(ls .git/objects/pack/*.bitmap) &&
>>  	test_when_finished "rm -f $bitmap" &&
>> -	head -c 512 <$bitmap >$bitmap.tmp &&
>> +	test_copy_bytes 512 <$bitmap >$bitmap.tmp &&
>>  	mv -f $bitmap.tmp $bitmap &&
>>  	git rev-list --use-bitmap-index --count --all >actual 2>stderr &&
>>  	test_cmp expect actual &&
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 8bb0f4348e..44288cbb59 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -867,7 +867,7 @@ then
>>  		# handle only executables, unless they are shell libraries that
>>  		# need to be in the exec-path.
>>  		test -x "$1" ||
>> -		test "# " = "$(head -c 2 <"$1")" ||
>> +		test "# " = "$(test_copy_bytes 2 <"$1")" ||
>>  		return;
>>
>>  		base=$(basename "$1")
>> @@ -882,7 +882,7 @@ then
>>  		# do not override scripts
>>  		if test -x "$symlink_target" &&
>>  		    test ! -d "$symlink_target" &&
>> -		    test "#!" != "$(head -c 2 < "$symlink_target")"
>> +		    test "#!" != "$(test_copy_bytes 2 <"$symlink_target")"
>>  		then
>>  			symlink_target=../valgrind.sh
>>  		fi