#556 Fallback to alternate mtime correctly

CHANGELOG.md

@@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Unreleased
 
+- Fix an issue with `fs.compress.write_zip` that would cause an error
+  interacting with `S3FS` directory entries.
+  ([#557](https://github.com/PyFilesystem/pyfilesystem2/pull/557))
+  Closes [#556](https://github.com/PyFilesystem/pyfilesystem2/issues/556).
+
 
 ### Added
 

CONTRIBUTORS.md

@@ -20,6 +20,7 @@ Many thanks to the following developers for contributing to this project:
 - [George Macon](https://github.com/gmacon)
 - [Giampaolo Cimino](https://github.com/gpcimino)
 - [@Hoboneer](https://github.com/Hoboneer)
+- [James Emerton](https://github.com/james-emerton)
 - [Jen Hagg](https://github.com/jenhagg)
 - [Joseph Atkins-Turkish](https://github.com/Spacerat)
 - [Joshua Tauberer](https://github.com/JoshData)

fs/compress.py

@@ -65,17 +65,15 @@ def write_zip(
                 # Python2 expects bytes filenames
                 zip_name = zip_name.encode(encoding, "replace")
 
-            if info.has_namespace("stat"):
-                # If the file has a stat namespace, get the
-                # zip time directory from the stat structure
-                st_mtime = info.get("stat", "st_mtime", None)
-                _mtime = time.localtime(st_mtime)
-                zip_time = _mtime[0:6]  # type: ZipTime
-            else:
-                # Otherwise, use the modified time from details
-                # namespace.
-                mt = info.modified or datetime.utcnow()
-                zip_time = (mt.year, mt.month, mt.day, mt.hour, mt.minute, mt.second)
+            # If the file has a stat namespace, get the
+            # zip time directory from the stat structure
+            st_mtime = info.get("stat", "st_mtime")
+            # Otherwise, use the modified time from details namespace.
+            if st_mtime is None:
+                st_mtime = info.get("details", "modified")
+
+            # If st_mtime is still None this will default to current time
+            zip_time = time.localtime(st_mtime)[:6]  # type: ZipTime
 
             # NOTE(@althonos): typeshed's `zipfile.py` on declares
             #     ZipInfo.__init__ for Python < 3 ?!

tests/test_zipfs.py

@@ -8,11 +8,13 @@
 import tempfile
 import unittest
 import zipfile
+from datetime import datetime, timedelta
 
 from fs import zipfs
 from fs.compress import write_zip
 from fs.enums import Seek
 from fs.errors import NoURL
+from fs.memoryfs import MemoryFS
 from fs.opener import open_fs
 from fs.opener.errors import NotWriteable
 from fs.test import FSTestCases
@@ -224,3 +226,38 @@ class TestOpener(unittest.TestCase):
     def test_not_writeable(self):
         with self.assertRaises(NotWriteable):
             open_fs("zip://foo.zip", writeable=True)
+
+
+class FSWithoutMtime(MemoryFS):
+    '''MemoryFS subclass that doesn't return details namespace
+    '''
+    def getinfo(self, path, namespaces):
+        if namespaces is not None:
+            namespaces = set(namespaces) - {'details'}
+        return super().getinfo(path, namespaces)
+
+
+class TestZipFSMtimeFallback(unittest.TestCase):
+    def setUp(self):
+        fh, self._temp_path = tempfile.mkstemp()
+        os.close(fh)
+
+    def tearDown(self):
+        os.remove(self._temp_path)
+
+    def test_no_mtime(self):
+        '''Fallback to current time when creating an archive of an fs that
+        doesn't support stat or details namespaces.
+        '''
+        src_fs = FSWithoutMtime()
+        with src_fs.open('test.txt', 'w') as f:
+            f.write('Hello World')
+
+        now = datetime.now()
+        write_zip(src_fs, self._temp_path)
+
+        zf = zipfile.ZipFile(self._temp_path)
+        info = zf.getinfo('test.txt')
+        zf.close()
+
+        self.assertLessEqual(now - datetime(*info.date_time), timedelta(seconds=2))