From 02673352b5db6ca4d3dc804965facbedfe66425d Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Fri, 28 Feb 2020 17:28:37 -0800 Subject: [PATCH] bpo-39769: Fix compileall ddir for subpkgs. (GH-18676) Fix compileall.compile_dir() ddir= behavior on sub-packages. Fixes compileall.compile_dir's ddir parameter and compileall command line flag `-d` to no longer write the wrong pathname to the generated pyc file for submodules beneath the root of the directory tree being compiled. This fixes a regression introduced with Python 3.5. Also marks the _new_ in 3.9 from PR #16012 parameters to compile_dir as keyword only (as that is the only way they will be used) and fixes an omission of them in one place from the docs. --- Doc/library/compileall.rst | 4 +- Lib/compileall.py | 11 ++++- Lib/test/test_compileall.py | 41 +++++++++++++++++++ Lib/test/test_importlib/util.py | 11 +++++ .../2020-02-27-00-40-21.bpo-39769.hJmxu4.rst | 4 ++ 5 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2020-02-27-00-40-21.bpo-39769.hJmxu4.rst diff --git a/Doc/library/compileall.rst b/Doc/library/compileall.rst index 394d60634f1..b1ae9d60e8a 100644 --- a/Doc/library/compileall.rst +++ b/Doc/library/compileall.rst @@ -143,7 +143,7 @@ runtime. Public functions ---------------- -.. function:: compile_dir(dir, maxlevels=sys.getrecursionlimit(), ddir=None, force=False, rx=None, quiet=0, legacy=False, optimize=-1, workers=1, invalidation_mode=None, stripdir=None, prependdir=None, limit_sl_dest=None) +.. function:: compile_dir(dir, maxlevels=sys.getrecursionlimit(), ddir=None, force=False, rx=None, quiet=0, legacy=False, optimize=-1, workers=1, invalidation_mode=None, \*, stripdir=None, prependdir=None, limit_sl_dest=None) Recursively descend the directory tree named by *dir*, compiling all :file:`.py` files along the way. Return a true value if all the files compiled successfully, @@ -221,7 +221,7 @@ Public functions .. versionchanged:: 3.9 Added *stripdir*, *prependdir* and *limit_sl_dest* arguments. -.. function:: compile_file(fullname, ddir=None, force=False, rx=None, quiet=0, legacy=False, optimize=-1, invalidation_mode=None) +.. function:: compile_file(fullname, ddir=None, force=False, rx=None, quiet=0, legacy=False, optimize=-1, invalidation_mode=None, \*, stripdir=None, prependdir=None, limit_sl_dest=None) Compile the file with path *fullname*. Return a true value if the file compiled successfully, and a false value otherwise. diff --git a/Lib/compileall.py b/Lib/compileall.py index 8cfde5b7b3e..1831ad749f2 100644 --- a/Lib/compileall.py +++ b/Lib/compileall.py @@ -46,7 +46,7 @@ def _walk_dir(dir, maxlevels, quiet=0): def compile_dir(dir, maxlevels=None, ddir=None, force=False, rx=None, quiet=0, legacy=False, optimize=-1, workers=1, - invalidation_mode=None, stripdir=None, + invalidation_mode=None, *, stripdir=None, prependdir=None, limit_sl_dest=None): """Byte-compile all modules in the given directory tree. @@ -72,6 +72,13 @@ def compile_dir(dir, maxlevels=None, ddir=None, force=False, the defined path """ ProcessPoolExecutor = None + if ddir is not None and (stripdir is not None or prependdir is not None): + raise ValueError(("Destination dir (ddir) cannot be used " + "in combination with stripdir or prependdir")) + if ddir is not None: + stripdir = dir + prependdir = ddir + ddir = None if workers < 0: raise ValueError('workers must be greater or equal to 0') if workers != 1: @@ -111,7 +118,7 @@ def compile_dir(dir, maxlevels=None, ddir=None, force=False, def compile_file(fullname, ddir=None, force=False, rx=None, quiet=0, legacy=False, optimize=-1, - invalidation_mode=None, stripdir=None, prependdir=None, + invalidation_mode=None, *, stripdir=None, prependdir=None, limit_sl_dest=None): """Byte-compile one file. diff --git a/Lib/test/test_compileall.py b/Lib/test/test_compileall.py index 2ebcb424095..cb59fd71b38 100644 --- a/Lib/test/test_compileall.py +++ b/Lib/test/test_compileall.py @@ -212,6 +212,47 @@ class CompileallTestsBase: compileall.compile_dir(self.directory, quiet=True, maxlevels=depth) self.assertTrue(os.path.isfile(pyc_filename)) + def _test_ddir_only(self, *, ddir, parallel=True): + """Recursive compile_dir ddir must contain package paths; bpo39769.""" + fullpath = ["test", "foo"] + path = self.directory + mods = [] + for subdir in fullpath: + path = os.path.join(path, subdir) + os.mkdir(path) + script_helper.make_script(path, "__init__", "") + mods.append(script_helper.make_script(path, "mod", + "def fn(): 1/0\nfn()\n")) + compileall.compile_dir( + self.directory, quiet=True, ddir=ddir, + workers=2 if parallel else 1) + self.assertTrue(mods) + for mod in mods: + self.assertTrue(mod.startswith(self.directory), mod) + modcode = importlib.util.cache_from_source(mod) + modpath = mod[len(self.directory+os.sep):] + _, _, err = script_helper.assert_python_failure(modcode) + expected_in = os.path.join(ddir, modpath) + mod_code_obj = test.test_importlib.util.get_code_from_pyc(modcode) + self.assertEqual(mod_code_obj.co_filename, expected_in) + self.assertIn(f'"{expected_in}"', os.fsdecode(err)) + + def test_ddir_only_one_worker(self): + """Recursive compile_dir ddir= contains package paths; bpo39769.""" + return self._test_ddir_only(ddir="", parallel=False) + + def test_ddir_multiple_workers(self): + """Recursive compile_dir ddir= contains package paths; bpo39769.""" + return self._test_ddir_only(ddir="", parallel=True) + + def test_ddir_empty_only_one_worker(self): + """Recursive compile_dir ddir='' contains package paths; bpo39769.""" + return self._test_ddir_only(ddir="", parallel=False) + + def test_ddir_empty_multiple_workers(self): + """Recursive compile_dir ddir='' contains package paths; bpo39769.""" + return self._test_ddir_only(ddir="", parallel=True) + def test_strip_only(self): fullpath = ["test", "build", "real", "path"] path = os.path.join(self.directory, *fullpath) diff --git a/Lib/test/test_importlib/util.py b/Lib/test/test_importlib/util.py index 5aaf277e1f3..de6e0b02560 100644 --- a/Lib/test/test_importlib/util.py +++ b/Lib/test/test_importlib/util.py @@ -7,6 +7,7 @@ import importlib from importlib import machinery, util, invalidate_caches from importlib.abc import ResourceReader import io +import marshal import os import os.path from pathlib import Path, PurePath @@ -118,6 +119,16 @@ def submodule(parent, name, pkg_dir, content=''): return '{}.{}'.format(parent, name), path +def get_code_from_pyc(pyc_path): + """Reads a pyc file and returns the unmarshalled code object within. + + No header validation is performed. + """ + with open(pyc_path, 'rb') as pyc_f: + pyc_f.seek(16) + return marshal.load(pyc_f) + + @contextlib.contextmanager def uncache(*names): """Uncache a module from sys.modules. diff --git a/Misc/NEWS.d/next/Library/2020-02-27-00-40-21.bpo-39769.hJmxu4.rst b/Misc/NEWS.d/next/Library/2020-02-27-00-40-21.bpo-39769.hJmxu4.rst new file mode 100644 index 00000000000..9b564bd10d3 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-02-27-00-40-21.bpo-39769.hJmxu4.rst @@ -0,0 +1,4 @@ +The :func:`compileall.compile_dir` function's *ddir* parameter and the +compileall command line flag `-d` no longer write the wrong pathname to the +generated pyc file for submodules beneath the root of the directory tree +being compiled. This fixes a regression introduced with Python 3.5.