feat: per-library destination subdirectories (photos.library_destinations)#456
Open
epheterson wants to merge 4 commits into
Open
feat: per-library destination subdirectories (photos.library_destinations)#456epheterson wants to merge 4 commits into
epheterson wants to merge 4 commits into
Conversation
…ions)
New optional config block: photos.library_destinations maps each iCloud
library to a subdirectory of photos.destination. When unset (default),
all libraries share the single photos.destination tree — preserving
the historical mandarons/icloud-docker behaviour, no breaking change.
Example:
photos:
destination: photos
library_destinations:
PrimarySync: personal
SharedLibrary: shared
Personal photos land in <photos.destination>/personal/, Shared library
photos in <photos.destination>/shared/.
Threaded through sync_photos.sync_photos,
_sync_all_photos_first_for_hardlinks, and _sync_albums_by_configuration.
New helper _library_destination resolves the per-library path.
Obsolete-file cleanup walks each per-library subdir independently when
library_destinations is set, falling back to the legacy single-destination
walk otherwise.
10 new tests in tests/test_library_destinations.py covering: config
helper defaults / dict / non-dict, _library_destination joins +
creates dirs / handles None mapping / handles missing library / handles
nested subdirs.
…ones Follow-up to a8810e5. Apple's modern iCloud Shared Photo Library is exposed by icloudpy under a GUID-based zone name like 'SharedSync-3C977B4A-C15A-46E4-9854-585B9342C409', not the literal string 'SharedLibrary' that early mandarons docs (and the v1 of this feature's README) used as the example. Without this fix, a user whose config follows the documented example ('SharedLibrary: Shared') would silently fall through to the base destination — shared-library photos would land alongside primary photos in /icloud/photos/ instead of in /icloud/photos/Shared/. Caught during a real boredazfcuk migration: dry-run output reported 'Photos libraries available: SharedSync-3C977B4A-..., PrimarySync' while the config expected the literal 'SharedLibrary'. _library_destination now has three rules, in priority order: 1. Exact match (unchanged). 2. If the zone name starts with 'SharedSync-' and config has a 'SharedLibrary' key, use that (new alias rule). 3. Fall through to base destination (unchanged). Three new tests in tests/test_library_destinations.py cover the alias behaviour: GUID zone matched via SharedLibrary key, alias does NOT catch non-SharedSync libraries, and exact-GUID config wins over the alias (predictability for users who pinned the GUID before this feature shipped). The base PR (a8810e5) and this follow-up will land as a single PR upstream; preserved as a separate commit here so the alias delta is reviewable in isolation against the original feature.
Upstream CI runs ruff as part of the test step; the prior commit had auto-fixable lint issues that ruff caught. Pure formatting, no semantic change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…86% → 100%) Upstream CI gates at 100% coverage; the new `if library_destinations: for library in libraries: remove_obsolete_files(...)` branch in sync_photos.sync_photos was the last 3 uncovered lines on this branch (because the existing remove_obsolete tests all hit the legacy single-destination path). Added TestSyncPhotosRemoveObsoletePerLibrary verifying remove_obsolete_files is invoked once per library_destinations subdir (not once on the parent — that would walk siblings' photo trees). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an optional photos.library_destinations config block that maps each iCloud library (e.g. PrimarySync, SharedLibrary/SharedSync-…) to a subdirectory under photos.destination, so personal and shared library content can be split on disk without running two containers. Behavior is unchanged when the new block is omitted.
Changes:
- New
config_parser.get_photos_library_destinations()returning adict[str, str](or{}when unset/invalid), with key/value coercion tostr. - New
_library_destination()helper insync_photos.pythat resolves per-library subdirs (exact match →SharedLibraryalias forSharedSync-…zones → fallthrough), creating the directory as needed, and threads it through_sync_all_photos_first_for_hardlinksand_sync_albums_by_configuration. - Obsolete-file cleanup now walks each configured library's subdir individually when the mapping is set, falling back to the legacy single-destination walk otherwise. Adds
tests/test_library_destinations.pycovering the helper and per-library cleanup.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/config_parser.py | Adds get_photos_library_destinations returning a normalized dict[str, str] mapping (or {}). |
| src/sync_photos.py | Plumbs library_destinations through sync entrypoints, adds _library_destination resolver, and per-library cleanup loop. |
| tests/test_library_destinations.py | Unit tests for the config helper, _library_destination rules (incl. SharedLibrary alias), and per-library remove_obsolete_files invocation. |
epheterson
added a commit
to epheterson/icloud-docker
that referenced
this pull request
May 31, 2026
…startup
Copilot's review on this PR caught a real runtime bug. On startup
``SyncState.{drive,photos}_time_remaining`` are both ``0``. The
marker-missing branch was returning early WITHOUT touching the
countdown, so the next iteration of the outer sync loop hit
``_calculate_next_sync_schedule`` with ``time_remaining=0`` and
computed a 0-second sleep. Result: tight busy loop logging the
"marker missing" error at full CPU until the user fixed the mount
and touched the marker file.
Fix: when ``_check_mount_marker`` returns False, reset the relevant
``*_time_remaining`` to the full configured interval before
returning ``None``. The next loop iteration now sleeps the proper
duration (default 30 minutes / 12 hours / whatever the user
configured) before re-checking.
Applied to both the Drive and Photos branches in ``sync.py``.
Tests:
- Updated ``test_drive_sync_returns_none_when_marker_missing`` to
assert the countdown reset (the bug only manifests when the
startup value is 0; test now sets that explicitly and verifies
the post-call value equals the interval).
- New ``test_photos_sync_resets_countdown_when_marker_missing``
mirrors the assertion for the Photos branch.
The third Copilot point on this PR -- that ``photos.library_destinations``
is a forward reference to feat/photos-library-destinations (mandarons#456) --
is intentional defensive scaffolding so this PR merges in any order
relative to mandarons#456. The existing comment already explains this; no
code change needed.
Verified on python:3.10 docker mirroring CI: ruff clean, 456
passed, 100.00% coverage.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
epheterson
added a commit
to epheterson/icloud-docker
that referenced
this pull request
May 31, 2026
…startup
Copilot's review on this PR caught a real runtime bug. On startup
``SyncState.{drive,photos}_time_remaining`` are both ``0``. The
marker-missing branch was returning early WITHOUT touching the
countdown, so the next iteration of the outer sync loop hit
``_calculate_next_sync_schedule`` with ``time_remaining=0`` and
computed a 0-second sleep. Result: tight busy loop logging the
"marker missing" error at full CPU until the user fixed the mount
and touched the marker file.
Fix: when ``_check_mount_marker`` returns False, reset the relevant
``*_time_remaining`` to the full configured interval before
returning ``None``. The next loop iteration now sleeps the proper
duration (default 30 minutes / 12 hours / whatever the user
configured) before re-checking.
Applied to both the Drive and Photos branches in ``sync.py``.
Tests:
- Updated ``test_drive_sync_returns_none_when_marker_missing`` to
assert the countdown reset (the bug only manifests when the
startup value is 0; test now sets that explicitly and verifies
the post-call value equals the interval).
- New ``test_photos_sync_resets_countdown_when_marker_missing``
mirrors the assertion for the Photos branch.
The third Copilot point on this PR -- that ``photos.library_destinations``
is a forward reference to feat/photos-library-destinations (mandarons#456) --
is intentional defensive scaffolding so this PR merges in any order
relative to mandarons#456. The existing comment already explains this; no
code change needed.
Verified on python:3.10 docker mirroring CI: ruff clean, 456
passed, 100.00% coverage.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New optional config block
photos.library_destinationsmaps each iCloudlibrary to a subdirectory of
photos.destination:```yaml
photos:
destination: photos
library_destinations:
PrimarySync: personal
SharedLibrary: shared
```
Personal photos then land in
<photos.destination>/personal/, SharedLibrary photos in
<photos.destination>/shared/. Whenlibrary_destinationsis unset (the default), all libraries share the single
photos.destinationtree — preserving the historical mandarons/icloud-docker behaviour, no
breaking change.
Motivation
Common request from users who want their iCloud Shared Library kept
separate from their personal photos on disk (for separate Plex libraries,
different sharing/backup policies, etc.). Currently the only way to get
this is to run two
mandarons/icloud-dockercontainers with differentlibraries:filters — which means two auth events and two cookie storesfor the same Apple ID.
Implementation
config_parser.get_photos_library_destinations(config)returns adict mapping library name → subdir (empty dict when unset).
sync_photos._library_destination(base, library, mapping)helperresolves the per-library destination and creates the subdir.
sync_photos.sync_photos,_sync_all_photos_first_for_hardlinks,and
_sync_albums_by_configurationas a kwarg withNonedefault(backward-compatible).
library_destinationsis set, falling back to the legacysingle-destination walk otherwise.
Tests
10 new tests in
tests/test_library_destinations.py:{}when unset / when not a dict / coercesnon-string keys+values to str, returns mapping when configured.
_library_destinationhelper: returns base when no mapping, returnsbase when library not in mapping, joins subdir + creates dir, handles
nested subdirs,
Nonemapping is safe.