Re: [PATCH] t7800 readlink not found

[Junio C Hamano <gitster@pobox.com>]

Armin Kunaschik <megabreit@googlemail.com> writes:

> On Tue, May 31, 2016 at 7:51 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Torsten Bögershausen <tboegi@web.de> writes:
>>
>>>> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
>>>> index 7ce4cd7..905035c 100755
>>>> --- a/t/t7800-difftool.sh
>>>> +++ b/t/t7800-difftool.sh
>>>> @@ -446,7 +446,7 @@ write_script .git/CHECK_SYMLINKS <<\EOF
>>>>   for f in file file2 sub/sub
>>>>   do
>>>>      echo "$f"
>>>> -    readlink "$2/$f"
>>>> +    ls -ld "$2/$f" | sed -e 's/.* -> //'
>>>>   done >actual
>>>>   EOF
>>>>
>>> I don't know how portable #ls -ld" really is.
>>
>> The parts with mode bits, nlinks, uid, gid, size, and date part do
>> have some variations.  For example, we have been burned on ACL
>> enabled systems having some funny suffix after the usual mode bits
>> stuff.
>>
>> However, as far as this test is concerned, I do not think "how
>> portable is the output from ls -ld" is an especially relevant
>> question.  None of the things we expect early in the output (the
>> fields I enumerated in the previous paragraph) would contain " -> ".
>> And we know that we do not use a filename that has " -> " (or "->")
>> as a substring in our tests.
>>
>> We don't have to use readlink, even on platforms where we do have
>> readlink.  Building the conditional to be checked at runtime and
>> providing a shell function read_link that uses "ls -ld | sed" or
>> "readlink" depending on the runtime check is wasteful.
>
> Just a short, curious question: Is this patch to be accepted/included some time?
> I didn't see it in 2.8.4 nor 2.9.0. Maybe it just fell off the table...

Yes, I think this fell off the table as I was waiting for some kind
of agreement or counter-proposal, neither of which came and the
thread was forgotten.

Unless Torsten still has strong objections (or better yet, a better
implementation), I am inclined to queue it as-is.

Thanks for pinging the thread.