https://hg.python.org/cpython/rev/71f071f2e074 changeset: 99695:71f071f2e074 user: Brett Cannon <brett@python.org> date: Sun Dec 27 13:17:04 2015 -0800 summary: Issue #25768: Make compileall functions return booleans and document the return values as well as test them. Thanks to Nicholas Chammas for the bug report and initial patch. files: Doc/library/compileall.rst | 12 +++++++--- Doc/whatsnew/3.6.rst | 5 ++++ Lib/compileall.py | 16 +++++++------- Lib/test/test_compileall.py | 26 +++++++++++++++++++++++- Misc/ACKS | 1 + Misc/NEWS | 3 ++ 6 files changed, 49 insertions(+), 14 deletions(-) diff --git a/Doc/library/compileall.rst b/Doc/library/compileall.rst --- a/Doc/library/compileall.rst +++ b/Doc/library/compileall.rst @@ -103,7 +103,8 @@ .. function:: compile_dir(dir, maxlevels=10, ddir=None, force=False, rx=None, quiet=0, legacy=False, optimize=-1, workers=1) Recursively descend the directory tree named by *dir*, compiling all :file:`.py` - files along the way. + files along the way. Return a true value if all the files compiled successfully, + and a false value otherwise. The *maxlevels* parameter is used to limit the depth of the recursion; it defaults to ``10``. @@ -155,7 +156,8 @@ .. function:: compile_file(fullname, ddir=None, force=False, rx=None, quiet=0, legacy=False, optimize=-1) - Compile the file with path *fullname*. + Compile the file with path *fullname*. Return a true value if the file + compiled successfully, and a false value otherwise. If *ddir* is given, it is prepended to the path to the file being compiled for use in compilation time tracebacks, and is also compiled in to the @@ -191,8 +193,10 @@ .. function:: compile_path(skip_curdir=True, maxlevels=0, force=False, quiet=0, legacy=False, optimize=-1) - Byte-compile all the :file:`.py` files found along ``sys.path``. If - *skip_curdir* is true (the default), the current directory is not included + Byte-compile all the :file:`.py` files found along ``sys.path``. Return a + true value if all the files compiled successfully, and a false value otherwise. + + If *skip_curdir* is true (the default), the current directory is not included in the search. All other parameters are passed to the :func:`compile_dir` function. Note that unlike the other compile functions, ``maxlevels`` defaults to ``0``. diff --git a/Doc/whatsnew/3.6.rst b/Doc/whatsnew/3.6.rst --- a/Doc/whatsnew/3.6.rst +++ b/Doc/whatsnew/3.6.rst @@ -230,6 +230,11 @@ Changes in the Python API ------------------------- +* The functions in the :mod:`compileall` module now return booleans instead + of ``1`` or ``0`` to represent success or failure, respectively. Thanks to + booleans being a subclass of integers, this should only be an issue if you + were doing identity checks for ``1`` or ``0``. See :issue:`25768`. + * Reading the :attr:`~urllib.parse.SplitResult.port` attribute of :func:`urllib.parse.urlsplit` and :func:`~urllib.parse.urlparse` results now raises :exc:`ValueError` for out-of-range values, rather than diff --git a/Lib/compileall.py b/Lib/compileall.py --- a/Lib/compileall.py +++ b/Lib/compileall.py @@ -68,7 +68,7 @@ """ files = _walk_dir(dir, quiet=quiet, maxlevels=maxlevels, ddir=ddir) - success = 1 + success = True if workers is not None and workers != 1 and ProcessPoolExecutor is not None: if workers < 0: raise ValueError('workers must be greater or equal to 0') @@ -81,12 +81,12 @@ legacy=legacy, optimize=optimize), files) - success = min(results, default=1) + success = min(results, default=True) else: for file in files: if not compile_file(file, ddir, force, rx, quiet, legacy, optimize): - success = 0 + success = False return success def compile_file(fullname, ddir=None, force=False, rx=None, quiet=0, @@ -104,7 +104,7 @@ legacy: if True, produce legacy pyc paths instead of PEP 3147 paths optimize: optimization level or -1 for level of the interpreter """ - success = 1 + success = True name = os.path.basename(fullname) if ddir is not None: dfile = os.path.join(ddir, name) @@ -144,7 +144,7 @@ ok = py_compile.compile(fullname, cfile, dfile, True, optimize=optimize) except py_compile.PyCompileError as err: - success = 0 + success = False if quiet >= 2: return success elif quiet: @@ -157,7 +157,7 @@ msg = msg.decode(sys.stdout.encoding) print(msg) except (SyntaxError, UnicodeError, OSError) as e: - success = 0 + success = False if quiet >= 2: return success elif quiet: @@ -167,7 +167,7 @@ print(e.__class__.__name__ + ':', e) else: if ok == 0: - success = 0 + success = False return success def compile_path(skip_curdir=1, maxlevels=0, force=False, quiet=0, @@ -183,7 +183,7 @@ legacy: as for compile_dir() (default False) optimize: as for compile_dir() (default -1) """ - success = 1 + success = True for dir in sys.path: if (not dir or dir == os.curdir) and skip_curdir: if quiet < 2: diff --git a/Lib/test/test_compileall.py b/Lib/test/test_compileall.py --- a/Lib/test/test_compileall.py +++ b/Lib/test/test_compileall.py @@ -1,6 +1,7 @@ import sys import compileall import importlib.util +import test.test_importlib.util import os import pathlib import py_compile @@ -40,6 +41,11 @@ def tearDown(self): shutil.rmtree(self.directory) + def add_bad_source_file(self): + self.bad_source_path = os.path.join(self.directory, '_test_bad.py') + with open(self.bad_source_path, 'w') as file: + file.write('x (\n') + def data(self): with open(self.bc_path, 'rb') as file: data = file.read(8) @@ -78,15 +84,31 @@ os.unlink(fn) except: pass - compileall.compile_file(self.source_path, force=False, quiet=True) + self.assertTrue(compileall.compile_file(self.source_path, + force=False, quiet=True)) self.assertTrue(os.path.isfile(self.bc_path) and not os.path.isfile(self.bc_path2)) os.unlink(self.bc_path) - compileall.compile_dir(self.directory, force=False, quiet=True) + self.assertTrue(compileall.compile_dir(self.directory, force=False, + quiet=True)) self.assertTrue(os.path.isfile(self.bc_path) and os.path.isfile(self.bc_path2)) os.unlink(self.bc_path) os.unlink(self.bc_path2) + # Test against bad files + self.add_bad_source_file() + self.assertFalse(compileall.compile_file(self.bad_source_path, + force=False, quiet=2)) + self.assertFalse(compileall.compile_dir(self.directory, + force=False, quiet=2)) + + def test_compile_path(self): + self.assertTrue(compileall.compile_path(quiet=2)) + + with test.test_importlib.util.import_state(path=[self.directory]): + self.add_bad_source_file() + self.assertFalse(compileall.compile_path(skip_curdir=False, + force=True, quiet=2)) def test_no_pycache_in_non_package(self): # Bug 8563 reported that __pycache__ directories got created by diff --git a/Misc/ACKS b/Misc/ACKS --- a/Misc/ACKS +++ b/Misc/ACKS @@ -235,6 +235,7 @@ Michael Cetrulo Dave Chambers Pascal Chambon +Nicholas Chammas John Chandler Hye-Shik Chang Jeffrey Chang diff --git a/Misc/NEWS b/Misc/NEWS --- a/Misc/NEWS +++ b/Misc/NEWS @@ -123,6 +123,9 @@ Library ------- +- Issue #25768: Have the functions in compileall return booleans instead of + ints and add proper documentation and tests for the return values. + - Issue #24103: Fixed possible use after free in ElementTree.XMLPullParser. - Issue #25860: os.fwalk() no longer skips remaining directories when error -- Repository URL: https://hg.python.org/cpython