diff --git a/Lib/compileall.py b/Lib/compileall.py index 541eecf9307..8c678e43bef 100644 --- a/Lib/compileall.py +++ b/Lib/compileall.py @@ -11,10 +11,11 @@ packages -- for now, you'll have to deal with packages separately.) See module py_compile for details of the actual byte-compilation. """ - import os import sys import py_compile +import struct +import imp __all__ = ["compile_dir","compile_path"] @@ -54,11 +55,17 @@ def compile_dir(dir, maxlevels=10, ddir=None, if os.path.isfile(fullname): head, tail = name[:-3], name[-3:] if tail == '.py': - cfile = fullname + (__debug__ and 'c' or 'o') - ftime = os.stat(fullname).st_mtime - try: ctime = os.stat(cfile).st_mtime - except os.error: ctime = 0 - if (ctime > ftime) and not force: continue + if not force: + try: + mtime = os.stat(fullname).st_mtime + expect = struct.pack('<4sl', imp.get_magic(), mtime) + cfile = fullname + (__debug__ and 'c' or 'o') + with open(cfile, 'rb') as chandle: + actual = chandle.read(8) + if expect == actual: + continue + except IOError: + pass if not quiet: print('Compiling', fullname, '...') try: @@ -86,7 +93,8 @@ def compile_dir(dir, maxlevels=10, ddir=None, name != os.curdir and name != os.pardir and \ os.path.isdir(fullname) and \ not os.path.islink(fullname): - if not compile_dir(fullname, maxlevels - 1, dfile, force, rx, quiet): + if not compile_dir(fullname, maxlevels - 1, dfile, force, rx, + quiet): success = 0 return success diff --git a/Lib/test/test_compileall.py b/Lib/test/test_compileall.py new file mode 100644 index 00000000000..49b644876c1 --- /dev/null +++ b/Lib/test/test_compileall.py @@ -0,0 +1,63 @@ +import compileall +import imp +import os +import py_compile +import shutil +import struct +import sys +import tempfile +import time +from test import support +import unittest + + +class CompileallTests(unittest.TestCase): + + def setUp(self): + self.directory = tempfile.mkdtemp() + self.source_path = os.path.join(self.directory, '_test.py') + self.bc_path = self.source_path + ('c' if __debug__ else 'o') + with open(self.source_path, 'w') as file: + file.write('x = 123\n') + + def tearDown(self): + shutil.rmtree(self.directory) + + def data(self): + with open(self.bc_path, 'rb') as file: + data = file.read(8) + mtime = int(os.stat(self.source_path).st_mtime) + compare = struct.pack('<4sl', imp.get_magic(), mtime) + return data, compare + + def recreation_check(self, metadata): + """Check that compileall recreates bytecode when the new metadata is + used.""" + if not hasattr(os, 'stat'): + return + py_compile.compile(self.source_path) + self.assertEqual(*self.data()) + with open(self.bc_path, 'rb') as file: + bc = file.read()[len(metadata):] + with open(self.bc_path, 'wb') as file: + file.write(metadata) + file.write(bc) + self.assertNotEqual(*self.data()) + compileall.compile_dir(self.directory, force=False, quiet=True) + self.assertTrue(*self.data()) + + def test_mtime(self): + # Test a change in mtime leads to a new .pyc. + self.recreation_check(struct.pack('<4sl', imp.get_magic(), 1)) + + def test_magic_number(self): + # Test a change in mtime leads to a new .pyc. + self.recreation_check(b'\0\0\0\0') + + +def test_main(): + support.run_unittest(CompileallTests) + + +if __name__ == "__main__": + test_main() diff --git a/Misc/ACKS b/Misc/ACKS index 2af19b5baa0..31f920e54ea 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -183,7 +183,7 @@ Virgil Dupras Andy Dustman Gary Duzan Eugene Dvurechenski -Josip Dzolonga +Josip Dzolonga Maxim Dzumanenko Walter Dörwald Hans Eckardt @@ -234,6 +234,7 @@ Geoff Furnish Ulisses Furquim Hagen Fürstenau Achim Gaedke +Martin von Gagern Lele Gaifax Santiago Gala Yitzchak Gale diff --git a/Misc/NEWS b/Misc/NEWS index 515785c287e..aebbcde2cdb 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -163,6 +163,10 @@ Core and Builtins Library ------- +- Issue #5128: Make compileall properly inspect bytecode to determine if needs + to be recreated. This avoids a timing hole thanks to the old reliance on the + ctime of the files involved. + - Issue #5122: Synchronize tk load failure check to prevent a potential deadlock.