Re: [PATCH 2/3] t1301-*.sh: Fix the 'forced modes' test on cygwin

[Ramsay Jones <ramsay@ramsay1.demon.co.uk>]

Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
[...]
>> This call to lstat() happens after git-init has set the "git_dir"
>> (so has_git_dir() returns true), but before the configuration has
>> been fully initialised. At this point git_config() does not find
>> any config files to parse and returns 0. Unfortunately, the code
>> used to determine the cygwin l/stat() function bindings did not
>> check the return from git_config() and assumed that the config
>> was complete and accessible once "git_dir" was set.
> 
> Ok, so this is not really about "a test fails so we will sweep the issue
> under rag", 

Er ... dunno! I don't quite understand what you mean by this. :(

> ... but "we try to optimize too early, before we have enough
> information, so let the code take slow path before we know what is in the
> configuration file".

Yes. While debugging this test failure, I noticed this behaviour, which
I consider to be incorrect (ie a bug), and so I determined to fix it up.
Of course, I knew that this would have the effect of delaying the binding
of l/stat to the WIN32 implementation, which in turn would have the
side-effect of fixing this test case!

So, yes, this is a "drive-by" bug-fix for this test; it could be broken
again by future patches which change the timing of various setup/config
function calls (I *don't* think it will actually, but don't quote me).

You could argue that, because of commit adbc0b6 et. seq. and commit
c869753 (which means that the test-suite is run with core.filemode as
false and core.ignorecygwinfstricks true) that the POSIXPERM prerequiste
should not be set (because the WIN32 l/stat implementation does not
support it). In that case, this test would not be run, and the whole
issue would be moot!  However, on NTFS at least, cygwin *does* support
POSIXPERM.

[Hmm, has anybody tried running the test-suite on a FAT32 filesystem
on Linux! *just joking*]

I *always* set core.filemode true in my cygwin repo(s) so that I don't
have to deal with these problems. :-P  (I would happily revert adbc0b6,
but then I don't have very large repos ...)

BTW, as far as I know, the only remaining problem with the test-suite
on cygwin is an intermittent failure of t4130-apply-criss-cross-rename.sh.
This would also not fail at all if the WIN32 l/stat were not used (this
time because of the inode emulation; just as on Linux, it forces git to
notice the file-change despite the timestamps). Note that t4130-*.sh
also fails intermittently on MinGW, for the same reason, but the frequency
of failure is about 3 times greater on cygwin.

ATB,
Ramsay Jones