[PATCH STABLE V2] windows: implement nlinks() using Python's ctypes (issue1922)

[Adrian Buehlmann]

On 2011-01-25 23:45, Matt Mackall wrote:
> On Sun, 2011-01-23 at 16:12 +0100, Adrian Buehlmann wrote:
>> On 2011-01-23 15:17, Adrian Buehlmann wrote:
>>> # HG changeset patch
>>> # User Adrian Buehlmann <adrian at cadifra.com>
>>> # Date 1295790385 -3600
>>> # Branch stable
>>> # Node ID 99f2b980756d1c22775cf6ec6e93c646bc910a55
>>> # Parent  d0e0d3d43e1439d63564ab4dddfe0daa69ae2d86
>>> windows: implement nlinks() using Python's ctypes (issue1922)
>>>
>>> If the pywin32 package was not installed, the import of win32 in
>>> windows.py silently failed and (before this patch) util.nlinks was then
>>> used as a fallback when running on Windows.
>>>
>>> util.nlinks() returned 0 for all files when called on Windows, because
>>> Python's
>>>
>>>     os.lstat(name).st_nlink
>>>
>>> is 0 on Windows for all files.
>>>
>>> If nlinks() returns 0, util.opener failed to break up hardlinks, which
>>> could lead to repository corruption when committing or pushing to a
>>> hardlinked clone (hg verify detects it).
>>>
>>> We now provide our own nlinks() in windows.py by using Python's ctypes
>>> library, so we don't depend on the pywin32 package being installed for
>>> nlinks().
>>>
>>>   ** Since Python's ctypes were introduced in Python 2.5, we now
>>>   ** require Python 2.5 or later for Mercurial on Windows
>>>
>>> Using ctypes also has the benefit that nlinks() also works correctly
>>> for the pure Python Mercurial.
>>>
>>> And we force breaking up hardlinks on every append file access in the
>>> opener if nlinks() returns < 1, thus making sure that we can't cause
>>> any hardlink repository corruption.
>>>
>>> It would have been possible to simply require the pywin32 package on
>>> Windows and abort with an import error if it's not installed, but such
>>> a policy change should be avoided on the stable branch. Previous packages
>>> like for example
>>>
>>>     mercurial-1.7.3-1.win32-py2.6.exe
>>>
>>> didn't make it obvious that pywin32 was needed as a dependency. It just
>>> silently caused repository corruption in hardlinked clones if pywin32
>>> was not installed.
> 
> Alright, now I've lost the plot. Let me see if I can recover it:
> 
> require pywin32
>  - adds a new dependency
>  - solves problem for native
>  - pure may or may not be able to use it (ie jython)
> extend osutils
>  - no new dependency
>  - solves problem for native
>  - pure still broken
> use ctypes
>  - requires py2.5 or separate ctypes
>  - solves problem for native
>  - pure may or may not be able to use it
> 
> It's worth noting that pure has pretty substantial problems on Windows
> due to the use of "native" open() and the like.
> 
> So in this analysis, osutils looks like the winner. Is your osutils
> patch good for 1.7.4?

Looking at your statements in this mail, I'm tempted to say no.

I think my osutil.c patch is quite risky for 1.7.4, it lacks a nlinks()
implementation for pure and it is going in a wrong direction.

IMHO, the right direction for the future is using Python's ctypes.

I'd rather like to see

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

alone in 1.7.4.

This simply forces a file copy on 'a'ppend if pywin32 is not installed.

I'll send a patch for this.

> Another way to look at it is "what else breaks without pywin32"?
> The answer looks like:
> 
> - finding stale locks
> - finding rc paths
> - finding usernames
> - signal handling
> - terminal width
> - window hiding??
> 
> Looks like pretty minor stuff. 
> 
> A related question is "can we replace pywin32 with ctypes"? If the
> answer is yes, it might make sense to do that so we'll have no
> dependencies when we move to Py2.5 a few years from now. Switching to
> ctypes might make sense for 1.8 or 1.9.
> 

I think we can get rid of the pywin32 dependency and we should.

I hope we can switch to ctypes for 1.8.

Can we require Python >=2.5 (at least for Windows) for 1.8?