Clean up byte-compilation code in packaging (#11254 followup).

- Don't use keyword arguments for debug_override; I find it more
  readable to have a comment explaining that True makes pyc and False
  pyo than to write out the non-obvious (when you haven’t read the doc)
  argument name

- Move duplicate code from build_py and install_lib into cmd

- Remove obsolete verbose argument of util.byte_compile

- Remove obsolete passing of -O/-OO to the Python process spawned by
  util.byte_compile (I’ll remove the whole spawning later, after I write
  more tests to check the contents of pyc and pyo files; now that
  byte_compile does not depend on the value of __debug__ in the calling
  Python, we can call py_compile or compileall directly)

5 files changed, 51 insertions, 73 deletions

diff --git a/Lib/packaging/command/build_py.py b/Lib/packaging/command/build_py.py
index 5faae39..0062140 100644
--- a/Lib/packaging/command/build_py.py
+++ b/Lib/packaging/command/build_py.py
@@ -18,7 +18,7 @@ class build_py(Command, Mixin2to3):
 
     description = "build pure Python modules (copy to build directory)"
 
-    # The options for controlling byte compilations are two independent sets;
+    # The options for controlling byte compilation are two independent sets;
     # more info in install_lib or the reST docs
 
     user_options = [
@@ -113,7 +113,8 @@ class build_py(Command, Mixin2to3):
             self.run_2to3(self._updated_files, self._doctests_2to3,
                                             self.use_2to3_fixers)
 
-        self.byte_compile(self.get_outputs(include_bytecode=False))
+        self.byte_compile(self.get_outputs(include_bytecode=False),
+                          prefix=self.build_lib)
 
     # -- Top-level worker functions ------------------------------------
 
@@ -335,11 +336,9 @@ class build_py(Command, Mixin2to3):
             outputs.append(filename)
             if include_bytecode:
                 if self.compile:
-                    outputs.append(imp.cache_from_source(filename,
-                                                         debug_override=True))
-                if self.optimize > 0:
-                    outputs.append(imp.cache_from_source(filename,
-                                                         debug_override=False))
+                    outputs.append(imp.cache_from_source(filename, True))
+                if self.optimize:
+                    outputs.append(imp.cache_from_source(filename, False))
 
         outputs += [
             os.path.join(build_dir, filename)
@@ -391,19 +390,3 @@ class build_py(Command, Mixin2to3):
             for package_, module, module_file in modules:
                 assert package == package_
                 self.build_module(module, module_file, package)
-
-    def byte_compile(self, files):
-        from packaging.util import byte_compile  # FIXME use compileall
-        prefix = self.build_lib
-        if prefix[-1] != os.sep:
-            prefix = prefix + os.sep
-
-        # XXX this code is essentially the same as the 'byte_compile()
-        # method of the "install_lib" command, except for the determination
-        # of the 'prefix' string.  Hmmm.
-        if self.compile:
-            byte_compile(files, optimize=0,
-                         force=self.force, prefix=prefix, dry_run=self.dry_run)
-        if self.optimize > 0:
-            byte_compile(files, optimize=self.optimize,
-                         force=self.force, prefix=prefix, dry_run=self.dry_run)
diff --git a/Lib/packaging/command/cmd.py b/Lib/packaging/command/cmd.py
index a88df02..333b99d 100644
--- a/Lib/packaging/command/cmd.py
+++ b/Lib/packaging/command/cmd.py
@@ -10,7 +10,7 @@ from packaging.errors import PackagingOptionError
 
 class Command:
     """Abstract base class for defining command classes, the "worker bees"
-    of the Packaging.  A useful analogy for command classes is to think of
+    of Packaging.  A useful analogy for command classes is to think of
     them as subroutines with local variables called "options".  The options
     are "declared" in 'initialize_options()' and "defined" (given their
     final values, aka "finalized") in 'finalize_options()', both of which
@@ -386,7 +386,6 @@ class Command:
         if self.dry_run:
             return  # see if we want to display something
 
-
         return util.copy_tree(infile, outfile, preserve_mode, preserve_times,
             preserve_symlinks, not self.force, dry_run=self.dry_run)
 
@@ -439,3 +438,20 @@ class Command:
         # Otherwise, print the "skip" message
         else:
             logger.debug(skip_msg)
+
+    def byte_compile(self, files, prefix=None):
+        """Byte-compile files to pyc and/or pyo files.
+
+        This method requires that the calling class define compile and
+        optimize options, like build_py and install_lib.  It also
+        automatically respects the force and dry-run options.
+
+        prefix, if given, is a string that will be stripped off the
+        filenames encoded in bytecode files.
+        """
+        if self.compile:
+            util.byte_compile(files, optimize=False, prefix=prefix,
+                              force=self.force, dry_run=self.dry_run)
+        if self.optimize:
+            util.byte_compile(files, optimize=self.optimize, prefix=prefix,
+                              force=self.force, dry_run=self.dry_run)
diff --git a/Lib/packaging/command/install_lib.py b/Lib/packaging/command/install_lib.py
index f6c785f..ffc5d45 100644
--- a/Lib/packaging/command/install_lib.py
+++ b/Lib/packaging/command/install_lib.py
@@ -2,7 +2,6 @@
 
 import os
 import imp
-import logging
 
 from packaging import logger
 from packaging.command.cmd import Command
@@ -21,7 +20,7 @@ class install_lib(Command):
 
     description = "install all modules (extensions and pure Python)"
 
-    # The options for controlling byte compilations are two independent sets:
+    # The options for controlling byte compilation are two independent sets:
     # 'compile' is strictly boolean, and only decides whether to
     # generate .pyc files.  'optimize' is three-way (0, 1, or 2), and
     # decides both whether to generate .pyo files and what level of
@@ -84,9 +83,14 @@ class install_lib(Command):
         # having a build directory!)
         outfiles = self.install()
 
-        # (Optionally) compile .py to .pyc
+        # (Optionally) compile .py to .pyc and/or .pyo
         if outfiles is not None and self.distribution.has_pure_modules():
-            self.byte_compile(outfiles)
+            # XXX comment from distutils: "This [prefix stripping] is far from
+            # complete, but it should at least generate usable bytecode in RPM
+            # distributions." -> need to find exact requirements for
+            # byte-compiled files and fix it
+            install_root = self.get_finalized_command('install_dist').root
+            self.byte_compile(outfiles, prefix=install_root)
 
     # -- Top-level worker functions ------------------------------------
     # (called from 'run()')
@@ -108,28 +112,6 @@ class install_lib(Command):
             return
         return outfiles
 
-    def byte_compile(self, files):
-        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
-        # encoded in bytecode files.  This is far from complete, but it
-        # should at least generate usable bytecode in RPM distributions.
-        install_root = self.get_finalized_command('install_dist').root
-
-        # Temporary kludge until we remove the verbose arguments and use
-        # logging everywhere
-        verbose = logger.getEffectiveLevel() >= logging.DEBUG
-
-        if self.compile:
-            byte_compile(files, optimize=0,
-                         force=self.force, prefix=install_root,
-                         verbose=verbose, dry_run=self.dry_run)
-        if self.optimize > 0:
-            byte_compile(files, optimize=self.optimize,
-                         force=self.force, prefix=install_root,
-                         verbose=verbose, dry_run=self.dry_run)
-
     # -- Utility methods -----------------------------------------------
 
     def _mutate_outputs(self, has_any, build_cmd, cmd_option, output_dir):
@@ -157,11 +139,9 @@ class install_lib(Command):
             if ext != PYTHON_SOURCE_EXTENSION:
                 continue
             if self.compile:
-                bytecode_files.append(imp.cache_from_source(
-                    py_file, debug_override=True))
-            if self.optimize > 0:
-                bytecode_files.append(imp.cache_from_source(
-                    py_file, debug_override=False))
+                bytecode_files.append(imp.cache_from_source(py_file, True))
+            if self.optimize:
+                bytecode_files.append(imp.cache_from_source(py_file, False))
 
         return bytecode_files
 
diff --git a/Lib/packaging/tests/test_command_install_lib.py b/Lib/packaging/tests/test_command_install_lib.py
index e58e812..79e8fa8 100644
--- a/Lib/packaging/tests/test_command_install_lib.py
+++ b/Lib/packaging/tests/test_command_install_lib.py
@@ -44,8 +44,8 @@ class InstallLibTestCase(support.TempdirManager,
         f = os.path.join(project_dir, 'foo.py')
         self.write_file(f, '# python file')
         cmd.byte_compile([f])
-        pyc_file = imp.cache_from_source('foo.py', debug_override=True)
-        pyo_file = imp.cache_from_source('foo.py', debug_override=False)
+        pyc_file = imp.cache_from_source('foo.py', True)
+        pyo_file = imp.cache_from_source('foo.py', False)
         self.assertTrue(os.path.exists(pyc_file))
         self.assertTrue(os.path.exists(pyo_file))
 
diff --git a/Lib/packaging/util.py b/Lib/packaging/util.py
index e76e3c3..e1d7ad0 100644
--- a/Lib/packaging/util.py
+++ b/Lib/packaging/util.py
@@ -296,7 +296,7 @@ def strtobool(val):
 
 
 def byte_compile(py_files, optimize=0, force=False, prefix=None,
-                 base_dir=None, verbose=0, dry_run=False, direct=None):
+                 base_dir=None, dry_run=False, direct=None):
     """Byte-compile a collection of Python source files to either .pyc
     or .pyo files in a __pycache__ subdirectory.
 
@@ -305,6 +305,9 @@ def byte_compile(py_files, optimize=0, force=False, prefix=None,
       0 - don't optimize (generate .pyc)
       1 - normal optimization (like "python -O")
       2 - extra optimization (like "python -OO")
+    This function is independent from the running Python's -O or -B options;
+    it is fully controlled by the parameters passed in.
+
     If 'force' is true, all files are recompiled regardless of
     timestamps.
 
@@ -325,10 +328,9 @@ def byte_compile(py_files, optimize=0, force=False, prefix=None,
     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.
-
-    This function is independent from the running Python's -O or -B options;
-    it is fully controlled by the parameters passed in.
     """
+    # FIXME use compileall + remove direct/indirect shenanigans
+
     # First, if the caller didn't force us into direct or indirect mode,
     # figure out which mode we should be in.  We take a conservative
     # approach: choose direct mode *only* if the current interpreter is
@@ -381,15 +383,11 @@ files = [
                 script.write("""
 byte_compile(files, optimize=%r, force=%r,
              prefix=%r, base_dir=%r,
-             verbose=%r, dry_run=False,
+             dry_run=False,
              direct=True)
-""" % (optimize, force, prefix, base_dir, verbose))
+""" % (optimize, force, prefix, base_dir))
 
         cmd = [sys.executable, script_name]
-        if optimize == 1:
-            cmd.insert(1, "-O")
-        elif optimize == 2:
-            cmd.insert(1, "-OO")
 
         env = os.environ.copy()
         env['PYTHONPATH'] = os.path.pathsep.join(sys.path)
@@ -415,8 +413,10 @@ byte_compile(files, optimize=%r, force=%r,
             # Terminology from the py_compile module:
             #   cfile - byte-compiled file
             #   dfile - purported source filename (same as 'file' by default)
-            debug_override = not optimize
-            cfile = imp.cache_from_source(file, debug_override)
+            # The second argument to cache_from_source forces the extension to
+            # be .pyc (if true) or .pyo (if false); without it, the extension
+            # would depend on the calling Python's -O option
+            cfile = imp.cache_from_source(file, not optimize)
             dfile = file
 
             if prefix:
@@ -1334,7 +1334,7 @@ def copy_tree(src, dst, preserve_mode=True, preserve_times=True,
               preserve_symlinks=False, update=False, verbose=True,
               dry_run=False):
     # FIXME use of this function is why we get spurious logging message on
-    # stdout when tests run; kill and replace by shuil!
+    # stdout when tests run; kill and replace by shutil!
     from distutils.file_util import copy_file
 
     if not dry_run and not os.path.isdir(src):
@@ -1448,8 +1448,7 @@ def encode_multipart(fields, files, boundary=None):
 
     Returns (content_type: bytes, body: bytes) ready for http.client.HTTP.
     """
-    # Taken from
-    # http://code.activestate.com/recipes/146306-http-client-to-post-using-multipartform-data/
+    # Taken from http://code.activestate.com/recipes/146306
 
     if boundary is None:
         boundary = b'--------------GHSKFJDLGDS7543FJKLFHRE75642756743254'