Quantcast

t7800 test failure

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: t7800 test failure

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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: t7800 test failure

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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: t7800 test failure

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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: t7800 test failure

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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: t7800 test failure

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
Loading...