[PATCH]change posix test for symlinks

Matt Mackall mpm at selenic.com
Sun Jul 28 17:48:37 CDT 2013


On Sun, 2013-07-28 at 17:28 +0000, DeathGorePain wrote:
> Hello,
> 
> I've been having problems with hg leaving artifacts when the repository
> / working directory is mounted on sshfs. hg-checklink-* linking to the
> `hg root` are left in root of the repository and render all operations
> slow. In somebody else's words (stackoverflow).
> <http://stackoverflow.com/questions/5213876/mercurial-hg-checklinks-recursive-symlink-over-nfs-samba-sshfs-network-drive>
> 
> I have not figured out how your tests work (confusing output) :/ But a
> local test (sudo cp posix.py ...) returned favorable results and the
> artifacts don't exist anymore.
> Please find the patch below.

This patch looks like voodoo.

The existing code was obviously correct and reasonable on normal
filesystems, and we've arrived at its current state after half a decade
of deployment and feedback. You threw that code out and completely
rewrote it apparently without ever diagnosing what the underlying
problem was. And without having done that, you can't tell us why the new
code works but the old code doesn't: voodoo.

(It's also got a few new bugs.)

Ideally, you'd first find the root cause of the problem, and then make a
_minimal_ change to the existing code to deal with it, with an
explanation in the comments or commit messages.

That said, the problem here is almost certainly sshfs's follow_symlinks
mount option, which is a horrible and magically broken feature. After
poking at its weirdness for a bit, I came up with this:

# HG changeset patch
# User Matt Mackall <mpm at selenic.com>
# Date 1375041752 18000
#      Sun Jul 28 15:02:32 2013 -0500
# Branch stable
# Node ID cfdae231ba781a590e2e27b3eeee4dabda884be5
# Parent  9e8298a324accd509cd8df1915cd98dfbdcdb512
checklink: work around sshfs brain-damage (issue3636)

With the follow_symlinks option, sshfs will successfully create links
while claiming it encountered an I/O error. In addition, depending on
the type of link, it may subsequently be impossible to delete the link
via sshfs. Our existing link to '.' will cause sshfs to think the link
is a directory, and thus cause unlink to give EISDIR. Links to
non-existent names or circular links will cause the created link to not
even be visible.

Thus, we need to create a new temporary file and link to that. We'll
still get a failure, but we'll be able to remove the link.

diff -r 9e8298a324ac -r cfdae231ba78 mercurial/posix.py
--- a/mercurial/posix.py	Sat Jul 27 19:31:14 2013 -0500
+++ b/mercurial/posix.py	Sun Jul 28 15:02:32 2013 -0500
@@ -154,10 +154,16 @@
     # file already exists
     name = tempfile.mktemp(dir=path, prefix='hg-checklink-')
     try:
-        os.symlink(".", name)
+        fd = tempfile.NamedTemporaryFile(dir=path, prefix='hg-checklink-')
+        os.symlink(os.path.basename(fd.name), name)
         os.unlink(name)
         return True
-    except (OSError, AttributeError):
+    except AttributeError:
+        return False
+    except OSError, inst:
+        # sshfs might report failure while successfully creating the link
+        if inst[0] == errno.EIO and os.path.exists(name):
+            os.unlink(name)
         return False
 
 def checkosfilename(path):


-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list