Issue #13645: pyc files now contain the size of the corresponding source

code, to avoid timestamp collisions (especially on filesystems with a low timestamp resolution) when checking for freshness of the bytecode.

Author: pitrou

Doc/library/importlib.rst

@@ -239,11 +239,30 @@ are also provided to help in implementing the core ABCs.
  optimization to speed up loading by removing the parsing step of Python's
  compiler, and so no bytecode-specific API is exposed.
 
+ .. method:: path_stats(self, path)
+
+ Optional abstract method which returns a :class:`dict` containing
+ metadata about the specifed path. Supported dictionary keys are:
+
+ - ``'mtime'`` (mandatory): an integer or floating-point number
+ representing the modification time of the source code;
+ - ``'size'`` (optional): the size in bytes of the source code.
+
+ Any other keys in the dictionary are ignored, to allow for future
+ extensions.
+
+ .. versionadded:: 3.3
+
  .. method:: path_mtime(self, path)
 
  Optional abstract method which returns the modification time for the
  specified path.
 
+ .. deprecated:: 3.3
+ This method is deprecated in favour of :meth:`path_stats`. You don't
+ have to implement it, but it is still available for compatibility
+ purposes.
+
  .. method:: set_data(self, path, data)
 
  Optional abstract method which writes the specified bytes to a file

Lib/importlib/_bootstrap.py

@@ -331,25 +331,40 @@ def is_package(self, fullname):
 filename=self.get_filename(fullname).rpartition(path_sep)[2]
 returnfilename.rsplit('.', 1)[0] =='__init__'
 
-def_bytes_from_bytecode(self, fullname, data, source_mtime):
+def_bytes_from_bytecode(self, fullname, data, source_stats):
 """Return the marshalled bytes from bytecode, verifying the magic
- numberand timestamp along the way.
+ number, timestamp and source size along the way.
 
- If source_mtime is None then skip the timestamp check.
+ If source_stats is None then skip the timestamp check.
 
  """
 magic=data[:4]
 raw_timestamp=data[4:8]
+raw_size=data[8:12]
 iflen(magic) !=4ormagic!=imp.get_magic():
 raiseImportError("bad magic number in{}".format(fullname))
 eliflen(raw_timestamp) !=4:
 raiseEOFError("bad timestamp in{}".format(fullname))
-elifsource_mtimeisnotNone:
-ifmarshal._r_long(raw_timestamp) !=source_mtime:
-raiseImportError("bytecode is stale for{}".format(fullname))
+eliflen(raw_size) !=4:
+raiseEOFError("bad size in{}".format(fullname))
+ifsource_statsisnotNone:
+try:
+source_mtime=int(source_stats['mtime'])
+exceptKeyError:
+pass
+else:
+ifmarshal._r_long(raw_timestamp) !=source_mtime:
+raiseImportError("bytecode is stale for{}".format(fullname))
+try:
+source_size=source_stats['size'] &0xFFFFFFFF
+exceptKeyError:
+pass
+else:
+ifmarshal._r_long(raw_size) !=source_size:
+raiseImportError("bytecode is stale for{}".format(fullname))
 # Can't return the code object as errors from marshal loading need to
 # propagate even when source is available.
-returndata[8:]
+returndata[12:]
 
 @module_for_loader
 def_load_module(self, module, *, sourceless=False):
@@ -377,11 +392,20 @@ class SourceLoader(_LoaderBasics):
 defpath_mtime(self, path):
 """Optional method that returns the modification time (an int) for the
  specified path, where path is a str.
+ """
+raiseNotImplementedError
 
- Implementing this method allows the loader to read bytecode files.
+defpath_stats(self, path):
+"""Optional method returning a metadata dict for the specified path
+ to by the path (str).
+ Possible keys:
+ - 'mtime' (mandatory) is the numeric timestamp of last source
+ code modification;
+ - 'size' (optional) is the size in bytes of the source code.
 
+ Implementing this method allows the loader to read bytecode files.
  """
-raiseNotImplementedError
+return{'mtime': self.path_mtime(path)}
 
 defset_data(self, path, data):
 """Optional method which writes data (bytes) to a file path (a str).
@@ -407,7 +431,7 @@ def get_source(self, fullname):
 defget_code(self, fullname):
 """Concrete implementation of InspectLoader.get_code.
 
- Reading of bytecode requires path_mtime to be implemented. To write
+ Reading of bytecode requires path_stats to be implemented. To write
  bytecode, set_data must also be implemented.
 
  """
@@ -416,18 +440,19 @@ def get_code(self, fullname):
 source_mtime=None
 ifbytecode_pathisnotNone:
 try:
-source_mtime=self.path_mtime(source_path)
+st=self.path_stats(source_path)
 exceptNotImplementedError:
 pass
 else:
+source_mtime=int(st['mtime'])
 try:
 data=self.get_data(bytecode_path)
 exceptIOError:
 pass
 else:
 try:
 bytes_data=self._bytes_from_bytecode(fullname, data,
-source_mtime)
+st)
 except (ImportError, EOFError):
 pass
 else:
@@ -448,6 +473,7 @@ def get_code(self, fullname):
 # throw an exception.
 data=bytearray(imp.get_magic())
 data.extend(marshal._w_long(source_mtime))
+data.extend(marshal._w_long(len(source_bytes)))
 data.extend(marshal.dumps(code_object))
 try:
 self.set_data(bytecode_path, data)
@@ -492,9 +518,10 @@ class _SourceFileLoader(_FileLoader, SourceLoader):
 
 """Concrete implementation of SourceLoader using the file system."""
 
-defpath_mtime(self, path):
-"""Return the modification time for the path."""
-returnint(_os.stat(path).st_mtime)
+defpath_stats(self, path):
+"""Return the metadat for the path."""
+st=_os.stat(path)
+return{'mtime': st.st_mtime, 'size': st.st_size}
 
 defset_data(self, path, data):
 """Write bytes data to a file."""

Lib/importlib/abc.py

@@ -123,7 +123,20 @@ class SourceLoader(_bootstrap.SourceLoader, ResourceLoader, ExecutionLoader):
 
 defpath_mtime(self, path):
 """Return the (int) modification time for the path (str)."""
-raiseNotImplementedError
+ifself.path_stats.__func__isSourceLoader.path_stats:
+raiseNotImplementedError
+returnint(self.path_stats(path)['mtime'])
+
+defpath_stats(self, path):
+"""Return a metadata dict for the source pointed to by the path (str).
+ Possible keys:
+ - 'mtime' (mandatory) is the numeric timestamp of last source
+ code modification;
+ - 'size' (optional) is the size in bytes of the source code.
+ """
+ifself.path_mtime.__func__isSourceLoader.path_mtime:
+raiseNotImplementedError
+return{'mtime': self.path_mtime(path)}
 
 defset_data(self, path, data):
 """Write the bytes to the path (if possible).

Lib/importlib/test/source/test_abc_loader.py

@@ -5,6 +5,7 @@
 from .. importutil
 from . importutilassource_util
 
+importcollections
 importimp
 importinspect
 importio
@@ -40,8 +41,10 @@ class SourceLoaderMock(SourceOnlyLoaderMock):
 def__init__(self, path, magic=imp.get_magic()):
 super().__init__(path)
 self.bytecode_path=imp.cache_from_source(self.path)
+self.source_size=len(self.source)
 data=bytearray(magic)
 data.extend(marshal._w_long(self.source_mtime))
+data.extend(marshal._w_long(self.source_size))
 code_object=compile(self.source, self.path, 'exec',
 dont_inherit=True)
 data.extend(marshal.dumps(code_object))
@@ -56,9 +59,9 @@ def get_data(self, path):
 else:
 raiseIOError
 
-defpath_mtime(self, path):
+defpath_stats(self, path):
 assertpath==self.path
-returnself.source_mtime
+return{'mtime': self.source_mtime, 'size': self.source_size}
 
 defset_data(self, path, data):
 self.written[path] =bytes(data)
@@ -657,6 +660,7 @@ def verify_code(self, code_object, *, bytecode_written=False):
 self.assertIn(self.cached, self.loader.written)
 data=bytearray(imp.get_magic())
 data.extend(marshal._w_long(self.loader.source_mtime))
+data.extend(marshal._w_long(self.loader.source_size))
 data.extend(marshal.dumps(code_object))
 self.assertEqual(self.loader.written[self.cached], bytes(data))
 
@@ -847,7 +851,7 @@ def test_SourceLoader(self):
 # Required abstractmethods.
 self.raises_NotImplementedError(ins, 'get_filename', 'get_data')
 # Optional abstractmethods.
-self.raises_NotImplementedError(ins,'path_mtime', 'set_data')
+self.raises_NotImplementedError(ins,'path_stats', 'set_data')
 
 deftest_PyLoader(self):
 self.raises_NotImplementedError(self.PyLoader(), 'source_path',

Lib/importlib/test/source/test_file_loader.py

@@ -70,11 +70,6 @@ def test_module_reuse(self):
 module_dict_id=id(module.__dict__)
 withopen(mapping['_temp'], 'w') asfile:
 file.write("testing_var = 42\n")
-# For filesystems where the mtime is only to a second granularity,
-# everything that has happened above can be too fast;
-# force an mtime on the source that is guaranteed to be different
-# than the original mtime.
-loader.path_mtime=self.fake_mtime(loader.path_mtime)
 module=loader.load_module('_temp')
 self.assertTrue('testing_var'inmodule.__dict__,
 "'testing_var' not in "
@@ -190,10 +185,17 @@ def _test_partial_timestamp(self, test, *, del_source=False):
 del_source=del_source)
 test('_temp', mapping, bc_path)
 
+def_test_partial_size(self, test, *, del_source=False):
+withsource_util.create_modules('_temp') asmapping:
+bc_path=self.manipulate_bytecode('_temp', mapping,
+lambdabc: bc[:11],
+del_source=del_source)
+test('_temp', mapping, bc_path)
+
 def_test_no_marshal(self, *, del_source=False):
 withsource_util.create_modules('_temp') asmapping:
 bc_path=self.manipulate_bytecode('_temp', mapping,
-lambdabc: bc[:8],
+lambdabc: bc[:12],
 del_source=del_source)
 file_path=mapping['_temp'] ifnotdel_sourceelsebc_path
 withself.assertRaises(EOFError):
@@ -202,7 +204,7 @@ def _test_no_marshal(self, *, del_source=False):
 def_test_non_code_marshal(self, *, del_source=False):
 withsource_util.create_modules('_temp') asmapping:
 bytecode_path=self.manipulate_bytecode('_temp', mapping,
-lambdabc: bc[:8] +marshal.dumps(b'abcd'),
+lambdabc: bc[:12] +marshal.dumps(b'abcd'),
 del_source=del_source)
 file_path=mapping['_temp'] ifnotdel_sourceelsebytecode_path
 withself.assertRaises(ImportError):
@@ -211,7 +213,7 @@ def _test_non_code_marshal(self, *, del_source=False):
 def_test_bad_marshal(self, *, del_source=False):
 withsource_util.create_modules('_temp') asmapping:
 bytecode_path=self.manipulate_bytecode('_temp', mapping,
-lambdabc: bc[:8] +b'<test>',
+lambdabc: bc[:12] +b'<test>',
 del_source=del_source)
 file_path=mapping['_temp'] ifnotdel_sourceelsebytecode_path
 withself.assertRaises(EOFError):
@@ -235,15 +237,15 @@ def test_empty_file(self):
 deftest(name, mapping, bytecode_path):
 self.import_(mapping[name], name)
 withopen(bytecode_path, 'rb') asfile:
-self.assertGreater(len(file.read()), 8)
+self.assertGreater(len(file.read()), 12)
 
 self._test_empty_file(test)
 
 deftest_partial_magic(self):
 deftest(name, mapping, bytecode_path):
 self.import_(mapping[name], name)
 withopen(bytecode_path, 'rb') asfile:
-self.assertGreater(len(file.read()), 8)
+self.assertGreater(len(file.read()), 12)
 
 self._test_partial_magic(test)
 
@@ -254,7 +256,7 @@ def test_magic_only(self):
 deftest(name, mapping, bytecode_path):
 self.import_(mapping[name], name)
 withopen(bytecode_path, 'rb') asfile:
-self.assertGreater(len(file.read()), 8)
+self.assertGreater(len(file.read()), 12)
 
 self._test_magic_only(test)
 
@@ -276,10 +278,21 @@ def test_partial_timestamp(self):
 deftest(name, mapping, bc_path):
 self.import_(mapping[name], name)
 withopen(bc_path, 'rb') asfile:
-self.assertGreater(len(file.read()), 8)
+self.assertGreater(len(file.read()), 12)
 
 self._test_partial_timestamp(test)
 
+@source_util.writes_bytecode_files
+deftest_partial_size(self):
+# When the size is partial, regenerate the .pyc, else
+# raise EOFError.
+deftest(name, mapping, bc_path):
+self.import_(mapping[name], name)
+withopen(bc_path, 'rb') asfile:
+self.assertGreater(len(file.read()), 12)
+
+self._test_partial_size(test)
+
 @source_util.writes_bytecode_files
 deftest_no_marshal(self):
 # When there is only the magic number and timestamp, raise EOFError.
@@ -375,6 +388,13 @@ def test(name, mapping, bytecode_path):
 
 self._test_partial_timestamp(test, del_source=True)
 
+deftest_partial_size(self):
+deftest(name, mapping, bytecode_path):
+withself.assertRaises(EOFError):
+self.import_(bytecode_path, name)
+
+self._test_partial_size(test, del_source=True)
+
 deftest_no_marshal(self):
 self._test_no_marshal(del_source=True)
 

Lib/pkgutil.py

@@ -21,7 +21,7 @@ def read_code(stream):
 ifmagic!=imp.get_magic():
 returnNone
 
-stream.read(4) # Skip timestamp
+stream.read(8) # Skip timestamp and size
 returnmarshal.load(stream)
 
 

Lib/py_compile.py

@@ -110,9 +110,11 @@ def compile(file, cfile=None, dfile=None, doraise=False, optimize=-1):
  """
 withtokenize.open(file) asf:
 try:
-timestamp=int(os.fstat(f.fileno()).st_mtime)
+st=os.fstat(f.fileno())
 exceptAttributeError:
-timestamp=int(os.stat(file).st_mtime)
+st=os.stat(file)
+timestamp=int(st.st_mtime)
+size=st.st_size&0xFFFFFFFF
 codestring=f.read()
 try:
 codeobject=builtins.compile(codestring, dfileorfile, 'exec',
@@ -139,6 +141,7 @@ def compile(file, cfile=None, dfile=None, doraise=False, optimize=-1):
 withopen(cfile, 'wb') asfc:
 fc.write(b'\0\0\0\0')
 wr_long(fc, timestamp)
+wr_long(fc, size)
 marshal.dump(codeobject, fc)
 fc.flush()
 fc.seek(0, 0)

Lib/test/test_import.py

@@ -380,7 +380,7 @@ def test_module_without_source(self):
 deftest_foreign_code(self):
 py_compile.compile(self.file_name)
 withopen(self.compiled_name, "rb") asf:
-header=f.read(8)
+header=f.read(12)
 code=marshal.load(f)
 constants=list(code.co_consts)
 foreign_code=test_main.__code__
@@ -644,6 +644,16 @@ def cleanup():
 self.assertEqual(sys.modules['pep3147.foo'].__cached__,
 os.path.join(os.curdir, foo_pyc))
 
+deftest_recompute_pyc_same_second(self):
+# Even when the source file doesn't change timestamp, a change in
+# source size is enough to trigger recomputation of the pyc file.
+__import__(TESTFN)
+unload(TESTFN)
+withopen(self.source, 'a') asfp:
+print("x = 5", file=fp)
+m=__import__(TESTFN)
+self.assertEqual(m.x, 5)
+
 
 classRelativeImportFromImportlibTests(test_relative_imports.RelativeImports):