[PATCH V1] util: move checknlink away from the dependency of a hard-coded filename
Yuya Nishihara
yuya at tcha.org
Wed Aug 24 08:14:31 EDT 2016
On Tue, 23 Aug 2016 10:39:29 -0700, ttung at fb.com wrote:
> # HG changeset patch
> # User Tony Tung <tonytung at merly.org>
> # Date 1471973931 25200
> # Tue Aug 23 10:38:51 2016 -0700
> # Node ID 800bdaf50847ed85c913d8d8701d7c6738ee082c
> # Parent 2efc5a05d80a6d4253767f2ce0c2fb062ba83cb6
> util: move checknlink away from the dependency of a hard-coded filename
>
> This somewhat insulates us against bugs in checknlink when a stale file
> left behind could result in checknlink always returning False.
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -1305,34 +1305,46 @@
> def checknlink(testfile):
> '''check whether hardlink count reporting works properly'''
>
> - # testfile may be open, so we need a separate file for checking to
> - # work around issue2543 (or testfile may get lost on Samba shares)
> - f1 = testfile + ".hgtmp1"
> - if os.path.lexists(f1):
> - return False
> + f1path, f2path, f1handle, f2handle = None, None, None, None
Nit: this could be f1path = f2path = ... = None.
> try:
> - posixfile(f1, 'w').close()
> - except IOError:
> + dirpath, filename = os.path.split(testfile)
> +
> + # testfile may be open, so we need a separate file for checking to
> + # work around issue2543 (or testfile may get lost on Samba shares)
> + f1handle, f1path = tempfile.mkstemp(prefix=filename, dir=dirpath)
> + os.close(f1handle)
> + f1handle = None
> +
> + f2handle, f2path = tempfile.mkstemp(prefix=filename, dir=dirpath)
> + os.close(f2handle)
> + f2handle = None
> +
> + # there's a small race condition that another file can jam itself in
> + # between the time we remove f2 and the time we create the hard link.
> + # in the unlikely scenario that happens, we'll treat it as nlink being
> + # unreliable.
> try:
> - os.unlink(f1)
> + os.unlink(f2path)
> + oslink(f1path, f2path)
> + # nlinks() may behave differently for files on Windows shares if
> + # the file is open.
> + f2handle = posixfile(f2path)
> + return nlinks(f2path) > 1
> except OSError:
> - pass
> - return False
> -
> - f2 = testfile + ".hgtmp2"
> - fd = None
> - try:
> - oslink(f1, f2)
> - # nlinks() may behave differently for files on Windows shares if
> - # the file is open.
> - fd = posixfile(f2)
> - return nlinks(f2) > 1
> - except OSError:
> - return False
> + return False
> finally:
> - if fd is not None:
> - fd.close()
> - for f in (f1, f2):
> + for f in (f1handle, f2handle):
> + try:
> + if f is None:
> + continue
> + elif isinstance(f, int):
> + os.close(f)
> + else:
> + f.close()
I don't think there's a benefit to store file descriptor or file object
to the same variable. And IIRC, f.close() would raise IOError on failure,
whereas os.close() would OSError.
> + except OSError:
> + pass
> + for f in (f1path, f2path):
f1path and f2path could be None.
> try:
> os.unlink(f)
More information about the Mercurial-devel
mailing list