On Wed, Feb 08 2023, Junio C Hamano wrote: > I see many failures around SYMLINKS prerequisite in Windows tests. > There are too many to point at, but the pattern seems to be the > same. Here is one example: > > https://github.com/git/git/actions/runs/4127147009/jobs/7130175639#step:5:502 > > where "ln -s x y && test -h y" succeeds and declares SYMLINKS > lazy prerequisite is satisfied, but then it fails to run > > "ln -s unrelated DS && git update-index --add DS" > > with: > > error: readlink("DS"): Function not implemented > > I wonder if something like this is in order? I don't have much to contribute on that front, but this is really missing some "why", this worked before, why is it failing now? Do we have any idea. We use "windows-latest", at first I suspected that this was the 2019->2022 migration, which seems to be happening around now: https://github.blog/changelog/2022-01-11-github-actions-jobs-running-on-windows-latest-are-now-running-on-windows-server-2022/ But looking at a previous successful run on master: https://github.com/git/git/actions/runs/4089369223/jobs/7052074004 V.s. this current failure: https://github.com/git/git/actions/runs/4127146869/jobs/7130173444 Shows that both run 2022, and seem to be running the same software, except that in "set up job" this is different, it was: Download action repository 'git-for-windows/setup-git-for-windows-sdk@v1' (SHA:cbe017cd7ae39629bf4e34fce8b1ccd211fec009) Now: Download action repository 'git-for-windows/setup-git-for-windows-sdk@v1' (SHA:848609620edfa4c2fc64838b85fbe19e534236ee) I have no idea if that's related though... > diff --git a/t/helper/test-readlink.c b/t/helper/test-readlink.c > new file mode 100644 > index 0000000000..c300dc8a1a > --- /dev/null > +++ b/t/helper/test-readlink.c > @@ -0,0 +1,19 @@ > +#include "test-tool.h" > +#include "strbuf.h" > + > +static const char *usage_msg = "test-tool readlink file"; > + > +int cmd__readlink(int ac, const char **av) > +{ > + struct strbuf buf; You're leaving the strbuf uninitialized here, use STRBUF_INIT... > + int ret; > + > + if (ac != 2 || !av[1]) > + usage(usage_msg); > + > + ret = strbuf_readlink(&buf, av[1], 0); ...it's used here, and there's no implicit strbuf_init() on strbuf_readlink(). The first thing it does is inspect sb->alloc. > + if (!ret) > + printf("%s\n", buf.buf); Nit: puts(buf.buf); All in all a simple helper, but isn't this redundant to "test_readlink"? That requires perl, and once we have this we could just replace it, but then let's do that here, no? > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 01e88781dd..c8094f643b 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -1773,7 +1773,8 @@ test_lazy_prereq PIPE ' > > test_lazy_prereq SYMLINKS ' > # test whether the filesystem supports symbolic links > - ln -s x y && test -h y > + ln -s x y && test -h y && test-tool readlink y >/dev/null && > + test "$(test-tool readlink y)" = x Why get the exit code, and then proceed to hide the exit code with the test "$()" pattern, we can just (untested): echo x >expect && test-tool readlink y >actual && test_cmp expect actual If you're trying to avoid leaving litter or cleaning up that's not needed anymore with these lazy prereqs for a while now (they get their throw-away temporary directory).
