[PATCH 0/3] v5 index branch breakage on cygwin

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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!).

The other two patches don't affect the correctness of the code, so please
feel free to ignore them if you like.

ATB,
Ramsay Jones


Ramsay Allan Jones (3):
  read-cache.c: Fix index reading breakage on cygwin
  read-cache.c: Pass 'struct stat' parameters by reference
  read-cache.c: Simplify do loop conditional expression

 read-cache.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

-- 
1.7.12

--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]