diff --git a/osxphotos/cli/export.py b/osxphotos/cli/export.py index cdb2fd8b..0c280a32 100644 --- a/osxphotos/cli/export.py +++ b/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: diff --git a/osxphotos/fileutil.py b/osxphotos/fileutil.py index db4f7a7c..fedff5e1 100644 --- a/osxphotos/fileutil.py +++ b/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 diff --git a/tests/test_cli.py b/tests/test_cli.py index 03162b81..73016b36 100644 --- a/tests/test_cli.py +++ b/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""" diff --git a/tests/test_fileutil.py b/tests/test_fileutil.py index b58f8788..8555ed8f 100644 --- a/tests/test_fileutil.py +++ b/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