opener: check hardlink count reporting (issue1866) stable
authorAdrian Buehlmann <adrian@cadifra.com>
Sun, 07 Nov 2010 18:21:29 +0100
branchstable
changeset 12938 bf826c0b9537
parent 12937 6ff784de7c3a
child 12939 0cb90f9c6d8e
child 12953 f3183932c487
opener: check hardlink count reporting (issue1866) The Linux CIFS kernel driver (even in 2.6.36) suffers from a hardlink count blindness bug (lstat() returning 1 in st_nlink when it is expected to return >1), which causes repository corruption if Mercurial running on Linux pushes or commits to a hardlinked repository stored on a Windows share, if that share is mounted using the CIFS driver. This patch works around issue1866 and improves the workaround done in 50523b4407f6 to fix issue761, by teaching the opener to lazily execute a runtime check (new function checknlink) to see if the hardlink count reported by nlinks() can be trusted. Since nlinks() is also known to return varying count values (1 or >1) depending on whether the file is open or not and depending on what client and server software combination is being used for accessing and serving the Windows share, we deliberately open the file before calling nlinks() in order to have a stable precondition. Trying to depend on the precondition "file closed" would be fragile, as the file could have been opened very easily somewhere else in the program.
mercurial/util.py
mercurial/win32.py
--- a/mercurial/util.py	Thu Nov 04 09:04:37 2010 +0100
+++ b/mercurial/util.py	Sun Nov 07 18:21:29 2010 +0100
@@ -716,6 +716,29 @@
     except (OSError, AttributeError):
         return False
 
+def checknlink(testfile):
+    '''check whether hardlink count reporting works properly'''
+    f = testfile + ".hgtmp"
+
+    try:
+        os_link(testfile, f)
+    except OSError, inst:
+        if inst.errno == errno.EINVAL:
+            # FS doesn't support creating hardlinks
+            return True
+        return False
+
+    try:
+        # nlinks() may behave differently for files on Windows shares if
+        # the file is open.
+        fd = open(f)
+        return nlinks(f) > 1
+    finally:
+        fd.close()
+        os.unlink(f)
+
+    return False
+
 def endswithsep(path):
     '''Check path ends with os.sep or os.altsep.'''
     return path.endswith(os.sep) or os.altsep and path.endswith(os.altsep)
@@ -840,6 +863,7 @@
         else:
             self.auditor = always
         self.createmode = None
+        self._trustnlink = None
 
     @propertycache
     def _can_symlink(self):
@@ -873,13 +897,20 @@
                     os.unlink(f)
                     nlink = 0
                 else:
+                    # nlinks() may behave differently for files on Windows
+                    # shares if the file is open.
+                    fd = open(f)
                     nlink = nlinks(f)
-            except OSError:
+                    fd.close()
+            except (OSError, IOError):
                 nlink = 0
                 if not os.path.isdir(dirname):
                     makedirs(dirname, self.createmode)
-            if nlink > 1:
-                rename(mktempcopy(f), f)
+            if nlink > 0:
+                if self._trustnlink is None:
+                    self._trustnlink = nlink > 1 or checknlink(f)
+                if nlink > 1 or not self._trustnlink:
+                    rename(mktempcopy(f), f)
         fp = posixfile(f, mode)
         if nlink == 0:
             if st_mode is None:
--- a/mercurial/win32.py	Thu Nov 04 09:04:37 2010 +0100
+++ b/mercurial/win32.py	Sun Nov 07 18:21:29 2010 +0100
@@ -43,17 +43,7 @@
 
 def nlinks(pathname):
     """Return number of hardlinks for the given file."""
-    links = _getfileinfo(pathname)[7]
-    if links < 2:
-        # Known to be wrong for most network drives
-        dirname = os.path.dirname(pathname)
-        if not dirname:
-            dirname = '.'
-        dt = win32file.GetDriveType(dirname + '\\')
-        if dt == 4 or dt == 1:
-            # Fake hardlink to force COW for network drives
-            links = 2
-    return links
+    return _getfileinfo(pathname)[7]
 
 def samefile(fpath1, fpath2):
     """Returns whether fpath1 and fpath2 refer to the same file. This is only