Re: [PATCH v2 1/2] tests: fix and add lint for non-portable head -c N

On Thu, Aug 23, 2018 at 03:25:01PM +0000, Ævar Arnfjörð Bjarmason wrote:

The "head -c BYTES" option is non-portable (not in POSIX[1]). Change
such invocations to use the test_copy_bytes wrapper added in
48860819e8 ("t9300: factor out portable "head -c" replacement",
2016-06-30).

This fixes a test added in 9d2e330b17 ("ewah_read_mmap: bounds-check
mmap reads", 2018-06-14), which has been breaking
t5310-pack-bitmaps.sh on OpenBSD since 2.18.0. The OpenBSD ports
already have a similar workaround after their upgrade to 2.18.0[2].

Heh, I even considered using this when writing that test. But the reason
I introduced test_copy_bytes is not because the target platform did not
have "head -c" at all, but because some tests need very specific
buffering guarantees when reading from a shared pipe.

That said, if OpenBSD's "head" doesn't have "-c" at all, I'm fine with
this as a fix (and it sounds like we know that IRIX lacks it, too).

Also, change a valgrind-specific codepath in test-lib.sh to use this
wrapper. Given where valgrind runs I don't think this would ever
become a portability issue in practice, but it's easier to just use
the wrapper than introduce some exception for the "make test-lint"
check being added here.

When working on 9d2e330b17, I recall finding these other "head -c"
invocations in the test suite when I did 9d2e330b17 and took them as
evidence that it was OK to use in vanilla cases. So even if these sites
don't affect any platforms in practice, I think it's worth it to ban
"head -c" completely.

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
 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(-)

Patch itself looks good to me. Thanks.

-Peff