Re: [PATCH] test: make SYMLINKS prerequisite more robust

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Feb 08 2023, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> 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.
>
> Your guess is as good as mine.  I do not do Windows.
>
> Thanks for independently noticing the uninitialized strbuf.  What I
> have queued has it fixed already locally but there isn't much point
> to send another copy out to the list ;-).
>
>> All in all a simple helper, but isn't this redundant to "test_readlink"?
>
> Not at all.  We need to avoid the Perl one for this purpose.  What
> matters is whether "git" considers if symlink is working.
>
> Perhaps our readlink(3) emulation we have in compat/ may hardcode
> our "knowledge" that symlink is not available in Windows, which may
> not match what the POSIX XCU emulation in our Windows environment
> offers, which apparently ran "ln -s x y && test -h y" successfully,
> and who knows what test_readlink that is written in Perl thinks?

Yeah that's fair, I wonder why we haven't replaced this already...

FWIW I think this is what perl will dispatch to on Windows, so it makes
your point, it has its own NIH Windows emulation layer:
https://github.com/Perl/perl5/blob/blead/win32/win32.c#L1983-L2026

> We
> are testing "git" with the test suite, so even if with some magic
> that is still unknown to compat/mingw.h it knows how to read what
> "ln -s x y" left in "y", until compat/mingw.h::readlink() learns the
> same trick, asking Perl to decide SYMLINKS prerequisite would not
> help our test scripts at all.

We could always see if they return the same answer :) But not worth
pursuing in this case.

>> 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).
>
> Indeed, I just did not want to add another cruft, but 'x' and 'y'
> are already such crufts, so I could have just done
>
> 	ln -s x y &&
> 	test -h y &&
> 	test-tool readlink y >x &&
> 	test $(cat x) = x
>
> to use one of them ;-)

Yeah, I mean you don't need to avoid cruft, because the whole directory
is about to be rm -rf'd

Now that I've dug it up I see you implemented that in 04083f278d1 (test:
allow prerequisite to be evaluated lazily, 2012-07-26).

I was recalling 53ff3b96a87 (tests: make sure nested lazy prereqs work
reliably, 2020-11-18) as the more recent change, but that just solved a
nested prereq edge case.