Subject: Re: [PATCH] Add a new lstat and fstat implementation based on Win32 API

From: Marius Storm-Olsen <[email-masked]>
Date: Mon, 03 Sep 2007 15:52:06 +0200

Johannes Schindelin said the following on 03.09.2007 15:33:
> On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:
>> There was a problem with racy conditions, which this revision fixes.
>> The problem was that fstat was using the builtin implementation, which for
>> for some reason is off by some amount of seconds. (This is probably due to
>> some leap-year issue in one of the implementations. However, Microsoft tells
>> us to use 116444736000000000 in http://support.microsoft.com/kb/167296, so
>> I'll stick with that.)
>> Also, since both stat and lstat proved to be rather slow, having our own
>> version of fstat is probably also wise. At least we now control all the
>> stat'ing, so we _know_ they are compatible.
>> Also note that this revision makes git_lstat call itself after modifying
>> the filename, instead of the builtin stat, for the same reasons.
> 
> At least some of these informations should go into the commit message, 
> too.

Sure

>> With the our own implementations of lstat & fstat, the following test cases
>> are now fixed:
>>     t4116-apply-reverte.sh
>>         ok 3: apply in reverse
>>     t4200-rerere.sh
>>         ok 17: young records still live
>> However, the following test cases seems to fail now:
>>     t6024-recursive-merge.sh
>>         FAIL 1: setup tests
>>         FAIL 3: result contains a conflict
>>         FAIL 4: virtual trees were processed
>>         FAIL 5: refuse to merge binaries
>>
>> See attached test case logs.
>> Are some of these test cases unstable, so the result will fluctuate on
>> Windows?
> 
> I saw some funny stuff on Windows, like test cases succeeding when run 
> interactively, but failing when run from "make test".

Ok, I ran 'make test', so maybe that's it? I'll rerun them later.

> BTW it would have been way easier to apply your patch, had you followed 
> SubmittingPatches...

Heh, I actually tried, using the Thunderbird way. Of course the 
attachments are non-conforming :-) What was the problem? Whitespace 
issues, Windows EOL, attachments, or all of the above? :-)

> To make it easier on others, I just uploaded it into the "teststat" branch 
> on 4msysgit.git (subject to removal in a few days).

Cool, thanks

> First comment: it seems git_fstat() is not declared properly, so there are 
> quite a few compiler warnings.

/me slaps self. Right, sorry 'bout that. I'll amend the declaration in 
git-compat-util.h.

> Running the tests now.

Great, thanks!
-- 
.marius