t7800 test failure

[Armin Kunaschik]

t7800 fails on systems where readlink (GNUism?) is not available.
An easy workaround for the very basic readlink usage of this test
would be to use a shell function like this:

---
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index ff7a9e9..be3d19f 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -420,6 +420,7 @@ run_dir_diff_test 'difftool --dir-diff when
worktree file is missing' '
 '

 write_script .git/CHECK_SYMLINKS <<\EOF
+readlink() { ls -ld "$1" | sed 's/.* -> //'; }
 for f in file file2 sub/sub
 do
        echo "$f"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Matthieu Moy-2]

Armin Kunaschik <[hidden email]> writes:

> t7800 fails on systems where readlink (GNUism?) is not available.

I don't think it's POSIX, but it is present on all POSIX-like systems I
know. On which system did you get the issue?

> +readlink() { ls -ld "$1" | sed 's/.* -> //'; }

This is much less robust than the actual readlink. For example, if ->
appears in the link name, it breaks.

It would be acceptable as a fall-back if readlink is not present, but
shouldn't activate the "ls" hack by default.

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Junio C Hamano]

Matthieu Moy <[hidden email]> writes:

> Armin Kunaschik <[hidden email]> writes:
>
>> t7800 fails on systems where readlink (GNUism?) is not available.
>
> I don't think it's POSIX, but it is present on all POSIX-like systems I
> know. On which system did you get the issue?
>
>> +readlink() { ls -ld "$1" | sed 's/.* -> //'; }
>
> This is much less robust than the actual readlink. For example, if ->
> appears in the link name, it breaks.

I wouldn't allow it in our scripted Porcelain, but the environment
of our test scripts are under our control, so I do not think it is a
problem ("ls piped to sed" has been an established idiom before
readlink(1) was widely accepted, by the way).

> It would be acceptable as a fall-back if readlink is not present, but
> shouldn't activate the "ls" hack by default.

Yup.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Armin Kunaschik]

On Tue, May 24, 2016 at 6:57 PM, Junio C Hamano <[hidden email]> wrote:
> Matthieu Moy <[hidden email]> writes:
>
>> Armin Kunaschik <[hidden email]> writes:
>>
>>> t7800 fails on systems where readlink (GNUism?) is not available.
>>
>> I don't think it's POSIX, but it is present on all POSIX-like systems I
>> know. On which system did you get the issue?

It's not available in AIX or HP-UX.

>>> +readlink() { ls -ld "$1" | sed 's/.* -> //'; }
>>
>> This is much less robust than the actual readlink. For example, if ->
>> appears in the link name, it breaks.
>
> I wouldn't allow it in our scripted Porcelain, but the environment
> of our test scripts are under our control, so I do not think it is a
> problem ("ls piped to sed" has been an established idiom before
> readlink(1) was widely accepted, by the way).

I think so too. Maybe I can improve the sed expression a bit, but
it will never be a universal readlink replacement. But it doesn't have to.
It's defined locally for this one test only and it does the specific job.

>> It would be acceptable as a fall-back if readlink is not present, but
>> shouldn't activate the "ls" hack by default.
>
> Yup.

Ok, how can this be implemented within the test environment?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Junio C Hamano]

Armin Kunaschik <[hidden email]> writes:

>> I wouldn't allow it in our scripted Porcelain, but the environment
>> of our test scripts are under our control, so I do not think it is a
>> problem ("ls piped to sed" has been an established idiom before
>> readlink(1) was widely accepted, by the way).
>
> I think so too. Maybe I can improve the sed expression a bit, but
> it will never be a universal readlink replacement. But it doesn't have to.
> It's defined locally for this one test only and it does the specific job.
>
>>> It would be acceptable as a fall-back if readlink is not present, but
>>> shouldn't activate the "ls" hack by default.
>>
>> Yup.
>
> Ok, how can this be implemented within the test environment?

I actually think an unconditional check like this is sufficient.

 t/t7800-difftool.sh | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 7ce4cd7..f304228 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -442,15 +442,16 @@ run_dir_diff_test 'difftool --dir-diff with unmerged files' '
  test_cmp expect actual
 '
 
-write_script .git/CHECK_SYMLINKS <<\EOF
-for f in file file2 sub/sub
-do
- echo "$f"
- readlink "$2/$f"
-done >actual
-EOF
-
 test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '
+
+ write_script .git/CHECK_SYMLINKS <<-\EOF &&
+ for f in file file2 sub/sub
+ do
+ echo "$f"
+ ls -ld "$2/$f" | sed -e "s/.* -> //"
+ done >actual
+ EOF
+
  cat >expect <<-EOF &&
  file
  $PWD/file
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Armin Kunaschik]

On Tue, May 24, 2016 at 7:36 PM, Junio C Hamano <[hidden email]> wrote:
> Armin Kunaschik <[hidden email]> writes:
>>
>> Ok, how can this be implemented within the test environment?
>
> I actually think an unconditional check like this is sufficient.

Ah, good. My thoughts were a bit more complicated.
Anyway, this works for me.
Thanks!

>  t/t7800-difftool.sh | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index 7ce4cd7..f304228 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -442,15 +442,16 @@ run_dir_diff_test 'difftool --dir-diff with unmerged files' '
>         test_cmp expect actual
>  '
>
> -write_script .git/CHECK_SYMLINKS <<\EOF
> -for f in file file2 sub/sub
> -do
> -       echo "$f"
> -       readlink "$2/$f"
> -done >actual
> -EOF
> -
>  test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '
> +
> +       write_script .git/CHECK_SYMLINKS <<-\EOF &&
> +       for f in file file2 sub/sub
> +       do
> +               echo "$f"
> +               ls -ld "$2/$f" | sed -e "s/.* -> //"
> +       done >actual
> +       EOF
> +
>         cat >expect <<-EOF &&
>         file
>         $PWD/file

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html