From f1cff388e9b4dcb5cbf77c22480b03f72051e58a Mon Sep 17 00:00:00 2001 From: MSP Date: Fri, 3 Jan 2025 18:02:33 +0000 Subject: [PATCH 1/6] enhancement-1774: first adjustments. Missing adjustment to find_first_file_in_branch. --- osxphotos/cli/export.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/osxphotos/cli/export.py b/osxphotos/cli/export.py index 56947b6c..c90f12a3 100644 --- a/osxphotos/cli/export.py +++ b/osxphotos/cli/export.py @@ -794,7 +794,7 @@ "--ignore-exportdb", "-F", is_flag=True, - help="If exporting to a directory that already contains an export database " + help="1) If exporting to a directory that already contains an export database " "and --update is not specified, do not prompt to continue but instead continue " "the export. " "Normally, if you export to a directory that already contains an export database " @@ -803,7 +803,10 @@ "Use --ignore-exportdb to skip this prompt and continue the export. " "The resulting export database will contain the combined state of both export sets. " "Short option is '-F' (mnemonic: force export). " - "See also --update.", + "2) For advanced use: when used with --update, --ignore-exportdb will skip searching " + "for export database files in the parent or child of the export directory; " + "thus avoiding what could be a time comsuming search. " + "3) See also --update.", ) @click.option( "--no-exportdb", @@ -1591,7 +1594,6 @@ def export_cli( ("syndicated", "not_syndicated"), ("saved_to_library", "not_saved_to_library"), ("shared_moment", "not_shared_moment"), - ("ignore_exportdb", "update"), ("no_exportdb", "update"), ("no_exportdb", "force_update"), ] @@ -1769,13 +1771,12 @@ def export_cli( return 1 # check that export isn't in the parent or child of a previously exported library - other_db_files = find_files_in_branch(dest, OSXPHOTOS_EXPORT_DB) - if other_db_files: + if not ignore_exportdb and (other_db_files := find_first_file_in_branch(dest, OSXPHOTOS_EXPORT_DB)): rich_click_echo( "[warning]WARNING: found other export database files in this destination directory branch. " - + "This likely means you are attempting to export files into a directory " - + "that is either the parent or a child directory of a previous export. " - + "Proceeding may cause your exported files to be overwritten.", + "This likely means you are attempting to export files into a directory " + "that is either the parent or a child directory of a previous export. " + "Proceeding may cause your exported files to be overwritten.", err=True, ) rich_click_echo( @@ -3000,7 +3001,7 @@ def get_dirnames_from_template( return dest_paths -def find_files_in_branch(pathname, filename): +def find_first_file_in_branch(pathname, filename): """Search a directory branch to find file(s) named filename The branch searched includes all folders below pathname and the parent tree of pathname but not pathname itself. From 55bb473ad8e2bafdf52ecc5f15c04cc4114d2d6f Mon Sep 17 00:00:00 2001 From: MSP Date: Fri, 3 Jan 2025 18:13:25 +0000 Subject: [PATCH 2/6] enhancement-1774: Adjustment to find_first_file_in_branch to return list of first file found. --- osxphotos/cli/export.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/osxphotos/cli/export.py b/osxphotos/cli/export.py index c90f12a3..f4959956 100644 --- a/osxphotos/cli/export.py +++ b/osxphotos/cli/export.py @@ -1771,7 +1771,9 @@ def export_cli( return 1 # check that export isn't in the parent or child of a previously exported library - if not ignore_exportdb and (other_db_files := find_first_file_in_branch(dest, OSXPHOTOS_EXPORT_DB)): + if not ignore_exportdb and ( + other_db_files := find_first_file_in_branch(dest, OSXPHOTOS_EXPORT_DB) + ): rich_click_echo( "[warning]WARNING: found other export database files in this destination directory branch. " "This likely means you are attempting to export files into a directory " @@ -3002,11 +3004,11 @@ def get_dirnames_from_template( def find_first_file_in_branch(pathname, filename): - """Search a directory branch to find file(s) named filename - The branch searched includes all folders below pathname and - the parent tree of pathname but not pathname itself. + """Search a directory branch to find file(s) named filename. + The branch searched includes all folders below pathname and + the parent tree of pathname but not pathname itself. - e.g. find filename in children folders and parent folders + e.g. find filename in children folders and parent folders Args: pathname: str, full path of directory to search @@ -3016,14 +3018,12 @@ def find_first_file_in_branch(pathname, filename): """ pathname = pathlib.Path(pathname).resolve() - files = [] # walk down the tree for root, _, filenames in os.walk(pathname): - # for directory in directories: for fname in filenames: if fname == filename and pathlib.Path(root) != pathname: - files.append(os.path.join(root, fname)) + return [os.path.join(root, fname)] # walk up the tree path = pathlib.Path(pathname) @@ -3032,9 +3032,9 @@ def find_first_file_in_branch(pathname, filename): for fname in filenames: filepath = os.path.join(root, fname) if fname == filename and os.path.isfile(filepath): - files.append(os.path.join(root, fname)) + return [filepath] - return files + return [] def collect_files_to_keep( From 71cc6846c3bf8f37f488084371058da8899d6ad3 Mon Sep 17 00:00:00 2001 From: MSP Date: Fri, 3 Jan 2025 19:20:18 +0000 Subject: [PATCH 3/6] =?UTF-8?q?*=20Reusing=20option=20--ignore-exportdb=20?= =?UTF-8?q?*=20Removed=20exclusivity=20of=20options=20--update=20and=20--i?= =?UTF-8?q?gnore-exportdb=20*=20Search=20for=20files=20logic=20will=20also?= =?UTF-8?q?=20**not=20be=20done**=20in=20case=20options=20--exportdb=20or?= =?UTF-8?q?=20--no-exportdb=20are=20used.=20*=20Adjusted=20option=20defini?= =?UTF-8?q?tion.=20Maybe=20a=20bit=20lengthy=20=F0=9F=98=9E=20!=20*=20Rena?= =?UTF-8?q?med=20function=20find=5Ffiles=5Fin=5Fbranch=20into=20find=5Ffir?= =?UTF-8?q?st=5Ffile=5Fin=5Fbranch=20and=20revised=20logic=20to=20return?= =?UTF-8?q?=20on=20first=20occurrence=20found,=20when=20--ignore-exportdb?= =?UTF-8?q?=20is=20not=20active.=20*=20When=20--update=20is=20not=20used,?= =?UTF-8?q?=20added=20warnings=20when=20--ignore-exportdb=20is=20in=20use?= =?UTF-8?q?=20to=20make=20it=20compatible=20with=20interactive=20questions?= =?UTF-8?q?.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- osxphotos/cli/export.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/osxphotos/cli/export.py b/osxphotos/cli/export.py index f4959956..89471dd4 100644 --- a/osxphotos/cli/export.py +++ b/osxphotos/cli/export.py @@ -1767,11 +1767,16 @@ def export_cli( "Please confirm that you want to continue without using --update", err=True, ) - if not ignore_exportdb and not click.confirm("Do you want to continue?"): + if ignore_exportdb: + rich_click_echo( + "[warning]Warning: option --ignore-exportdb enabled: ignoring export database; " + "osxphotos will not consider state of previous export which may result in duplicate files." + ) + elif not click.confirm("Do you want to continue?"): return 1 # check that export isn't in the parent or child of a previously exported library - if not ignore_exportdb and ( + if not (ignore_exportdb or exportdb or no_exportdb) and ( other_db_files := find_first_file_in_branch(dest, OSXPHOTOS_EXPORT_DB) ): rich_click_echo( From 01f11773bb697fba71446b1ef895e439034d1044 Mon Sep 17 00:00:00 2001 From: MSP Date: Sat, 4 Jan 2025 17:59:01 +0000 Subject: [PATCH 4/6] Addressing topics: - find_first_file_in_branch should now return either None or a single str with the file path instead of a list or empty list. - Change other_db_files to other_db_file and the subsequent warning that lists out the files to now say "file" vs "files". --- osxphotos/cli/export.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/osxphotos/cli/export.py b/osxphotos/cli/export.py index 89471dd4..1aa3c5cb 100644 --- a/osxphotos/cli/export.py +++ b/osxphotos/cli/export.py @@ -1777,20 +1777,18 @@ def export_cli( # check that export isn't in the parent or child of a previously exported library if not (ignore_exportdb or exportdb or no_exportdb) and ( - other_db_files := find_first_file_in_branch(dest, OSXPHOTOS_EXPORT_DB) + other_db_file := find_first_file_in_branch(dest, OSXPHOTOS_EXPORT_DB) ): rich_click_echo( - "[warning]WARNING: found other export database files in this destination directory branch. " + "[warning]WARNING: found other export database file in this destination directory branch. " "This likely means you are attempting to export files into a directory " "that is either the parent or a child directory of a previous export. " "Proceeding may cause your exported files to be overwritten.", err=True, ) rich_click_echo( - f"You are exporting to {dest}, found {OSXPHOTOS_EXPORT_DB} files in:" + f"You are exporting to {dest}, found {OSXPHOTOS_EXPORT_DB} file in: {other_db_file}" ) - for other_db in other_db_files: - rich_click_echo(f"{other_db}") if not click.confirm("Do you want to continue?"): return 1 @@ -3028,7 +3026,7 @@ def find_first_file_in_branch(pathname, filename): for root, _, filenames in os.walk(pathname): for fname in filenames: if fname == filename and pathlib.Path(root) != pathname: - return [os.path.join(root, fname)] + return os.path.join(root, fname) # walk up the tree path = pathlib.Path(pathname) @@ -3037,9 +3035,9 @@ def find_first_file_in_branch(pathname, filename): for fname in filenames: filepath = os.path.join(root, fname) if fname == filename and os.path.isfile(filepath): - return [filepath] + return filepath - return [] + return None def collect_files_to_keep( From ebf4b4b060770c8f7546f5f39d1411febb8e5955 Mon Sep 17 00:00:00 2001 From: MSP Date: Sat, 4 Jan 2025 18:08:31 +0000 Subject: [PATCH 5/6] adjusted test "test_export_update_parent_folder" and "test_export_update_child_folder" --- tests/test_cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index f661c71f..473fc15c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -5623,7 +5623,7 @@ def test_export_update_child_folder(): input="N\n", ) assert result.exit_code != 0 - assert "WARNING: found other export database files" in result.output + assert "WARNING: found other export database file" in result.output def test_export_update_parent_folder(): @@ -5647,7 +5647,7 @@ def test_export_update_parent_folder(): input="N\n", ) assert result.exit_code != 0 - assert "WARNING: found other export database files" in result.output + assert "WARNING: found other export database file" in result.output @pytest.mark.skipif(exiftool is None, reason="exiftool not installed") From 677ce7ff2821a01f46b0642e2552438e6f0ad17c Mon Sep 17 00:00:00 2001 From: MSP Date: Sat, 4 Jan 2025 18:21:58 +0000 Subject: [PATCH 6/6] * Error in Tests: [Fatal Python error: Segmentation fault](https://github.com/RhetTbull/osxphotos/actions/runs/12612637331/job/35149593923#step:9:261) * Unable to re-run Tests up-stream :( * Adjusting some long lines and forcing commit! --- osxphotos/cli/export.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osxphotos/cli/export.py b/osxphotos/cli/export.py index 1aa3c5cb..8ce34dfa 100644 --- a/osxphotos/cli/export.py +++ b/osxphotos/cli/export.py @@ -1744,12 +1744,15 @@ def export_cli( if not no_exportdb and exportdb and exportdb != str(expected_exportdb): if expected_exportdb.exists(): rich_click_echo( - f"[warning]Warning: export database is '{exportdb}' but found '{OSXPHOTOS_EXPORT_DB}' in {dest}; using '{exportdb}'", + f"[warning]Warning: export database is '{exportdb}' but found " + f"'{OSXPHOTOS_EXPORT_DB}' in {dest}; using '{exportdb}'", err=True, ) if pathlib.Path(exportdb).resolve().parent != pathlib.Path(dest): rich_click_echo( - f"[warning]Warning: export database '{pathlib.Path(exportdb).resolve()}' is in a different directory than export destination '{dest}'", + f"[warning]Warning: export database " + f"'{pathlib.Path(exportdb).resolve()}' is in a different " + f"directory than export destination '{dest}'", err=True, )