Junio C Hamano wrote:
> Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx> writes:
>
>> Michael Haggerty and Jeff King have been re-vamping the reference
>> handling code. The failures noted above were provoked by patches
>> in the 'mh/ref-races' branch. At the time I wrote this patch, that
>> branch was only included in 'pu', but I notice that this topic has
>> now progressed to 'next' (see commit 71f1a182).
>
> I had an impression that up to 98eeb09e (for_each_ref: load all
> loose refs before packed refs, 2013-06-20) that is now in 'next'
> does not agressively use the lstat timestamp of the packed-refs
> file, and the "optional" bit 5d478f5c (refs: do not invalidate the
> packed-refs cache unnecessarily, 2013-06-20), and the one in 'next'
> should be safe with the cheating-lstat. Isn't it the case?
The next branch (from a couple of days ago, namely commit 4f488db) is
currently broken on cygwin, like so:
$ cd t
$ ./t3211-peel-ref.sh -i -v
[ ... ]
ok 7 - refs are peeled outside of refs/tags (old packed)
expecting success:
git pack-refs --all &&
cp .git/packed-refs fully-peeled &&
git branch yadda &&
git pack-refs --all &&
git branch -d yadda &&
test_cmp fully-peeled .git/packed-refs
fatal: internal error: packed-ref cache cleared while locked
not ok 8 - peeled refs survive deletion of packed ref
#
# git pack-refs --all &&
# cp .git/packed-refs fully-peeled &&
# git branch yadda &&
# git pack-refs --all &&
# git branch -d yadda &&
# test_cmp fully-peeled .git/packed-refs
#
$ cd trash\ directory.t3211-peel-ref/
$ ls
actual base.t expect
$ ../../bin-wrappers/git pack-refs --all
fatal: internal error: packed-ref cache cleared while locked
$ gdb ../../git.exe
GNU gdb 6.5.50.20060706-cvs (cygwin-special)
Copyright (C) 2006 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB. Type "show warranty" for details.
This GDB was configured as "i686-pc-cygwin"...
(gdb) b stat_validity_check
Breakpoint 1 at 0x48a33f: file read-cache.c, line 1965.
(gdb) run pack-refs --all
Starting program: /home/ramsay/git/git.exe pack-refs --all
Loaded symbols for /cygdrive/c/WINDOWS/system32/ntdll.dll
Loaded symbols for /cygdrive/c/WINDOWS/system32/kernel32.dll
Loaded symbols for /usr/bin/cygcrypto-0.9.8.dll
Loaded symbols for /usr/bin/cygwin1.dll
Loaded symbols for /cygdrive/c/WINDOWS/system32/advapi32.dll
Loaded symbols for /cygdrive/c/WINDOWS/system32/rpcrt4.dll
Loaded symbols for /cygdrive/c/WINDOWS/system32/secur32.dll
Loaded symbols for /usr/bin/cygiconv-2.dll
Loaded symbols for /usr/bin/cygz.dll
Breakpoint 1, stat_validity_check (sv=0xa611a4,
path=0x57d98c ".git/packed-refs") at read-cache.c:1965
1965 if (stat(path, &st) < 0)
(gdb) n
1967 if (!sv->sd)
(gdb) p sv
$1 = (struct stat_validity *) 0xa611a4
(gdb) p *sv
$2 = {sd = 0xa62478}
(gdb) p sv->sd
$3 = (struct stat_data *) 0xa62478
(gdb) p *sv->sd
$4 = {sd_ctime = {sec = 1372194879, nsec = 671875000}, sd_mtime = {
sec = 1372194879, nsec = 656250000}, sd_dev = 2899104371,
sd_ino = 180184, sd_uid = 1005, sd_gid = 513, sd_size = 296}
(gdb) p st
$5 = {st_dev = 0, st_ino = 0, st_mode = 33152, st_nlink = 1, st_uid = 0,
st_gid = 0, st_rdev = 0, st_size = 296, st_atim = {tv_sec = 1372195048,
tv_nsec = 890625000}, st_mtim = {tv_sec = 1372194879,
tv_nsec = 656250000}, st_ctim = {tv_sec = 1372194879,
tv_nsec = 15625000}, st_blksize = 5636381, st_blocks = 1, st_spare4 = {
10883480, 5757324}}
(gdb) c
Continuing.
fatal: internal error: packed-ref cache cleared while locked
Program exited with code 0200.
(gdb) quit
$ ../../test-stat .git/packed-refs
stat for '.git/packed-refs':
*dev: -1395862925, 0
*ino: 180184, 0
*mode: 100644 -rw-, 100600 -rw-
nlink: 1, 1
*uid: 1005, 0
*gid: 513, 0
*rdev: -1395862925, 0
size: 296, 296
atime: 1372195048, 1372195048 Tue Jun 25 22:17:28 2013
mtime: 1372194879, 1372194879 Tue Jun 25 22:14:39 2013
ctime: 1372194879, 1372194879 Tue Jun 25 22:14:39 2013
$
Note that, as noted before, (at least) the following tests fail
due to the 'mh/ref-races' branch:
t3210-pack-refs.sh (Wstat: 256 Tests: 16 Failed: 12)
Failed tests: 4-9, 11-16
Non-zero exit status: 1
t3211-peel-ref.sh (Wstat: 256 Tests: 8 Failed: 1)
Failed test: 8
Non-zero exit status: 1
t5500-fetch-pack.sh (Wstat: 256 Tests: 54 Failed: 1)
Failed test: 43
Non-zero exit status: 1
> In any case, if removing the cheating-lstat will improve the
> robustness without hurting performance, I am all for it.
I'm sure it will improve robustness (and help with maintainability),
but as for performance ...
> The less platform specific hacks, the better ;-).
Agreed.
ATB,
Ramsay Jones
--
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