From 1327a6386412ab2d9f0fa1082d2ea1a126daeb04 Mon Sep 17 00:00:00 2001 From: atollk Date: Sat, 20 Mar 2021 17:56:39 +0100 Subject: [PATCH 01/14] Added `preserve_time` parameter to all copy/move/mirror functions. If `preserve_time` is set to True, the call tries to preserve the atime, ctime, and mtime timestamps on the file after the copy. The value is currently not actually used; places in code that have to be adapted are marked with `TODO(preserve_time)`. --- fs/_bulk.py | 9 ++++--- fs/base.py | 35 ++++++++++++++++++++------ fs/copy.py | 69 ++++++++++++++++++++++++++++++++++++++++++++++------ fs/mirror.py | 15 +++++++++--- fs/move.py | 37 ++++++++++++++++++++++++---- fs/osfs.py | 10 +++++--- fs/wrap.py | 4 +-- fs/wrapfs.py | 12 ++++----- 8 files changed, 153 insertions(+), 38 deletions(-) diff --git a/fs/_bulk.py b/fs/_bulk.py index 9b0b8b79..63cecc01 100644 --- a/fs/_bulk.py +++ b/fs/_bulk.py @@ -124,13 +124,16 @@ def __exit__( if traceback is None and self.errors: raise BulkCopyFailed(self.errors) - def copy(self, src_fs, src_path, dst_fs, dst_path): - # type: (FS, Text, FS, Text) -> None + def copy(self, src_fs, src_path, dst_fs, dst_path, preserve_time=False): + # type: (FS, Text, FS, Text, bool) -> None """Copy a file from one fs to another.""" if self.queue is None: # This should be the most performant for a single-thread - copy_file_internal(src_fs, src_path, dst_fs, dst_path) + copy_file_internal( + src_fs, src_path, dst_fs, dst_path, preserve_time=preserve_time + ) else: + # TODO(preserve_time) src_file = src_fs.openbin(src_path, "r") try: dst_file = dst_fs.openbin(dst_path, "w") diff --git a/fs/base.py b/fs/base.py index 7a81c5cb..03d4dac8 100644 --- a/fs/base.py +++ b/fs/base.py @@ -387,8 +387,14 @@ def close(self): """ self._closed = True - def copy(self, src_path, dst_path, overwrite=False): - # type: (Text, Text, bool) -> None + def copy( + self, + src_path, # type: Text + dst_path, # type: Text + overwrite=False, # type: bool + preserve_time=False, # type: bool + ): + # type: (...) -> None """Copy file contents from ``src_path`` to ``dst_path``. Arguments: @@ -396,6 +402,8 @@ def copy(self, src_path, dst_path, overwrite=False): dst_path (str): Path to destination file. overwrite (bool): If `True`, overwrite the destination file if it exists (defaults to `False`). + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resource (defaults to `False`). Raises: fs.errors.DestinationExists: If ``dst_path`` exists, @@ -404,6 +412,7 @@ def copy(self, src_path, dst_path, overwrite=False): ``dst_path`` does not exist. """ + # TODO(preserve_time) with self._lock: if not overwrite and self.exists(dst_path): raise errors.DestinationExists(dst_path) @@ -411,8 +420,14 @@ def copy(self, src_path, dst_path, overwrite=False): # FIXME(@althonos): typing complains because open return IO self.upload(dst_path, read_file) # type: ignore - def copydir(self, src_path, dst_path, create=False): - # type: (Text, Text, bool) -> None + def copydir( + self, + src_path, # type: Text + dst_path, # type: Text + create=False, # type: bool + preserve_time=False, # type: bool + ): + # type: (...) -> None """Copy the contents of ``src_path`` to ``dst_path``. Arguments: @@ -420,6 +435,8 @@ def copydir(self, src_path, dst_path, create=False): dst_path (str): Path to destination directory. create (bool): If `True`, then ``dst_path`` will be created if it doesn't exist already (defaults to `False`). + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resource (defaults to `False`). Raises: fs.errors.ResourceNotFound: If the ``dst_path`` @@ -431,7 +448,7 @@ def copydir(self, src_path, dst_path, create=False): raise errors.ResourceNotFound(dst_path) if not self.getinfo(src_path).is_dir: raise errors.DirectoryExpected(src_path) - copy.copy_dir(self, src_path, self, dst_path) + copy.copy_dir(self, src_path, self, dst_path, preserve_time=preserve_time) def create(self, path, wipe=False): # type: (Text, bool) -> bool @@ -1004,8 +1021,8 @@ def lock(self): """ return self._lock - def movedir(self, src_path, dst_path, create=False): - # type: (Text, Text, bool) -> None + def movedir(self, src_path, dst_path, create=False, preserve_time=False): + # type: (Text, Text, bool, bool) -> None """Move directory ``src_path`` to ``dst_path``. Arguments: @@ -1013,6 +1030,8 @@ def movedir(self, src_path, dst_path, create=False): dst_path (str): Path to destination directory. create (bool): If `True`, then ``dst_path`` will be created if it doesn't exist already (defaults to `False`). + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resources (defaults to `False`). Raises: fs.errors.ResourceNotFound: if ``dst_path`` does not exist, @@ -1022,7 +1041,7 @@ def movedir(self, src_path, dst_path, create=False): with self._lock: if not create and not self.exists(dst_path): raise errors.ResourceNotFound(dst_path) - move.move_dir(self, src_path, self, dst_path) + move.move_dir(self, src_path, self, dst_path, preserve_time=preserve_time) def makedirs( self, diff --git a/fs/copy.py b/fs/copy.py index 6cd34392..6f9fc701 100644 --- a/fs/copy.py +++ b/fs/copy.py @@ -24,6 +24,7 @@ def copy_fs( walker=None, # type: Optional[Walker] on_copy=None, # type: Optional[_OnCopy] workers=0, # type: int + preserve_time=False, # type: bool ): # type: (...) -> None """Copy the contents of one filesystem to another. @@ -39,6 +40,8 @@ def copy_fs( dst_path)``. workers (int): Use `worker` threads to copy data, or ``0`` (default) for a single-threaded copy. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resources (defaults to `False`). """ return copy_dir( @@ -52,6 +55,7 @@ def copy_fs_if_newer( walker=None, # type: Optional[Walker] on_copy=None, # type: Optional[_OnCopy] workers=0, # type: int + preserve_time=False, # type: bool ): # type: (...) -> None """Copy the contents of one filesystem to another, checking times. @@ -72,10 +76,19 @@ def copy_fs_if_newer( dst_path)``. workers (int): Use ``worker`` threads to copy data, or ``0`` (default) for a single-threaded copy. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resources (defaults to `False`). """ return copy_dir_if_newer( - src_fs, "/", dst_fs, "/", walker=walker, on_copy=on_copy, workers=workers + src_fs, + "/", + dst_fs, + "/", + walker=walker, + on_copy=on_copy, + workers=workers, + preserve_time=preserve_time, ) @@ -113,6 +126,7 @@ def copy_file( src_path, # type: Text dst_fs, # type: Union[FS, Text] dst_path, # type: Text + preserve_time=False, # type: bool ): # type: (...) -> None """Copy a file from one filesystem to another. @@ -124,6 +138,8 @@ def copy_file( src_path (str): Path to a file on the source filesystem. dst_fs (FS or str): Destination filesystem (instance or URL). dst_path (str): Path to a file on the destination filesystem. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resource (defaults to `False`). """ with manage_fs(src_fs, writeable=False) as _src_fs: @@ -131,8 +147,11 @@ def copy_file( if _src_fs is _dst_fs: # Same filesystem, so we can do a potentially optimized # copy - _src_fs.copy(src_path, dst_path, overwrite=True) + _src_fs.copy( + src_path, dst_path, overwrite=True, preserve_time=preserve_time + ) else: + # TODO(preserve_time) # Standard copy with _src_fs.lock(), _dst_fs.lock(): if _dst_fs.hassyspath(dst_path): @@ -148,6 +167,7 @@ def copy_file_internal( src_path, # type: Text dst_fs, # type: FS dst_path, # type: Text + preserve_time=False, # type: bool ): # type: (...) -> None """Copy a file at low level, without calling `manage_fs` or locking. @@ -162,12 +182,15 @@ def copy_file_internal( src_path (str): Path to a file on the source filesystem. dst_fs (FS): Destination filesystem. dst_path (str): Path to a file on the destination filesystem. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resource (defaults to `False`). """ if src_fs is dst_fs: # Same filesystem, so we can do a potentially optimized # copy - src_fs.copy(src_path, dst_path, overwrite=True) + src_fs.copy(src_path, dst_path, overwrite=True, preserve_time=preserve_time) + # TODO(preserve_time) elif dst_fs.hassyspath(dst_path): with dst_fs.openbin(dst_path, "w") as write_file: src_fs.download(src_path, write_file) @@ -181,6 +204,7 @@ def copy_file_if_newer( src_path, # type: Text dst_fs, # type: Union[FS, Text] dst_path, # type: Text + preserve_time=False, # type: bool ): # type: (...) -> bool """Copy a file from one filesystem to another, checking times. @@ -196,6 +220,8 @@ def copy_file_if_newer( src_path (str): Path to a file on the source filesystem. dst_fs (FS or str): Destination filesystem (instance or URL). dst_path (str): Path to a file on the destination filesystem. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resource (defaults to `False`). Returns: bool: `True` if the file copy was executed, `False` otherwise. @@ -207,7 +233,9 @@ def copy_file_if_newer( # Same filesystem, so we can do a potentially optimized # copy if _source_is_newer(_src_fs, src_path, _dst_fs, dst_path): - _src_fs.copy(src_path, dst_path, overwrite=True) + _src_fs.copy( + src_path, dst_path, overwrite=True, preserve_time=preserve_time + ) return True else: return False @@ -215,7 +243,13 @@ def copy_file_if_newer( # Standard copy with _src_fs.lock(), _dst_fs.lock(): if _source_is_newer(_src_fs, src_path, _dst_fs, dst_path): - copy_file_internal(_src_fs, src_path, _dst_fs, dst_path) + copy_file_internal( + _src_fs, + src_path, + _dst_fs, + dst_path, + preserve_time=preserve_time, + ) return True else: return False @@ -225,6 +259,7 @@ def copy_structure( src_fs, # type: Union[FS, Text] dst_fs, # type: Union[FS, Text] walker=None, # type: Optional[Walker] + preserve_time=False, # type: bool ): # type: (...) -> None """Copy directories (but not files) from ``src_fs`` to ``dst_fs``. @@ -235,6 +270,8 @@ def copy_structure( walker (~fs.walk.Walker, optional): A walker object that will be used to scan for files in ``src_fs``. Set this if you only want to consider a sub-set of the resources in ``src_fs``. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resource (defaults to `False`). """ walker = walker or Walker() @@ -253,6 +290,7 @@ def copy_dir( walker=None, # type: Optional[Walker] on_copy=None, # type: Optional[_OnCopy] workers=0, # type: int + preserve_time=False, # type: bool ): # type: (...) -> None """Copy a directory from one filesystem to another. @@ -270,6 +308,8 @@ def copy_dir( ``(src_fs, src_path, dst_fs, dst_path)``. workers (int): Use ``worker`` threads to copy data, or ``0`` (default) for a single-threaded copy. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resources (defaults to `False`). """ on_copy = on_copy or (lambda *args: None) @@ -297,7 +337,13 @@ def dst(): for info in files: src_path = info.make_path(dir_path) dst_path = info.make_path(copy_path) - copier.copy(_src_fs, src_path, _dst_fs, dst_path) + copier.copy( + _src_fs, + src_path, + _dst_fs, + dst_path, + preserve_time=preserve_time, + ) on_copy(_src_fs, src_path, _dst_fs, dst_path) @@ -309,6 +355,7 @@ def copy_dir_if_newer( walker=None, # type: Optional[Walker] on_copy=None, # type: Optional[_OnCopy] workers=0, # type: int + preserve_time=False, # type: bool ): # type: (...) -> None """Copy a directory from one filesystem to another, checking times. @@ -331,6 +378,8 @@ def copy_dir_if_newer( ``(src_fs, src_path, dst_fs, dst_path)``. workers (int): Use ``worker`` threads to copy data, or ``0`` (default) for a single-threaded copy. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resources (defaults to `False`). """ on_copy = on_copy or (lambda *args: None) @@ -381,5 +430,11 @@ def dst(): ) if do_copy: - copier.copy(_src_fs, dir_path, _dst_fs, copy_path) + copier.copy( + _src_fs, + dir_path, + _dst_fs, + copy_path, + preserve_time=preserve_time, + ) on_copy(_src_fs, dir_path, _dst_fs, copy_path) diff --git a/fs/mirror.py b/fs/mirror.py index 6b989e63..6123aed2 100644 --- a/fs/mirror.py +++ b/fs/mirror.py @@ -57,6 +57,7 @@ def mirror( walker=None, # type: Optional[Walker] copy_if_newer=True, # type: bool workers=0, # type: int + preserve_time=False, # type: bool ): # type: (...) -> None """Mirror files / directories from one filesystem to another. @@ -73,6 +74,8 @@ def mirror( workers (int): Number of worker threads used (0 for single threaded). Set to a relatively low number for network filesystems, 4 would be a good start. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resources (defaults to `False`). """ @@ -92,13 +95,19 @@ def dst(): walker=walker, copy_if_newer=copy_if_newer, copy_file=copier.copy, + preserve_time=preserve_time, ) def _mirror( - src_fs, dst_fs, walker=None, copy_if_newer=True, copy_file=copy_file_internal + src_fs, # type: FS + dst_fs, # type: FS + walker=None, # type: Optional[Walker] + copy_if_newer=True, # type: bool + copy_file=copy_file_internal, # type: Callable[[FS, str, FS, str, bool], None] + preserve_time=False, # type: bool ): - # type: (FS, FS, Optional[Walker], bool, Callable[[FS, str, FS, str], None]) -> None + # type: (...) -> None walker = walker or Walker() walk = walker.walk(src_fs, namespaces=["details"]) for path, dirs, files in walk: @@ -122,7 +131,7 @@ def _mirror( # Compare file info if copy_if_newer and not _compare(_file, dst_file): continue - copy_file(src_fs, _path, dst_fs, _path) + copy_file(src_fs, _path, dst_fs, _path, preserve_time) # Make directories for _dir in dirs: diff --git a/fs/move.py b/fs/move.py index 1d8e26c1..da941479 100644 --- a/fs/move.py +++ b/fs/move.py @@ -15,8 +15,13 @@ from typing import Text, Union -def move_fs(src_fs, dst_fs, workers=0): - # type: (Union[Text, FS], Union[Text, FS], int) -> None +def move_fs( + src_fs, # type: Union[Text, FS] + dst_fs, # type:Union[Text, FS] + workers=0, # type: int + preserve_time=False, # type: bool +): + # type: (...) -> None """Move the contents of a filesystem to another filesystem. Arguments: @@ -24,9 +29,11 @@ def move_fs(src_fs, dst_fs, workers=0): dst_fs (FS or str): Destination filesystem (instance or URL). workers (int): Use `worker` threads to copy data, or ``0`` (default) for a single-threaded copy. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resources (defaults to `False`). """ - move_dir(src_fs, "/", dst_fs, "/", workers=workers) + move_dir(src_fs, "/", dst_fs, "/", workers=workers, preserve_time=preserve_time) def move_file( @@ -34,6 +41,7 @@ def move_file( src_path, # type: Text dst_fs, # type: Union[Text, FS] dst_path, # type: Text + preserve_time=False, # type: bool ): # type: (...) -> None """Move a file from one filesystem to another. @@ -43,8 +51,11 @@ def move_file( src_path (str): Path to a file on ``src_fs``. dst_fs (FS or str): Destination filesystem (instance or URL). dst_path (str): Path to a file on ``dst_fs``. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resources (defaults to `False`). """ + # TODO(preserve_time) with manage_fs(src_fs) as _src_fs: with manage_fs(dst_fs, create=True) as _dst_fs: if _src_fs is _dst_fs: @@ -53,7 +64,13 @@ def move_file( else: # Standard copy and delete with _src_fs.lock(), _dst_fs.lock(): - copy_file(_src_fs, src_path, _dst_fs, dst_path) + copy_file( + _src_fs, + src_path, + _dst_fs, + dst_path, + preserve_time=preserve_time, + ) _src_fs.remove(src_path) @@ -63,6 +80,7 @@ def move_dir( dst_fs, # type: Union[Text, FS] dst_path, # type: Text workers=0, # type: int + preserve_time=False, # type: bool ): # type: (...) -> None """Move a directory from one filesystem to another. @@ -74,6 +92,8 @@ def move_dir( dst_path (str): Path to a directory on ``dst_fs``. workers (int): Use ``worker`` threads to copy data, or ``0`` (default) for a single-threaded copy. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resources (defaults to `False`). """ @@ -86,5 +106,12 @@ def dst(): with src() as _src_fs, dst() as _dst_fs: with _src_fs.lock(), _dst_fs.lock(): _dst_fs.makedir(dst_path, recreate=True) - copy_dir(src_fs, src_path, dst_fs, dst_path, workers=workers) + copy_dir( + src_fs, + src_path, + dst_fs, + dst_path, + workers=workers, + preserve_time=preserve_time, + ) _src_fs.removetree(src_path) diff --git a/fs/osfs.py b/fs/osfs.py index 3b35541c..8c071c3d 100644 --- a/fs/osfs.py +++ b/fs/osfs.py @@ -431,8 +431,9 @@ def _check_copy(self, src_path, dst_path, overwrite=False): if hasattr(errno, "ENOTSUP"): _sendfile_error_codes.add(errno.ENOTSUP) - def copy(self, src_path, dst_path, overwrite=False): - # type: (Text, Text, bool) -> None + def copy(self, src_path, dst_path, overwrite=False, preserve_time=False): + # type: (Text, Text, bool, bool) -> None + # TODO(preserve_time) with self._lock: # validate and canonicalise paths _src_path, _dst_path = self._check_copy(src_path, dst_path, overwrite) @@ -461,8 +462,9 @@ def copy(self, src_path, dst_path, overwrite=False): else: - def copy(self, src_path, dst_path, overwrite=False): - # type: (Text, Text, bool) -> None + def copy(self, src_path, dst_path, overwrite=False, preserve_time=False): + # type: (Text, Text, bool, bool) -> None + # TODO(preserve_time) with self._lock: _src_path, _dst_path = self._check_copy(src_path, dst_path, overwrite) shutil.copy2(self.getsyspath(_src_path), self.getsyspath(_dst_path)) diff --git a/fs/wrap.py b/fs/wrap.py index 8685e9f6..9ceae351 100644 --- a/fs/wrap.py +++ b/fs/wrap.py @@ -225,8 +225,8 @@ def settimes(self, path, accessed=None, modified=None): self.check() raise ResourceReadOnly(path) - def copy(self, src_path, dst_path, overwrite=False): - # type: (Text, Text, bool) -> None + def copy(self, src_path, dst_path, overwrite=False, preserve_time=False): + # type: (Text, Text, bool, bool) -> None self.check() raise ResourceReadOnly(dst_path) diff --git a/fs/wrapfs.py b/fs/wrapfs.py index e40a7a83..7be9c235 100644 --- a/fs/wrapfs.py +++ b/fs/wrapfs.py @@ -256,17 +256,17 @@ def touch(self, path): with unwrap_errors(path): _fs.touch(_path) - def copy(self, src_path, dst_path, overwrite=False): - # type: (Text, Text, bool) -> None + def copy(self, src_path, dst_path, overwrite=False, preserve_time=False): + # type: (Text, Text, bool, bool) -> None src_fs, _src_path = self.delegate_path(src_path) dst_fs, _dst_path = self.delegate_path(dst_path) with unwrap_errors({_src_path: src_path, _dst_path: dst_path}): if not overwrite and dst_fs.exists(_dst_path): raise errors.DestinationExists(_dst_path) - copy_file(src_fs, _src_path, dst_fs, _dst_path) + copy_file(src_fs, _src_path, dst_fs, _dst_path, preserve_time=preserve_time) - def copydir(self, src_path, dst_path, create=False): - # type: (Text, Text, bool) -> None + def copydir(self, src_path, dst_path, create=False, preserve_time=False): + # type: (Text, Text, bool, bool) -> None src_fs, _src_path = self.delegate_path(src_path) dst_fs, _dst_path = self.delegate_path(dst_path) with unwrap_errors({_src_path: src_path, _dst_path: dst_path}): @@ -274,7 +274,7 @@ def copydir(self, src_path, dst_path, create=False): raise errors.ResourceNotFound(dst_path) if not src_fs.getinfo(_src_path).is_dir: raise errors.DirectoryExpected(src_path) - copy_dir(src_fs, _src_path, dst_fs, _dst_path) + copy_dir(src_fs, _src_path, dst_fs, _dst_path, preserve_time=preserve_time) def create(self, path, wipe=False): # type: (Text, bool) -> bool From ec39f9d37b457176c66f015f493196ab02b1222f Mon Sep 17 00:00:00 2001 From: atollk Date: Sat, 20 Mar 2021 19:05:57 +0100 Subject: [PATCH 02/14] Added unit tests for the `preserve_time` parameter. At this point in time, all the new tests are failing right now; as they should, because the functionality of `preserve_time` has not been implemented yet. --- setup.cfg | 1 + tests/requirements.txt | 2 ++ tests/test_copy.py | 71 +++++++++++++++++++++++++++++++----------- tests/test_memoryfs.py | 16 ++++++++++ tests/test_mirror.py | 53 +++++++++++++++---------------- tests/test_move.py | 36 ++++++++++++++++++++- tests/test_opener.py | 16 ++++++++-- tests/test_osfs.py | 20 ++++++++++++ 8 files changed, 167 insertions(+), 48 deletions(-) diff --git a/setup.cfg b/setup.cfg index b90ebe47..f667b1a1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -133,6 +133,7 @@ sitepackages = false skip_missing_interpreters = true requires = setuptools >=38.3.0 + parameterized ~=0.8 [testenv] commands = pytest --cov={toxinidir}/fs {posargs} {toxinidir}/tests diff --git a/tests/requirements.txt b/tests/requirements.txt index 9e7ece32..b7ff3ce4 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -14,3 +14,5 @@ pysendfile ~=2.0 ; python_version <= "3.3" # mock v4+ doesn't support Python 2.7 anymore mock ~=3.0 ; python_version < "3.3" +# parametrized to prevent code duplication in tests. +parameterized ~=0.8 \ No newline at end of file diff --git a/tests/test_copy.py b/tests/test_copy.py index 63e550e9..77c7f756 100644 --- a/tests/test_copy.py +++ b/tests/test_copy.py @@ -8,25 +8,44 @@ import shutil import calendar +from parameterized import parameterized + import fs.copy from fs import open_fs class TestCopy(unittest.TestCase): - def test_copy_fs(self): - for workers in (0, 1, 2, 4): - src_fs = open_fs("mem://") - src_fs.makedirs("foo/bar") - src_fs.makedirs("foo/empty") - src_fs.touch("test.txt") - src_fs.touch("foo/bar/baz.txt") + @parameterized.expand([(0,), (1,), (2,), (4,)]) + def test_copy_fs(self, workers): + namespaces = ("details", "accessed", "metadata_changed", "modified") - dst_fs = open_fs("mem://") - fs.copy.copy_fs(src_fs, dst_fs, workers=workers) + src_fs = open_fs("mem://") + src_fs.makedirs("foo/bar") + src_fs.makedirs("foo/empty") + src_fs.touch("test.txt") + src_fs.touch("foo/bar/baz.txt") + src_file1_info = src_fs.getinfo("test.txt", namespaces) + src_file2_info = src_fs.getinfo("foo/bar/baz.txt", namespaces) - self.assertTrue(dst_fs.isdir("foo/empty")) - self.assertTrue(dst_fs.isdir("foo/bar")) - self.assertTrue(dst_fs.isfile("test.txt")) + dst_fs = open_fs("mem://") + fs.copy.copy_fs(src_fs, dst_fs, workers=workers, preserve_time=True) + + self.assertTrue(dst_fs.isdir("foo/empty")) + self.assertTrue(dst_fs.isdir("foo/bar")) + self.assertTrue(dst_fs.isfile("test.txt")) + + dst_file1_info = dst_fs.getinfo("test.txt", namespaces) + dst_file2_info = dst_fs.getinfo("foo/bar/baz.txt", namespaces) + self.assertEqual(dst_file1_info.modified, src_file1_info.modified) + self.assertEqual(dst_file1_info.accessed, src_file1_info.accessed) + self.assertEqual( + dst_file1_info.metadata_changed, src_file1_info.metadata_changed + ) + self.assertEqual(dst_file2_info.modified, src_file2_info.modified) + self.assertEqual(dst_file2_info.accessed, src_file2_info.accessed) + self.assertEqual( + dst_file2_info.metadata_changed, src_file2_info.metadata_changed + ) def test_copy_value_error(self): src_fs = open_fs("mem://") @@ -34,18 +53,32 @@ def test_copy_value_error(self): with self.assertRaises(ValueError): fs.copy.copy_fs(src_fs, dst_fs, workers=-1) - def test_copy_dir(self): + @parameterized.expand([(0,), (1,), (2,), (4,)]) + def test_copy_dir(self, workers): + namespaces = ("details", "accessed", "metadata_changed", "modified") + src_fs = open_fs("mem://") src_fs.makedirs("foo/bar") src_fs.makedirs("foo/empty") src_fs.touch("test.txt") src_fs.touch("foo/bar/baz.txt") - for workers in (0, 1, 2, 4): - with open_fs("mem://") as dst_fs: - fs.copy.copy_dir(src_fs, "/foo", dst_fs, "/", workers=workers) - self.assertTrue(dst_fs.isdir("bar")) - self.assertTrue(dst_fs.isdir("empty")) - self.assertTrue(dst_fs.isfile("bar/baz.txt")) + src_file1_info = src_fs.getinfo("test.txt", namespaces) + src_file2_info = src_fs.getinfo("foo/bar/baz.txt", namespaces) + + with open_fs("mem://") as dst_fs: + fs.copy.copy_dir( + src_fs, "/foo", dst_fs, "/", workers=workers, preserve_time=True + ) + self.assertTrue(dst_fs.isdir("bar")) + self.assertTrue(dst_fs.isdir("empty")) + self.assertTrue(dst_fs.isfile("bar/baz.txt")) + + dst_file2_info = dst_fs.getinfo("bar/baz.txt", namespaces) + self.assertEqual(dst_file2_info.modified, src_file2_info.modified) + self.assertEqual(dst_file2_info.accessed, src_file2_info.accessed) + self.assertEqual( + dst_file2_info.metadata_changed, src_file2_info.metadata_changed + ) def test_copy_large(self): data1 = b"foo" * 512 * 1024 diff --git a/tests/test_memoryfs.py b/tests/test_memoryfs.py index 0b26a576..59502cbe 100644 --- a/tests/test_memoryfs.py +++ b/tests/test_memoryfs.py @@ -66,3 +66,19 @@ def test_close_mem_free(self): "Memory usage increased after closing the file system; diff is %0.2f KiB." % (diff_close.size_diff / 1024.0), ) + + def test_copy_preserve_time(self): + self.fs.makedir("foo") + self.fs.makedir("bar") + self.fs.touch("foo/file.txt") + + namespaces = ("details", "accessed", "metadata_changed", "modified") + src_info = self.fs.getinfo("foo/file.txt", namespaces) + + self.fs.copy("foo/file.txt", "bar/file.txt", preserve_time=True) + self.assertTrue(self.fs.exists("bar/file.txt")) + + dst_info = self.fs.getinfo("bar/file.txt", namespaces) + self.assertEqual(dst_info.modified, src_info.modified) + self.assertEqual(dst_info.accessed, src_info.accessed) + self.assertEqual(dst_info.metadata_changed, src_info.metadata_changed) diff --git a/tests/test_mirror.py b/tests/test_mirror.py index a0e8ac53..f204178f 100644 --- a/tests/test_mirror.py +++ b/tests/test_mirror.py @@ -2,15 +2,17 @@ import unittest +from parameterized import parameterized_class + from fs.mirror import mirror from fs import open_fs +@parameterized_class(("WORKERS",), [(0,), (1,), (2,), (4,)]) class TestMirror(unittest.TestCase): - WORKERS = 0 # Single threaded - def _contents(self, fs): """Extract an FS in to a simple data structure.""" + namespaces = ("details", "accessed", "metadata_changed", "modified") contents = [] for path, dirs, files in fs.walk(): for info in dirs: @@ -18,7 +20,18 @@ def _contents(self, fs): contents.append((_path, "dir", b"")) for info in files: _path = info.make_path(path) - contents.append((_path, "file", fs.readbytes(_path))) + _bytes = fs.readbytes(_path) + _info = fs.getinfo(_path, namespaces) + contents.append( + ( + _path, + "file", + _bytes, + _info.modified, + _info.accessed, + _info.metadata_changed, + ) + ) return sorted(contents) def assert_compare_fs(self, fs1, fs2): @@ -28,14 +41,14 @@ def assert_compare_fs(self, fs1, fs2): def test_empty_mirror(self): m1 = open_fs("mem://") m2 = open_fs("mem://") - mirror(m1, m2, workers=self.WORKERS) + mirror(m1, m2, workers=self.WORKERS, preserve_time=True) self.assert_compare_fs(m1, m2) def test_mirror_one_file(self): m1 = open_fs("mem://") m1.writetext("foo", "hello") m2 = open_fs("mem://") - mirror(m1, m2, workers=self.WORKERS) + mirror(m1, m2, workers=self.WORKERS, preserve_time=True) self.assert_compare_fs(m1, m2) def test_mirror_one_file_one_dir(self): @@ -43,7 +56,7 @@ def test_mirror_one_file_one_dir(self): m1.writetext("foo", "hello") m1.makedir("bar") m2 = open_fs("mem://") - mirror(m1, m2, workers=self.WORKERS) + mirror(m1, m2, workers=self.WORKERS, preserve_time=True) self.assert_compare_fs(m1, m2) def test_mirror_delete_replace(self): @@ -51,13 +64,13 @@ def test_mirror_delete_replace(self): m1.writetext("foo", "hello") m1.makedir("bar") m2 = open_fs("mem://") - mirror(m1, m2, workers=self.WORKERS) + mirror(m1, m2, workers=self.WORKERS, preserve_time=True) self.assert_compare_fs(m1, m2) m2.remove("foo") - mirror(m1, m2, workers=self.WORKERS) + mirror(m1, m2, workers=self.WORKERS, preserve_time=True) self.assert_compare_fs(m1, m2) m2.removedir("bar") - mirror(m1, m2, workers=self.WORKERS) + mirror(m1, m2, workers=self.WORKERS, preserve_time=True) self.assert_compare_fs(m1, m2) def test_mirror_extra_dir(self): @@ -66,7 +79,7 @@ def test_mirror_extra_dir(self): m1.makedir("bar") m2 = open_fs("mem://") m2.makedir("baz") - mirror(m1, m2, workers=self.WORKERS) + mirror(m1, m2, workers=self.WORKERS, preserve_time=True) self.assert_compare_fs(m1, m2) def test_mirror_extra_file(self): @@ -76,7 +89,7 @@ def test_mirror_extra_file(self): m2 = open_fs("mem://") m2.makedir("baz") m2.touch("egg") - mirror(m1, m2, workers=self.WORKERS) + mirror(m1, m2, workers=self.WORKERS, preserve_time=True) self.assert_compare_fs(m1, m2) def test_mirror_wrong_type(self): @@ -86,7 +99,7 @@ def test_mirror_wrong_type(self): m2 = open_fs("mem://") m2.makedir("foo") m2.touch("bar") - mirror(m1, m2, workers=self.WORKERS) + mirror(m1, m2, workers=self.WORKERS, preserve_time=True) self.assert_compare_fs(m1, m2) def test_mirror_update(self): @@ -94,20 +107,8 @@ def test_mirror_update(self): m1.writetext("foo", "hello") m1.makedir("bar") m2 = open_fs("mem://") - mirror(m1, m2, workers=self.WORKERS) + mirror(m1, m2, workers=self.WORKERS, preserve_time=True) self.assert_compare_fs(m1, m2) m2.appendtext("foo", " world!") - mirror(m1, m2, workers=self.WORKERS) + mirror(m1, m2, workers=self.WORKERS, preserve_time=True) self.assert_compare_fs(m1, m2) - - -class TestMirrorWorkers1(TestMirror): - WORKERS = 1 - - -class TestMirrorWorkers2(TestMirror): - WORKERS = 2 - - -class TestMirrorWorkers4(TestMirror): - WORKERS = 4 diff --git a/tests/test_move.py b/tests/test_move.py index d87d2bd6..8bba8857 100644 --- a/tests/test_move.py +++ b/tests/test_move.py @@ -2,29 +2,54 @@ import unittest +from parameterized import parameterized_class + import fs.move from fs import open_fs +@parameterized_class(("preserve_time",), [(True,), (False,)]) class TestMove(unittest.TestCase): def test_move_fs(self): + namespaces = ("details", "accessed", "metadata_changed", "modified") + src_fs = open_fs("mem://") src_fs.makedirs("foo/bar") src_fs.touch("test.txt") src_fs.touch("foo/bar/baz.txt") + src_file1_info = src_fs.getinfo("test.txt", namespaces) + src_file2_info = src_fs.getinfo("foo/bar/baz.txt", namespaces) dst_fs = open_fs("mem://") fs.move.move_fs(src_fs, dst_fs) self.assertTrue(dst_fs.isdir("foo/bar")) self.assertTrue(dst_fs.isfile("test.txt")) + self.assertTrue(dst_fs.isfile("foo/bar/baz.txt")) self.assertTrue(src_fs.isempty("/")) - def test_copy_dir(self): + if self.preserve_time: + dst_file1_info = dst_fs.getinfo("test.txt", namespaces) + dst_file2_info = dst_fs.getinfo("foo/bar/baz.txt", namespaces) + self.assertEqual(dst_file1_info.modified, src_file1_info.modified) + self.assertEqual(dst_file1_info.accessed, src_file1_info.accessed) + self.assertEqual( + dst_file1_info.metadata_changed, src_file1_info.metadata_changed + ) + self.assertEqual(dst_file2_info.modified, src_file2_info.modified) + self.assertEqual(dst_file2_info.accessed, src_file2_info.accessed) + self.assertEqual( + dst_file2_info.metadata_changed, src_file2_info.metadata_changed + ) + + def test_move_dir(self): + namespaces = ("details", "accessed", "metadata_changed", "modified") + src_fs = open_fs("mem://") src_fs.makedirs("foo/bar") src_fs.touch("test.txt") src_fs.touch("foo/bar/baz.txt") + src_file2_info = src_fs.getinfo("foo/bar/baz.txt", namespaces) dst_fs = open_fs("mem://") fs.move.move_dir(src_fs, "/foo", dst_fs, "/") @@ -32,3 +57,12 @@ def test_copy_dir(self): self.assertTrue(dst_fs.isdir("bar")) self.assertTrue(dst_fs.isfile("bar/baz.txt")) self.assertFalse(src_fs.exists("foo")) + self.assertTrue(src_fs.isfile("test.txt")) + + if self.preserve_time: + dst_file2_info = dst_fs.getinfo("bar/baz.txt", namespaces) + self.assertEqual(dst_file2_info.modified, src_file2_info.modified) + self.assertEqual(dst_file2_info.accessed, src_file2_info.accessed) + self.assertEqual( + dst_file2_info.metadata_changed, src_file2_info.metadata_changed + ) diff --git a/tests/test_opener.py b/tests/test_opener.py index e7fae983..bc2f5cd7 100644 --- a/tests/test_opener.py +++ b/tests/test_opener.py @@ -300,14 +300,26 @@ def test_user_data_opener(self, app_dir): def test_open_ftp(self, mock_FTPFS): open_fs("ftp://foo:bar@ftp.example.org") mock_FTPFS.assert_called_once_with( - "ftp.example.org", passwd="bar", port=21, user="foo", proxy=None, timeout=10, tls=False + "ftp.example.org", + passwd="bar", + port=21, + user="foo", + proxy=None, + timeout=10, + tls=False, ) @mock.patch("fs.ftpfs.FTPFS") def test_open_ftps(self, mock_FTPFS): open_fs("ftps://foo:bar@ftp.example.org") mock_FTPFS.assert_called_once_with( - "ftp.example.org", passwd="bar", port=21, user="foo", proxy=None, timeout=10, tls=True + "ftp.example.org", + passwd="bar", + port=21, + user="foo", + proxy=None, + timeout=10, + tls=True, ) @mock.patch("fs.ftpfs.FTPFS") diff --git a/tests/test_osfs.py b/tests/test_osfs.py index e43635f4..56927367 100644 --- a/tests/test_osfs.py +++ b/tests/test_osfs.py @@ -7,6 +7,7 @@ import shutil import tempfile import sys +import time import unittest from fs import osfs, open_fs @@ -87,6 +88,25 @@ def test_expand_vars(self): self.assertIn("TYRIONLANISTER", fs1.getsyspath("/")) self.assertNotIn("TYRIONLANISTER", fs2.getsyspath("/")) + def test_copy_preserve_time(self): + self.fs.makedir("foo") + self.fs.makedir("bar") + now = time.time() - 10000 + if not self.fs.create("foo/file.txt"): + raw_info = {"details": {"accessed": now, "modified": now}} + self.fs.setinfo("foo/file.txt", raw_info) + + namespaces = ("details", "accessed", "metadata_changed", "modified") + src_info = self.fs.getinfo("foo/file.txt", namespaces) + + self.fs.copy("foo/file.txt", "bar/file.txt", preserve_time=True) + self.assertTrue(self.fs.exists("bar/file.txt")) + + dst_info = self.fs.getinfo("bar/file.txt", namespaces) + self.assertEqual(dst_info.modified, src_info.modified) + self.assertEqual(dst_info.accessed, src_info.accessed) + self.assertEqual(dst_info.metadata_changed, src_info.metadata_changed) + @unittest.skipUnless(osfs.sendfile, "sendfile not supported") @unittest.skipIf( sys.version_info >= (3, 8), From f016533871b16b621b6b9ba542bea2e35225ea12 Mon Sep 17 00:00:00 2001 From: atollk Date: Sat, 20 Mar 2021 19:43:40 +0100 Subject: [PATCH 03/14] Added fs.copy.copy_metadata. All logic for the added `preserve_time` parameter is contained in that function, so other pieces of code just have to make a single call. It's still not implemented at this point. --- fs/_bulk.py | 43 +++++++++++++++++++++++-------------------- fs/base.py | 14 +++++++++----- fs/copy.py | 39 +++++++++++++++++++++++++++++++++------ fs/mirror.py | 6 +++--- fs/move.py | 5 +++-- fs/osfs.py | 2 -- fs/wrap.py | 4 ++-- fs/wrapfs.py | 6 +++--- 8 files changed, 76 insertions(+), 43 deletions(-) diff --git a/fs/_bulk.py b/fs/_bulk.py index 63cecc01..ec832945 100644 --- a/fs/_bulk.py +++ b/fs/_bulk.py @@ -56,20 +56,30 @@ def __call__(self): class _CopyTask(_Task): """A callable that copies from one file another.""" - def __init__(self, src_file, dst_file): - # type: (IO, IO) -> None - self.src_file = src_file - self.dst_file = dst_file + def __init__( + self, + src_fs, # type: FS + src_path, # type: Text + dst_fs, # type: FS + dst_path, # type: Text + preserve_time, # type: bool + ): + # type: (...) -> None + self.src_fs = src_fs + self.src_path = src_path + self.dst_fs = dst_fs + self.dst_path = dst_path + self.preserve_time = preserve_time def __call__(self): # type: () -> None - try: - copy_file_data(self.src_file, self.dst_file, chunk_size=1024 * 1024) - finally: - try: - self.src_file.close() - finally: - self.dst_file.close() + copy_file_internal( + self.src_fs, + self.src_path, + self.dst_fs, + self.dst_path, + preserve_time=self.preserve_time, + ) class Copier(object): @@ -88,7 +98,7 @@ def __init__(self, num_workers=4): def start(self): """Start the workers.""" if self.num_workers: - self.queue = Queue(maxsize=self.num_workers) + self.queue = Queue() self.workers = [_Worker(self) for _ in range(self.num_workers)] for worker in self.workers: worker.start() @@ -133,12 +143,5 @@ def copy(self, src_fs, src_path, dst_fs, dst_path, preserve_time=False): src_fs, src_path, dst_fs, dst_path, preserve_time=preserve_time ) else: - # TODO(preserve_time) - src_file = src_fs.openbin(src_path, "r") - try: - dst_file = dst_fs.openbin(dst_path, "w") - except Exception: - src_file.close() - raise - task = _CopyTask(src_file, dst_file) + task = _CopyTask(src_fs, src_path, dst_fs, dst_path, preserve_time) self.queue.put(task) diff --git a/fs/base.py b/fs/base.py index 03d4dac8..c86a5de6 100644 --- a/fs/base.py +++ b/fs/base.py @@ -22,6 +22,7 @@ import six from . import copy, errors, fsencode, iotools, move, tools, walk, wildcard +from .copy import copy_metadata from .glob import BoundGlobber from .mode import validate_open_mode from .path import abspath, join, normpath @@ -412,13 +413,13 @@ def copy( ``dst_path`` does not exist. """ - # TODO(preserve_time) with self._lock: if not overwrite and self.exists(dst_path): raise errors.DestinationExists(dst_path) with closing(self.open(src_path, "rb")) as read_file: # FIXME(@althonos): typing complains because open return IO self.upload(dst_path, read_file) # type: ignore + copy_metadata(self, src_path, self, dst_path) def copydir( self, @@ -1030,8 +1031,8 @@ def movedir(self, src_path, dst_path, create=False, preserve_time=False): dst_path (str): Path to destination directory. create (bool): If `True`, then ``dst_path`` will be created if it doesn't exist already (defaults to `False`). - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resources (defaults to `False`). + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resources (defaults to `False`). Raises: fs.errors.ResourceNotFound: if ``dst_path`` does not exist, @@ -1086,8 +1087,8 @@ def makedirs( raise return self.opendir(path) - def move(self, src_path, dst_path, overwrite=False): - # type: (Text, Text, bool) -> None + def move(self, src_path, dst_path, overwrite=False, preserve_time=False): + # type: (Text, Text, bool, bool) -> None """Move a file from ``src_path`` to ``dst_path``. Arguments: @@ -1096,6 +1097,8 @@ def move(self, src_path, dst_path, overwrite=False): file will be written to. overwrite (bool): If `True`, destination path will be overwritten if it exists. + preserve_time (bool): If `True`, try to preserve atime, ctime, + and mtime of the resources (defaults to `False`). Raises: fs.errors.FileExpected: If ``src_path`` maps to a @@ -1128,6 +1131,7 @@ def move(self, src_path, dst_path, overwrite=False): # FIXME(@althonos): typing complains because open return IO self.upload(dst_path, read_file) # type: ignore self.remove(src_path) + copy_metadata(self, src_path, self, dst_path) def open( self, diff --git a/fs/copy.py b/fs/copy.py index 6f9fc701..8cd2a55f 100644 --- a/fs/copy.py +++ b/fs/copy.py @@ -151,7 +151,6 @@ def copy_file( src_path, dst_path, overwrite=True, preserve_time=preserve_time ) else: - # TODO(preserve_time) # Standard copy with _src_fs.lock(), _dst_fs.lock(): if _dst_fs.hassyspath(dst_path): @@ -160,6 +159,7 @@ def copy_file( else: with _src_fs.openbin(src_path) as read_file: _dst_fs.upload(dst_path, read_file) + copy_metadata(_src_fs, src_path, _dst_fs, dst_path) def copy_file_internal( @@ -190,14 +190,17 @@ def copy_file_internal( # Same filesystem, so we can do a potentially optimized # copy src_fs.copy(src_path, dst_path, overwrite=True, preserve_time=preserve_time) - # TODO(preserve_time) - elif dst_fs.hassyspath(dst_path): + return + + if dst_fs.hassyspath(dst_path): with dst_fs.openbin(dst_path, "w") as write_file: src_fs.download(src_path, write_file) else: with src_fs.openbin(src_path) as read_file: dst_fs.upload(dst_path, read_file) + copy_metadata(src_fs, src_path, dst_fs, dst_path) + def copy_file_if_newer( src_fs, # type: Union[FS, Text] @@ -326,9 +329,10 @@ def dst(): from ._bulk import Copier with src() as _src_fs, dst() as _dst_fs: - with _src_fs.lock(), _dst_fs.lock(): - _thread_safe = is_thread_safe(_src_fs, _dst_fs) - with Copier(num_workers=workers if _thread_safe else 0) as copier: + _thread_safe = is_thread_safe(_src_fs, _dst_fs) + copier = Copier(num_workers=workers if _thread_safe else 0) + with copier: + with _src_fs.lock(), _dst_fs.lock(): _dst_fs.makedir(_dst_path, recreate=True) for dir_path, dirs, files in walker.walk(_src_fs, _src_path): copy_path = combine(_dst_path, frombase(_src_path, dir_path)) @@ -345,6 +349,7 @@ def dst(): preserve_time=preserve_time, ) on_copy(_src_fs, src_path, _dst_fs, dst_path) + pass def copy_dir_if_newer( @@ -438,3 +443,25 @@ def dst(): preserve_time=preserve_time, ) on_copy(_src_fs, dir_path, _dst_fs, copy_path) + + +def copy_metadata( + src_fs, # type: Union[FS, Text] + src_path, # type: Text + dst_fs, # type: Union[FS, Text] + dst_path, # type: Text +): + # type: (...) -> None + """Copies metadata from one file to another. + + Arguments: + src_fs (FS or str): Source filesystem (instance or URL). + src_path (str): Path to a directory on the source filesystem. + dst_fs (FS or str): Destination filesystem (instance or URL). + dst_path (str): Path to a directory on the destination filesystem. + + """ + # TODO(preserve_time) + with manage_fs(src_fs, writeable=False) as _src_fs: + with manage_fs(dst_fs, create=True) as _dst_fs: + pass diff --git a/fs/mirror.py b/fs/mirror.py index 6123aed2..1c674cbc 100644 --- a/fs/mirror.py +++ b/fs/mirror.py @@ -86,9 +86,9 @@ def dst(): return manage_fs(dst_fs, create=True) with src() as _src_fs, dst() as _dst_fs: - with _src_fs.lock(), _dst_fs.lock(): - _thread_safe = is_thread_safe(_src_fs, _dst_fs) - with Copier(num_workers=workers if _thread_safe else 0) as copier: + _thread_safe = is_thread_safe(_src_fs, _dst_fs) + with Copier(num_workers=workers if _thread_safe else 0) as copier: + with _src_fs.lock(), _dst_fs.lock(): _mirror( _src_fs, _dst_fs, diff --git a/fs/move.py b/fs/move.py index da941479..56dbcb98 100644 --- a/fs/move.py +++ b/fs/move.py @@ -55,12 +55,13 @@ def move_file( and mtime of the resources (defaults to `False`). """ - # TODO(preserve_time) with manage_fs(src_fs) as _src_fs: with manage_fs(dst_fs, create=True) as _dst_fs: if _src_fs is _dst_fs: # Same filesystem, may be optimized - _src_fs.move(src_path, dst_path, overwrite=True) + _src_fs.move( + src_path, dst_path, overwrite=True, preserve_time=preserve_time + ) else: # Standard copy and delete with _src_fs.lock(), _dst_fs.lock(): diff --git a/fs/osfs.py b/fs/osfs.py index 8c071c3d..47d612be 100644 --- a/fs/osfs.py +++ b/fs/osfs.py @@ -433,7 +433,6 @@ def _check_copy(self, src_path, dst_path, overwrite=False): def copy(self, src_path, dst_path, overwrite=False, preserve_time=False): # type: (Text, Text, bool, bool) -> None - # TODO(preserve_time) with self._lock: # validate and canonicalise paths _src_path, _dst_path = self._check_copy(src_path, dst_path, overwrite) @@ -464,7 +463,6 @@ def copy(self, src_path, dst_path, overwrite=False, preserve_time=False): def copy(self, src_path, dst_path, overwrite=False, preserve_time=False): # type: (Text, Text, bool, bool) -> None - # TODO(preserve_time) with self._lock: _src_path, _dst_path = self._check_copy(src_path, dst_path, overwrite) shutil.copy2(self.getsyspath(_src_path), self.getsyspath(_dst_path)) diff --git a/fs/wrap.py b/fs/wrap.py index 9ceae351..ce7e9d3c 100644 --- a/fs/wrap.py +++ b/fs/wrap.py @@ -181,8 +181,8 @@ def makedir( self.check() raise ResourceReadOnly(path) - def move(self, src_path, dst_path, overwrite=False): - # type: (Text, Text, bool) -> None + def move(self, src_path, dst_path, overwrite=False, preserve_time=False): + # type: (Text, Text, bool, bool) -> None self.check() raise ResourceReadOnly(dst_path) diff --git a/fs/wrapfs.py b/fs/wrapfs.py index 7be9c235..1419b5aa 100644 --- a/fs/wrapfs.py +++ b/fs/wrapfs.py @@ -167,15 +167,15 @@ def makedir( with unwrap_errors(path): return _fs.makedir(_path, permissions=permissions, recreate=recreate) - def move(self, src_path, dst_path, overwrite=False): - # type: (Text, Text, bool) -> None + def move(self, src_path, dst_path, overwrite=False, preserve_time=False): + # type: (Text, Text, bool, bool) -> None # A custom move permits a potentially optimized code path src_fs, _src_path = self.delegate_path(src_path) dst_fs, _dst_path = self.delegate_path(dst_path) with unwrap_errors({_src_path: src_path, _dst_path: dst_path}): if not overwrite and dst_fs.exists(_dst_path): raise errors.DestinationExists(_dst_path) - move_file(src_fs, _src_path, dst_fs, _dst_path) + move_file(src_fs, _src_path, dst_fs, _dst_path, preserve_time=preserve_time) def movedir(self, src_path, dst_path, create=False): # type: (Text, Text, bool) -> None From 34d26bce9c261dd8b3f67f1928ced5a56578a8c1 Mon Sep 17 00:00:00 2001 From: atollk Date: Sat, 20 Mar 2021 20:09:22 +0100 Subject: [PATCH 04/14] Removed preservation of access time in copy/move/mirror functions as changing the access time itself was an access. --- fs/_bulk.py | 3 +-- fs/base.py | 6 +++--- fs/copy.py | 18 ++++++++++++------ fs/wrapfs.py | 4 ++-- tests/test_copy.py | 26 ++++++++++++++++++++++---- tests/test_memoryfs.py | 1 - tests/test_mirror.py | 3 +-- tests/test_move.py | 3 --- 8 files changed, 41 insertions(+), 23 deletions(-) diff --git a/fs/_bulk.py b/fs/_bulk.py index ec832945..7ee1a2c7 100644 --- a/fs/_bulk.py +++ b/fs/_bulk.py @@ -13,12 +13,11 @@ from .copy import copy_file_internal from .errors import BulkCopyFailed -from .tools import copy_file_data if typing.TYPE_CHECKING: from .base import FS from types import TracebackType - from typing import IO, List, Optional, Text, Type + from typing import List, Optional, Text, Type class _Worker(threading.Thread): diff --git a/fs/base.py b/fs/base.py index c86a5de6..377bba72 100644 --- a/fs/base.py +++ b/fs/base.py @@ -22,7 +22,7 @@ import six from . import copy, errors, fsencode, iotools, move, tools, walk, wildcard -from .copy import copy_metadata +from .copy import copy_cmtime from .glob import BoundGlobber from .mode import validate_open_mode from .path import abspath, join, normpath @@ -419,7 +419,7 @@ def copy( with closing(self.open(src_path, "rb")) as read_file: # FIXME(@althonos): typing complains because open return IO self.upload(dst_path, read_file) # type: ignore - copy_metadata(self, src_path, self, dst_path) + copy_cmtime(self, src_path, self, dst_path) def copydir( self, @@ -1130,8 +1130,8 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): with self.open(src_path, "rb") as read_file: # FIXME(@althonos): typing complains because open return IO self.upload(dst_path, read_file) # type: ignore + copy_cmtime(self, src_path, self, dst_path) self.remove(src_path) - copy_metadata(self, src_path, self, dst_path) def open( self, diff --git a/fs/copy.py b/fs/copy.py index 8cd2a55f..5a252891 100644 --- a/fs/copy.py +++ b/fs/copy.py @@ -159,7 +159,7 @@ def copy_file( else: with _src_fs.openbin(src_path) as read_file: _dst_fs.upload(dst_path, read_file) - copy_metadata(_src_fs, src_path, _dst_fs, dst_path) + copy_cmtime(_src_fs, src_path, _dst_fs, dst_path) def copy_file_internal( @@ -199,7 +199,7 @@ def copy_file_internal( with src_fs.openbin(src_path) as read_file: dst_fs.upload(dst_path, read_file) - copy_metadata(src_fs, src_path, dst_fs, dst_path) + copy_cmtime(src_fs, src_path, dst_fs, dst_path) def copy_file_if_newer( @@ -445,14 +445,14 @@ def dst(): on_copy(_src_fs, dir_path, _dst_fs, copy_path) -def copy_metadata( +def copy_cmtime( src_fs, # type: Union[FS, Text] src_path, # type: Text dst_fs, # type: Union[FS, Text] dst_path, # type: Text ): # type: (...) -> None - """Copies metadata from one file to another. + """Copy modified time metadata from one file to another. Arguments: src_fs (FS or str): Source filesystem (instance or URL). @@ -461,7 +461,13 @@ def copy_metadata( dst_path (str): Path to a directory on the destination filesystem. """ - # TODO(preserve_time) + namespaces = ("details", "metadata_changed", "modified") with manage_fs(src_fs, writeable=False) as _src_fs: with manage_fs(dst_fs, create=True) as _dst_fs: - pass + src_meta = _src_fs.getinfo(src_path, namespaces) + src_details = src_meta.raw.get("details", {}) + dst_details = {} + for value in ("metadata_changed", "modified"): + if value in src_details: + dst_details[value] = src_details[value] + _dst_fs.setinfo(dst_path, {"details": dst_details}) diff --git a/fs/wrapfs.py b/fs/wrapfs.py index 1419b5aa..071878c2 100644 --- a/fs/wrapfs.py +++ b/fs/wrapfs.py @@ -177,8 +177,8 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): raise errors.DestinationExists(_dst_path) move_file(src_fs, _src_path, dst_fs, _dst_path, preserve_time=preserve_time) - def movedir(self, src_path, dst_path, create=False): - # type: (Text, Text, bool) -> None + def movedir(self, src_path, dst_path, create=False, preserve_time=False): + # type: (Text, Text, bool, bool) -> None src_fs, _src_path = self.delegate_path(src_path) dst_fs, _dst_path = self.delegate_path(dst_path) with unwrap_errors({_src_path: src_path, _dst_path: dst_path}): diff --git a/tests/test_copy.py b/tests/test_copy.py index 77c7f756..f2f279ba 100644 --- a/tests/test_copy.py +++ b/tests/test_copy.py @@ -37,12 +37,10 @@ def test_copy_fs(self, workers): dst_file1_info = dst_fs.getinfo("test.txt", namespaces) dst_file2_info = dst_fs.getinfo("foo/bar/baz.txt", namespaces) self.assertEqual(dst_file1_info.modified, src_file1_info.modified) - self.assertEqual(dst_file1_info.accessed, src_file1_info.accessed) self.assertEqual( dst_file1_info.metadata_changed, src_file1_info.metadata_changed ) self.assertEqual(dst_file2_info.modified, src_file2_info.modified) - self.assertEqual(dst_file2_info.accessed, src_file2_info.accessed) self.assertEqual( dst_file2_info.metadata_changed, src_file2_info.metadata_changed ) @@ -53,6 +51,28 @@ def test_copy_value_error(self): with self.assertRaises(ValueError): fs.copy.copy_fs(src_fs, dst_fs, workers=-1) + def test_copy_dir0(self): + namespaces = ("details", "accessed", "metadata_changed", "modified") + + src_fs = open_fs("mem://") + src_fs.makedirs("foo/bar") + src_fs.makedirs("foo/empty") + src_fs.touch("test.txt") + src_fs.touch("foo/bar/baz.txt") + src_file2_info = src_fs.getinfo("foo/bar/baz.txt", namespaces) + + with open_fs("mem://") as dst_fs: + fs.copy.copy_dir(src_fs, "/foo", dst_fs, "/", workers=0, preserve_time=True) + self.assertTrue(dst_fs.isdir("bar")) + self.assertTrue(dst_fs.isdir("empty")) + self.assertTrue(dst_fs.isfile("bar/baz.txt")) + + dst_file2_info = dst_fs.getinfo("bar/baz.txt", namespaces) + self.assertEqual(dst_file2_info.modified, src_file2_info.modified) + self.assertEqual( + dst_file2_info.metadata_changed, src_file2_info.metadata_changed + ) + @parameterized.expand([(0,), (1,), (2,), (4,)]) def test_copy_dir(self, workers): namespaces = ("details", "accessed", "metadata_changed", "modified") @@ -62,7 +82,6 @@ def test_copy_dir(self, workers): src_fs.makedirs("foo/empty") src_fs.touch("test.txt") src_fs.touch("foo/bar/baz.txt") - src_file1_info = src_fs.getinfo("test.txt", namespaces) src_file2_info = src_fs.getinfo("foo/bar/baz.txt", namespaces) with open_fs("mem://") as dst_fs: @@ -75,7 +94,6 @@ def test_copy_dir(self, workers): dst_file2_info = dst_fs.getinfo("bar/baz.txt", namespaces) self.assertEqual(dst_file2_info.modified, src_file2_info.modified) - self.assertEqual(dst_file2_info.accessed, src_file2_info.accessed) self.assertEqual( dst_file2_info.metadata_changed, src_file2_info.metadata_changed ) diff --git a/tests/test_memoryfs.py b/tests/test_memoryfs.py index 59502cbe..253cbb1f 100644 --- a/tests/test_memoryfs.py +++ b/tests/test_memoryfs.py @@ -80,5 +80,4 @@ def test_copy_preserve_time(self): dst_info = self.fs.getinfo("bar/file.txt", namespaces) self.assertEqual(dst_info.modified, src_info.modified) - self.assertEqual(dst_info.accessed, src_info.accessed) self.assertEqual(dst_info.metadata_changed, src_info.metadata_changed) diff --git a/tests/test_mirror.py b/tests/test_mirror.py index f204178f..1cce3d59 100644 --- a/tests/test_mirror.py +++ b/tests/test_mirror.py @@ -12,7 +12,7 @@ class TestMirror(unittest.TestCase): def _contents(self, fs): """Extract an FS in to a simple data structure.""" - namespaces = ("details", "accessed", "metadata_changed", "modified") + namespaces = ("details", "metadata_changed", "modified") contents = [] for path, dirs, files in fs.walk(): for info in dirs: @@ -28,7 +28,6 @@ def _contents(self, fs): "file", _bytes, _info.modified, - _info.accessed, _info.metadata_changed, ) ) diff --git a/tests/test_move.py b/tests/test_move.py index 8bba8857..04a93628 100644 --- a/tests/test_move.py +++ b/tests/test_move.py @@ -32,12 +32,10 @@ def test_move_fs(self): dst_file1_info = dst_fs.getinfo("test.txt", namespaces) dst_file2_info = dst_fs.getinfo("foo/bar/baz.txt", namespaces) self.assertEqual(dst_file1_info.modified, src_file1_info.modified) - self.assertEqual(dst_file1_info.accessed, src_file1_info.accessed) self.assertEqual( dst_file1_info.metadata_changed, src_file1_info.metadata_changed ) self.assertEqual(dst_file2_info.modified, src_file2_info.modified) - self.assertEqual(dst_file2_info.accessed, src_file2_info.accessed) self.assertEqual( dst_file2_info.metadata_changed, src_file2_info.metadata_changed ) @@ -62,7 +60,6 @@ def test_move_dir(self): if self.preserve_time: dst_file2_info = dst_fs.getinfo("bar/baz.txt", namespaces) self.assertEqual(dst_file2_info.modified, src_file2_info.modified) - self.assertEqual(dst_file2_info.accessed, src_file2_info.accessed) self.assertEqual( dst_file2_info.metadata_changed, src_file2_info.metadata_changed ) From 01f2d344696c421808fbe25dc27c1280f6398598 Mon Sep 17 00:00:00 2001 From: atollk Date: Sat, 27 Mar 2021 17:37:32 +0100 Subject: [PATCH 05/14] Removed previous change from setup.cfg. --- setup.cfg | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index f667b1a1..b90ebe47 100644 --- a/setup.cfg +++ b/setup.cfg @@ -133,7 +133,6 @@ sitepackages = false skip_missing_interpreters = true requires = setuptools >=38.3.0 - parameterized ~=0.8 [testenv] commands = pytest --cov={toxinidir}/fs {posargs} {toxinidir}/tests From b466bcb572311bc7b32b70f715ca5510a0e2c690 Mon Sep 17 00:00:00 2001 From: atollk Date: Thu, 1 Apr 2021 12:26:01 +0200 Subject: [PATCH 06/14] Added `preserve_time` parameter that was missing from `MemoryFS.move` and `MemoryFS.movedir`. --- fs/memoryfs.py | 4 ++-- tests/test_osfs.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/fs/memoryfs.py b/fs/memoryfs.py index 7965a135..60c9b365 100644 --- a/fs/memoryfs.py +++ b/fs/memoryfs.py @@ -444,7 +444,7 @@ def makedir( parent_dir.set_entry(dir_name, new_dir) return self.opendir(path) - def move(self, src_path, dst_path, overwrite=False): + def move(self, src_path, dst_path, overwrite=False, preserve_time=False): src_dir, src_name = split(self.validatepath(src_path)) dst_dir, dst_name = split(self.validatepath(dst_path)) @@ -465,7 +465,7 @@ def move(self, src_path, dst_path, overwrite=False): dst_dir_entry.set_entry(dst_name, src_entry) src_dir_entry.remove_entry(src_name) - def movedir(self, src_path, dst_path, create=False): + def movedir(self, src_path, dst_path, create=False, preserve_time=False): src_dir, src_name = split(self.validatepath(src_path)) dst_dir, dst_name = split(self.validatepath(dst_path)) diff --git a/tests/test_osfs.py b/tests/test_osfs.py index 704dee5e..ec5d8957 100644 --- a/tests/test_osfs.py +++ b/tests/test_osfs.py @@ -113,7 +113,6 @@ def test_copy_preserve_time(self): dst_info = self.fs.getinfo("bar/file.txt", namespaces) self.assertEqual(dst_info.modified, src_info.modified) - self.assertEqual(dst_info.accessed, src_info.accessed) self.assertEqual(dst_info.metadata_changed, src_info.metadata_changed) @unittest.skipUnless(osfs.sendfile, "sendfile not supported") From e6789572a2b7c5c1c56516c85c26050e3ff2358e Mon Sep 17 00:00:00 2001 From: atollk Date: Thu, 1 Apr 2021 12:40:41 +0200 Subject: [PATCH 07/14] For the newly added `preserve_time` parameter, reserved atime and ctime guarantees. As it turns out, there are platforms which do not allow changing either atime or ctime to custom values. Thus, the proposed behavior was impossible to implement. `preserve_time` now only makes guarantees about mtime. --- CHANGELOG.md | 3 +++ fs/base.py | 16 ++++++++-------- fs/copy.py | 32 ++++++++++++++++---------------- fs/mirror.py | 4 ++-- fs/move.py | 12 ++++++------ tests/test_copy.py | 18 +++--------------- tests/test_memoryfs.py | 6 ++---- tests/test_move.py | 13 ++----------- tests/test_osfs.py | 3 +-- tests/test_wrap.py | 8 ++++---- 10 files changed, 47 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bbf977f..41f3ff18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Better documentation of the `writable` parameter of `fs.open_fs`, and hint about using `fs.wrap.read_only` when a read-only filesystem is required. Closes [#441](https://github.com/PyFilesystem/pyfilesystem2/issues/441). +- Copy and move operations now provide a parameter `preserve_time` that, when + passed as `True`, makes sure the "mtime" of the destination file will be + the same as that of the source file. ### Changed diff --git a/fs/base.py b/fs/base.py index 7b2bf099..86644377 100644 --- a/fs/base.py +++ b/fs/base.py @@ -409,8 +409,8 @@ def copy( dst_path (str): Path to destination file. overwrite (bool): If `True`, overwrite the destination file if it exists (defaults to `False`). - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resource (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resource (defaults to `False`). Raises: fs.errors.DestinationExists: If ``dst_path`` exists, @@ -443,8 +443,8 @@ def copydir( dst_path (str): Path to destination directory. create (bool): If `True`, then ``dst_path`` will be created if it doesn't exist already (defaults to `False`). - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resource (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resource (defaults to `False`). Raises: fs.errors.ResourceNotFound: If the ``dst_path`` @@ -1054,8 +1054,8 @@ def movedir(self, src_path, dst_path, create=False, preserve_time=False): dst_path (str): Path to destination directory. create (bool): If `True`, then ``dst_path`` will be created if it doesn't exist already (defaults to `False`). - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resources (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resources (defaults to `False`). Raises: fs.errors.ResourceNotFound: if ``dst_path`` does not exist, @@ -1122,8 +1122,8 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): file will be written to. overwrite (bool): If `True`, destination path will be overwritten if it exists. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resources (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resources (defaults to `False`). Raises: fs.errors.FileExpected: If ``src_path`` maps to a diff --git a/fs/copy.py b/fs/copy.py index 5a252891..600d301b 100644 --- a/fs/copy.py +++ b/fs/copy.py @@ -40,8 +40,8 @@ def copy_fs( dst_path)``. workers (int): Use `worker` threads to copy data, or ``0`` (default) for a single-threaded copy. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resources (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resources (defaults to `False`). """ return copy_dir( @@ -76,8 +76,8 @@ def copy_fs_if_newer( dst_path)``. workers (int): Use ``worker`` threads to copy data, or ``0`` (default) for a single-threaded copy. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resources (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resources (defaults to `False`). """ return copy_dir_if_newer( @@ -138,8 +138,8 @@ def copy_file( src_path (str): Path to a file on the source filesystem. dst_fs (FS or str): Destination filesystem (instance or URL). dst_path (str): Path to a file on the destination filesystem. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resource (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resource (defaults to `False`). """ with manage_fs(src_fs, writeable=False) as _src_fs: @@ -182,8 +182,8 @@ def copy_file_internal( src_path (str): Path to a file on the source filesystem. dst_fs (FS): Destination filesystem. dst_path (str): Path to a file on the destination filesystem. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resource (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resource (defaults to `False`). """ if src_fs is dst_fs: @@ -223,8 +223,8 @@ def copy_file_if_newer( src_path (str): Path to a file on the source filesystem. dst_fs (FS or str): Destination filesystem (instance or URL). dst_path (str): Path to a file on the destination filesystem. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resource (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resource (defaults to `False`). Returns: bool: `True` if the file copy was executed, `False` otherwise. @@ -273,8 +273,8 @@ def copy_structure( walker (~fs.walk.Walker, optional): A walker object that will be used to scan for files in ``src_fs``. Set this if you only want to consider a sub-set of the resources in ``src_fs``. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resource (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resource (defaults to `False`). """ walker = walker or Walker() @@ -311,8 +311,8 @@ def copy_dir( ``(src_fs, src_path, dst_fs, dst_path)``. workers (int): Use ``worker`` threads to copy data, or ``0`` (default) for a single-threaded copy. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resources (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resources (defaults to `False`). """ on_copy = on_copy or (lambda *args: None) @@ -383,8 +383,8 @@ def copy_dir_if_newer( ``(src_fs, src_path, dst_fs, dst_path)``. workers (int): Use ``worker`` threads to copy data, or ``0`` (default) for a single-threaded copy. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resources (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resources (defaults to `False`). """ on_copy = on_copy or (lambda *args: None) diff --git a/fs/mirror.py b/fs/mirror.py index 1c674cbc..15b067fe 100644 --- a/fs/mirror.py +++ b/fs/mirror.py @@ -74,8 +74,8 @@ def mirror( workers (int): Number of worker threads used (0 for single threaded). Set to a relatively low number for network filesystems, 4 would be a good start. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resources (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resources (defaults to `False`). """ diff --git a/fs/move.py b/fs/move.py index 56dbcb98..56e9e5ca 100644 --- a/fs/move.py +++ b/fs/move.py @@ -29,8 +29,8 @@ def move_fs( dst_fs (FS or str): Destination filesystem (instance or URL). workers (int): Use `worker` threads to copy data, or ``0`` (default) for a single-threaded copy. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resources (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resources (defaults to `False`). """ move_dir(src_fs, "/", dst_fs, "/", workers=workers, preserve_time=preserve_time) @@ -51,8 +51,8 @@ def move_file( src_path (str): Path to a file on ``src_fs``. dst_fs (FS or str): Destination filesystem (instance or URL). dst_path (str): Path to a file on ``dst_fs``. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resources (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resources (defaults to `False`). """ with manage_fs(src_fs) as _src_fs: @@ -93,8 +93,8 @@ def move_dir( dst_path (str): Path to a directory on ``dst_fs``. workers (int): Use ``worker`` threads to copy data, or ``0`` (default) for a single-threaded copy. - preserve_time (bool): If `True`, try to preserve atime, ctime, - and mtime of the resources (defaults to `False`). + preserve_time (bool): If `True`, try to preserve mtime of the + resources (defaults to `False`). """ diff --git a/tests/test_copy.py b/tests/test_copy.py index f2f279ba..5c4fd0c8 100644 --- a/tests/test_copy.py +++ b/tests/test_copy.py @@ -17,7 +17,7 @@ class TestCopy(unittest.TestCase): @parameterized.expand([(0,), (1,), (2,), (4,)]) def test_copy_fs(self, workers): - namespaces = ("details", "accessed", "metadata_changed", "modified") + namespaces = ("details", "modified") src_fs = open_fs("mem://") src_fs.makedirs("foo/bar") @@ -37,13 +37,7 @@ def test_copy_fs(self, workers): dst_file1_info = dst_fs.getinfo("test.txt", namespaces) dst_file2_info = dst_fs.getinfo("foo/bar/baz.txt", namespaces) self.assertEqual(dst_file1_info.modified, src_file1_info.modified) - self.assertEqual( - dst_file1_info.metadata_changed, src_file1_info.metadata_changed - ) self.assertEqual(dst_file2_info.modified, src_file2_info.modified) - self.assertEqual( - dst_file2_info.metadata_changed, src_file2_info.metadata_changed - ) def test_copy_value_error(self): src_fs = open_fs("mem://") @@ -52,7 +46,7 @@ def test_copy_value_error(self): fs.copy.copy_fs(src_fs, dst_fs, workers=-1) def test_copy_dir0(self): - namespaces = ("details", "accessed", "metadata_changed", "modified") + namespaces = ("details", "modified") src_fs = open_fs("mem://") src_fs.makedirs("foo/bar") @@ -69,13 +63,10 @@ def test_copy_dir0(self): dst_file2_info = dst_fs.getinfo("bar/baz.txt", namespaces) self.assertEqual(dst_file2_info.modified, src_file2_info.modified) - self.assertEqual( - dst_file2_info.metadata_changed, src_file2_info.metadata_changed - ) @parameterized.expand([(0,), (1,), (2,), (4,)]) def test_copy_dir(self, workers): - namespaces = ("details", "accessed", "metadata_changed", "modified") + namespaces = ("details", "modified") src_fs = open_fs("mem://") src_fs.makedirs("foo/bar") @@ -94,9 +85,6 @@ def test_copy_dir(self, workers): dst_file2_info = dst_fs.getinfo("bar/baz.txt", namespaces) self.assertEqual(dst_file2_info.modified, src_file2_info.modified) - self.assertEqual( - dst_file2_info.metadata_changed, src_file2_info.metadata_changed - ) def test_copy_large(self): data1 = b"foo" * 512 * 1024 diff --git a/tests/test_memoryfs.py b/tests/test_memoryfs.py index ea6b62c9..bb5096f9 100644 --- a/tests/test_memoryfs.py +++ b/tests/test_memoryfs.py @@ -72,7 +72,7 @@ def test_copy_preserve_time(self): self.fs.makedir("bar") self.fs.touch("foo/file.txt") - namespaces = ("details", "accessed", "metadata_changed", "modified") + namespaces = ("details", "modified") src_info = self.fs.getinfo("foo/file.txt", namespaces) self.fs.copy("foo/file.txt", "bar/file.txt", preserve_time=True) @@ -80,11 +80,9 @@ def test_copy_preserve_time(self): dst_info = self.fs.getinfo("bar/file.txt", namespaces) self.assertEqual(dst_info.modified, src_info.modified) - self.assertEqual(dst_info.metadata_changed, src_info.metadata_changed) - -class TestMemoryFile(unittest.TestCase): +class TestMemoryFile(unittest.TestCase): def setUp(self): self.fs = memoryfs.MemoryFS() diff --git a/tests/test_move.py b/tests/test_move.py index 04a93628..5d5a4059 100644 --- a/tests/test_move.py +++ b/tests/test_move.py @@ -11,7 +11,7 @@ @parameterized_class(("preserve_time",), [(True,), (False,)]) class TestMove(unittest.TestCase): def test_move_fs(self): - namespaces = ("details", "accessed", "metadata_changed", "modified") + namespaces = ("details", "modified") src_fs = open_fs("mem://") src_fs.makedirs("foo/bar") @@ -32,16 +32,10 @@ def test_move_fs(self): dst_file1_info = dst_fs.getinfo("test.txt", namespaces) dst_file2_info = dst_fs.getinfo("foo/bar/baz.txt", namespaces) self.assertEqual(dst_file1_info.modified, src_file1_info.modified) - self.assertEqual( - dst_file1_info.metadata_changed, src_file1_info.metadata_changed - ) self.assertEqual(dst_file2_info.modified, src_file2_info.modified) - self.assertEqual( - dst_file2_info.metadata_changed, src_file2_info.metadata_changed - ) def test_move_dir(self): - namespaces = ("details", "accessed", "metadata_changed", "modified") + namespaces = ("details", "modified") src_fs = open_fs("mem://") src_fs.makedirs("foo/bar") @@ -60,6 +54,3 @@ def test_move_dir(self): if self.preserve_time: dst_file2_info = dst_fs.getinfo("bar/baz.txt", namespaces) self.assertEqual(dst_file2_info.modified, src_file2_info.modified) - self.assertEqual( - dst_file2_info.metadata_changed, src_file2_info.metadata_changed - ) diff --git a/tests/test_osfs.py b/tests/test_osfs.py index ec5d8957..6d2b11f6 100644 --- a/tests/test_osfs.py +++ b/tests/test_osfs.py @@ -105,7 +105,7 @@ def test_copy_preserve_time(self): raw_info = {"details": {"accessed": now, "modified": now}} self.fs.setinfo("foo/file.txt", raw_info) - namespaces = ("details", "accessed", "metadata_changed", "modified") + namespaces = ("details", "modified") src_info = self.fs.getinfo("foo/file.txt", namespaces) self.fs.copy("foo/file.txt", "bar/file.txt", preserve_time=True) @@ -113,7 +113,6 @@ def test_copy_preserve_time(self): dst_info = self.fs.getinfo("bar/file.txt", namespaces) self.assertEqual(dst_info.modified, src_info.modified) - self.assertEqual(dst_info.metadata_changed, src_info.metadata_changed) @unittest.skipUnless(osfs.sendfile, "sendfile not supported") @unittest.skipIf( diff --git a/tests/test_wrap.py b/tests/test_wrap.py index 89a91187..e11ded4a 100644 --- a/tests/test_wrap.py +++ b/tests/test_wrap.py @@ -177,7 +177,7 @@ def test_scandir(self): ] with mock.patch.object(self.fs, "scandir", wraps=self.fs.scandir) as scandir: self.assertEqual(sorted(self.cached.scandir("/"), key=key), expected) - scandir.assert_has_calls([mock.call('/', namespaces=None, page=None)]) + scandir.assert_has_calls([mock.call("/", namespaces=None, page=None)]) with mock.patch.object(self.fs, "scandir", wraps=self.fs.scandir) as scandir: self.assertEqual(sorted(self.cached.scandir("/"), key=key), expected) scandir.assert_not_called() @@ -187,7 +187,7 @@ def test_isdir(self): self.assertTrue(self.cached.isdir("foo")) self.assertFalse(self.cached.isdir("egg")) # is file self.assertFalse(self.cached.isdir("spam")) # doesn't exist - scandir.assert_has_calls([mock.call('/', namespaces=None, page=None)]) + scandir.assert_has_calls([mock.call("/", namespaces=None, page=None)]) with mock.patch.object(self.fs, "scandir", wraps=self.fs.scandir) as scandir: self.assertTrue(self.cached.isdir("foo")) self.assertFalse(self.cached.isdir("egg")) @@ -199,7 +199,7 @@ def test_isfile(self): self.assertTrue(self.cached.isfile("egg")) self.assertFalse(self.cached.isfile("foo")) # is dir self.assertFalse(self.cached.isfile("spam")) # doesn't exist - scandir.assert_has_calls([mock.call('/', namespaces=None, page=None)]) + scandir.assert_has_calls([mock.call("/", namespaces=None, page=None)]) with mock.patch.object(self.fs, "scandir", wraps=self.fs.scandir) as scandir: self.assertTrue(self.cached.isfile("egg")) self.assertFalse(self.cached.isfile("foo")) @@ -211,7 +211,7 @@ def test_getinfo(self): self.assertEqual(self.cached.getinfo("foo"), self.fs.getinfo("foo")) self.assertEqual(self.cached.getinfo("/"), self.fs.getinfo("/")) self.assertNotFound(self.cached.getinfo, "spam") - scandir.assert_has_calls([mock.call('/', namespaces=None, page=None)]) + scandir.assert_has_calls([mock.call("/", namespaces=None, page=None)]) with mock.patch.object(self.fs, "scandir", wraps=self.fs.scandir) as scandir: self.assertEqual(self.cached.getinfo("foo"), self.fs.getinfo("foo")) self.assertEqual(self.cached.getinfo("/"), self.fs.getinfo("/")) From 9b432a2612f8c0dc8292a88aefec185e1a017c77 Mon Sep 17 00:00:00 2001 From: atollk Date: Thu, 1 Apr 2021 13:01:22 +0200 Subject: [PATCH 08/14] Fixed `OSFS.copy(..., preserve_time=True)`. --- fs/base.py | 6 +++--- fs/copy.py | 6 +++--- fs/osfs.py | 3 +++ tests/test_osfs.py | 7 +++---- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/fs/base.py b/fs/base.py index 86644377..1363931a 100644 --- a/fs/base.py +++ b/fs/base.py @@ -22,7 +22,7 @@ import six from . import copy, errors, fsencode, iotools, move, tools, walk, wildcard -from .copy import copy_cmtime +from .copy import copy_mtime from .glob import BoundGlobber from .mode import validate_open_mode from .path import abspath, join, normpath @@ -426,7 +426,7 @@ def copy( with closing(self.open(src_path, "rb")) as read_file: # FIXME(@althonos): typing complains because open return IO self.upload(dst_path, read_file) # type: ignore - copy_cmtime(self, src_path, self, dst_path) + copy_mtime(self, src_path, self, dst_path) def copydir( self, @@ -1155,7 +1155,7 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): with self.open(src_path, "rb") as read_file: # FIXME(@althonos): typing complains because open return IO self.upload(dst_path, read_file) # type: ignore - copy_cmtime(self, src_path, self, dst_path) + copy_mtime(self, src_path, self, dst_path) self.remove(src_path) def open( diff --git a/fs/copy.py b/fs/copy.py index 600d301b..15a9fc84 100644 --- a/fs/copy.py +++ b/fs/copy.py @@ -159,7 +159,7 @@ def copy_file( else: with _src_fs.openbin(src_path) as read_file: _dst_fs.upload(dst_path, read_file) - copy_cmtime(_src_fs, src_path, _dst_fs, dst_path) + copy_mtime(_src_fs, src_path, _dst_fs, dst_path) def copy_file_internal( @@ -199,7 +199,7 @@ def copy_file_internal( with src_fs.openbin(src_path) as read_file: dst_fs.upload(dst_path, read_file) - copy_cmtime(src_fs, src_path, dst_fs, dst_path) + copy_mtime(src_fs, src_path, dst_fs, dst_path) def copy_file_if_newer( @@ -445,7 +445,7 @@ def dst(): on_copy(_src_fs, dir_path, _dst_fs, copy_path) -def copy_cmtime( +def copy_mtime( src_fs, # type: Union[FS, Text] src_path, # type: Text dst_fs, # type: Union[FS, Text] diff --git a/fs/osfs.py b/fs/osfs.py index 359876dc..4ffe7188 100644 --- a/fs/osfs.py +++ b/fs/osfs.py @@ -49,6 +49,7 @@ from .mode import Mode, validate_open_mode from .errors import FileExpected, NoURL from ._url_tools import url_quote +from .copy import copy_mtime if typing.TYPE_CHECKING: from typing import ( @@ -452,6 +453,8 @@ def copy(self, src_path, dst_path, overwrite=False, preserve_time=False): while sent > 0: sent = sendfile(fd_dst, fd_src, offset, maxsize) offset += sent + if preserve_time: + copy_mtime(self, src_path, self, dst_path) except OSError as e: # the error is not a simple "sendfile not supported" error if e.errno not in self._sendfile_error_codes: diff --git a/tests/test_osfs.py b/tests/test_osfs.py index 6d2b11f6..ff3af9e5 100644 --- a/tests/test_osfs.py +++ b/tests/test_osfs.py @@ -100,10 +100,9 @@ def test_expand_vars(self): def test_copy_preserve_time(self): self.fs.makedir("foo") self.fs.makedir("bar") - now = time.time() - 10000 - if not self.fs.create("foo/file.txt"): - raw_info = {"details": {"accessed": now, "modified": now}} - self.fs.setinfo("foo/file.txt", raw_info) + self.fs.create("foo/file.txt") + raw_info = {"details": {"modified": time.time() - 10000}} + self.fs.setinfo("foo/file.txt", raw_info) namespaces = ("details", "modified") src_info = self.fs.getinfo("foo/file.txt", namespaces) From 09cf4dc628e98df294835abf91813ea4e17008a1 Mon Sep 17 00:00:00 2001 From: atollk Date: Wed, 7 Apr 2021 19:28:13 +0200 Subject: [PATCH 09/14] Added support for MFMT command to `FTPFS.setinfo` to set the last modified time of a file. --- fs/copy.py | 2 +- fs/ftpfs.py | 27 +++++++++++++++++++++++---- tests/test_ftpfs.py | 26 +++++++++++++++++++++++--- 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/fs/copy.py b/fs/copy.py index 15a9fc84..0260b30b 100644 --- a/fs/copy.py +++ b/fs/copy.py @@ -461,7 +461,7 @@ def copy_mtime( dst_path (str): Path to a directory on the destination filesystem. """ - namespaces = ("details", "metadata_changed", "modified") + namespaces = ("details",) with manage_fs(src_fs, writeable=False) as _src_fs: with manage_fs(dst_fs, create=True) as _dst_fs: src_meta = _src_fs.getinfo(src_path, namespaces) diff --git a/fs/ftpfs.py b/fs/ftpfs.py index 515709df..79462de0 100644 --- a/fs/ftpfs.py +++ b/fs/ftpfs.py @@ -6,6 +6,7 @@ import array import calendar +import datetime import io import itertools import socket @@ -19,8 +20,7 @@ from ftplib import FTP_TLS except ImportError as err: FTP_TLS = err # type: ignore -from ftplib import error_perm -from ftplib import error_temp +from ftplib import error_perm, error_temp from typing import cast from six import PY2 @@ -836,8 +836,27 @@ def writebytes(self, path, contents): def setinfo(self, path, info): # type: (Text, RawInfo) -> None - if not self.exists(path): - raise errors.ResourceNotFound(path) + current_info = self.getinfo(path) + if current_info.is_file and "MFMT" in self.features: + mtime = 0.0 + if "modified" in info: + mtime = float(cast(float, info["modified"]["modified"])) + if "details" in info: + mtime = float(cast(float, info["details"]["modified"])) + if mtime: + with ftp_errors(self, path): + cmd = ( + "MFMT " + + datetime.datetime.fromtimestamp(mtime).strftime( + "%Y%m%d%H%M%S" + ) + + " " + + _encode(path, self.ftp.encoding) + ) + try: + self.ftp.sendcmd(cmd) + except error_perm: + pass def readbytes(self, path): # type: (Text) -> bytes diff --git a/tests/test_ftpfs.py b/tests/test_ftpfs.py index b0e0c1f0..86d72ef0 100644 --- a/tests/test_ftpfs.py +++ b/tests/test_ftpfs.py @@ -11,6 +11,7 @@ import time import unittest import uuid +import datetime try: from unittest import mock @@ -144,7 +145,6 @@ def test_manager_with_host(self): @mark.slow @unittest.skipIf(platform.python_implementation() == "PyPy", "ftp unreliable with PyPy") class TestFTPFS(FSTestCases, unittest.TestCase): - user = "user" pasw = "1234" @@ -162,7 +162,7 @@ def setUpClass(cls): cls.server.shutdown_after = -1 cls.server.handler.authorizer = DummyAuthorizer() cls.server.handler.authorizer.add_user( - cls.user, cls.pasw, cls._temp_path, perm="elradfmw" + cls.user, cls.pasw, cls._temp_path, perm="elradfmwT" ) cls.server.handler.authorizer.add_anonymous(cls._temp_path) cls.server.start() @@ -223,6 +223,27 @@ def test_geturl(self): ), ) + def test_setinfo(self): + # TODO: temporary test, since FSTestCases.test_setinfo is broken. + self.fs.create("bar") + time1 = time.mktime(datetime.datetime.now().timetuple()) + time2 = time1 + 60 + self.fs.setinfo("bar", {"details": {"modified": time1}}) + mtime1 = self.fs.getinfo("bar", ("details",)).modified + self.fs.setinfo("bar", {"details": {"modified": time2}}) + mtime2 = self.fs.getinfo("bar", ("details",)).modified + replacement = {} + if mtime1.microsecond == 0 or mtime2.microsecond == 0: + mtime1 = mtime1.replace(microsecond=0) + mtime2 = mtime2.replace(microsecond=0) + if mtime1.second == 0 or mtime2.second == 0: + mtime1 = mtime1.replace(second=0) + mtime2 = mtime2.replace(second=0) + mtime2_modified = mtime2.replace(minute=mtime2.minute - 1) + self.assertEqual( + mtime1.replace(**replacement), mtime2_modified.replace(**replacement) + ) + def test_host(self): self.assertEqual(self.fs.host, self.server.host) @@ -301,7 +322,6 @@ def test_features(self): @mark.slow @unittest.skipIf(platform.python_implementation() == "PyPy", "ftp unreliable with PyPy") class TestAnonFTPFS(FSTestCases, unittest.TestCase): - user = "anonymous" pasw = "" From b4b3478e315806bfc387cd269052f80e5c4aaa10 Mon Sep 17 00:00:00 2001 From: atollk Date: Wed, 7 Apr 2021 19:59:28 +0200 Subject: [PATCH 10/14] Fixed the newly added preserve_time parameter in fs.copy actually being used, rather than being assumed to be True always. --- fs/copy.py | 15 ++++++++++----- tests/test_move.py | 4 ++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/fs/copy.py b/fs/copy.py index 0260b30b..521517cf 100644 --- a/fs/copy.py +++ b/fs/copy.py @@ -45,7 +45,14 @@ def copy_fs( """ return copy_dir( - src_fs, "/", dst_fs, "/", walker=walker, on_copy=on_copy, workers=workers + src_fs, + "/", + dst_fs, + "/", + walker=walker, + on_copy=on_copy, + workers=workers, + preserve_time=preserve_time, ) @@ -199,7 +206,8 @@ def copy_file_internal( with src_fs.openbin(src_path) as read_file: dst_fs.upload(dst_path, read_file) - copy_mtime(src_fs, src_path, dst_fs, dst_path) + if preserve_time: + copy_mtime(src_fs, src_path, dst_fs, dst_path) def copy_file_if_newer( @@ -262,7 +270,6 @@ def copy_structure( src_fs, # type: Union[FS, Text] dst_fs, # type: Union[FS, Text] walker=None, # type: Optional[Walker] - preserve_time=False, # type: bool ): # type: (...) -> None """Copy directories (but not files) from ``src_fs`` to ``dst_fs``. @@ -273,8 +280,6 @@ def copy_structure( walker (~fs.walk.Walker, optional): A walker object that will be used to scan for files in ``src_fs``. Set this if you only want to consider a sub-set of the resources in ``src_fs``. - preserve_time (bool): If `True`, try to preserve mtime of the - resource (defaults to `False`). """ walker = walker or Walker() diff --git a/tests/test_move.py b/tests/test_move.py index 5d5a4059..72f48210 100644 --- a/tests/test_move.py +++ b/tests/test_move.py @@ -21,7 +21,7 @@ def test_move_fs(self): src_file2_info = src_fs.getinfo("foo/bar/baz.txt", namespaces) dst_fs = open_fs("mem://") - fs.move.move_fs(src_fs, dst_fs) + fs.move.move_fs(src_fs, dst_fs, preserve_time=self.preserve_time) self.assertTrue(dst_fs.isdir("foo/bar")) self.assertTrue(dst_fs.isfile("test.txt")) @@ -44,7 +44,7 @@ def test_move_dir(self): src_file2_info = src_fs.getinfo("foo/bar/baz.txt", namespaces) dst_fs = open_fs("mem://") - fs.move.move_dir(src_fs, "/foo", dst_fs, "/") + fs.move.move_dir(src_fs, "/foo", dst_fs, "/", preserve_time=self.preserve_time) self.assertTrue(dst_fs.isdir("bar")) self.assertTrue(dst_fs.isfile("bar/baz.txt")) From 224952cf42f45470fcf3c3b1f57a1d3557552179 Mon Sep 17 00:00:00 2001 From: atollk Date: Thu, 8 Apr 2021 11:44:01 +0200 Subject: [PATCH 11/14] Changes proposed by code review. --- fs/_bulk.py | 64 ++++++++++++++++++++++++++-------------------- fs/base.py | 10 +++++--- fs/copy.py | 21 +++++++-------- fs/memoryfs.py | 7 +++++ fs/mirror.py | 4 ++- fs/osfs.py | 4 +-- fs/wrapfs.py | 2 +- tests/test_move.py | 6 +++++ 8 files changed, 73 insertions(+), 45 deletions(-) diff --git a/fs/_bulk.py b/fs/_bulk.py index 7ee1a2c7..12eef85e 100644 --- a/fs/_bulk.py +++ b/fs/_bulk.py @@ -11,13 +11,14 @@ from six.moves.queue import Queue -from .copy import copy_file_internal +from .copy import copy_file_internal, copy_modified_time from .errors import BulkCopyFailed +from .tools import copy_file_data if typing.TYPE_CHECKING: from .base import FS from types import TracebackType - from typing import List, Optional, Text, Type + from typing import List, Optional, Text, Type, IO, Tuple class _Worker(threading.Thread): @@ -55,40 +56,32 @@ def __call__(self): class _CopyTask(_Task): """A callable that copies from one file another.""" - def __init__( - self, - src_fs, # type: FS - src_path, # type: Text - dst_fs, # type: FS - dst_path, # type: Text - preserve_time, # type: bool - ): - # type: (...) -> None - self.src_fs = src_fs - self.src_path = src_path - self.dst_fs = dst_fs - self.dst_path = dst_path - self.preserve_time = preserve_time + def __init__(self, src_file, dst_file): + # type: (IO, IO) -> None + self.src_file = src_file + self.dst_file = dst_file def __call__(self): # type: () -> None - copy_file_internal( - self.src_fs, - self.src_path, - self.dst_fs, - self.dst_path, - preserve_time=self.preserve_time, - ) + try: + copy_file_data(self.src_file, self.dst_file, chunk_size=1024 * 1024) + finally: + try: + self.src_file.close() + finally: + self.dst_file.close() class Copier(object): """Copy files in worker threads.""" - def __init__(self, num_workers=4): - # type: (int) -> None + def __init__(self, num_workers=4, preserve_time=False): + # type: (int, bool) -> None if num_workers < 0: raise ValueError("num_workers must be >= 0") self.num_workers = num_workers + self.preserve_time = preserve_time + self.all_tasks = [] # type: List[Tuple[FS, Text, FS, Text]] self.queue = None # type: Optional[Queue[_Task]] self.workers = [] # type: List[_Worker] self.errors = [] # type: List[Exception] @@ -97,7 +90,7 @@ def __init__(self, num_workers=4): def start(self): """Start the workers.""" if self.num_workers: - self.queue = Queue() + self.queue = Queue(maxsize=self.num_workers) self.workers = [_Worker(self) for _ in range(self.num_workers)] for worker in self.workers: worker.start() @@ -106,10 +99,18 @@ def start(self): def stop(self): """Stop the workers (will block until they are finished).""" if self.running and self.num_workers: + # Notify the workers that all tasks have arrived + # and wait for them to finish. for _worker in self.workers: self.queue.put(None) for worker in self.workers: worker.join() + + # If the "last modified" time is to be preserved, do it now. + if self.preserve_time: + for args in self.all_tasks: + copy_modified_time(*args) + # Free up references held by workers del self.workers[:] self.queue.join() @@ -139,8 +140,15 @@ def copy(self, src_fs, src_path, dst_fs, dst_path, preserve_time=False): if self.queue is None: # This should be the most performant for a single-thread copy_file_internal( - src_fs, src_path, dst_fs, dst_path, preserve_time=preserve_time + src_fs, src_path, dst_fs, dst_path, preserve_time=self.preserve_time ) else: - task = _CopyTask(src_fs, src_path, dst_fs, dst_path, preserve_time) + self.all_tasks.append((src_fs, src_path, dst_fs, dst_path)) + src_file = src_fs.openbin(src_path, "r") + try: + dst_file = dst_fs.openbin(dst_path, "w") + except Exception: + src_file.close() + raise + task = _CopyTask(src_file, dst_file) self.queue.put(task) diff --git a/fs/base.py b/fs/base.py index 1363931a..8d8ee4c8 100644 --- a/fs/base.py +++ b/fs/base.py @@ -22,7 +22,7 @@ import six from . import copy, errors, fsencode, iotools, move, tools, walk, wildcard -from .copy import copy_mtime +from .copy import copy_modified_time from .glob import BoundGlobber from .mode import validate_open_mode from .path import abspath, join, normpath @@ -426,7 +426,8 @@ def copy( with closing(self.open(src_path, "rb")) as read_file: # FIXME(@althonos): typing complains because open return IO self.upload(dst_path, read_file) # type: ignore - copy_mtime(self, src_path, self, dst_path) + if preserve_time: + copy_modified_time(self, src_path, self, dst_path) def copydir( self, @@ -1150,12 +1151,15 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): except OSError: pass else: + if preserve_time: + copy_modified_time(self, src_path, self, dst_path) return with self._lock: with self.open(src_path, "rb") as read_file: # FIXME(@althonos): typing complains because open return IO self.upload(dst_path, read_file) # type: ignore - copy_mtime(self, src_path, self, dst_path) + if preserve_time: + copy_modified_time(self, src_path, self, dst_path) self.remove(src_path) def open( diff --git a/fs/copy.py b/fs/copy.py index 521517cf..701d8215 100644 --- a/fs/copy.py +++ b/fs/copy.py @@ -166,7 +166,7 @@ def copy_file( else: with _src_fs.openbin(src_path) as read_file: _dst_fs.upload(dst_path, read_file) - copy_mtime(_src_fs, src_path, _dst_fs, dst_path) + copy_modified_time(_src_fs, src_path, _dst_fs, dst_path) def copy_file_internal( @@ -207,7 +207,7 @@ def copy_file_internal( dst_fs.upload(dst_path, read_file) if preserve_time: - copy_mtime(src_fs, src_path, dst_fs, dst_path) + copy_modified_time(src_fs, src_path, dst_fs, dst_path) def copy_file_if_newer( @@ -334,10 +334,11 @@ def dst(): from ._bulk import Copier with src() as _src_fs, dst() as _dst_fs: - _thread_safe = is_thread_safe(_src_fs, _dst_fs) - copier = Copier(num_workers=workers if _thread_safe else 0) - with copier: - with _src_fs.lock(), _dst_fs.lock(): + with _src_fs.lock(), _dst_fs.lock(): + _thread_safe = is_thread_safe(_src_fs, _dst_fs) + with Copier( + num_workers=workers if _thread_safe else 0, preserve_time=preserve_time + ) as copier: _dst_fs.makedir(_dst_path, recreate=True) for dir_path, dirs, files in walker.walk(_src_fs, _src_path): copy_path = combine(_dst_path, frombase(_src_path, dir_path)) @@ -351,7 +352,6 @@ def dst(): src_path, _dst_fs, dst_path, - preserve_time=preserve_time, ) on_copy(_src_fs, src_path, _dst_fs, dst_path) pass @@ -408,7 +408,9 @@ def dst(): with src() as _src_fs, dst() as _dst_fs: with _src_fs.lock(), _dst_fs.lock(): _thread_safe = is_thread_safe(_src_fs, _dst_fs) - with Copier(num_workers=workers if _thread_safe else 0) as copier: + with Copier( + num_workers=workers if _thread_safe else 0, preserve_time=preserve_time + ) as copier: _dst_fs.makedir(_dst_path, recreate=True) namespace = ("details", "modified") dst_state = { @@ -445,12 +447,11 @@ def dst(): dir_path, _dst_fs, copy_path, - preserve_time=preserve_time, ) on_copy(_src_fs, dir_path, _dst_fs, copy_path) -def copy_mtime( +def copy_modified_time( src_fs, # type: Union[FS, Text] src_path, # type: Text dst_fs, # type: Union[FS, Text] diff --git a/fs/memoryfs.py b/fs/memoryfs.py index 60c9b365..c34d60a7 100644 --- a/fs/memoryfs.py +++ b/fs/memoryfs.py @@ -15,6 +15,7 @@ from . import errors from .base import FS +from .copy import copy_modified_time from .enums import ResourceType, Seek from .info import Info from .mode import Mode @@ -465,6 +466,9 @@ def move(self, src_path, dst_path, overwrite=False, preserve_time=False): dst_dir_entry.set_entry(dst_name, src_entry) src_dir_entry.remove_entry(src_name) + if preserve_time: + copy_modified_time(self, src_path, self, dst_path) + def movedir(self, src_path, dst_path, create=False, preserve_time=False): src_dir, src_name = split(self.validatepath(src_path)) dst_dir, dst_name = split(self.validatepath(dst_path)) @@ -484,6 +488,9 @@ def movedir(self, src_path, dst_path, create=False, preserve_time=False): dst_dir_entry.set_entry(dst_name, src_entry) src_dir_entry.remove_entry(src_name) + if preserve_time: + copy_modified_time(self, src_path, self, dst_path) + def openbin(self, path, mode="r", buffering=-1, **options): # type: (Text, Text, int, **Any) -> BinaryIO _mode = Mode(mode) diff --git a/fs/mirror.py b/fs/mirror.py index 15b067fe..dd00ff7b 100644 --- a/fs/mirror.py +++ b/fs/mirror.py @@ -87,7 +87,9 @@ def dst(): with src() as _src_fs, dst() as _dst_fs: _thread_safe = is_thread_safe(_src_fs, _dst_fs) - with Copier(num_workers=workers if _thread_safe else 0) as copier: + with Copier( + num_workers=workers if _thread_safe else 0, preserve_time=preserve_time + ) as copier: with _src_fs.lock(), _dst_fs.lock(): _mirror( _src_fs, diff --git a/fs/osfs.py b/fs/osfs.py index 4ffe7188..ac43471a 100644 --- a/fs/osfs.py +++ b/fs/osfs.py @@ -49,7 +49,7 @@ from .mode import Mode, validate_open_mode from .errors import FileExpected, NoURL from ._url_tools import url_quote -from .copy import copy_mtime +from .copy import copy_modified_time if typing.TYPE_CHECKING: from typing import ( @@ -454,7 +454,7 @@ def copy(self, src_path, dst_path, overwrite=False, preserve_time=False): sent = sendfile(fd_dst, fd_src, offset, maxsize) offset += sent if preserve_time: - copy_mtime(self, src_path, self, dst_path) + copy_modified_time(self, src_path, self, dst_path) except OSError as e: # the error is not a simple "sendfile not supported" error if e.errno not in self._sendfile_error_codes: diff --git a/fs/wrapfs.py b/fs/wrapfs.py index 57b605f3..00edd7af 100644 --- a/fs/wrapfs.py +++ b/fs/wrapfs.py @@ -186,7 +186,7 @@ def movedir(self, src_path, dst_path, create=False, preserve_time=False): raise errors.ResourceNotFound(dst_path) if not src_fs.getinfo(_src_path).is_dir: raise errors.DirectoryExpected(src_path) - move_dir(src_fs, _src_path, dst_fs, _dst_path) + move_dir(src_fs, _src_path, dst_fs, _dst_path, preserve_time=preserve_time) def openbin(self, path, mode="r", buffering=-1, **options): # type: (Text, Text, int, **Any) -> BinaryIO diff --git a/tests/test_move.py b/tests/test_move.py index 72f48210..6b12b2b6 100644 --- a/tests/test_move.py +++ b/tests/test_move.py @@ -21,6 +21,9 @@ def test_move_fs(self): src_file2_info = src_fs.getinfo("foo/bar/baz.txt", namespaces) dst_fs = open_fs("mem://") + dst_fs.create("test.txt") + dst_fs.setinfo("test.txt", {"details": {"modified": 1000000}}) + fs.move.move_fs(src_fs, dst_fs, preserve_time=self.preserve_time) self.assertTrue(dst_fs.isdir("foo/bar")) @@ -44,6 +47,9 @@ def test_move_dir(self): src_file2_info = src_fs.getinfo("foo/bar/baz.txt", namespaces) dst_fs = open_fs("mem://") + dst_fs.create("test.txt") + dst_fs.setinfo("test.txt", {"details": {"modified": 1000000}}) + fs.move.move_dir(src_fs, "/foo", dst_fs, "/", preserve_time=self.preserve_time) self.assertTrue(dst_fs.isdir("bar")) From 8a1bf572f26759c5abecb5af5e540dee5fbf11b7 Mon Sep 17 00:00:00 2001 From: atollk Date: Thu, 8 Apr 2021 11:56:16 +0200 Subject: [PATCH 12/14] Ran black to format code after merge. --- fs/copy.py | 50 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/fs/copy.py b/fs/copy.py index 9a9eb963..b9daea4b 100644 --- a/fs/copy.py +++ b/fs/copy.py @@ -45,7 +45,9 @@ def copy_fs( resources (defaults to `False`). """ - return copy_fs_if(src_fs, dst_fs, "always", walker, on_copy, workers, preserve_time=preserve_time) + return copy_fs_if( + src_fs, dst_fs, "always", walker, on_copy, workers, preserve_time=preserve_time + ) def copy_fs_if_newer( @@ -66,7 +68,9 @@ def copy_fs_if_newer( warnings.warn( "copy_fs_if_newer is deprecated. Use copy_fs_if instead.", DeprecationWarning ) - return copy_fs_if(src_fs, dst_fs, "newer", walker, on_copy, workers, preserve_time=preserve_time) + return copy_fs_if( + src_fs, dst_fs, "newer", walker, on_copy, workers, preserve_time=preserve_time + ) def copy_fs_if( @@ -135,7 +139,9 @@ def copy_file( resource (defaults to `False`). """ - copy_file_if(src_fs, src_path, dst_fs, dst_path, "always", preserve_time=preserve_time) + copy_file_if( + src_fs, src_path, dst_fs, dst_path, "always", preserve_time=preserve_time + ) def copy_file_if_newer( @@ -156,7 +162,9 @@ def copy_file_if_newer( "copy_file_if_newer is deprecated. Use copy_file_if instead.", DeprecationWarning, ) - return copy_file_if(src_fs, src_path, dst_fs, dst_path, "newer", preserve_time=preserve_time) + return copy_file_if( + src_fs, src_path, dst_fs, dst_path, "newer", preserve_time=preserve_time + ) def copy_file_if( @@ -210,7 +218,14 @@ def copy_file_if( _src_fs, src_path, _dst_fs, dst_path, condition ) if do_copy: - copy_file_internal(_src_fs, src_path, _dst_fs, dst_path, preserve_time=preserve_time, lock=True,) + copy_file_internal( + _src_fs, + src_path, + _dst_fs, + dst_path, + preserve_time=preserve_time, + lock=True, + ) return do_copy @@ -264,7 +279,6 @@ def _copy_locked(): _copy_locked() - def copy_structure( src_fs, # type: Union[FS, Text] dst_fs, # type: Union[FS, Text] @@ -327,7 +341,17 @@ def copy_dir( resources (defaults to `False`). """ - copy_dir_if(src_fs, src_path, dst_fs, dst_path, "always", walker, on_copy, workers, preserve_time=preserve_time) + copy_dir_if( + src_fs, + src_path, + dst_fs, + dst_path, + "always", + walker, + on_copy, + workers, + preserve_time=preserve_time, + ) def copy_dir_if_newer( @@ -350,7 +374,17 @@ def copy_dir_if_newer( warnings.warn( "copy_dir_if_newer is deprecated. Use copy_dir_if instead.", DeprecationWarning ) - copy_dir_if(src_fs, src_path, dst_fs, dst_path, "newer", walker, on_copy, workers, preserve_time=preserve_time) + copy_dir_if( + src_fs, + src_path, + dst_fs, + dst_path, + "newer", + walker, + on_copy, + workers, + preserve_time=preserve_time, + ) def copy_dir_if( From a20070840522116a99736539cb5c886e8ad3ca6a Mon Sep 17 00:00:00 2001 From: atollk Date: Thu, 8 Apr 2021 12:32:39 +0200 Subject: [PATCH 13/14] Fixed minor bug related to new `preserve_time` parameter in fs.copy. In a certain special case, the parameter was not passed on correctly but always `False` instead. --- fs/copy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/copy.py b/fs/copy.py index b9daea4b..95650b94 100644 --- a/fs/copy.py +++ b/fs/copy.py @@ -258,7 +258,7 @@ def copy_file_internal( if src_fs is dst_fs: # Same filesystem, so we can do a potentially optimized # copy - src_fs.copy(src_path, dst_path, overwrite=True) + src_fs.copy(src_path, dst_path, overwrite=True, preserve_time=preserve_time) return def _copy_locked(): From ebcd1a9d4bd9e41068d915014beb7a5ee0bbf119 Mon Sep 17 00:00:00 2001 From: atollk Date: Fri, 9 Apr 2021 17:30:32 +0200 Subject: [PATCH 14/14] Optimization in `FTPFS.setinfo`. Before, `setinfo` would always start by sending a MLST or LIST command to find whether the resources exists. This is actually not necessary if MFMT is sent anyway, so the command is now skipped in that case. --- fs/ftpfs.py | 45 +++++++++++++++++++++++++-------------------- tests/test_ftpfs.py | 31 ++++++++++++++----------------- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/fs/ftpfs.py b/fs/ftpfs.py index 79462de0..86f04958 100644 --- a/fs/ftpfs.py +++ b/fs/ftpfs.py @@ -836,27 +836,32 @@ def writebytes(self, path, contents): def setinfo(self, path, info): # type: (Text, RawInfo) -> None - current_info = self.getinfo(path) - if current_info.is_file and "MFMT" in self.features: - mtime = 0.0 + use_mfmt = False + if "MFMT" in self.features: + info_details = None if "modified" in info: - mtime = float(cast(float, info["modified"]["modified"])) - if "details" in info: - mtime = float(cast(float, info["details"]["modified"])) - if mtime: - with ftp_errors(self, path): - cmd = ( - "MFMT " - + datetime.datetime.fromtimestamp(mtime).strftime( - "%Y%m%d%H%M%S" - ) - + " " - + _encode(path, self.ftp.encoding) - ) - try: - self.ftp.sendcmd(cmd) - except error_perm: - pass + info_details = info["modified"] + elif "details" in info: + info_details = info["details"] + if info_details and "modified" in info_details: + use_mfmt = True + mtime = cast(float, info_details["modified"]) + + if use_mfmt: + with ftp_errors(self, path): + cmd = ( + "MFMT " + + datetime.datetime.utcfromtimestamp(mtime).strftime("%Y%m%d%H%M%S") + + " " + + _encode(path, self.ftp.encoding) + ) + try: + self.ftp.sendcmd(cmd) + except error_perm: + pass + else: + if not self.exists(path): + raise errors.ResourceNotFound(path) def readbytes(self, path): # type: (Text) -> bytes diff --git a/tests/test_ftpfs.py b/tests/test_ftpfs.py index 86d72ef0..43028da7 100644 --- a/tests/test_ftpfs.py +++ b/tests/test_ftpfs.py @@ -3,6 +3,7 @@ from __future__ import print_function from __future__ import unicode_literals +import calendar import os import platform import shutil @@ -226,23 +227,19 @@ def test_geturl(self): def test_setinfo(self): # TODO: temporary test, since FSTestCases.test_setinfo is broken. self.fs.create("bar") - time1 = time.mktime(datetime.datetime.now().timetuple()) - time2 = time1 + 60 - self.fs.setinfo("bar", {"details": {"modified": time1}}) - mtime1 = self.fs.getinfo("bar", ("details",)).modified - self.fs.setinfo("bar", {"details": {"modified": time2}}) - mtime2 = self.fs.getinfo("bar", ("details",)).modified - replacement = {} - if mtime1.microsecond == 0 or mtime2.microsecond == 0: - mtime1 = mtime1.replace(microsecond=0) - mtime2 = mtime2.replace(microsecond=0) - if mtime1.second == 0 or mtime2.second == 0: - mtime1 = mtime1.replace(second=0) - mtime2 = mtime2.replace(second=0) - mtime2_modified = mtime2.replace(minute=mtime2.minute - 1) - self.assertEqual( - mtime1.replace(**replacement), mtime2_modified.replace(**replacement) - ) + original_modified = self.fs.getinfo("bar", ("details",)).modified + new_modified = original_modified - datetime.timedelta(hours=1) + new_modified_stamp = calendar.timegm(new_modified.timetuple()) + self.fs.setinfo("bar", {"details": {"modified": new_modified_stamp}}) + new_modified_get = self.fs.getinfo("bar", ("details",)).modified + if original_modified.microsecond == 0 or new_modified_get.microsecond == 0: + original_modified = original_modified.replace(microsecond=0) + new_modified_get = new_modified_get.replace(microsecond=0) + if original_modified.second == 0 or new_modified_get.second == 0: + original_modified = original_modified.replace(second=0) + new_modified_get = new_modified_get.replace(second=0) + new_modified_get = new_modified_get + datetime.timedelta(hours=1) + self.assertEqual(original_modified, new_modified_get) def test_host(self): self.assertEqual(self.fs.host, self.server.host)