Re: [PATCH] t: use portable wrapper for readlink(1)
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jun 4, 2021, 21:09 UTC
- Message-ID
- <87r1hh5o97.fsf@evledraar.gmail.com>
- In-Reply-To
- <YLk0Zm2J6VOA/lks@coredump.intra.peff.net>
On Thu, Jun 03 2021, Jeff King wrote:
Show 12 quoted lines
> 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.
Show 81 quoted lines
> -- >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' "$@"
> +}