Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add —cleanup-move that cleans up by moving instead of deleting #1786

Open
infused-kim opened this issue Jan 20, 2025 · 3 comments
Open

Add —cleanup-move that cleans up by moving instead of deleting #1786

infused-kim opened this issue Jan 20, 2025 · 3 comments
Labels
feature request New feature or request

Comments

@infused-kim
Copy link

Is your feature request related to a problem? Please describe.

I would like to have better visibility into what has been deleted from the library.

Currently, when you use the cleanup option, it deletes photos from the export that have been deleted in Apple Photos.

This makes it hard to verify if all the deletes were intentional.

Describe the solution you'd like

It would be great if there was an option that wouldn’t delete the files, but instead move the deleted files into a different directory instead.

Ideally, it would preserve the same directory structure as before.

This would allow us to review what has been deleted in Apple photos since the last backup.

Describe alternatives you've considered

Until now I have been using the —report option and reviewed deletions by looking at the file paths, but being able to actually look at the photos would make this much more effective.

Additional context

If you are open to adding this feature, but don’t have the time yourself, I could contribute the code for it.

@infused-kim infused-kim added the feature request New feature or request label Jan 20, 2025
@RhetTbull
Copy link
Owner

RhetTbull commented Jan 21, 2025

This is a pretty niche feature but I can see how some might find it useful. I think a --cleanup-command or re-using the --post-command option would be more useful and flexible. This would let you run a command or a shell script on the files identified for cleanup

For example, the following --post-command adds the file path of all exported files to a text file:

--post-command exported "echo {filepath|shell_quote} >>{export_dir}/exported.txt"

In this option, exported specifies to apply this to all files exported. Other valid categories are: new, updated, skipped, missing, exif_updated, touched, converted_to_jpeg, sidecar_json_written, sidecar_json_skipped, sidecar_exiftool_written, sidecar_exiftool_skipped, sidecar_xmp_written, sidecar_xmp_skipped, error. These allow you to run a command on nearly any category of file evaluated by osxphotos during the export. Adding cleanup to this fits the mental model of what this option intends.

When the command gets run, the {filepath} template is set to the value of the file being processed.

One option is to add a cleanup category to use the same method. This is clean and avoids another option (in my opinion, osxphotos has an overwhelming number of options). There are some problems with this from an implementation standpoint. First, the post command gets run immediately after each photo is exported. The cleanup code cannot run until all photos are exported because osxphotos needs to know which files are part of the export set (and thus should not be cleaned up) and this is not known until the end of the export. Second, the current run_post_command() function (below), expects a PhotoInfo object which would not make sense for a file being cleaned up (there might be no associated photo because it was deleted or it was a file added to export directory by the user).

def run_post_command(
photo: osxphotos.PhotoInfo,
post_command: tuple[tuple[str, str]],
export_results: ExportResults,
export_dir: str | pathlib.Path,
dry_run: bool,
exiftool_path: str,
on_error: Literal["break", "continue"] | None,
verbose: Callable[[Any], None],
):
"""Run --post-command commands"""
# todo: pass in RenderOptions from export? (e.g. so it contains strip, etc?)
for category, command_template in post_command:
files = getattr(export_results, category)
for f in files:
# some categories, like error, return a tuple of (file, error str)
if isinstance(f, tuple):
f = f[0]
render_options = RenderOptions(export_dir=export_dir, filepath=f)
template = PhotoTemplate(photo, exiftool_path=exiftool_path)
command, _ = template.render(command_template, options=render_options)
command = command[0] if command else None
if command:
verbose(f'Running command: "{command}"')
if not dry_run:
cwd = pathlib.Path(f).parent
run_error = None
run_results = None
try:
run_results = subprocess.run(command, shell=True, cwd=cwd)
except Exception as e:
run_error = e
finally:
returncode = run_results.returncode if run_results else None
if run_error or returncode:
# there was an error running the command
error_str = f'Error running command "{command}": return code: {returncode}, exception: {run_error}'
rich_echo_error(f"[error]{error_str}[/]")
if not on_error:
# no error handling specified, raise exception
raise RuntimeError(error_str)
if on_error == "break":
# break out of loop and return
return
# else on_error must be continue

One way to solve this is to change the definition of run_post_command so the photo argument can be None and to call it again at the end with a list of files to be cleaned up. (Would also require changing the export_results argument, possibly others. This is messy. Perhaps a run_cleanup_command would be better. There'd be some repetition of the logic but it would only look at commands associated with the cleanup category. The run_post_command() would then need to skip any commands associated with the cleanup category.

I think having a separate run_cleanup_command option is cleaner (no pun intended!) than overloading run_post_command but this still allows us to re-use the --post-command scaffolding and the associated --post-command-error which specifies what to do in the case of an error (stop or continue). It also allows you to have multiple commands that get run (you can specify --post-command repeatedly), for example, one command to log the file path and another to move it.

--cleanup and --post-command would be independent so you can run a cleanup command without running --cleanup or you could run them together, e.g. use the command to log the file path and then let cleanup delete the file.

The cleanup logic happens here:

# cleanup files and do report if needed
if cleanup:
db_file = str(pathlib.Path(export_db_path).resolve())
db_files = [db_file, db_file + "-wal", db_file + "-shm"]
keep_file = str(pathlib.Path(dest) / ".osxphotos_keep")
all_files = (
results.exported
+ results.skipped
+ results.exif_updated
+ results.touched
+ results.converted_to_jpeg
+ results.aae_written
+ results.sidecar_json_written
+ results.sidecar_json_skipped
+ results.sidecar_exiftool_written
+ results.sidecar_exiftool_skipped
+ results.sidecar_xmp_written
+ results.sidecar_xmp_skipped
+ results.sidecar_user_written
+ results.sidecar_user_skipped
+ results.user_written
+ results.user_skipped
# include missing so a file that was already in export directory
# but was missing on --update doesn't get deleted
# (better to have old version than none)
+ results.missing
# include files that have error in case they exist from previous export
+ [r[0] for r in results.error]
# don't delete export database files
+ db_files
# include the .osxphotos_keep file
+ [keep_file]
)
# if --report, add report file to keep list to prevent it from being deleted
if report:
all_files.append(report)
# gather any files that should be kept from both .osxphotos_keep and --keep
dirs_to_keep = []
files_to_keep, dirs_to_keep = collect_files_to_keep(keep, dest)
all_files += files_to_keep
rich_echo(f"Cleaning up [filepath]{dest}")
cleaned_files, cleaned_dirs = cleanup_files(
dest, all_files, dirs_to_keep, fileutil, verbose=verbose
)
file_str = "files" if len(cleaned_files) != 1 else "file"
dir_str = "directories" if len(cleaned_dirs) != 1 else "directory"
rich_echo(
f"Deleted: [num]{len(cleaned_files)}[/num] {file_str}, [num]{len(cleaned_dirs)}[/num] {dir_str}"
)
report_writer.write(
ExportResults(deleted_files=cleaned_files, deleted_directories=cleaned_dirs)
)
results.deleted_files = cleaned_files
results.deleted_directories = cleaned_dirs

The other option is to add a separate --cleanup-command option but I think I like the simplicity adding cleanup to the --post-command categories. Open to feedback and discussion though!

If you want to take a stab at implementing this I'm happy to accept a PR once we settle on a preferred approach.

@RhetTbull
Copy link
Owner

@all-contributors please add @infused-kim for ideas

Copy link
Contributor

@RhetTbull

@infused-kim already contributed before to ideas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants