Re: [PATCH] t: use portable wrapper for readlink(1)

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Jun 03 2021, Jeff King wrote:

> On Thu, Jun 03, 2021 at 03:39:53PM -0400, Jeff King wrote:
>
>> > FWIW this broke tests on AIX because we can't assume readlink(1) exists
>> > at all. See d2addc3b96 (t7800: readlink may not be available,
>> > 2016-05-31) for a workaround.
>> 
>> Hmm. So obviously we can use a fix similar to the one in t7800 (though
>> it's sufficiently complicated that I'd be tempted to wrap it in a helper
>> function). There are a few other calls that could be changed, too.
>
> Here's a patch to do that. Can you confirm that it fixes your test
> failure?

It does, thanks, t3210*.sh is broken on current master on AIX, fixed
with this patch. I don't have p4 on that machine (or any box I hack on),
so I can't test t9802*.sh.

> -- >8 --
> Subject: [PATCH] t: use portable wrapper for readlink(1)
>
> Not all systems have a readlink program available for use by the shell.
> This causes t3210 to fail on at least AIX. Let's provide a perl
> one-liner to do the same thing, and use it there.
>
> I also updated calls in t9802. Nobody reported failure there, but it's
> the same issue. Presumably nobody actually tests with p4 on AIX in the
> first place (if it is even available there).
>
> I left the use of readlink in the "--valgrind" setup in test-lib.sh, as
> valgrind isn't available on exotic platforms anyway (and I didn't want
> to increase dependencies between test-lib.sh and test-lib-functions.sh).
>
> There's one other curious case. Commit d2addc3b96 (t7800: readlink may
> not be available, 2016-05-31) fixed a similar case. We can't use our
> wrapper function there, though, as it's inside a sub-script triggered by
> Git. It uses a slightly different technique ("ls" piped to "sed"). I
> chose not to use that here as it gives confusing "ls -l" output if the
> file is unexpectedly not a symlink (which is OK for its limited use, but
> potentially confusing for general use within the test suite). The perl
> version emits the empty string.
>
> Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/t3210-pack-refs.sh       | 2 +-
>  t/t9802-git-p4-filetype.sh | 4 ++--
>  t/test-lib-functions.sh    | 6 ++++++
>  3 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
> index 3b7cdc56ec..577f32dc71 100755
> --- a/t/t3210-pack-refs.sh
> +++ b/t/t3210-pack-refs.sh
> @@ -253,7 +253,7 @@ test_expect_success SYMLINKS 'pack symlinked packed-refs' '
>  	git for-each-ref >all-refs-packed &&
>  	test_cmp all-refs-before all-refs-packed &&
>  	test -h .git/packed-refs &&
> -	test "$(readlink .git/packed-refs)" = "my-deviant-packed-refs"
> +	test "$(test_readlink .git/packed-refs)" = "my-deviant-packed-refs"
>  '
>  
>  test_done
> diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
> index 94edebe272..19073c6e9f 100755
> --- a/t/t9802-git-p4-filetype.sh
> +++ b/t/t9802-git-p4-filetype.sh
> @@ -263,7 +263,7 @@ test_expect_success SYMLINKS 'ensure p4 symlink parsed correctly' '
>  	(
>  		cd "$git" &&
>  		test -L symlink &&
> -		test $(readlink symlink) = symlink-target
> +		test $(test_readlink symlink) = symlink-target
>  	)
>  '
>  
> @@ -329,7 +329,7 @@ test_expect_success SYMLINKS 'empty symlink target' '
>  	git p4 clone --dest="$git" //depot@all &&
>  	(
>  		cd "$git" &&
> -		test $(readlink empty-symlink) = target2
> +		test $(test_readlink empty-symlink) = target2
>  	)
>  '
>  
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index b823c14027..661f376077 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -1692,3 +1692,9 @@ test_region () {
>  
>  	return 0
>  }
> +
> +# Print the destination of symlink(s) provided as arguments. Basically
> +# the same as the readlink command, but it's not available everywhere.
> +test_readlink () {
> +	perl -le 'print readlink($_) for @ARGV' "$@"
> +}