Clean up packaging.util: add __all__, remove some unused functions.

This huge module is the heir of six distutils modules, and contains
a number of miscellaneous functions.  I have attempted to help readers
of the source code with an annoted __all__.  Removed or deprecated
functions have been removed from the documentation; I’m working on
another patch to document the remaining public functions.

For the curious:

The unzip_file and untar_file were used by (or intended to be used by)
“pysetup install path/to/archive.tar.gz”, but the code presently used
shutil.unpack_archive and an helper function, so I just deleted them.
They’re still in the repository if we need them in the future.

The find_packages function is not used anymore but I want to discuss
module and package auto-discovery (in “pysetup create”) again before
removing it.

subst_vars now lives in sysconfig; rfc822_escape is inlined in
packaging.metadata.  Other functions are for internal use only, or
deprecated; I have left them out of __all__ and sprinkled TODO notes
for future cleanups.

Doc/library/packaging.util.rst

@@ -90,34 +90,6 @@ This module contains various helpers for the other modules.
    Search the path for a given executable name.
 
 
-.. function:: subst_vars(s, local_vars)
-
-   Perform shell/Perl-style variable substitution on *s*.  Every occurrence of
-   ``$`` followed by a name is considered a variable, and variable is
-   substituted by the value found in the *local_vars* dictionary, or in
-   ``os.environ`` if it's not in *local_vars*. *os.environ* is first
-   checked/augmented to guarantee that it contains certain values: see
-   :func:`check_environ`.  Raise :exc:`ValueError` for any variables not found
-   in either *local_vars* or ``os.environ``.
-
-   Note that this is not a fully-fledged string interpolation function. A valid
-   ``$variable`` can consist only of upper and lower case letters, numbers and
-   an underscore. No { } or ( ) style quoting is available.
-
-
-.. function:: split_quoted(s)
-
-   Split a string up according to Unix shell-like rules for quotes and
-   backslashes. In short: words are delimited by spaces, as long as those spaces
-   are not escaped by a backslash, or inside a quoted string. Single and double
-   quotes are equivalent, and the quote characters can be backslash-escaped.
-   The backslash is stripped from any two-character escape sequence, leaving
-   only the escaped character.  The quote characters are stripped from any
-   quoted string.  Returns a list of words.
-
-   .. TODO Should probably be moved into the standard library.
-
-
 .. function:: execute(func, args[, msg=None, verbose=0, dry_run=0])
 
    Perform some action that affects the outside world (for instance, writing to
@@ -175,12 +147,3 @@ This module contains various helpers for the other modules.
    figure out to use direct compilation or not (see the source for details).
    The *direct* flag is used by the script generated in indirect mode; unless
    you know what you're doing, leave it set to ``None``.
-
-
-.. function:: rfc822_escape(header)
-
-   Return a version of *header* escaped for inclusion in an :rfc:`822` header, by
-   ensuring there are 8 spaces space after each newline.  Note that it does no
-   other modification of the string.
-
-   .. TODO this _can_ be replaced

Lib/packaging/command/build_py.py

@@ -393,7 +393,7 @@ class build_py(Command, Mixin2to3):
                            self.get_command_name())
             return
 
-        from packaging.util import byte_compile
+        from packaging.util import byte_compile  # FIXME use compileall
         prefix = self.build_lib
         if prefix[-1] != os.sep:
             prefix = prefix + os.sep

Lib/packaging/command/install_lib.py

@@ -122,7 +122,7 @@ class install_lib(Command):
                            self.get_command_name())
             return
 
-        from packaging.util import byte_compile
+        from packaging.util import byte_compile  # FIXME use compileall
 
         # Get the "--root" directory supplied to the "install_dist" command,
         # and use it as a prefix to strip off the purported filename

Lib/packaging/command/register.py

@@ -2,7 +2,6 @@
 
 # Contributed by Richard Jones
 
-import io
 import getpass
 import urllib.error
 import urllib.parse

Lib/packaging/command/upload.py

@@ -5,7 +5,6 @@ import socket
 import logging
 import platform
 import urllib.parse
-from io import BytesIO
 from base64 import standard_b64encode
 from hashlib import md5
 from urllib.error import HTTPError

Lib/packaging/config.py

@@ -216,7 +216,7 @@ class Config:
             for data in files.get('package_data', []):
                 data = data.split('=')
                 if len(data) != 2:
-                    continue  # XXX error should never pass silently
+                    continue  # FIXME errors should never pass silently
                 key, value = data
                 self.dist.package_data[key.strip()] = value.strip()
 

Lib/packaging/create.py

@@ -36,7 +36,7 @@ from packaging._trove import all_classifiers as _CLASSIFIERS_LIST
 from packaging.version import is_valid_version
 
 _FILENAME = 'setup.cfg'
-_DEFAULT_CFG = '.pypkgcreate'
+_DEFAULT_CFG = '.pypkgcreate'  # FIXME use a section in user .pydistutils.cfg
 
 _helptext = {
     'name': '''
@@ -127,6 +127,10 @@ def ask_yn(question, default=None, helptext=None):
         print('\nERROR: You must select "Y" or "N".\n')
 
 
+# XXX use util.ask
+# FIXME: if prompt ends with '?', don't add ':'
+
+
 def ask(question, default=None, helptext=None, required=True,
         lengthy=False, multiline=False):
     prompt = '%s: ' % (question,)

Lib/packaging/tests/test_util.py

@@ -15,7 +15,7 @@ from packaging.errors import (
 from packaging import util
 from packaging.dist import Distribution
 from packaging.util import (
-    convert_path, change_root, split_quoted, strtobool, rfc822_escape,
+    convert_path, change_root, split_quoted, strtobool,
     get_compiler_versions, _MAC_OS_X_LD_VERSION, byte_compile, find_packages,
     spawn, get_pypirc_path, generate_pypirc, read_pypirc, resolve_name, iglob,
     RICH_GLOB, egginfo_to_distinfo, is_setuptools, is_distutils, is_packaging,
@@ -255,13 +255,6 @@ class UtilTestCase(support.EnvironRestorer,
         for n in no:
             self.assertFalse(strtobool(n))
 
-    def test_rfc822_escape(self):
-        header = 'I am a\npoor\nlonesome\nheader\n'
-        res = rfc822_escape(header)
-        wanted = ('I am a%(8s)spoor%(8s)slonesome%(8s)s'
-                  'header%(8s)s') % {'8s': '\n' + 8 * ' '}
-        self.assertEqual(res, wanted)
-
     def test_find_exe_version(self):
         # the ld version scheme under MAC OS is:
         #   ^@(#)PROGRAM:ld  PROJECT:ld64-VERSION

Lib/packaging/util.py

@@ -8,8 +8,6 @@ import errno
 import shutil
 import string
 import hashlib
-import tarfile
-import zipfile
 import posixpath
 import subprocess
 import sysconfig
@@ -23,6 +21,30 @@ from packaging.errors import (PackagingPlatformError, PackagingFileError,
                               PackagingByteCompileError, PackagingExecError,
                               InstallationException, PackagingInternalError)
 
+__all__ = [
+    # file dependencies
+    'newer', 'newer_group',
+    # helpers for commands (dry-run system)
+    'execute', 'write_file',
+    # spawning programs
+    'find_executable', 'spawn',
+    # path manipulation
+    'convert_path', 'change_root',
+    # 2to3 conversion
+    'Mixin2to3', 'run_2to3',
+    # packaging compatibility helpers
+    'cfg_to_args', 'generate_setup_py',
+    'egginfo_to_distinfo',
+    'get_install_method',
+    # misc
+    'ask', 'check_environ', 'encode_multipart', 'resolve_name',
+    # querying for information  TODO move to sysconfig
+    'get_compiler_versions', 'get_platform', 'set_platform',
+    # configuration  TODO move to packaging.config
+    'get_pypirc_path', 'read_pypirc', 'generate_pypirc',
+    'strtobool', 'split_multiline',
+]
+
 _PLATFORM = None
 _DEFAULT_INSTALLER = 'packaging'
 
@@ -152,31 +174,6 @@ def check_environ():
     _environ_checked = True
 
 
-def subst_vars(s, local_vars):
-    """Perform shell/Perl-style variable substitution on 'string'.
-
-    Every occurrence of '$' followed by a name is considered a variable, and
-    variable is substituted by the value found in the 'local_vars'
-    dictionary, or in 'os.environ' if it's not in 'local_vars'.
-    'os.environ' is first checked/augmented to guarantee that it contains
-    certain values: see 'check_environ()'.  Raise ValueError for any
-    variables not found in either 'local_vars' or 'os.environ'.
-    """
-    check_environ()
-
-    def _subst(match, local_vars=local_vars):
-        var_name = match.group(1)
-        if var_name in local_vars:
-            return str(local_vars[var_name])
-        else:
-            return os.environ[var_name]
-
-    try:
-        return re.sub(r'\$([a-zA-Z_][a-zA-Z_0-9]*)', _subst, s)
-    except KeyError as var:
-        raise ValueError("invalid variable '$%s'" % var)
-
-
 # Needed by 'split_quoted()'
 _wordchars_re = _squote_re = _dquote_re = None
 
@@ -188,6 +185,8 @@ def _init_regex():
     _dquote_re = re.compile(r'"(?:[^"\\]|\\.)*"')
 
 
+# TODO replace with shlex.split after testing
+
 def split_quoted(s):
     """Split a string up according to Unix shell-like rules for quotes and
     backslashes.
@@ -435,15 +434,6 @@ byte_compile(files, optimize=%r, force=%r,
                               file, cfile_base)
 
 
-def rfc822_escape(header):
-    """Return a form of *header* suitable for inclusion in an RFC 822-header.
-
-    This function ensures there are 8 spaces after each newline.
-    """
-    lines = header.split('\n')
-    sep = '\n' + 8 * ' '
-    return sep.join(lines)
-
 _RE_VERSION = re.compile('(\d+\.\d+(\.\d+)*)')
 _MAC_OS_X_LD_VERSION = re.compile('^@\(#\)PROGRAM:ld  '
                                   'PROJECT:ld64-((\d+)(\.\d+)*)')
@@ -543,6 +533,10 @@ def write_file(filename, contents):
     """Create *filename* and write *contents* to it.
 
     *contents* is a sequence of strings without line terminators.
+
+    This functions is not intended to replace the usual with open + write
+    idiom in all cases, only with Command.execute, which runs depending on
+    the dry_run argument and also logs its arguments).
     """
     with open(filename, "w") as f:
         for line in contents:
@@ -562,6 +556,7 @@ def _is_archive_file(name):
 
 
 def _under(path, root):
+    # XXX use os.path
     path = path.split(os.sep)
     root = root.split(os.sep)
     if len(root) > len(path):
@@ -664,103 +659,11 @@ def splitext(path):
     return base, ext
 
 
-def unzip_file(filename, location, flatten=True):
-    """Unzip the file *filename* into the *location* directory."""
-    if not os.path.exists(location):
-        os.makedirs(location)
-    with open(filename, 'rb') as zipfp:
-        zip = zipfile.ZipFile(zipfp)
-        leading = has_leading_dir(zip.namelist()) and flatten
-        for name in zip.namelist():
-            data = zip.read(name)
-            fn = name
-            if leading:
-                fn = split_leading_dir(name)[1]
-            fn = os.path.join(location, fn)
-            dir = os.path.dirname(fn)
-            if not os.path.exists(dir):
-                os.makedirs(dir)
-            if fn.endswith('/') or fn.endswith('\\'):
-                # A directory
-                if not os.path.exists(fn):
-                    os.makedirs(fn)
-            else:
-                with open(fn, 'wb') as fp:
-                    fp.write(data)
-
-
-def untar_file(filename, location):
-    """Untar the file *filename* into the *location* directory."""
-    if not os.path.exists(location):
-        os.makedirs(location)
-    if filename.lower().endswith('.gz') or filename.lower().endswith('.tgz'):
-        mode = 'r:gz'
-    elif (filename.lower().endswith('.bz2')
-          or filename.lower().endswith('.tbz')):
-        mode = 'r:bz2'
-    elif filename.lower().endswith('.tar'):
-        mode = 'r'
-    else:
-        mode = 'r:*'
-    with tarfile.open(filename, mode) as tar:
-        leading = has_leading_dir(member.name for member in tar.getmembers())
-        for member in tar.getmembers():
-            fn = member.name
-            if leading:
-                fn = split_leading_dir(fn)[1]
-            path = os.path.join(location, fn)
-            if member.isdir():
-                if not os.path.exists(path):
-                    os.makedirs(path)
-            else:
-                try:
-                    fp = tar.extractfile(member)
-                except (KeyError, AttributeError):
-                    # Some corrupt tar files seem to produce this
-                    # (specifically bad symlinks)
-                    continue
-                try:
-                    if not os.path.exists(os.path.dirname(path)):
-                        os.makedirs(os.path.dirname(path))
-                        with open(path, 'wb') as destfp:
-                            shutil.copyfileobj(fp, destfp)
-                finally:
-                    fp.close()
-
-
-def has_leading_dir(paths):
-    """Return true if all the paths have the same leading path name.
-
-    In other words, check that everything is in one subdirectory in an
-    archive.
-    """
-    common_prefix = None
-    for path in paths:
-        prefix, rest = split_leading_dir(path)
-        if not prefix:
-            return False
-        elif common_prefix is None:
-            common_prefix = prefix
-        elif prefix != common_prefix:
-            return False
-    return True
-
-
-def split_leading_dir(path):
-    path = str(path)
-    path = path.lstrip('/').lstrip('\\')
-    if '/' in path and (('\\' in path and path.find('/') < path.find('\\'))
-                        or '\\' not in path):
-        return path.split('/', 1)
-    elif '\\' in path:
-        return path.split('\\', 1)
-    else:
-        return path, ''
-
 if sys.platform == 'darwin':
     _cfg_target = None
     _cfg_target_split = None
 
+
 def spawn(cmd, search_path=True, verbose=0, dry_run=False, env=None):
     """Run another program specified as a command list 'cmd' in a new process.
 
@@ -1510,7 +1413,7 @@ def encode_multipart(fields, files, boundary=None):
     for key, values in fields:
         # handle multiple entries for the same name
         if not isinstance(values, (tuple, list)):
-            values=[values]
+            values = [values]
 
         for value in values:
             l.extend((