[PATCH 01 of 14] posix: use local reference to unlink
Jun Wu
quark at fb.com
Mon Mar 20 23:27:02 EDT 2017
The series is a nice clean-up and looks good to me, except a nit about the
return value "tryunlink", which could probably be fixed in flight.
Excerpts from Ryan McElroy's message of 2017-03-20 19:10:44 -0700:
> # HG changeset patch
> # User Ryan McElroy <rmcelroy at fb.com>
> # Date 1490059858 25200
> # Mon Mar 20 18:30:58 2017 -0700
> # Node ID 9e525e28e3c76792f57355989bdd652f0617541d
> # Parent 8fbdaf1533bb20cb471679be6ab2dc799fd7e634
> posix: use local reference to unlink
>
> We have a local reference to os.unlink in module scope, but we still used
> os.unlink inside functions. This changes util to use the local reference,
> which will pave the way for combining duplicated code in future patches.
>
> diff --git a/mercurial/posix.py b/mercurial/posix.py
> --- a/mercurial/posix.py
> +++ b/mercurial/posix.py
> @@ -105,7 +105,7 @@ def setflags(f, l, x):
> fp = open(f)
> data = fp.read()
> fp.close()
> - os.unlink(f)
> + unlink(f)
> try:
> os.symlink(data, f)
> except OSError:
> @@ -118,7 +118,7 @@ def setflags(f, l, x):
> if stat.S_ISLNK(s):
> # switch link to file
> data = os.readlink(f)
> - os.unlink(f)
> + unlink(f)
> fp = open(f, "w")
> fp.write(data)
> fp.close()
> @@ -187,9 +187,9 @@ def checkexec(path):
> # check-exec is exec and check-no-exec is not exec
> return True
> # checknoexec exists but is exec - delete it
> - os.unlink(checknoexec)
> + unlink(checknoexec)
> # checkisexec exists but is not exec - delete it
> - os.unlink(checkisexec)
> + unlink(checkisexec)
>
> # check using one file, leave it as checkisexec
> checkdir = cachedir
> @@ -210,7 +210,7 @@ def checkexec(path):
> return True
> finally:
> if fn is not None:
> - os.unlink(fn)
> + unlink(fn)
> except (IOError, OSError):
> # we don't care, the user probably won't be able to commit anyway
> return False
> @@ -245,12 +245,12 @@ def checklink(path):
> try:
> os.symlink(target, name)
> if cachedir is None:
> - os.unlink(name)
> + unlink(name)
> else:
> try:
> os.rename(name, checklink)
> except OSError:
> - os.unlink(name)
> + unlink(name)
> return True
> except OSError as inst:
> # link creation might race, try again
> @@ -265,7 +265,7 @@ def checklink(path):
> except OSError as inst:
> # sshfs might report failure while successfully creating the link
> if inst[0] == errno.EIO and os.path.exists(name):
> - os.unlink(name)
> + unlink(name)
> return False
>
> def checkosfilename(path):
> @@ -536,7 +536,7 @@ def makedir(path, notindexed):
> def unlinkpath(f, ignoremissing=False):
> """unlink and remove the directory if it is empty"""
> try:
> - os.unlink(f)
> + unlink(f)
> except OSError as e:
> if not (ignoremissing and e.errno == errno.ENOENT):
> raise
More information about the Mercurial-devel
mailing list