[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