Added --alt-copy method for #807 (#835)

osxphotos/cli/export.py

@@ -50,7 +50,7 @@ from osxphotos.datetime_formatter import DateTimeFormatter
 from osxphotos.debug import is_debug, set_debug
 from osxphotos.exiftool import get_exiftool_path
 from osxphotos.export_db import ExportDB, ExportDBInMemory
-from osxphotos.fileutil import FileUtil, FileUtilNoOp
+from osxphotos.fileutil import FileUtil, FileUtilNoOp, FileUtilShUtil
 from osxphotos.path_utils import is_valid_filepath, sanitize_filename, sanitize_filepath
 from osxphotos.photoexporter import ExportOptions, ExportResults, PhotoExporter
 from osxphotos.photoinfo import PhotoInfoNone
@@ -651,6 +651,15 @@ from .verbose import get_verbose_console, time_stamp, verbose_print
     "the Photos library.",
     type=click.Path(dir_okay=True, file_okay=False, exists=True),
 )
+@click.option(
+    "--alt-copy",
+    is_flag=True,
+    help="Use alternate copy method that may be more reliable for some "
+    "network attached storage (NAS) devices. Use --alt-copy if you experience "
+    "problems exporting to a NAS device or SMB volume. "
+    "Unlike the default copy method, --alt-copy does not support "
+    "copy-on-write on APFS volumes nor does it preserve filesystem metadata.",
+)
 @click.option(
     "--load-config",
     required=False,
@@ -745,6 +754,7 @@ def export(
     added_in_last,
     album_keyword,
     album,
+    alt_copy,
     append,
     beta,
     burst,
@@ -969,6 +979,7 @@ def export(
         add_skipped_to_album = cfg.add_skipped_to_album
         album = cfg.album
         album_keyword = cfg.album_keyword
+        alt_copy = cfg.alt_copy
         append = cfg.append
         beta = cfg.beta
         burst = cfg.burst
@@ -1333,7 +1344,7 @@ def export(
             if ramdb
             else ExportDB(dbfile=export_db_path, export_dir=dest)
         )
-        fileutil = FileUtil
+        fileutil = FileUtilShUtil if alt_copy else FileUtil
 
     if verbose_:
         if export_db.was_created:

osxphotos/fileutil.py

@@ -2,6 +2,7 @@
 
 import os
 import pathlib
+import shutil
 import stat
 import tempfile
 import typing as t
@@ -12,7 +13,7 @@ import Foundation
 
 from .imageconverter import ImageConverter
 
-__all__ = ["FileUtilABC", "FileUtilMacOS", "FileUtil", "FileUtilNoOp"]
+__all__ = ["FileUtilABC", "FileUtilMacOS", "FileUtilShUtil", "FileUtil", "FileUtilNoOp"]
 
 
 class FileUtilABC(ABC):
@@ -25,7 +26,7 @@ class FileUtilABC(ABC):
 
     @classmethod
     @abstractmethod
-    def copy(cls, src, dest, norsrc=False):
+    def copy(cls, src, dest):
         pass
 
     @classmethod
@@ -250,6 +251,43 @@ class FileUtilMacOS(FileUtilABC):
         return (stat.S_IFMT(st.st_mode), st.st_size, int(st.st_mtime))
 
 
+class FileUtilShUtil(FileUtilMacOS):
+    """Various file utilities, uses shutil.copy to copy files instead of NSFileManager (#807)"""
+
+    @classmethod
+    def copy(cls, src, dest):
+        """Copies a file from src path to dest path using shutil.copy
+
+        Args:
+            src: source path as string; must be a valid file path
+            dest: destination path as string
+                  dest may be either directory or file; in either case, src file must not exist in dest
+            Note: src and dest may be either a string or a pathlib.Path object
+
+        Returns:
+            True if copy succeeded
+
+        Raises:
+            OSError if copy fails
+            TypeError if either path is None
+        """
+        if not isinstance(src, pathlib.Path):
+            src = pathlib.Path(src)
+
+        if not isinstance(dest, pathlib.Path):
+            dest = pathlib.Path(dest)
+
+        if dest.is_dir():
+            dest /= src.name
+
+        try:
+            shutil.copy(str(src), str(dest))
+        except Exception as e:
+            raise OSError(f"Error copying {src} to {dest}: {e}") from e
+
+        return True
+
+
 class FileUtil(FileUtilMacOS):
     """Various file utilities"""
 
@@ -280,7 +318,7 @@ class FileUtilNoOp(FileUtil):
         pass
 
     @classmethod
-    def copy(cls, src, dest, norsrc=False):
+    def copy(cls, src, dest):
         pass
 
     @classmethod

tests/test_cli.py

@@ -1397,6 +1397,20 @@ def test_export():
         assert sorted(files) == sorted(CLI_EXPORT_FILENAMES)
 
 
+def test_export_alt_copy():
+    """test basic export with --alt-copy"""
+    runner = CliRunner()
+    cwd = os.getcwd()
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        result = runner.invoke(
+            export, [os.path.join(cwd, CLI_PHOTOS_DB), ".", "--alt-copy", "-V"]
+        )
+        assert result.exit_code == 0
+        files = glob.glob("*")
+        assert sorted(files) == sorted(CLI_EXPORT_FILENAMES)
+
+
 def test_export_tmpdir():
     """test basic export with --tmpdir"""
     runner = CliRunner()
@@ -4862,7 +4876,7 @@ def test_export_then_hardlink():
 
 
 def test_export_dry_run():
-    """test export with dry-run flag"""
+    """test export with --dry-run flag"""
 
     runner = CliRunner()
     cwd = os.getcwd()
@@ -4881,6 +4895,27 @@ def test_export_dry_run():
             assert not os.path.isfile(normalize_fs_path(filepath))
 
 
+def test_export_dry_run_alt_copy():
+    """test export with --dry-run flag and --alt-copy"""
+
+    runner = CliRunner()
+    cwd = os.getcwd()
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        result = runner.invoke(
+            export,
+            [os.path.join(cwd, CLI_PHOTOS_DB), ".", "-V", "--alt-copy", "--dry-run"],
+        )
+        assert result.exit_code == 0
+        assert (
+            f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, missing: 3, error: 0"
+            in result.output
+        )
+        for filepath in CLI_EXPORT_FILENAMES_DRY_RUN:
+            assert re.search(r"Exported.*" + f"{re.escape(filepath)}", result.output)
+            assert not os.path.isfile(normalize_fs_path(filepath))
+
+
 def test_export_update_edits_dry_run():
     """test export then update after removing and editing files with dry-run flag"""
 

tests/test_fileutil.py

@@ -5,7 +5,7 @@ import pathlib
 
 import pytest
 
-from osxphotos.fileutil import FileUtil
+from osxphotos.fileutil import FileUtil, FileUtilShUtil
 
 TEST_HEIC = "tests/test-images/IMG_3092.heic"
 TEST_RAW = "tests/test-images/DSC03584.dng"
@@ -38,6 +38,33 @@ def test_copy_file_invalid():
     assert e.type == OSError
 
 
+def test_copy_file_valid_shutil():
+    # copy file with valid src, dest with the shutil implementation
+    import os.path
+    import tempfile
+
+    from osxphotos.fileutil import FileUtil
+
+    temp_dir = tempfile.TemporaryDirectory(prefix="osxphotos_")
+    src = "tests/test-images/wedding.jpg"
+    result = FileUtilShUtil.copy(src, temp_dir.name)
+    assert result
+    assert os.path.isfile(os.path.join(temp_dir.name, "wedding.jpg"))
+
+
+def test_copy_file_invalid_shutil():
+    # copy file with invalid src with the shutil implementation
+    import tempfile
+
+    from osxphotos.fileutil import FileUtil
+
+    temp_dir = tempfile.TemporaryDirectory(prefix="osxphotos_")
+    with pytest.raises(Exception) as e:
+        src = "tests/test-images/wedding_DOES_NOT_EXIST.jpg"
+        assert FileUtilShUtil.copy(src, temp_dir.name)
+    assert e.type == OSError
+
+
 def test_hardlink_file_valid():
     # hardlink file with valid src, dest
     import os.path