Merge tweaks to fix for #375

--HG--
branch : distribute

CHANGES.txt

@@ -10,6 +10,9 @@ CHANGES
 * Fix possible issue in GUI launchers where the subsystem was not supplied to
   the linker.
 * Launcher build script now refactored for robustness.
+* Issue #375: Resources extracted from a zip egg to the file system now also
+  check the contents of the file against the zip contents during each
+  invocation of get_resource_filename.
 
 0.6.38
 ------

pkg_resources.py

@@ -1392,6 +1392,16 @@ class ZipProvider(EggProvider):
                 self._extract_resource(manager, self._eager_to_zip(name))
         return self._extract_resource(manager, zip_path)
 
+    @staticmethod
+    def _get_date_and_size(zip_stat):
+        t,d,size = zip_stat[5], zip_stat[6], zip_stat[3]
+        date_time = (
+            (d>>9)+1980, (d>>5)&0xF, d&0x1F,                      # ymd
+            (t&0xFFFF)>>11, (t>>5)&0x3F, (t&0x1F) * 2, 0, 0, -1   # hms, etc.
+        )
+        timestamp = time.mktime(date_time)
+        return timestamp, size
+
     def _extract_resource(self, manager, zip_path):
 
         if zip_path in self._index():
@@ -1401,28 +1411,19 @@ class ZipProvider(EggProvider):
                 )
             return os.path.dirname(last)  # return the extracted directory name
 
-        zip_stat = self.zipinfo[zip_path]
-        t,d,size = zip_stat[5], zip_stat[6], zip_stat[3]
-        date_time = (
-            (d>>9)+1980, (d>>5)&0xF, d&0x1F,                      # ymd
-            (t&0xFFFF)>>11, (t>>5)&0x3F, (t&0x1F) * 2, 0, 0, -1   # hms, etc.
-        )
-        timestamp = time.mktime(date_time)
+        timestamp, size = self._get_date_and_size(self.zipinfo[zip_path])
 
+        if not WRITE_SUPPORT:
+            raise IOError('"os.rename" and "os.unlink" are not supported '
+                          'on this platform')
         try:
-            if not WRITE_SUPPORT:
-                raise IOError('"os.rename" and "os.unlink" are not supported '
-                              'on this platform')
 
             real_path = manager.get_cache_path(
                 self.egg_name, self._parts(zip_path)
             )
 
-            if os.path.isfile(real_path):
-                stat = os.stat(real_path)
-                if stat.st_size==size and stat.st_mtime==timestamp:
-                    # size and stamp match, don't bother extracting
-                    return real_path
+            if self._is_current(real_path, zip_path):
+                return real_path
 
             outf, tmpnam = _mkstemp(".$extract", dir=os.path.dirname(real_path))
             os.write(outf, self.loader.get_data(zip_path))
@@ -1435,11 +1436,9 @@ class ZipProvider(EggProvider):
 
             except os.error:
                 if os.path.isfile(real_path):
-                    stat = os.stat(real_path)
-
-                    if stat.st_size==size and stat.st_mtime==timestamp:
-                        # size and stamp match, somebody did it just ahead of
-                        # us, so we're done
+                    if self._is_current(real_path, zip_path):
+                        # the file became current since it was checked above,
+                        #  so proceed.
                         return real_path
                     elif os.name=='nt':     # Windows, del old file and retry
                         unlink(real_path)
@@ -1452,6 +1451,23 @@ class ZipProvider(EggProvider):
 
         return real_path
 
+    def _is_current(self, file_path, zip_path):
+        """
+        Return True if the file_path is current for this zip_path
+        """
+        timestamp, size = self._get_date_and_size(self.zipinfo[zip_path])
+        if not os.path.isfile(file_path):
+            return False
+        stat = os.stat(file_path)
+        if stat.st_size!=size or stat.st_mtime!=timestamp:
+            return False
+        # check that the contents match
+        zip_contents = self.loader.get_data(zip_path)
+        f = open(file_path, 'rb')
+        file_contents = f.read()
+        f.close()
+        return zip_contents == file_contents
+
     def _get_eager_resources(self):
         if self.eagers is None:
             eagers = []

tests/test_pkg_resources.py

+import sys
+import tempfile
+import os
+import zipfile
+
+import pkg_resources
+
+class EggRemover(unicode):
+	def __call__(self):
+		if self in sys.path:
+			sys.path.remove(self)
+		if os.path.exists(self):
+			os.remove(self)
+
+class TestZipProvider(object):
+	finalizers = []
+
+	@classmethod
+	def setup_class(cls):
+		"create a zip egg and add it to sys.path"
+		egg = tempfile.NamedTemporaryFile(suffix='.egg', delete=False)
+		zip_egg = zipfile.ZipFile(egg, 'w')
+		zip_info = zipfile.ZipInfo()
+		zip_info.filename = 'mod.py'
+		zip_info.date_time = 2013, 5, 12, 13, 25, 0
+		zip_egg.writestr(zip_info, 'x = 3\n')
+		zip_info = zipfile.ZipInfo()
+		zip_info.filename = 'data.dat'
+		zip_info.date_time = 2013, 5, 12, 13, 25, 0
+		zip_egg.writestr(zip_info, 'hello, world!')
+		zip_egg.close()
+		egg.close()
+
+		sys.path.append(egg.name)
+		cls.finalizers.append(EggRemover(egg.name))
+
+	@classmethod
+	def teardown_class(cls):
+		for finalizer in cls.finalizers:
+			finalizer()
+
+	def test_resource_filename_rewrites_on_change(self):
+		"""
+		If a previous call to get_resource_filename has saved the file, but
+		the file has been subsequently mutated with different file of the
+		same size and modification time, it should not be overwritten on a
+		subsequent call to get_resource_filename.
+		"""
+		import mod
+		manager = pkg_resources.ResourceManager()
+		zp = pkg_resources.ZipProvider(mod)
+		filename = zp.get_resource_filename(manager, 'data.dat')
+		assert os.stat(filename).st_mtime == 1368379500
+		f = open(filename, 'wb')
+		f.write('hello, world?')
+		f.close()
+		os.utime(filename, (1368379500, 1368379500))
+		filename = zp.get_resource_filename(manager, 'data.dat')
+		f = open(filename)
+		assert f.read() == 'hello, world!'
+		manager.cleanup_resources()