osxphotos: --exiftool and --update don't catch changes in keyword template

This is almost working. The “should update” logic above misses one case: if user ran export with --exiftool then ran again with --update but not --exiftool, the logic above would lead to skipping the export even though the file should be re-exported (the signature on disk, which is the exiftool signature, will match what’s in the database).

I haven’t tested it yet but I think the same will be true for --convert-to-jpeg

Update: solved this by adding a bit mask field to the database that stores a bit mask representing the options that make a difference (like exiftool) so it’s easy to check if exporting with a different set of options

All tests passing!

====== 1192 passed, 13 skipped in 554.38s (0:09:14) ======

If you want to give the new version a try, it’s in the exportdb_refactor branch

I want to do some more more testing with my own library before releasing but all the tests are passing including a couple of complex --update, and --force-update cases built specifically to catch the errors for this issue.

Also, this release adds a new (hidden) exportdb command to osxphotos for doing maintenance on the export database. For example, you can use osxphotos exportdb --check-signatures to find any exported photos with a signature different than what’s in the database or osxphotos exportdb --touch-file to update the touch time for all photos. This is useful if you used --touch-time previous to this version as it’ll prevent osxphotos from re-exporting everything when you run --update. The export database also now saves the configuration for the last 10 exports and you can recover the most recent (in case you didn’t save the toml file) with osxphotos --save-config config.toml.

If you’re going to try this version, I recommend you backup your export database as osxphotos will migrate the database to a new schema and this cannot be rolled back.

$ osxphotos help exportdb
Usage: osxphotos exportdb [OPTIONS] EXPORT_DATABASE

  Utilities for working with the osxphotos export database

Options:
  --version                  Print export database version and exit.
  --vacuum                   Run VACUUM to defragment the database.
  --check-signatures         Check signatures for all exported photos in the
                             database to find signatures that don't match.
  --update-signatures        Update signatures for all exported photos in the
                             database to match on-disk signatures.
  --touch-file               Touch files on disk to match created date in
                             Photos library and update export database
                             signatures
  --last-run                 Show last run osxphotos commands used with this
                             database.
  --save-config CONFIG_FILE  Save last run configuration to TOML file for use
                             by --load-config.
  --export-dir DIRECTORY     Optional path to export directory (if not parent
                             of export database).
  -V, --verbose              Print verbose output.
  --dry-run                  Run in dry-run mode (don't actually update
                             files), e.g. for use with --update-signatures.
  -h, --help                 Show this message and exit.

Would the re-export overwrite files and keep the same name or create ‘(1)’ files and would require a second run to establish the base file name?

It will by default add (1), etc. to each file unless you specify --overwrite

You can also run --update --cleanup twice to get the same result (this of course results in redundant exports. The first cleanup deletes the pre-existing files and the second --update will use the now available original names and --cleanup will delete the files from the last --update.

I’m almost done with this major refactor of the export logic. The code is much simpler to understand and will be far more maintainable as well as doing the “right” thing in all cases I can think of. However, I’m at an impasse with one issue: osxphotos depends on the export database for state information about the export to do --update. In previous versions, I’ve gone to great lengths in the code to try to do the right thing if for some reason the the export database was missing and user running with --update. I think I’ve found a case where this can be indeterminate and thus may have to drop this “feature” meaning if you delete or lose your export database, you won’t be able to do --update without first doing a full export to rebuild the database. The edge case is this:

1. User runs with --exiftool --touch-file
2. User deletes the export database
3. User runs --update
4. osxphotos will try to determine if the file is the right file and should be exported by comparing the source file in Photos and the destination file on disk. The only information it has to do this (without comparing hashes, which could be slower than just re-exporting) is to look at the file signature. osxphotos currently looks at mode, mtime, and size. If the mtime is the same (it will be if the source file already matched the touch time) and the mode is the same (very likely) then the only information is the size. It’s very likely the size will be different due to running --exiftool but it is possible to run exiftool and have the new file size the same. In this case, the file won’t be updated even though it should be.

I think the only safe solution to guarantee the integrity of the export is to remove the code that attempts to rebuild the export database and just require the user to do a full export if the export database is lost.

Progress! This test uses @oPromessa 's toml file to run multiple exports whilst editing photos, etc and verifies --update behaves as expected.

[I] ➜ pytest tests/test_cli.py::test_export_update_complex
=========================================================== test session starts ============================================================
platform darwin -- Python 3.9.5, pytest-6.2.4, py-1.11.0, pluggy-0.13.1
rootdir: /Users/rhet/Dropbox/Code/osxphotos
plugins: mock-3.6.1
collected 1 item

tests/test_cli.py .                                                                                                                  [100%]

============================================================ 1 passed in 10.71s ============================================================

Jotting all this down so I don’t forget it… I’ve been thinking through how to refactor the export code and I think I have a simpler design I can implement. But–this will likely require some schema changes to the export database and may mean everything gets re-exported the first time you run the new code. The current export logic is a bit of a mess because it’s evolved over time as I’ve added features that affect --update: --exiftool, --convert-to-jpeg, --touch-time, etc. In each case, I’ve tried to adjust the export DB so it doesn’t require a new export and “just works” except that now, with some combinations of options, it “just doesn’t work”.

Here’s the current paths a photo can take to get exported (also, yay that Github markdown now supports mermaid):

graph LR
    A[Export] -->B(Convert to JPEG)
    A --> C(exiftool)
    A --> E[Destination]
    A --> D(Touch File)
    B --> C
    B --> D
    B --> E
    C --> D
    C --> E
    D --> E

To properly do --update, --force-update, --ignore-signature, etc. osxphotos needs to know the following information:

1. Did the destination file change from the last export (e.g. was post-processing done on the file or was the file edited?). If so, osxphotos will re-export the file unless --ignore-signature was specified. (looks at destination file signature)
2. Is exiftool being run? If so, did any of the metadata information that exiftool will write change? Did any of the inputs to this process (e.g. templates, --person-keyword, etc) change? If so, export the photo. (Looks at stored json string of the inputs that osxphotos would feed to exiftool which captures both changes in Photos and changes in the command line options/templates)
3. Is the edited version being exported and if so, was the photo edited in Photos since the last export? (looks at stored file signature of the edited image)
4. Is the file being converted to jpeg? Because this case is covered by step 1 (destination changed), step 2 (photo was edited), step 3 (exiftool also being run), I don’t think any additional checks need to be made for this case. But do need to handle the case where there’s a name collision. E.g. user ran without --convert-to-jpeg and had source images named IMG_1234.HEIC and IMG_1234.JPEG then ran again with --update --convert-to-jpeg, osxphotos will need to know that the existing IMG_1234.JPEG is not the converted version of IMG_1234.HEIC. It does this by looking up the UUID associated with a destination file in the export database.
5. Is touch time needed? As long as the stored signature happens after the file is touched (step 1) then no additional check is needed here. (That’s not currently how osxphotos does it so that will be one schema change). However, if user first ran export without --touch-file then ran again with --update --touch-file, all the files will look different and will be re-exported. osxphotos currently tries to handle this by computing what the file signature would have been had --touch-time been specified the first time but this over-complicates the code. I may need to drop this check. I could add a --touch-only option that doesn’t export anything but touches files and updates the database in case user didn’t use --touch-file then changed their mind.
6. In the case of --force-update, need to compare if any metadata for the image changed in Photos since last export. This is currently done by computing a hash of the json string of all photo metadata and storing that in the database.

Given this logic, I think something like the following should work (pseudo-code):

def should_update(destination):
    if not ignore_signature:
        if not compare_signature(destination, last_signature):
            # destination file doesn't match what was last exported
            return True

    if exiftool:
        if current_exif_data != last_exif_data:
            # either metadata in Photos changed or input templates changed, re-export and run exiftool
            return True

    if edited:
        if not compare_signature(source, edited_signature):
            # edited file in Photos doesn't match what was last exported
            return True

    if force_update:
        if metadata != last_metadata:
            # metadata in Photos changed, force update
            return True
 
    # photo should not be updated
    return False

Detecting the template change is actually pretty easy (the current export database stores a json string of all the fields sent to exiftool) – it’s the interplay of all the other options that’s been tough. I think I’m just going to rewrite this part of the code from scratch. As I was looking at the code this weekend there are parts where I have no idea why I did something…not good for maintainability! I’m mapping out all the different options that affect --update and --force-update and will try to come up with some cleaner logic for detecting changes. Where at all possible, I’d like osxphotos to “do the right thing” even if the user changes export options between updates. But this could take a little while.

--touch-time is definitely a desirable feature so I want to make sure the other options work with it and --update.

No worries and no stress for a quick fix. I have time. And there is always the workaround to delete files on the destination and force

The reason why I use the touch-time is to make sure other apps get the pic ‘date taken’ right 'cause they some times get it from the modify time of the pic/video for files like mpg (yikes… I’m converting them to mov soon…) the modify date of the file would be the current exported time.

Maybe you could leave it to the user to indicate ‘hey keyword_template changed!’ to make it re-export… and simplify the osxphotos logic.

Working on a fix but it’ll take some time. The interplay between all these options you’re using is ferreting out some bugs in the export logic. Very difficult to determine if a file should be exported with --update as so many things can affect it: did data in Photos change? Did template change? Did photo get edited? Was touch time update? Was photo converted to jpeg? And every combination of each of these. osxphotos currently tries to export a photo only if absolutely needed. (e.g. maybe photo wasn’t changed but the touch-time changed) What if user ran with --touch-time then ran again without it? osxphotos currently won’t export the photos all over it’ll just fix the touch time. But this is complicated by all the other options. It might be easier to export if anything changed regardless of whether it really needs to or not.

It’s touch_file. I found a bug in the logic: if touch_file is True and the touch signatures of source and destination file match, then the file is not re-exported even if the exifdata changed. It’ll take a little work to re-do the logic.

https://github.com/RhetTbull/osxphotos/blob/9c26e5519b2d48f3a0ae80d1cc4a765c12b62d40/osxphotos/photoexporter.py#L1042

In this case, the touch signatures match (cmp_touch) even though the exif data doesn’t match (cmp_orig)

I just tried with your .toml and got the same results you did. So it’s something about the combination of options. I’ll dig deeper!

I’ll take a look. This should have been fixed (I even have a test case for this that shows it’s working). I’ll try with your .toml and see if I can figure out what’s going on.

I just did a test and it worked as expected:

Clean export: osxphotos export ~/Desktop/export --keyword-template "{keyword}" --exiftool --db tests/Test-10.15.7.photoslibrary Processed: 27 photos, exported: 33, missing: 3, error: 1

Run update, no files updated osxphotos export ~/Desktop/export --keyword-template "{keyword}" --exiftool --db tests/Test-10.15.7.photoslibrary --update Processed: 27 photos, exported: 0, updated: 0, skipped: 33, updated EXIF data: 0, missing: 3, error: 0

Add a new keyword-template, some files updated as expected osxphotos export ~/Desktop/export --keyword-template "{keyword}" --keyword-template "{label}" --exiftool --db tests/Test-10.15.7.photoslibrary --update Processed: 27 photos, exported: 0, updated: 6, skipped: 27, updated EXIF data: 6, missing: 3, error: 1

Run --update again, no files should be exported osxphotos export ~/Desktop/export --keyword-template "{keyword}" --keyword-template "{label}" --exiftool --db tests/Test-10.15.7.photoslibrary --update Processed: 27 photos, exported: 0, updated: 0, skipped: 33, updated EXIF data: 0, missing: 3, error: 0

The first run with update option I queried the .osxphotos db and get no data on the metadata field. Is this correct?

Yes, this is correct. The metadata field is only calculated when --force-update is used.

--update and --force-update work differently and should not be used interchangeably – you should pick one or the other depending on your use case. --force-update forces the photo to be updated if any of the metadata in Photos changes, even if this change wouldn’t otherwise trigger an export (e.g. you weren’t using --exiftool). --update only exports the photo if the exported photo is different than the one in Photos or if --exiftool would produce different metadata. In your case, you want --update and it should trigger an update based on the new keyword template.

OK, I think it’s fixed for real this time in v0.45.10