{"thread":{"id":"42437","subject":"t7800 test failure","startedAt":"2016-05-24T15:53:25Z","lastAt":"2016-06-21T20:30:39Z","messageCount":14,"participants":["Armin Kunaschik","Matthieu Moy","Junio C Hamano","David Aguilar","Torsten Bögershausen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"287417","messageId":"CALR6jEiJwx14zAyond9ggz29Q64Fz84URtjr8zaddjnrdY7TjA@mail.gmail.com","threadId":"42437","inReplyTo":null,"subject":"t7800 test failure","fromName":"Armin Kunaschik","fromEmail":"megabreit@googlemail.com","sentAt":"2016-05-24T15:53:25Z","receivedAt":"2016-05-24T15:53:25Z","isPatch":false,"sender":{"key":"megabreit@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/10429218?v=4"},"body":"t7800 fails on systems where readlink (GNUism?) is not available.\nAn easy workaround for the very basic readlink usage of this test\nwould be to use a shell function like this:\n\n---\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex ff7a9e9..be3d19f 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -420,6 +420,7 @@ run_dir_diff_test 'difftool --dir-diff when\nworktree file is missing' '\n '\n\n write_script .git/CHECK_SYMLINKS <<\\EOF\n+readlink() { ls -ld \"$1\" | sed 's/.* -> //'; }\n for f in file file2 sub/sub\n do\n        echo \"$f\"\n"},{"id":"287424","messageId":"vpqk2ijs8p2.fsf@anie.imag.fr","threadId":"42437","inReplyTo":"CALR6jEiJwx14zAyond9ggz29Q64Fz84URtjr8zaddjnrdY7TjA@mail.gmail.com","subject":"Re: t7800 test failure","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-24T16:48:57Z","receivedAt":"2016-05-24T16:48:57Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Armin Kunaschik <megabreit@googlemail.com> writes:\n\n> t7800 fails on systems where readlink (GNUism?) is not available.\n\nI don't think it's POSIX, but it is present on all POSIX-like systems I\nknow. On which system did you get the issue?\n\n> +readlink() { ls -ld \"$1\" | sed 's/.* -> //'; }\n\nThis is much less robust than the actual readlink. For example, if ->\nappears in the link name, it breaks.\n\nIt would be acceptable as a fall-back if readlink is not present, but\nshouldn't activate the \"ls\" hack by default.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287426","messageId":"xmqq1t4r75sv.fsf@gitster.mtv.corp.google.com","threadId":"42437","inReplyTo":"vpqk2ijs8p2.fsf@anie.imag.fr","subject":"Re: t7800 test failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-24T16:57:04Z","receivedAt":"2016-05-24T16:57:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Armin Kunaschik <megabreit@googlemail.com> writes:\n>\n>> t7800 fails on systems where readlink (GNUism?) is not available.\n>\n> I don't think it's POSIX, but it is present on all POSIX-like systems I\n> know. On which system did you get the issue?\n>\n>> +readlink() { ls -ld \"$1\" | sed 's/.* -> //'; }\n>\n> This is much less robust than the actual readlink. For example, if ->\n> appears in the link name, it breaks.\n\nI wouldn't allow it in our scripted Porcelain, but the environment\nof our test scripts are under our control, so I do not think it is a\nproblem (\"ls piped to sed\" has been an established idiom before\nreadlink(1) was widely accepted, by the way).\n\n> It would be acceptable as a fall-back if readlink is not present, but\n> shouldn't activate the \"ls\" hack by default.\n\nYup.\n"},{"id":"287428","messageId":"CALR6jEj67MA7CCHQ_jfdtAuGoo9wjPie0+a=e-BqJjoYtJ9oHw@mail.gmail.com","threadId":"42437","inReplyTo":"xmqq1t4r75sv.fsf@gitster.mtv.corp.google.com","subject":"Re: t7800 test failure","fromName":"Armin Kunaschik","fromEmail":"megabreit@googlemail.com","sentAt":"2016-05-24T17:20:21Z","receivedAt":"2016-05-24T17:20:21Z","isPatch":false,"sender":{"key":"megabreit@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/10429218?v=4"},"body":"On Tue, May 24, 2016 at 6:57 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> Armin Kunaschik <megabreit@googlemail.com> writes:\n>>\n>>> t7800 fails on systems where readlink (GNUism?) is not available.\n>>\n>> I don't think it's POSIX, but it is present on all POSIX-like systems I\n>> know. On which system did you get the issue?\n\nIt's not available in AIX or HP-UX.\n\n>>> +readlink() { ls -ld \"$1\" | sed 's/.* -> //'; }\n>>\n>> This is much less robust than the actual readlink. For example, if ->\n>> appears in the link name, it breaks.\n>\n> I wouldn't allow it in our scripted Porcelain, but the environment\n> of our test scripts are under our control, so I do not think it is a\n> problem (\"ls piped to sed\" has been an established idiom before\n> readlink(1) was widely accepted, by the way).\n\nI think so too. Maybe I can improve the sed expression a bit, but\nit will never be a universal readlink replacement. But it doesn't have to.\nIt's defined locally for this one test only and it does the specific job.\n\n>> It would be acceptable as a fall-back if readlink is not present, but\n>> shouldn't activate the \"ls\" hack by default.\n>\n> Yup.\n\nOk, how can this be implemented within the test environment?\n"},{"id":"287430","messageId":"xmqqfut75peg.fsf@gitster.mtv.corp.google.com","threadId":"42437","inReplyTo":"CALR6jEj67MA7CCHQ_jfdtAuGoo9wjPie0+a=e-BqJjoYtJ9oHw@mail.gmail.com","subject":"Re: t7800 test failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-24T17:36:39Z","receivedAt":"2016-05-24T17:36:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Armin Kunaschik <megabreit@googlemail.com> writes:\n\n>> I wouldn't allow it in our scripted Porcelain, but the environment\n>> of our test scripts are under our control, so I do not think it is a\n>> problem (\"ls piped to sed\" has been an established idiom before\n>> readlink(1) was widely accepted, by the way).\n>\n> I think so too. Maybe I can improve the sed expression a bit, but\n> it will never be a universal readlink replacement. But it doesn't have to.\n> It's defined locally for this one test only and it does the specific job.\n>\n>>> It would be acceptable as a fall-back if readlink is not present, but\n>>> shouldn't activate the \"ls\" hack by default.\n>>\n>> Yup.\n>\n> Ok, how can this be implemented within the test environment?\n\nI actually think an unconditional check like this is sufficient.\n\n t/t7800-difftool.sh | 17 +++++++++--------\n 1 file changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 7ce4cd7..f304228 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -442,15 +442,16 @@ run_dir_diff_test 'difftool --dir-diff with unmerged files' '\n \ttest_cmp expect actual\n '\n \n-write_script .git/CHECK_SYMLINKS <<\\EOF\n-for f in file file2 sub/sub\n-do\n-\techo \"$f\"\n-\treadlink \"$2/$f\"\n-done >actual\n-EOF\n-\n test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '\n+\n+\twrite_script .git/CHECK_SYMLINKS <<-\\EOF &&\n+\tfor f in file file2 sub/sub\n+\tdo\n+\t\techo \"$f\"\n+\t\tls -ld \"$2/$f\" | sed -e \"s/.* -> //\"\n+\tdone >actual\n+\tEOF\n+\n \tcat >expect <<-EOF &&\n \tfile\n \t$PWD/file\n"},{"id":"287491","messageId":"CALR6jEixZitA1CTE_kDkDEHv59ALT9zkCOgd28unMhLUZKt48Q@mail.gmail.com","threadId":"42437","inReplyTo":"xmqqfut75peg.fsf@gitster.mtv.corp.google.com","subject":"Re: t7800 test failure","fromName":"Armin Kunaschik","fromEmail":"megabreit@googlemail.com","sentAt":"2016-05-25T09:33:33Z","receivedAt":"2016-05-25T09:33:33Z","isPatch":false,"sender":{"key":"megabreit@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/10429218?v=4"},"body":"On Tue, May 24, 2016 at 7:36 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Armin Kunaschik <megabreit@googlemail.com> writes:\n>>\n>> Ok, how can this be implemented within the test environment?\n>\n> I actually think an unconditional check like this is sufficient.\n\nAh, good. My thoughts were a bit more complicated.\nAnyway, this works for me.\nThanks!\n\n>  t/t7800-difftool.sh | 17 +++++++++--------\n>  1 file changed, 9 insertions(+), 8 deletions(-)\n>\n> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> index 7ce4cd7..f304228 100755\n> --- a/t/t7800-difftool.sh\n> +++ b/t/t7800-difftool.sh\n> @@ -442,15 +442,16 @@ run_dir_diff_test 'difftool --dir-diff with unmerged files' '\n>         test_cmp expect actual\n>  '\n>\n> -write_script .git/CHECK_SYMLINKS <<\\EOF\n> -for f in file file2 sub/sub\n> -do\n> -       echo \"$f\"\n> -       readlink \"$2/$f\"\n> -done >actual\n> -EOF\n> -\n>  test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '\n> +\n> +       write_script .git/CHECK_SYMLINKS <<-\\EOF &&\n> +       for f in file file2 sub/sub\n> +       do\n> +               echo \"$f\"\n> +               ls -ld \"$2/$f\" | sed -e \"s/.* -> //\"\n> +       done >actual\n> +       EOF\n> +\n>         cat >expect <<-EOF &&\n>         file\n>         $PWD/file\n"},{"id":"287649","messageId":"20160527041944.GA17438@gmail.com","threadId":"42437","inReplyTo":"CALR6jEixZitA1CTE_kDkDEHv59ALT9zkCOgd28unMhLUZKt48Q@mail.gmail.com","subject":"Re: t7800 test failure","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-05-27T04:19:44Z","receivedAt":"2016-05-27T04:19:44Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Wed, May 25, 2016 at 11:33:33AM +0200, Armin Kunaschik wrote:\n> On Tue, May 24, 2016 at 7:36 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> > Armin Kunaschik <megabreit@googlemail.com> writes:\n> >>\n> >> Ok, how can this be implemented within the test environment?\n> >\n> > I actually think an unconditional check like this is sufficient.\n> \n> Ah, good. My thoughts were a bit more complicated.\n> Anyway, this works for me.\n> Thanks!\n\nWould you mind submitting a patch so that we can support these\ntests when running on AIX/HP-UX?\n\n\n> >  t/t7800-difftool.sh | 17 +++++++++--------\n> >  1 file changed, 9 insertions(+), 8 deletions(-)\n> >\n> > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> > index 7ce4cd7..f304228 100755\n> > --- a/t/t7800-difftool.sh\n> > +++ b/t/t7800-difftool.sh\n> > @@ -442,15 +442,16 @@ run_dir_diff_test 'difftool --dir-diff with unmerged files' '\n> >         test_cmp expect actual\n> >  '\n> >\n> > -write_script .git/CHECK_SYMLINKS <<\\EOF\n> > -for f in file file2 sub/sub\n> > -do\n> > -       echo \"$f\"\n> > -       readlink \"$2/$f\"\n> > -done >actual\n> > -EOF\n> > -\n> >  test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '\n> > +\n> > +       write_script .git/CHECK_SYMLINKS <<-\\EOF &&\n> > +       for f in file file2 sub/sub\n> > +       do\n> > +               echo \"$f\"\n> > +               ls -ld \"$2/$f\" | sed -e \"s/.* -> //\"\n> > +       done >actual\n> > +       EOF\n> > +\n> >         cat >expect <<-EOF &&\n> >         file\n> >         $PWD/file\n\nI was curious so I whipped together a small tweak to\nt/check-non-portable-shell.pl below.\n\nThe difftool tests are not the only ones that use readlink.\nMy guess is you haven't run the p4 tests because AIX/HP-UX doesn't have p4?\n\n\t$ make test-lint-shell-syntax\n\tt7800-difftool.sh:449: error: readlink is not portable (please use ls -ld | sed):       readlink \"$2/$f\"\n\tt9802-git-p4-filetype.sh:266: error: readlink is not portable (please use ls -ld | sed):                test $(readlink symlink) = symlink-target\n\tt9802-git-p4-filetype.sh:332: error: readlink is not portable (please use ls -ld | sed):                test $(readlink empty-symlink) = target2\n\ttest-lib.sh:757: error: readlink is not portable (please use ls -ld | sed):             test \"$1\" = \"$(readlink \"$2\")\" || {\n\nIf we want to ban use of readlink from the test suite we could\nadd it to the check script.  test-lib.sh also includes it for\nvalgrind support so I'm not really sure whether we'd want to\napply this, but I figured I'd bring it up for discussion.\n\nIf we end up fixing all of these then I can send this to the\nlist as a proper patch.\n\nCurious, is there an easy way to get readlink and mktemp installed on AIX?\n\nAnother alternative is that we can compile our own\n\"git-readlink\" and \"git-mktemp\" programs and use those instead,\nbut that seems like a big maintenance burden compared to the\nrelative simplicity of the test-suite workarounds.\n\nThanks for fixing my non-portable tests ;-)\n\n--- 8< --- 8< ---\nFrom 40c41402dfa24ca16b41062172c34f238d77b42c Mon Sep 17 00:00:00 2001\nFrom: David Aguilar <davvid@gmail.com>\nDate: Thu, 26 May 2016 02:42:52 -0700\nSubject: [PATCH] tests: add shell portability check for \"readlink\"\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n t/check-non-portable-shell.pl | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex b170cbc..2707e42 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -18,6 +18,7 @@ while (<>) {\n \tchomp;\n \t/\\bsed\\s+-i/ and err 'sed -i is not portable';\n \t/\\becho\\s+-n/ and err 'echo -n is not portable (please use printf)';\n+\t/\\breadlink\\s+/ and err 'readlink is not portable (please use ls -ld | sed)';\n \t/^\\s*declare\\s+/ and err 'arrays/declare not portable';\n \t/^\\s*[^#]\\s*which\\s/ and err 'which is not portable (please use type)';\n \t/\\btest\\s+[^=]*==/ and err '\"test a == b\" is not portable (please use =)';\n-- \n2.7.0.rc3\n\n-- \nDavid\n"},{"id":"287663","messageId":"vpq4m9kvt4k.fsf@anie.imag.fr","threadId":"42437","inReplyTo":"20160527041944.GA17438@gmail.com","subject":"Re: t7800 test failure","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-27T07:48:27Z","receivedAt":"2016-05-27T07:48:27Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> Another alternative is that we can compile our own\n> \"git-readlink\" and \"git-mktemp\" programs and use those instead,\n> but that seems like a big maintenance burden compared to the\n> relative simplicity of the test-suite workarounds.\n\nNot _that_ big burden as we already have the necessary infrastructure:\ngit-mktemp would be t/helper/test-mktemp.c already available to tests as\ntest-mktemp, and it would be easy to do it for readlink too.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287864","messageId":"574CDA24.1020906@googlemail.com","threadId":"42437","inReplyTo":"20160527041944.GA17438@gmail.com","subject":"[PATCH] t7800 readlink not found","fromName":"Armin Kunaschik","fromEmail":"megabreit@googlemail.com","sentAt":"2016-05-31T00:26:12Z","receivedAt":"2016-05-31T00:26:12Z","isPatch":true,"sender":{"key":"megabreit@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/10429218?v=4"},"body":"On 05/27/2016 06:19 AM, David Aguilar wrote:\n> On Wed, May 25, 2016 at 11:33:33AM +0200, Armin Kunaschik wrote:\n> \n> Would you mind submitting a patch so that we can support these\n> tests when running on AIX/HP-UX?\n\nI don't feel comfortable to submit patches for tests I can't verify. I\ndon't have valgrind and python/p4 here. Looking to the code I'd say,\npatching the p4 tests with \"ls -ld | sed\" looks quite save.\nBut I'm not sure about the test-lib.sh. When you are really super\nparanoid, as written in the comment, you should probably use perl like\n\nperl -e 'print readlink $ARGV[0]' $name\n\nas a replacement.\n\nSo, as suggested by Junio, here the readlink workaround for t7800 only.\n(hopefully whitespace clean this time)\n\n--- 8< --- 8< ---\nFrom: Armin Kunaschik <megabreit@googlemail.com>\nSubject: t7800: readlink is not portable\n\nThe readlink(1) command is not available on all platforms (notably not\non AIX and HP-UX) and can be replaced in this test with the \"workaround\"\n\nls -ld <name> | sed -e 's/.* -> //'\n\nThis is no universal readlink replacement but works in the controlled\ntest environment good enough.\n\nSigned-off-by: Armin Kunaschik <megabreit@googlemail.com>\n---\n\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 7ce4cd7..905035c 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -446,7 +446,7 @@ write_script .git/CHECK_SYMLINKS <<\\EOF\n for f in file file2 sub/sub\n do\n \techo \"$f\"\n-\treadlink \"$2/$f\"\n+\tls -ld \"$2/$f\" | sed -e 's/.* -> //'\n done >actual\n EOF\n"},{"id":"287871","messageId":"574D1BEA.5020409@web.de","threadId":"42437","inReplyTo":"574CDA24.1020906@googlemail.com","subject":"Re: [PATCH] t7800 readlink not found","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-05-31T05:06:50Z","receivedAt":"2016-05-31T05:06:50Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 05/31/2016 02:26 AM, Armin Kunaschik wrote:\n> On 05/27/2016 06:19 AM, David Aguilar wrote:\n>> On Wed, May 25, 2016 at 11:33:33AM +0200, Armin Kunaschik wrote:\n>>\n>> Would you mind submitting a patch so that we can support these\n>> tests when running on AIX/HP-UX?\n> I don't feel comfortable to submit patches for tests I can't verify. I\n> don't have valgrind and python/p4 here. Looking to the code I'd say,\n> patching the p4 tests with \"ls -ld | sed\" looks quite save.\n> But I'm not sure about the test-lib.sh. When you are really super\n> paranoid, as written in the comment, you should probably use perl like\n>\n> perl -e 'print readlink $ARGV[0]' $name\n>\n> as a replacement.\n>\n> So, as suggested by Junio, here the readlink workaround for t7800 only.\n> (hopefully whitespace clean this time)\n>\n> --- 8< --- 8< ---\n> From: Armin Kunaschik <megabreit@googlemail.com>\n> Subject: t7800: readlink is not portable\n>\n> The readlink(1) command is not available on all platforms (notably not\n> on AIX and HP-UX) and can be replaced in this test with the \"workaround\"\n>\n> ls -ld <name> | sed -e 's/.* -> //'\n>\n> This is no universal readlink replacement but works in the controlled\n> test environment good enough.\n>\n> Signed-off-by: Armin Kunaschik <megabreit@googlemail.com>\n> ---\n>\n> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> index 7ce4cd7..905035c 100755\n> --- a/t/t7800-difftool.sh\n> +++ b/t/t7800-difftool.sh\n> @@ -446,7 +446,7 @@ write_script .git/CHECK_SYMLINKS <<\\EOF\n>   for f in file file2 sub/sub\n>   do\n>   \techo \"$f\"\n> -\treadlink \"$2/$f\"\n> +\tls -ld \"$2/$f\" | sed -e 's/.* -> //'\n>   done >actual\n>   EOF\n>\nI don't know how portable #ls -ld\" really is.\nIf there is one platform, that doesn't support readlink, would it\nmake sense to implement readlink() in test-lib.sh,\nsimilar to what we have for MINGW, e.g. sort() or find() ?\nAnd keep t7800 as it is ?\n"},{"id":"287873","messageId":"xmqqzir67p1y.fsf@gitster.mtv.corp.google.com","threadId":"42437","inReplyTo":"574D1BEA.5020409@web.de","subject":"Re: [PATCH] t7800 readlink not found","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-31T05:51:37Z","receivedAt":"2016-05-31T05:51:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n>> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n>> index 7ce4cd7..905035c 100755\n>> --- a/t/t7800-difftool.sh\n>> +++ b/t/t7800-difftool.sh\n>> @@ -446,7 +446,7 @@ write_script .git/CHECK_SYMLINKS <<\\EOF\n>>   for f in file file2 sub/sub\n>>   do\n>>   \techo \"$f\"\n>> -\treadlink \"$2/$f\"\n>> +\tls -ld \"$2/$f\" | sed -e 's/.* -> //'\n>>   done >actual\n>>   EOF\n>>\n> I don't know how portable #ls -ld\" really is.\n\nThe parts with mode bits, nlinks, uid, gid, size, and date part do\nhave some variations.  For example, we have been burned on ACL\nenabled systems having some funny suffix after the usual mode bits\nstuff.\n\nHowever, as far as this test is concerned, I do not think \"how\nportable is the output from ls -ld\" is an especially relevant\nquestion.  None of the things we expect early in the output (the\nfields I enumerated in the previous paragraph) would contain \" -> \".\nAnd we know that we do not use a filename that has \" -> \" (or \"->\")\nas a substring in our tests.\n\nWe don't have to use readlink, even on platforms where we do have\nreadlink.  Building the conditional to be checked at runtime and\nproviding a shell function read_link that uses \"ls -ld | sed\" or\n\"readlink\" depending on the runtime check is wasteful.\n"},{"id":"289773","messageId":"CALR6jEgAtvuecJ4OPOAcDGh3o02oM_WP5_CM8Y52eW2hjuLDNg@mail.gmail.com","threadId":"42437","inReplyTo":"xmqqzir67p1y.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t7800 readlink not found","fromName":"Armin Kunaschik","fromEmail":"megabreit@googlemail.com","sentAt":"2016-06-21T14:44:06Z","receivedAt":"2016-06-21T14:45:09Z","isPatch":true,"sender":{"key":"megabreit@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/10429218?v=4"},"body":"On Tue, May 31, 2016 at 7:51 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Torsten Bögershausen <tboegi@web.de> writes:\n>\n>>> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n>>> index 7ce4cd7..905035c 100755\n>>> --- a/t/t7800-difftool.sh\n>>> +++ b/t/t7800-difftool.sh\n>>> @@ -446,7 +446,7 @@ write_script .git/CHECK_SYMLINKS <<\\EOF\n>>>   for f in file file2 sub/sub\n>>>   do\n>>>      echo \"$f\"\n>>> -    readlink \"$2/$f\"\n>>> +    ls -ld \"$2/$f\" | sed -e 's/.* -> //'\n>>>   done >actual\n>>>   EOF\n>>>\n>> I don't know how portable #ls -ld\" really is.\n>\n> The parts with mode bits, nlinks, uid, gid, size, and date part do\n> have some variations.  For example, we have been burned on ACL\n> enabled systems having some funny suffix after the usual mode bits\n> stuff.\n>\n> However, as far as this test is concerned, I do not think \"how\n> portable is the output from ls -ld\" is an especially relevant\n> question.  None of the things we expect early in the output (the\n> fields I enumerated in the previous paragraph) would contain \" -> \".\n> And we know that we do not use a filename that has \" -> \" (or \"->\")\n> as a substring in our tests.\n>\n> We don't have to use readlink, even on platforms where we do have\n> readlink.  Building the conditional to be checked at runtime and\n> providing a shell function read_link that uses \"ls -ld | sed\" or\n> \"readlink\" depending on the runtime check is wasteful.\n>\n\nJust a short, curious question: Is this patch to be accepted/included some time?\nI didn't see it in 2.8.4 nor 2.9.0. Maybe it just fell off the table...\n\nArmin\n"},{"id":"289792","messageId":"xmqqy45ypesl.fsf@gitster.mtv.corp.google.com","threadId":"42437","inReplyTo":"CALR6jEgAtvuecJ4OPOAcDGh3o02oM_WP5_CM8Y52eW2hjuLDNg@mail.gmail.com","subject":"Re: [PATCH] t7800 readlink not found","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-21T18:39:22Z","receivedAt":"2016-06-21T18:39:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Armin Kunaschik <megabreit@googlemail.com> writes:\n\n> On Tue, May 31, 2016 at 7:51 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Torsten Bögershausen <tboegi@web.de> writes:\n>>\n>>>> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n>>>> index 7ce4cd7..905035c 100755\n>>>> --- a/t/t7800-difftool.sh\n>>>> +++ b/t/t7800-difftool.sh\n>>>> @@ -446,7 +446,7 @@ write_script .git/CHECK_SYMLINKS <<\\EOF\n>>>>   for f in file file2 sub/sub\n>>>>   do\n>>>>      echo \"$f\"\n>>>> -    readlink \"$2/$f\"\n>>>> +    ls -ld \"$2/$f\" | sed -e 's/.* -> //'\n>>>>   done >actual\n>>>>   EOF\n>>>>\n>>> I don't know how portable #ls -ld\" really is.\n>>\n>> The parts with mode bits, nlinks, uid, gid, size, and date part do\n>> have some variations.  For example, we have been burned on ACL\n>> enabled systems having some funny suffix after the usual mode bits\n>> stuff.\n>>\n>> However, as far as this test is concerned, I do not think \"how\n>> portable is the output from ls -ld\" is an especially relevant\n>> question.  None of the things we expect early in the output (the\n>> fields I enumerated in the previous paragraph) would contain \" -> \".\n>> And we know that we do not use a filename that has \" -> \" (or \"->\")\n>> as a substring in our tests.\n>>\n>> We don't have to use readlink, even on platforms where we do have\n>> readlink.  Building the conditional to be checked at runtime and\n>> providing a shell function read_link that uses \"ls -ld | sed\" or\n>> \"readlink\" depending on the runtime check is wasteful.\n>\n> Just a short, curious question: Is this patch to be accepted/included some time?\n> I didn't see it in 2.8.4 nor 2.9.0. Maybe it just fell off the table...\n\nYes, I think this fell off the table as I was waiting for some kind\nof agreement or counter-proposal, neither of which came and the\nthread was forgotten.\n\nUnless Torsten still has strong objections (or better yet, a better\nimplementation), I am inclined to queue it as-is.\n\nThanks for pinging the thread.\n"},{"id":"289804","messageId":"5769A3D4.5060506@web.de","threadId":"42437","inReplyTo":"xmqqy45ypesl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t7800 readlink not found","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2016-06-21T20:30:12Z","receivedAt":"2016-06-21T20:30:39Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 06/21/2016 08:39 PM, Junio C Hamano wrote:\n> Armin Kunaschik <megabreit@googlemail.com> writes:\n>\n>> On Tue, May 31, 2016 at 7:51 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Torsten Bögershausen <tboegi@web.de> writes:\n>>>\n>>>>> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n>>>>> index 7ce4cd7..905035c 100755\n>>>>> --- a/t/t7800-difftool.sh\n>>>>> +++ b/t/t7800-difftool.sh\n>>>>> @@ -446,7 +446,7 @@ write_script .git/CHECK_SYMLINKS <<\\EOF\n>>>>>    for f in file file2 sub/sub\n>>>>>    do\n>>>>>       echo \"$f\"\n>>>>> -    readlink \"$2/$f\"\n>>>>> +    ls -ld \"$2/$f\" | sed -e 's/.* -> //'\n>>>>>    done >actual\n>>>>>    EOF\n>>>>>\n>>>> I don't know how portable #ls -ld\" really is.\n>>> The parts with mode bits, nlinks, uid, gid, size, and date part do\n>>> have some variations.  For example, we have been burned on ACL\n>>> enabled systems having some funny suffix after the usual mode bits\n>>> stuff.\n>>>\n>>> However, as far as this test is concerned, I do not think \"how\n>>> portable is the output from ls -ld\" is an especially relevant\n>>> question.  None of the things we expect early in the output (the\n>>> fields I enumerated in the previous paragraph) would contain \" -> \".\n>>> And we know that we do not use a filename that has \" -> \" (or \"->\")\n>>> as a substring in our tests.\n>>>\n>>> We don't have to use readlink, even on platforms where we do have\n>>> readlink.  Building the conditional to be checked at runtime and\n>>> providing a shell function read_link that uses \"ls -ld | sed\" or\n>>> \"readlink\" depending on the runtime check is wasteful.\n>> Just a short, curious question: Is this patch to be accepted/included some time?\n>> I didn't see it in 2.8.4 nor 2.9.0. Maybe it just fell off the table...\n> Yes, I think this fell off the table as I was waiting for some kind\n> of agreement or counter-proposal, neither of which came and the\n> thread was forgotten.\n>\n> Unless Torsten still has strong objections (or better yet, a better\n> implementation), I am inclined to queue it as-is.\n>\nI just double-checked the man pages for Mac OS and opengroup:\nNo better implementation from my side -> No objections\n\n"}]}