[PATCH] util: eliminate wildcard imports

[Adrian Buehlmann]

On 2011-07-23 15:34, Adrian Buehlmann wrote:
> # HG changeset patch
> # User Adrian Buehlmann <adrian at cadifra.com>
> # Date 1311416992 -7200
> # Node ID f512f94029d77b9dd67455ae7c5668539f695edd
> # Parent  ebdfdba0faafc5728b2dd4084fed50d06a6fad60
> util: eliminate wildcard imports
> 
> diff --git a/mercurial/posix.py b/mercurial/posix.py
> --- a/mercurial/posix.py
> +++ b/mercurial/posix.py
> @@ -348,3 +348,6 @@
>      child process under Windows, unneeded on other systems.
>      """
>      pass
> +
> +def executablepath():
> +    return None # available on Windows only
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -20,9 +20,54 @@
>  import imp, socket, urllib
>  
>  if os.name == 'nt':
> -    from windows import *
> +    import windows as platform
>  else:
> -    from posix import *
> +    import posix as platform
> +
> +checkexec = platform.checkexec
> +checklink = platform.checklink
> +executablepath = platform.executablepath
> +expandglobs = platform.expandglobs
> +explainexit = platform.explainexit
> +findexe = platform.findexe
> +gethgcmd = platform.gethgcmd
> +getuser = platform.getuser
> +groupmembers = platform.groupmembers
> +groupname = platform.groupname
> +hidewindow = platform.hidewindow
> +isexec = platform.isexec
> +isowner = platform.isowner
> +localpath = platform.localpath
> +lookupreg = platform.lookupreg
> +makedir = platform.makedir
> +nlinks = platform.nlinks
> +normpath = platform.normpath
> +nulldev = platform.nulldev
> +openhardlinks = platform.openhardlinks
> +oslink = platform.oslink
> +parsepatchoutput = platform.parsepatchoutput
> +pconvert = platform.pconvert
> +popen = platform.popen
> +posixfile = platform.posixfile
> +quotecommand = platform.quotecommand
> +realpath = platform.realpath
> +rename = platform.rename
> +samedevice = platform.samedevice
> +samefile = platform.samefile
> +samestat = platform.samestat
> +setbinary = platform.setbinary
> +setflags = platform.setflags
> +setsignalhandler = platform.setsignalhandler
> +shellquote = platform.shellquote
> +spawndetached = platform.spawndetached
> +sshargs = platform.sshargs
> +statfiles = platform.statfiles
> +termwidth = platform.termwidth
> +testpid = platform.testpid
> +umask = platform.umask
> +unlink = platform.unlink
> +unlinkpath = platform.unlinkpath
> +username = platform.username
>  
>  # Python compatibility
>  
> @@ -482,6 +527,8 @@
>  
>  if os.name == 'nt':
>      checkosfilename = checkwinfilename
> +else:
> +    checkosfilename = platform.checkosfilename
>  
>  def makelock(info, pathname):
>      try:
> diff --git a/mercurial/windows.py b/mercurial/windows.py
> --- a/mercurial/windows.py
> +++ b/mercurial/windows.py
> @@ -281,6 +281,9 @@
>      # Don't support groups on Windows for now
>      raise KeyError()
>  
> +def isexec(f):
> +    return False
> +
>  from win32 import *
>  
>  expandglobs = True
> diff --git a/tests/test-check-pyflakes.t b/tests/test-check-pyflakes.t
> --- a/tests/test-check-pyflakes.t
> +++ b/tests/test-check-pyflakes.t
> @@ -7,8 +7,6 @@
>    mercurial/commands.py:*: 'mpatch' imported but unused (glob)
>    mercurial/commands.py:*: 'osutil' imported but unused (glob)
>    hgext/inotify/linux/__init__.py:*: 'from _inotify import *' used; unable to detect undefined names (glob)
> -  mercurial/util.py:*: 'from posix import *' used; unable to detect undefined names (glob)
>    mercurial/windows.py:*: 'from win32 import *' used; unable to detect undefined names (glob)
> -  mercurial/util.py:*: 'from windows import *' used; unable to detect undefined names (glob)

As you can see, doing this exposed two more problems:

(1)
isexec was only defined in posix.py, but not in windows.py. It worked
like this before this patch, because the code happenes to not call
isexec if we are running on windows. But it makes sense to require that
there are implementations for this function in both posix.py and
windows.py. Applying this patch ensures that

(2)
executablepath was only defined in win32.py, but not in posix.py. It
worked before, because executablepath is only called in code parts that
are only called when running on windows. This patch here exposes the
fact that this function is missing in posix.py. Assuming we would apply
this patch here, I could look into finding a better way with this
executablepath.

It was Mads who first proposed this import pattern here and I was
skeptical about it myself first. But I tried it anyway to see how it
would look like in detail and decided to send this patch for review.

Looking at all the arguments I think I tend to agree with Mads. The list
is a bit tedious but it's safer (with regards to coding bugs) and more
self contained.