From 11f563a47926798295e24872bc0efcaaba35906f Mon Sep 17 00:00:00 2001 From: Rhet Turnbull Date: Sun, 6 Dec 2020 22:02:47 -0800 Subject: [PATCH 1/5] Fix for issue #262 --- osxphotos/__main__.py | 1 - osxphotos/_version.py | 2 +- tests/test_cli.py | 23 ----------------------- 3 files changed, 1 insertion(+), 25 deletions(-) diff --git a/osxphotos/__main__.py b/osxphotos/__main__.py index c48800d8..90e9f948 100644 --- a/osxphotos/__main__.py +++ b/osxphotos/__main__.py @@ -1588,7 +1588,6 @@ def export( (shared, not_shared), (has_comment, no_comment), (has_likes, no_likes), - (export_as_hardlink, cleanup), ] if any(all(bb) for bb in exclusive): click.echo("Incompatible export options", err=True) diff --git a/osxphotos/_version.py b/osxphotos/_version.py index 8c4789cc..ec26987e 100644 --- a/osxphotos/_version.py +++ b/osxphotos/_version.py @@ -1,4 +1,4 @@ """ version info """ -__version__ = "0.37.6" +__version__ = "0.37.7" diff --git a/tests/test_cli.py b/tests/test_cli.py index 10871d0a..ec974ded 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3961,26 +3961,3 @@ def test_export_cleanup(): assert not pathlib.Path("./delete_me.txt").is_file() assert not pathlib.Path("./foo/delete_me_too.txt").is_file() - -def test_export_cleanup_export_as_hardling(): - """ test export with incompatible option """ - import os - import os.path - from osxphotos.__main__ import export - - 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", - "--export-as-hardlink", - "--cleanup", - ], - ) - assert "Incompatible export options" in result.output - From d8de86cb6f17aad13ca6dd80d017d40ef74ee93a Mon Sep 17 00:00:00 2001 From: Rhet Turnbull Date: Sun, 6 Dec 2020 22:10:20 -0800 Subject: [PATCH 2/5] Updated CHANGELOG.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcfb4208..89f73f25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. Dates are d Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). +#### [v0.37.7](https://github.com/RhetTbull/osxphotos/compare/v0.37.6...v0.37.7) + +> 7 December 2020 + +- Fix for issue #262 [`11f563a`](https://github.com/RhetTbull/osxphotos/commit/11f563a47926798295e24872bc0efcaaba35906f) + #### [v0.37.6](https://github.com/RhetTbull/osxphotos/compare/v0.37.5...v0.37.6) > 6 December 2020 From ec4b53ed9dd2bc1e6b71349efdaf0b81c6d797e5 Mon Sep 17 00:00:00 2001 From: Rhet Turnbull Date: Thu, 10 Dec 2020 20:29:47 -0800 Subject: [PATCH 3/5] Refactored FileUtil to use copy-on-write no APFS, issue #287 --- README.md | 9 +--- osxphotos/__main__.py | 15 ------- osxphotos/_version.py | 2 +- osxphotos/fileutil.py | 53 +++++++++++++----------- osxphotos/photoinfo/_photoinfo_export.py | 12 +----- osxphotos/photosdb/photosdb.py | 9 ++-- tests/test_export_catalina_10_15_7.py | 22 ---------- tests/test_fileutil.py | 17 +------- 8 files changed, 38 insertions(+), 101 deletions(-) diff --git a/README.md b/README.md index 94deb39b..a9837e39 100644 --- a/README.md +++ b/README.md @@ -388,11 +388,6 @@ Options: photo would be named 'filename_original.ext'. The default suffix is '' (no suffix). - --no-extended-attributes Don't copy extended attributes when - exporting. You only need this if exporting - to a filesystem that doesn't support Mac OS - extended attributes. Only use this if you - get an error while exporting. --use-photos-export Force the use of AppleScript or PhotoKit to export even if not missing (see also '-- download-missing' and '--use-photokit'). @@ -1487,7 +1482,7 @@ Returns a JSON representation of all photo info. Returns a dictionary representation of all photo info. #### `export()` -`export(dest, *filename, edited=False, live_photo=False, export_as_hardlink=False, overwrite=False, increment=True, sidecar_json=False, sidecar_xmp=False, use_photos_export=False, timeout=120, exiftool=False, no_xattr=False, use_albums_as_keywords=False, use_persons_as_keywords=False)` +`export(dest, *filename, edited=False, live_photo=False, export_as_hardlink=False, overwrite=False, increment=True, sidecar_json=False, sidecar_xmp=False, use_photos_export=False, timeout=120, exiftool=False, use_albums_as_keywords=False, use_persons_as_keywords=False)` Export photo from the Photos library to another destination on disk. - dest: must be valid destination path as str (or exception raised). @@ -1502,7 +1497,6 @@ Export photo from the Photos library to another destination on disk. - use_photos_export: boolean; (default=False), if True will attempt to export photo via applescript interaction with Photos; useful for forcing download of missing photos. This only works if the Photos library being used is the default library (last opened by Photos) as applescript will directly interact with whichever library Photos is currently using. - timeout: (int, default=120) timeout in seconds used with use_photos_export - exiftool: (boolean, default = False) if True, will use [exiftool](https://exiftool.org/) to write metadata directly to the exported photo; exiftool must be installed and in the system path -- no_xattr: (boolean, default = False); if True, exports file without preserving extended attributes - use_albums_as_keywords: (boolean, default = False); if True, will use album names as keywords when exporting metadata with exiftool or sidecar - use_persons_as_keywords: (boolean, default = False); if True, will use person names as keywords when exporting metadata with exiftool or sidecar @@ -1524,7 +1518,6 @@ Then If overwrite=False and increment=False, export will fail if destination file already exists -**Implementation Note**: Because the usual python file copy methods don't preserve all the metadata available on MacOS, export uses `/usr/bin/ditto` to do the copy for export. ditto preserves most metadata such as extended attributes, permissions, ACLs, etc. #### `render_template()` diff --git a/osxphotos/__main__.py b/osxphotos/__main__.py index 90e9f948..9cf0a9a2 100644 --- a/osxphotos/__main__.py +++ b/osxphotos/__main__.py @@ -1408,14 +1408,6 @@ def query( "'filename.ext'. For example, with '--original-suffix _original', the original photo " "would be named 'filename_original.ext'. The default suffix is '' (no suffix).", ) -@click.option( - "--no-extended-attributes", - is_flag=True, - default=False, - help="Don't copy extended attributes when exporting. You only need this if exporting " - "to a filesystem that doesn't support Mac OS extended attributes. Only use this if you get " - "an error while exporting.", -) @click.option( "--use-photos-export", is_flag=True, @@ -1529,7 +1521,6 @@ def export( no_comment, has_likes, no_likes, - no_extended_attributes, label, deleted, deleted_only, @@ -1808,7 +1799,6 @@ def export( exiftool=exiftool, directory=directory, filename_template=filename_template, - no_extended_attributes=no_extended_attributes, export_raw=export_raw, album_keyword=album_keyword, person_keyword=person_keyword, @@ -1867,7 +1857,6 @@ def export( exiftool=exiftool, directory=directory, filename_template=filename_template, - no_extended_attributes=no_extended_attributes, export_raw=export_raw, album_keyword=album_keyword, person_keyword=person_keyword, @@ -2438,7 +2427,6 @@ def export_photo( exiftool=None, directory=None, filename_template=None, - no_extended_attributes=None, export_raw=None, album_keyword=None, person_keyword=None, @@ -2473,7 +2461,6 @@ def export_photo( exiftool: use exiftool to write EXIF metadata directly to exported photo directory: template used to determine output directory filename_template: template use to determine output file - no_extended_attributes: boolean; if True, exports photo without preserving extended attributes export_raw: boolean; if True exports raw image associate with the photo export_edited: boolean; if True exports edited version of photo if there is one skip_original_if_edited: boolean; if True does not export original if photo has been edited @@ -2616,7 +2603,6 @@ def export_photo( overwrite=overwrite, use_photos_export=use_photos_export, exiftool=exiftool, - no_xattr=no_extended_attributes, use_albums_as_keywords=album_keyword, use_persons_as_keywords=person_keyword, keyword_template=keyword_template, @@ -2700,7 +2686,6 @@ def export_photo( edited=True, use_photos_export=use_photos_export, exiftool=exiftool, - no_xattr=no_extended_attributes, use_albums_as_keywords=album_keyword, use_persons_as_keywords=person_keyword, keyword_template=keyword_template, diff --git a/osxphotos/_version.py b/osxphotos/_version.py index ec26987e..95945b8a 100644 --- a/osxphotos/_version.py +++ b/osxphotos/_version.py @@ -1,4 +1,4 @@ """ version info """ -__version__ = "0.37.7" +__version__ = "0.38.0" diff --git a/osxphotos/fileutil.py b/osxphotos/fileutil.py index ed06a594..9d31321b 100644 --- a/osxphotos/fileutil.py +++ b/osxphotos/fileutil.py @@ -7,6 +7,8 @@ import subprocess import sys from abc import ABC, abstractmethod +import CoreFoundation + from .imageconverter import ImageConverter @@ -82,35 +84,38 @@ class FileUtilMacOS(FileUtilABC): raise e @classmethod - def copy(cls, src, dest, norsrc=False): + def copy(cls, src, dest): """ Copies a file from src path to dest path - src: source path as string + + Args: + src: source path as string; must be a valid file path dest: destination path as string - norsrc: (bool) if True, uses --norsrc flag with ditto so it will not copy - resource fork or extended attributes. May be useful on volumes that - don't work with extended attributes (likely only certain SMB mounts) - default is False - Uses ditto to perform copy; will silently overwrite dest if it exists - Raises exception if copy fails or either path is None """ + 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 src is None or dest is None: - raise ValueError("src and dest must not be None", src, dest) + if not isinstance(dest, pathlib.Path): + dest = pathlib.Path(dest) - if not os.path.exists(src): - raise FileNotFoundError("src file does not appear to exist", src) + if dest.is_dir(): + dest /= src.name - if norsrc: - command = ["/usr/bin/ditto", "--norsrc", src, dest] - else: - command = ["/usr/bin/ditto", src, dest] - - # if error on copy, subprocess will raise CalledProcessError - try: - result = subprocess.run(command, check=True, stderr=subprocess.PIPE) - except subprocess.CalledProcessError as e: - raise e - - return result.returncode + filemgr = CoreFoundation.NSFileManager.defaultManager() + error = filemgr.copyItemAtPath_toPath_error_(str(src), str(dest), None) + # error is a tuple of (bool, error_string) + # error[0] is True if copy succeeded + if not error[0]: + raise OSError(error[1]) + return True @classmethod def unlink(cls, filepath): diff --git a/osxphotos/photoinfo/_photoinfo_export.py b/osxphotos/photoinfo/_photoinfo_export.py index 76b49e76..66164c93 100644 --- a/osxphotos/photoinfo/_photoinfo_export.py +++ b/osxphotos/photoinfo/_photoinfo_export.py @@ -246,7 +246,6 @@ def export( use_photos_export=False, timeout=120, exiftool=False, - no_xattr=False, use_albums_as_keywords=False, use_persons_as_keywords=False, keyword_template=None, @@ -279,7 +278,6 @@ def export( use_photos_export: (boolean, default=False); if True will attempt to export photo via applescript interaction with Photos timeout: (int, default=120) timeout in seconds used with use_photos_export exiftool: (boolean, default = False); if True, will use exiftool to write metadata to export file - no_xattr: (boolean, default = False); if True, exports file without preserving extended attributes returns list of full paths to the exported files use_albums_as_keywords: (boolean, default = False); if True, will include album names in keywords when exporting metadata with exiftool or sidecar @@ -306,7 +304,6 @@ def export( use_photos_export=use_photos_export, timeout=timeout, exiftool=exiftool, - no_xattr=no_xattr, use_albums_as_keywords=use_albums_as_keywords, use_persons_as_keywords=use_persons_as_keywords, keyword_template=keyword_template, @@ -331,7 +328,6 @@ def export2( use_photos_export=False, timeout=120, exiftool=False, - no_xattr=False, use_albums_as_keywords=False, use_persons_as_keywords=False, keyword_template=None, @@ -372,7 +368,6 @@ def export2( use_photos_export: (boolean, default=False); if True will attempt to export photo via applescript interaction with Photos timeout: (int, default=120) timeout in seconds used with use_photos_export exiftool: (boolean, default = False); if True, will use exiftool to write metadata to export file - no_xattr: (boolean, default = False); if True, exports file without preserving extended attributes use_albums_as_keywords: (boolean, default = False); if True, will include album names in keywords when exporting metadata with exiftool or sidecar use_persons_as_keywords: (boolean, default = False); if True, will include person names in keywords @@ -604,7 +599,6 @@ def export2( update, export_db, overwrite, - no_xattr, export_as_hardlink, exiftool, touch_file, @@ -632,7 +626,6 @@ def export2( update, export_db, overwrite, - no_xattr, export_as_hardlink, exiftool, touch_file, @@ -658,7 +651,6 @@ def export2( update, export_db, overwrite, - no_xattr, export_as_hardlink, exiftool, touch_file, @@ -964,7 +956,6 @@ def _export_photo( update, export_db, overwrite, - no_xattr, export_as_hardlink, exiftool, touch_file, @@ -985,7 +976,6 @@ def _export_photo( update: bool export_db: instance of ExportDB that conforms to ExportDB_ABC interface overwrite: bool - no_xattr: don't copy extended attributes export_as_hardlink: bool exiftool: bool touch_file: bool @@ -1100,7 +1090,7 @@ def _export_photo( converted_stat = fileutil.file_sig(dest_str) converted_to_jpeg_files.append(dest_str) else: - fileutil.copy(src, dest_str, norsrc=no_xattr) + fileutil.copy(src, dest_str) export_db.set_data( filename=dest_str, diff --git a/osxphotos/photosdb/photosdb.py b/osxphotos/photosdb/photosdb.py index 75e5a360..f85df9a6 100644 --- a/osxphotos/photosdb/photosdb.py +++ b/osxphotos/photosdb/photosdb.py @@ -12,7 +12,6 @@ import sys import tempfile from datetime import datetime, timedelta, timezone from pprint import pformat -from shutil import copyfile from .._constants import ( _DB_TABLE_NAMES, @@ -532,14 +531,14 @@ class PhotosDB: try: dest_name = pathlib.Path(fname).name dest_path = os.path.join(self._tempdir_name, dest_name) - copyfile(fname, dest_path) + FileUtil.copy(fname, dest_path) # copy write-ahead log and shared memory files (-wal and -shm) files if they exist if os.path.exists(f"{fname}-wal"): - copyfile(f"{fname}-wal", f"{dest_path}-wal") + FileUtil.copy(f"{fname}-wal", f"{dest_path}-wal") if os.path.exists(f"{fname}-shm"): - copyfile(f"{fname}-shm", f"{dest_path}-shm") + FileUtil.copy(f"{fname}-shm", f"{dest_path}-shm") except: - print("Error copying " + fname + " to " + dest_path, file=sys.stderr) + print(f"Error copying{fname} to {dest_path}", file=sys.stderr) raise Exception if _debug(): diff --git a/tests/test_export_catalina_10_15_7.py b/tests/test_export_catalina_10_15_7.py index e0c77ea7..267121bc 100644 --- a/tests/test_export_catalina_10_15_7.py +++ b/tests/test_export_catalina_10_15_7.py @@ -414,28 +414,6 @@ def test_export_13(): assert e.type == type(FileNotFoundError()) -def test_export_no_xattr(): - # test basic export with no_xattr=True - # get an unedited image and export it using default filename - import os - import os.path - import tempfile - - import osxphotos - - tempdir = tempfile.TemporaryDirectory(prefix="osxphotos_") - dest = tempdir.name - photosdb = osxphotos.PhotosDB(dbfile=PHOTOS_DB) - photos = photosdb.photos(uuid=[UUID_DICT["export"]]) - - filename = photos[0].filename - expected_dest = os.path.join(dest, filename) - got_dest = photos[0].export(dest, no_xattr=True)[0] - - assert got_dest == expected_dest - assert os.path.isfile(got_dest) - - def test_dd_to_dms_str_1(): import osxphotos diff --git a/tests/test_fileutil.py b/tests/test_fileutil.py index 1109f400..c52978a8 100644 --- a/tests/test_fileutil.py +++ b/tests/test_fileutil.py @@ -16,7 +16,7 @@ def test_copy_file_valid(): temp_dir = tempfile.TemporaryDirectory(prefix="osxphotos_") src = "tests/test-images/wedding.jpg" result = FileUtil.copy(src, temp_dir.name) - assert result == 0 + assert result assert os.path.isfile(os.path.join(temp_dir.name, "wedding.jpg")) @@ -29,20 +29,7 @@ def test_copy_file_invalid(): with pytest.raises(Exception) as e: src = "tests/test-images/wedding_DOES_NOT_EXIST.jpg" assert FileUtil.copy(src, temp_dir.name) - assert e.type == FileNotFoundError - - -def test_copy_file_norsrc(): - # copy file with --norsrc - import os.path - import tempfile - from osxphotos.fileutil import FileUtil - - temp_dir = tempfile.TemporaryDirectory(prefix="osxphotos_") - src = "tests/test-images/wedding.jpg" - result = FileUtil.copy(src, temp_dir.name, norsrc=True) - assert result == 0 - assert os.path.isfile(os.path.join(temp_dir.name, "wedding.jpg")) + assert e.type == OSError def test_hardlink_file_valid(): From e17ee0e3887ad515cb59a688ea164d31bdf5593c Mon Sep 17 00:00:00 2001 From: Rhet Turnbull Date: Thu, 10 Dec 2020 20:48:19 -0800 Subject: [PATCH 4/5] Updated CHANGELOG.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89f73f25..c707a7a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. Dates are d Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). +#### [v0.38.0](https://github.com/RhetTbull/osxphotos/compare/v0.37.7...v0.38.0) + +> 11 December 2020 + +- Refactored FileUtil to use copy-on-write no APFS, issue #287 [`ec4b53e`](https://github.com/RhetTbull/osxphotos/commit/ec4b53ed9dd2bc1e6b71349efdaf0b81c6d797e5) + #### [v0.37.7](https://github.com/RhetTbull/osxphotos/compare/v0.37.6...v0.37.7) > 7 December 2020 From 73f936e06126ceb3bc49470f02577c95492804de Mon Sep 17 00:00:00 2001 From: Rhet Turnbull Date: Fri, 11 Dec 2020 06:19:05 -0800 Subject: [PATCH 5/5] Added link to discussions --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index a9837e39..ed0934b4 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ OSXPhotos provides the ability to interact with and query Apple's Photos.app lib Only works on MacOS (aka Mac OS X). Tested on MacOS 10.12.6 / Photos 2.0, 10.13.6 / Photos 3.0, MacOS 10.14.5, 10.14.6 / Photos 4.0, MacOS 10.15.1 - 10.15.6 / Photos 5.0. -Alpha support for MacOS 10.16/MacOS 11 Big Sur Beta / Photos 6.0. +Beta support for MacOS 10.16/MacOS 11 Big Sur Beta / Photos 6.0. Requires python >= 3.7. @@ -2175,7 +2175,7 @@ if __name__ == "__main__": ## Contributing -Contributing is easy! if you find bugs or want to suggest additional features/changes, please open an [issue](https://github.com/rhettbull/osxphotos/issues/). +Contributing is easy! if you find bugs or want to suggest additional features/changes, please open an [issue](https://github.com/rhettbull/osxphotos/issues/) or join the [discussion](https://github.com/RhetTbull/osxphotos/discussions). I'll gladly consider pull requests for bug fixes or feature implementations. @@ -2245,4 +2245,4 @@ For additional details about how osxphotos is implemented or if you would like t ## Acknowledgements This project was originally inspired by [photo-export](https://github.com/patrikhson/photo-export) by Patrick Fältström, Copyright (c) 2015 Patrik Fältström paf@frobbit.se -I use [py-applescript](https://github.com/rdhyee/py-applescript) by "Raymond Yee / rdhyee" to interact with Photos. Rather than import this package, I included the entire package (which is published as public domain code) in a private package to prevent ambiguity with other applescript packages on PyPi. py-applescript uses a native bridge via PyObjC and is very fast compared to the other osascript based packages. \ No newline at end of file +I use [py-applescript](https://github.com/rdhyee/py-applescript) by "Raymond Yee / rdhyee" to interact with Photos. Rather than import this package, I included the entire package (which is published as public domain code) in a private package to prevent ambiguity with other applescript packages on PyPi. py-applescript uses a native bridge via PyObjC and is very fast compared to the other osascript based packages.