Re: [patch] munmap-before-rename, cygwin need

From: Junio C Hamano <junkio@cox.net>
Date: May 7, 2006, 21:14 UTC
Message-ID: <7vslnlk04v.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <f36b08ee0605071258s7a0cb085r3f08e9981234255a@mail.gmail.com>

"Yakov Lerner" <iler.ml@gmail.com> 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?

 - As long as you can clear the second point, I do not see a
   particular reason to make this an option; we should be able
   to do so everywhere.