Patchwork [06,of,12] osutil: proxy through util (and platform) modules (API)

# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1493213188 -32400
#      Wed Apr 26 22:26:28 2017 +0900
# Node ID 04d511906826eaa7f1c51c9b250dc6cbc290ebeb
# Parent  3c2b0b5bf7a694cc39e1ff320fb764e4f755ed8b
osutil: proxy through util (and platform) modules (API)

See the previous commit for why. Marked as API change since osutil.listdir()
seems widely used in third-party extensions.

The win32mbcs extension is updated to wrap both util. and windows. aliases.

On Sat, May 6, 2017 at 6:45 PM, Yuya Nishihara <yuya@tcha.org> wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1493213188 -32400
> #      Wed Apr 26 22:26:28 2017 +0900
> # Node ID 04d511906826eaa7f1c51c9b250dc6cbc290ebeb
> # Parent  3c2b0b5bf7a694cc39e1ff320fb764e4f755ed8b
> osutil: proxy through util (and platform) modules (API)
>
> See the previous commit for why. Marked as API change since osutil.listdir()
> seems widely used in third-party extensions.
>
> The win32mbcs extension is updated to wrap both util. and windows. aliases.
>
> diff --git a/hgext/win32mbcs.py b/hgext/win32mbcs.py
> --- a/hgext/win32mbcs.py
> +++ b/hgext/win32mbcs.py
> @@ -182,7 +182,8 @@ def extsetup(ui):
>          if pycompat.osname == 'nt':
>              for f in winfuncs.split():
>                  wrapname(f, wrapper)
> -        wrapname("mercurial.osutil.listdir", wrapperforlistdir)
> +        wrapname("mercurial.util.listdir", wrapperforlistdir)
> +        wrapname("mercurial.windows.listdir", wrapperforlistdir)
>          # wrap functions to be called with local byte string arguments
>          for f in rfuncs.split():
>              wrapname(f, reversewrapper)
> diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
> --- a/mercurial/chgserver.py
> +++ b/mercurial/chgserver.py
> @@ -54,7 +54,6 @@ from . import (
>      encoding,
>      error,
>      extensions,
> -    osutil,
>      pycompat,
>      util,
>  )
> @@ -313,7 +312,7 @@ class chgcmdserver(commandserver.server)
>          # tell client to sendmsg() with 1-byte payload, which makes it
>          # distinctive from "attachio\n" command consumed by client.read()
>          self.clientsock.sendall(struct.pack('>cI', 'I', 1))
> -        clientfds = osutil.recvfds(self.clientsock.fileno())
> +        clientfds = util.recvfds(self.clientsock.fileno())
>          _log('received fds: %r\n' % clientfds)
>
>          ui = self.ui
> @@ -458,12 +457,12 @@ class chgcmdserver(commandserver.server)
>                           'setenv': setenv,
>                           'setumask': setumask})
>
> -    if util.safehasattr(osutil, 'setprocname'):
> +    if util.safehasattr(util, 'setprocname'):
>          def setprocname(self):
>              """Change process title"""
>              name = self._readstr()
>              _log('setprocname: %r\n' % name)
> -            osutil.setprocname(name)
> +            util.setprocname(name)
>          capabilities['setprocname'] = setprocname
>
>  def _tempaddress(address):
> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
> --- a/mercurial/dirstate.py
> +++ b/mercurial/dirstate.py
> @@ -18,7 +18,6 @@ from . import (
>      encoding,
>      error,
>      match as matchmod,
> -    osutil,
>      parsers,
>      pathutil,
>      pycompat,
> @@ -988,7 +987,7 @@ class dirstate(object):
>          matchalways = match.always()
>          matchtdir = match.traversedir
>          dmap = self._map
> -        listdir = osutil.listdir
> +        listdir = util.listdir
>          lstat = os.lstat
>          dirkind = stat.S_IFDIR
>          regkind = stat.S_IFREG
> diff --git a/mercurial/rcutil.py b/mercurial/rcutil.py
> --- a/mercurial/rcutil.py
> +++ b/mercurial/rcutil.py
> @@ -11,7 +11,6 @@ import os
>
>  from . import (
>      encoding,
> -    osutil,
>      pycompat,
>      util,
>  )
> @@ -30,7 +29,7 @@ def _expandrcpath(path):
>      p = util.expandpath(path)
>      if os.path.isdir(p):
>          join = os.path.join
> -        return [join(p, f) for f, k in osutil.listdir(p) if f.endswith('.rc')]
> +        return [join(p, f) for f, k in util.listdir(p) if f.endswith('.rc')]
>      return [p]
>
>  def envrcitems(env=None):
> diff --git a/mercurial/scmposix.py b/mercurial/scmposix.py
> --- a/mercurial/scmposix.py
> +++ b/mercurial/scmposix.py
> @@ -8,8 +8,8 @@ import sys
>
>  from . import (
>      encoding,
> -    osutil,
>      pycompat,
> +    util,
>  )
>
>  # BSD 'more' escapes ANSI color sequences by default. This can be disabled by
> @@ -23,7 +23,7 @@ def _rcfiles(path):
>      rcdir = os.path.join(path, 'hgrc.d')
>      try:
>          rcs.extend([os.path.join(rcdir, f)
> -                    for f, kind in osutil.listdir(rcdir)
> +                    for f, kind in util.listdir(rcdir)
>                      if f.endswith(".rc")])
>      except OSError:
>          pass
> diff --git a/mercurial/scmwindows.py b/mercurial/scmwindows.py
> --- a/mercurial/scmwindows.py
> +++ b/mercurial/scmwindows.py
> @@ -4,7 +4,6 @@ import os
>
>  from . import (
>      encoding,
> -    osutil,
>      pycompat,
>      util,
>      win32,
> @@ -29,7 +28,7 @@ def systemrcpath():
>      # Use hgrc.d found in directory with hg.exe
>      progrcd = os.path.join(os.path.dirname(filename), 'hgrc.d')
>      if os.path.isdir(progrcd):
> -        for f, kind in osutil.listdir(progrcd):
> +        for f, kind in util.listdir(progrcd):
>              if f.endswith('.rc'):
>                  rcpath.append(os.path.join(progrcd, f))
>      # else look for a system rcpath in the registry
> @@ -42,7 +41,7 @@ def systemrcpath():
>          if p.lower().endswith('mercurial.ini'):
>              rcpath.append(p)
>          elif os.path.isdir(p):
> -            for f, kind in osutil.listdir(p):
> +            for f, kind in util.listdir(p):
>                  if f.endswith('.rc'):
>                      rcpath.append(os.path.join(p, f))
>      return rcpath
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -106,6 +106,7 @@ groupname = platform.groupname
>  hidewindow = platform.hidewindow
>  isexec = platform.isexec
>  isowner = platform.isowner
> +listdir = osutil.listdir
>  localpath = platform.localpath
>  lookupreg = platform.lookupreg
>  makedir = platform.makedir
> @@ -143,6 +144,15 @@ umask = platform.umask
>  unlink = platform.unlink
>  username = platform.username
>
> +try:
> +    recvfds = osutil.recvfds
> +except AttributeError:
> +    pass
> +try:
> +    setprocname = osutil.setprocname
> +except AttributeError:
> +    pass
> +

What happens with the users of util.{recvfds,setprocname} in the
AttributeError case? Also, when would that happen? Doesn't the C
version also export those methods?

>  # Python compatibility
>
>  _notset = object()
> @@ -1165,7 +1175,7 @@ def copyfiles(src, dst, hardlink=None, p
>                          os.stat(os.path.dirname(dst)).st_dev)
>          topic = gettopic()
>          os.mkdir(dst)
> -        for name, kind in osutil.listdir(src):
> +        for name, kind in listdir(src):
>              srcname = os.path.join(src, name)
>              dstname = os.path.join(dst, name)
>              def nprog(t, pos):
> diff --git a/mercurial/vfs.py b/mercurial/vfs.py
> --- a/mercurial/vfs.py
> +++ b/mercurial/vfs.py
> @@ -17,7 +17,6 @@ import threading
>  from .i18n import _
>  from . import (
>      error,
> -    osutil,
>      pathutil,
>      pycompat,
>      util,
> @@ -163,7 +162,7 @@ class abstractvfs(object):
>              return fd, fname
>
>      def readdir(self, path=None, stat=None, skip=None):
> -        return osutil.listdir(self.join(path), stat, skip)
> +        return util.listdir(self.join(path), stat, skip)
>
>      def readlock(self, path):
>          return util.readlock(self.join(path))
> diff --git a/mercurial/windows.py b/mercurial/windows.py
> --- a/mercurial/windows.py
> +++ b/mercurial/windows.py
> @@ -136,6 +136,9 @@ def posixfile(name, mode='r', buffering=
>          # convert to a friendlier exception
>          raise IOError(err.errno, '%s: %s' % (name, err.strerror))
>
> +# may be wrapped by win32mbcs extension
> +listdir = osutil.listdir
> +
>  class winstdout(object):
>      '''stdout on windows misbehaves if sent through a pipe'''
>
> @@ -349,7 +352,7 @@ def statfiles(files):
>          if cache is None:
>              try:
>                  dmap = dict([(normcase(n), s)
> -                             for n, k, s in osutil.listdir(dir, True)
> +                             for n, k, s in listdir(dir, True)
>                               if getkind(s.st_mode) in _wantedkinds])
>              except OSError as err:
>                  # Python >= 2.5 returns ENOENT and adds winerror field
> @@ -376,7 +379,7 @@ def groupname(gid=None):
>  def removedirs(name):
>      """special version of os.removedirs that does not remove symlinked
>      directories or junction points if they actually contain files"""
> -    if osutil.listdir(name):
> +    if listdir(name):
>          return
>      os.rmdir(name)
>      head, tail = os.path.split(name)
> @@ -384,7 +387,7 @@ def removedirs(name):
>          head, tail = os.path.split(head)
>      while head and tail:
>          try:
> -            if osutil.listdir(head):
> +            if listdir(head):
>                  return
>              os.rmdir(head)
>          except (ValueError, OSError):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

On Fri, May 12, 2017 at 09:40:08AM -0700, Martin von Zweigbergk via Mercurial-devel wrote:
> On Sat, May 6, 2017 at 6:45 PM, Yuya Nishihara <yuya@tcha.org> wrote:

[...]

> > diff --git a/mercurial/util.py b/mercurial/util.py
> > --- a/mercurial/util.py
> > +++ b/mercurial/util.py
> > @@ -106,6 +106,7 @@ groupname = platform.groupname
> >  hidewindow = platform.hidewindow
> >  isexec = platform.isexec
> >  isowner = platform.isowner
> > +listdir = osutil.listdir
> >  localpath = platform.localpath
> >  lookupreg = platform.lookupreg
> >  makedir = platform.makedir
> > @@ -143,6 +144,15 @@ umask = platform.umask
> >  unlink = platform.unlink
> >  username = platform.username
> >
> > +try:
> > +    recvfds = osutil.recvfds
> > +except AttributeError:
> > +    pass
> > +try:
> > +    setprocname = osutil.setprocname
> > +except AttributeError:
> > +    pass
> > +
>
> What happens with the users of util.{recvfds,setprocname} in the
> AttributeError case? Also, when would that happen? Doesn't the C
> version also export those methods?

Both of those methods are only used by chgserver.py. The former
appears to be used unconditionally (but I think that's unavoidable?)
adn the latter is only used if it exists (it's guarded by
util.safehasattr).

I wouldn't worry about it in this case since chgserver is only useful
in cases where a compiler exists to compile chg.

On Fri, 12 May 2017 22:03:59 -0400, Augie Fackler wrote:
> On Fri, May 12, 2017 at 09:40:08AM -0700, Martin von Zweigbergk via Mercurial-devel wrote:
> > On Sat, May 6, 2017 at 6:45 PM, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> [...]
> 
> > > diff --git a/mercurial/util.py b/mercurial/util.py
> > > --- a/mercurial/util.py
> > > +++ b/mercurial/util.py
> > > @@ -106,6 +106,7 @@ groupname = platform.groupname
> > >  hidewindow = platform.hidewindow
> > >  isexec = platform.isexec
> > >  isowner = platform.isowner
> > > +listdir = osutil.listdir
> > >  localpath = platform.localpath
> > >  lookupreg = platform.lookupreg
> > >  makedir = platform.makedir
> > > @@ -143,6 +144,15 @@ umask = platform.umask
> > >  unlink = platform.unlink
> > >  username = platform.username
> > >
> > > +try:
> > > +    recvfds = osutil.recvfds
> > > +except AttributeError:
> > > +    pass
> > > +try:
> > > +    setprocname = osutil.setprocname
> > > +except AttributeError:
> > > +    pass
> > > +
> >
> > What happens with the users of util.{recvfds,setprocname} in the
> > AttributeError case? Also, when would that happen? Doesn't the C
> > version also export those methods?
> 
> Both of those methods are only used by chgserver.py. The former
> appears to be used unconditionally (but I think that's unavoidable?)
> adn the latter is only used if it exists (it's guarded by
> util.safehasattr).
> 
> I wouldn't worry about it in this case since chgserver is only useful
> in cases where a compiler exists to compile chg.

Yeah, they are for chgserver.py and unavailable on Windows (and some Unices.)
I could wrap them by e.g. maybesetprocname() but I didn't do that in this
series.

On Sat, May 13, 2017 at 2:41 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Fri, 12 May 2017 22:03:59 -0400, Augie Fackler wrote:
>> On Fri, May 12, 2017 at 09:40:08AM -0700, Martin von Zweigbergk via Mercurial-devel wrote:
>> > On Sat, May 6, 2017 at 6:45 PM, Yuya Nishihara <yuya@tcha.org> wrote:
>>
>> [...]
>>
>> > > diff --git a/mercurial/util.py b/mercurial/util.py
>> > > --- a/mercurial/util.py
>> > > +++ b/mercurial/util.py
>> > > @@ -106,6 +106,7 @@ groupname = platform.groupname
>> > >  hidewindow = platform.hidewindow
>> > >  isexec = platform.isexec
>> > >  isowner = platform.isowner
>> > > +listdir = osutil.listdir
>> > >  localpath = platform.localpath
>> > >  lookupreg = platform.lookupreg
>> > >  makedir = platform.makedir
>> > > @@ -143,6 +144,15 @@ umask = platform.umask
>> > >  unlink = platform.unlink
>> > >  username = platform.username
>> > >
>> > > +try:
>> > > +    recvfds = osutil.recvfds
>> > > +except AttributeError:
>> > > +    pass
>> > > +try:
>> > > +    setprocname = osutil.setprocname
>> > > +except AttributeError:
>> > > +    pass
>> > > +
>> >
>> > What happens with the users of util.{recvfds,setprocname} in the
>> > AttributeError case? Also, when would that happen? Doesn't the C
>> > version also export those methods?
>>
>> Both of those methods are only used by chgserver.py. The former
>> appears to be used unconditionally (but I think that's unavoidable?)
>> adn the latter is only used if it exists (it's guarded by
>> util.safehasattr).
>>
>> I wouldn't worry about it in this case since chgserver is only useful
>> in cases where a compiler exists to compile chg.
>
> Yeah, they are for chgserver.py and unavailable on Windows (and some Unices.)

Aha, that's what I had been missing (i.e. that even the pure version
is unavailable on Windows).

> I could wrap them by e.g. maybesetprocname() but I didn't do that in this
> series.