[PATCH STABLE] windows: make win32 mandatory (issue1922)

[Adrian Buehlmann]

On 2011-01-22 23:48, Steve Borho wrote:
> On Sat, Jan 22, 2011 at 3:53 PM, Adrian Buehlmann <adrian at cadifra.com> wrote:
>> On 2011-01-22 20:06, Patrick Mézard wrote:
>>> Le 20/01/11 20:04, Adrian Buehlmann a écrit :
>>>> On 2011-01-20 19:59, Aaron Cohen wrote:
>>>>> Would using ctypes directly help?
>>>>
>>>> Yeah. ctypes are cool. But they require python 2.5.
>>>
>>> Still, requiring python 2.5 on Windows might not be such a big deal considering the advantages it might bring:
>>> - It's standard
>>> - In theory, with some work, it may replace pywin32 completely. I don't know if there are any issues with 32bits vs 64 bits OS.
>>> - Who knows, it may improve the situation with pure module
>>
>> I was thinking the same about replacing pywin32, but:
>>
>> - How mature is this ctypes thing?
>> - Is it used? = Does it work as expected? (in my experience,
>>  things which are not used by lots of people tend to have more
>>  bugs)
>> - What about speed? Isn't there a lot of forth and back
>>  going on between the types inside the dll's and the
>>  Python objects?
>>
>> I must say though I have absolutely no experience with ctypes,
>> so this might be pure FUD and ctypes are just fine.
>>
>> At the moment I have a pretty good feeling about my C
>> implementation of nlinks() I sent in the other patch today.
>> I played quite a lot with it, also together with my fsdebug
>> extension [1]. nlinks() is such a basic building block for mercurial,
>> it should really be rock solid, best using tried and known technology.
>> And interfacing to C seems to be a pretty mature part of Python.
>> But getting everything correct in the C part isn't easy, my
>> C code patch might even be too risky for stable (although it is
>> quite analogous to the posixfile C code, so there is some reuse
>> of known successful coding patterns there.)
>>
>> Another question is: is anyone actually using pure mercurial on
>> Windows?
> 
> setup.py uses the pure extensions to build the version file.  Beyond
> that, it's difficult to guess.  My guess is the number of regular pure
> users > number of Python 2.4 users on Windows.
> 
>> I'm already lucky enough if I get everything working
>> as intended using the canonical mercurial code (i.e. using the
>> parts in C).
>>
>> But there are two questions here:
>> - What shall we do with this current hardlink bug in stable?
>>  Is it a problem to now require pywin32? Or not? I wasn't entirely
>>  clear about Mads' opinion on that. IHMO, this new hardlink bug is
>>  pretty serious. After all Mercurial is corrupting its own
>>  repositories...
> 
> FWIW, I'm +1 on requiring pywin32.  It's one less variable to deal with.
> 

Wee. Now that I've wasted my time on doing a C implementation of
nlinks()... :p (just joking, it's no problem that I've looked into doing
a C nlinks(). It was a nice learning experience for me, I think I'm
almost prepared to do some more interesting C hacks for mercurial :)

Seriously. Another absolutely minimalistic option would be to just push
this patch alone on stable:

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -913,6 +913,8 @@ class opener(object):
                     # shares if the file is open.
                     fd = open(f)
                     nlink = nlinks(f)
+                    if nlink == 0:
+                        nlink = 2 # work around issue 1922
                     fd.close()
             except (OSError, IOError):
                 nlink = 0

this causes a file copy on every append file access (happens only when
writing in the store) if pywin32 is not installed. The hardlinks would
be guaranteed to be broken up by this. But the speed would be reduced if
pywin32 is not installed.

If pywin32 is installed, speed would be the same as today and hardlinks
are correctly broken up (no change to 1.7.3). The line "nlink = 2" would
never be reached if pywin32 is installed.

So with this minimalistic patch, people would always get correct and
safe behavior but reduced speed if pywin32 is not installed (some users
might complain about this being a performance regression, but we could
tell them they shall install pywin32.).

Such a small patch would have the benefit of the lowest risk to break
anything. After all, the next stable release of mercurial should be one
of the most stable releases of the 1.7 series... (but it still should
please not ship with a hardlink repository corruption bug).

Sigh.