Merged revisions 69481 via svnmerge from

svn+ssh://pythondev@svn.python.org/python/trunk

........
  r69481 | brett.cannon | 2009-02-09 18:07:38 -0800 (Mon, 09 Feb 2009) | 4 lines

  compileall used the ctime of bytecode and source to determine if the bytecode
  should be recreated. This created a timing hole. Fixed by just doing what
  import does; check the mtime and magic number.
........

4 files changed, 84 insertions, 8 deletions

diff --git a/Lib/compileall.py b/Lib/compileall.py
index 541eecf..8c678e4 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 0000000..49b6448
--- /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 2af19b5..31f920e 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 515785c..aebbcde 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.