On 5/7/06, Junio C Hamano <junkio@xxxxxxx> wrote:
"Yakov Lerner" <iler.ml@xxxxxxxxx> writes: > I found that mmap() works on cygwin, but needs a patch. > On Cygwin, rename() fails if target file has active mmap(). > The patch below adds munmap() before rename(). This is interesting in three counts. - I from time to time test Cygwin version on my day-job machine (W2K) and my wife's machine (XP); on both machines I usually have less than two weeks old Cygwin installation, and I have not seen the breakage. I wonder how reproducible this is. Also previously people reported mmap() works for some and fake mmap is needed for others. Would this patch make things work for everybody? - The part you patched is commit_index_file(). This typically is called just before program exit, but some callers, like apply.c, may want to still look at the index after calling it, fully aware that the changes after commit_index will not be written out. Although I haven't traced the codepath fully in apply.c yet, unmapping would break the access to the index (i.e. active_cache[]). Does apply still work with your patch?
You are right. Apply did not work when I gave it more than one patchfile on
commandline (and --index option). I fixed this by zeroing active_nr and freeing
active_cache in unmap_cache(). Then I got infinite loop in
remove_lock_file (after multiple calls to hold_index_file_for_update
with same cf, cache_file_list points to cf and cf->next points to
cf creating infinite loop.) The fix in index.c is easy.
The patch below works for me. However, it changes internal
working of apply.c in the scenario 'git-apply --index patch1 patch2 ...'.
(1) With the patch below, apply.c repeats mmap() on index after every patch
argument (because index gets unmapped after every patchfile argument).
(2) Current apply.c does single mmap() at the beginning. It modfies index
on disk and cache in memory and it does not repeat mmap. This mmap
is to original (now deleted) index, if i understand correctly (the
no-name inode).
But (2) this does not work in cygwin. The end results of (1) and
(2) are the same, I think. (2) looks to me bit faster (I didn't do
measurements).
It's up to you whether to make it under #ifdef MUNMAP_BEFORE_RENAME
of not. The changes are now bigger that in original patch.
The fix to index.c prevents circular list. I got infinite loop in
cache_file_list
every time I tried more than 1 patch on commandline of git-apply. I tried
other solution with the function below, but what I put in the atached patch
is shorter than the alternative below.
Yakov
P.S. I am attaching not inlining bacause
gmail totally removes leading tabs from inline text.
P.P.S.
Alternate fix for index.c:
static void clean_cache_file_list(struct cache_file *cf)
{
struct cache_file *ppcf = &cache_file_list;
cf->lockfile[0] = 0;
while( *ppcf ) {
if(*ppcf == cf ) {
*ppcf = cf->next;
} else
ppcf = &(cf->next);
}
}
- cf->lockfile[0] = 0;
+ clean_cache_file_list(cf);
- cf->lockfile[0] = 0;
+ clean_cache_file_list(cf);
Attachment:
patch2-unmap-before-rename
Description: Binary data