On 09/04, Ramsay Jones wrote: > > Hi Thomas, > > The current pu branch is *very* broken on cygwin; practically every > test fails. A git-bisect session points to: > > $ git bisect good > a4f6670277d5dc0b082139efe162100c6d7a91b8 is the first bad commit > commit a4f6670277d5dc0b082139efe162100c6d7a91b8 > Author: Thomas Gummerer <t.gummerer@xxxxxxxxx> > Date: Thu Aug 16 11:58:38 2012 +0200 > > read-cache.c: Re-read index if index file changed > > Add the possibility of re-reading the index file, if it changed > while reading. > > The index file might change during the read, causing outdated > information to be displayed. We check if the index file changed > by using its stat data as heuristic. > > Signed-off-by: Thomas Gummerer <t.gummerer@xxxxxxxxx> > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > > :100644 100644 6a8b4b1e6859b7a8df2624e80b9da47df6cd1648 cdd8480cc7c3ab373dab1ca9 > 4d77a3154f7d0fbd M read-cache.c > $ > > Since this appears to be a cygwin specific problem, I wasn't too surprised > to see that the code added in this commit involves the stat functions. (Yes, > this is another example of problems caused by the "schizophrenic stat() > functions"). > > A little debugging shows that the index_changed() function was always returning > true, so that the loop was executed 50 times and git then die()s. > > We note that fstat() is a "cygwin native" function, whereas lstat() is executing > the "WIN32 optimised" function. The problematic 'struct stat' fields are st_uid, > st_gid and st_ino, which fstat() fills with "appropriate" values (st_uid=1005, > st_gid=513, st_ino=<non-zero value>), whereas the the WIN32 version of lstat() > always returns zero in these fields. > > I have no wish to spread the insanity, but nonetheless I implemented a "WIN32 > optimised" version of fstat(). This was a great improvement, but there were > still a few failing tests. git-stash, in particular, seemed to be problematic. > A git-bisect session pointed at exactly the same commit as above! *ahem* > > A spot of debugging shows that index_changed() was always returning true ... > Here, the new fstat() function was returning zero in those fields, whereas the > lstat() function was returning "appropriate values" ... > > The difference here is caused by git-stash calling git with an GIT_INDEX_FILE > set to an _absolute_ path, such as: > > /home/ramsay/git/t/trash directory.t3903-stash/.git/index.stash.2440 > > This has the effect of disabling the "optimisation" and calling the "cygwin > native" lstat() function. *ho hum* > > So, the only sensible fix is to not include those fields in the index_changed > predicate on cygwin; which is what the first patch does. The testsuite now > runs just fine (well as fine as before, anyway!). Thanks for noticing that problem, and sending the patch to fix this. > The other two patches don't affect the correctness of the code, so please > feel free to ignore them if you like. Thanks for those patches, they make sense to me. I'm just not sure what to do with the series at the moment, because of the lack of comments for the rest of the series, and Junios comment in the latest What's cooking in git.git: > A GSoC project. Was waiting for comments from mentors and > stakeholders, but nothing seems to be happening, other than breakage > fixes on Cygwin. May discard. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html