diff --git a/Doc/library/py_compile.rst b/Doc/library/py_compile.rst index 0c8c99d3bfd..bae8450b7c9 100644 --- a/Doc/library/py_compile.rst +++ b/Doc/library/py_compile.rst @@ -41,6 +41,13 @@ byte-code cache files in the directory containing the source code. is raised. This function returns the path to byte-compiled file, i.e. whatever *cfile* value was used. + If the path that *cfile* becomes (either explicitly specified or computed) + is a symlink or non-regular file, :exc:`FileExistsError` will be raised. + This is to act as a warning that import will turn those paths into regular + files if it is allowed to write byte-compiled files to those paths. This is + a side-effect of import using file renaming to place the final byte-compiled + file into place to prevent concurrent file writing issues. + *optimize* controls the optimization level and is passed to the built-in :func:`compile` function. The default of ``-1`` selects the optimization level of the current interpreter. @@ -53,7 +60,9 @@ byte-code cache files in the directory containing the source code. .. versionchanged:: 3.4 Changed code to use :mod:`importlib` for the byte-code cache file writing. This means file creation/writing semantics now match what :mod:`importlib` - does, e.g. permissions, write-and-move semantics, etc. + does, e.g. permissions, write-and-move semantics, etc. Also added the + caveat that :exc:`FileExistsError` is raised if *cfile* is a symlink or + non-regular file. .. function:: main(args=None) diff --git a/Doc/whatsnew/3.4.rst b/Doc/whatsnew/3.4.rst index 1054c687b69..90b10e9b80b 100644 --- a/Doc/whatsnew/3.4.rst +++ b/Doc/whatsnew/3.4.rst @@ -272,3 +272,8 @@ that may require changes to your code. * :c:func:`PyErr_SetImportError` now sets :exc:`TypeError` when its **msg** argument is not set. Previously only ``NULL`` was returned. + +* :func:`py_compile.compile` now raises :exc:`FileExistsError` if the file path + it would write to is a symlink or a non-regular file. This is to act as a + warning that import will overwrite those files with a regular file regardless + of what type of file path they were originally. diff --git a/Lib/py_compile.py b/Lib/py_compile.py index 701e8acb9a6..cee35a5b6b1 100644 --- a/Lib/py_compile.py +++ b/Lib/py_compile.py @@ -7,6 +7,7 @@ import imp import importlib._bootstrap import importlib.machinery import os +import os.path import sys import traceback @@ -96,12 +97,25 @@ def compile(file, cfile=None, dfile=None, doraise=False, optimize=-1): See compileall.py for a script/module that uses this module to byte-compile all installed files (or all files in selected directories). + + Do note that FileExistsError is raised if cfile ends up pointing at a + non-regular file or symlink. Because the compilation uses a file renaming, + the resulting file would be regular and thus not the same type of file as + it was previously. """ if cfile is None: if optimize >= 0: cfile = imp.cache_from_source(file, debug_override=not optimize) else: cfile = imp.cache_from_source(file) + if os.path.islink(cfile): + msg = ('{} is a symlink and will be changed into a regular file if ' + 'import writes a byte-compiled file to it') + raise FileExistsError(msg.format(file, cfile)) + elif os.path.exists(cfile) and not os.path.isfile(cfile): + msg = ('{} is a non-regular file and will be changed into a regular ' + 'one if import writes a byte-compiled file to it') + raise FileExistsError(msg.format(file, cfile)) loader = importlib.machinery.SourceFileLoader('', file) source_bytes = loader.get_data(file) try: diff --git a/Lib/test/test_py_compile.py b/Lib/test/test_py_compile.py index e1795546e1f..54dc59660ad 100644 --- a/Lib/test/test_py_compile.py +++ b/Lib/test/test_py_compile.py @@ -36,6 +36,26 @@ class PyCompileTests(unittest.TestCase): self.assertTrue(os.path.exists(self.pyc_path)) self.assertFalse(os.path.exists(self.cache_path)) + def test_do_not_overwrite_symlinks(self): + # In the face of a cfile argument being a symlink, bail out. + # Issue #17222 + try: + os.symlink(self.pyc_path + '.actual', self.pyc_path) + except OSError: + self.skipTest('need to be able to create a symlink for a file') + else: + assert os.path.islink(self.pyc_path) + with self.assertRaises(FileExistsError): + py_compile.compile(self.source_path, self.pyc_path) + + @unittest.skipIf(not os.path.exists(os.devnull) or os.path.isfile(os.devnull), + 'requires os.devnull and for it to be a non-regular file') + def test_do_not_overwrite_nonregular_files(self): + # In the face of a cfile argument being a non-regular file, bail out. + # Issue #17222 + with self.assertRaises(FileExistsError): + py_compile.compile(self.source_path, os.devnull) + def test_cache_path(self): py_compile.compile(self.source_path) self.assertTrue(os.path.exists(self.cache_path)) diff --git a/Misc/NEWS b/Misc/NEWS index 1fcb9c9cf59..fb1fedf4c62 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -1005,6 +1005,9 @@ Library Python file. Patch by Ben Morgan. - Have py_compile use importlib as much as possible to avoid code duplication. + Code now raises FileExistsError if the file path to be used for the + byte-compiled file is a symlink or non-regular file as a warning that import + will not keep the file path type if it writes to that path. - Issue #180022: Have site.addpackage() consider already known paths even when none are explicitly passed in. Bug report and fix by Kirill.