From patchwork Sun Jul 28 22:48:37 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: change posix test for symlinks From: Matt Mackall X-Patchwork-Id: 1978 Message-Id: <1375051717.5856.42.camel@calx> To: DeathGorePain Cc: mercurial-devel@selenic.com Date: Sun, 28 Jul 2013 17:48:37 -0500 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). > > > 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 # 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):