diff --git a/CLAUDE.md b/CLAUDE.md index db2be07..35360f2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,16 +61,32 @@ persistence) → GUI (Qt widgets that read the manager and connect to its signal union is **anchor-based** (`merge_track_order`): a track only one copy has is re-inserted after the nearest track both share, not appended at the tail, so a middle insert stays in the middle. Whose order wins is decided by - `Playlist.date_modified` — bumped *only* in - `LibraryManager._set_track_ids` — not by the file's mtime, which moves for - cosmetic reasons. `library_manager._reconcile_playlist` uses the same helper - for the live-reload path. Since Round 42 every `ConflictSummary` carries a + `Playlist.date_modified`, not by the file's mtime, which moves for cosmetic + reasons. Since Round 43 a *stamped* copy also beats an *unstamped* one (a + stamp exists only once LinTunes recorded an edit, so that is real evidence); + mtime is the fallback only when neither side has ever been edited. Two rules + follow: a merge that changes nothing writes nothing (an unconditional write + reset the kept file's mtime and biased that fallback a little more every + round), and a merge whose result is a **union neither copy had** stamps + `date_modified` — that content is newer than both, and saying so is what stops + two machines trading the same tracks back and forth. + `library_manager._reconcile_playlist` is the same decision on the live-reload + path and must read the stamps too; it is reached only for playlists in + `_dirty_playlist_content` (real content edits), never for one that is merely + cosmetically dirty from a column drag. Since Round 42 every `ConflictSummary` carries a **`level`** (`WARNING` / `CHANGE` / `INFO`): a merge that only reconciled column widths, or a smart playlist whose rules are byte-identical on both sides, is `INFO` and must never read as an edit the user made. The dialog (`gui/conflict_dialog.py`) shows one level *and above* and opens at the highest level in the batch, so a routine merge never steals focus but a - blank window is impossible either. + blank window is impossible either. Round 43 made the summaries say something + actionable: which copy won and when it was edited, who wrote the copy + Syncthing set aside (the 7-char device token in the conflict filename, named + via `sync_identity.py`), and every re-inserted track by `Artist — Title` and + position — six in the window, all of them in `what-changed.txt` at the top of + the backup snapshot. **Never label a copy "this machine" from which file + holds the plain name**: that is Syncthing's choice, and it sets the local copy + aside as readily as a remote one. - **`lintunes/storage/play_journal.py`** — why play counts can't conflict. Since Round 38 `library.json` holds only a **base** count and each machine owns @@ -153,6 +169,12 @@ persistence) → GUI (Qt widgets that read the manager and connect to its signal back to `Path("Music")`, which resolved against a working directory GNOME's dash doesn't set predictably. +- **`lintunes/sync_identity.py`** — turns a conflict filename's 7-char device + token into a device name, by reading Syncthing's `config.xml` and deriving + *our own* device ID from `cert.pem` (base32 of the SHA-256 of the DER cert). + Stdlib only, cached, and every failure path returns `None` — a machine with no + Syncthing must still merge, just without naming anyone. + - **`lintunes/mpris.py`** — registers `org.mpris.MediaPlayer2.lintunes` over D-Bus so the desktop's media keys / now-playing popup control playback. Spacebar and arrow keys are handled locally via `MainWindow.eventFilter`. diff --git a/lintunes/__init__.py b/lintunes/__init__.py index ca5e082..1730c34 100644 --- a/lintunes/__init__.py +++ b/lintunes/__init__.py @@ -1,3 +1,3 @@ """LinTunes — iTunes-style music library manager and player for Linux.""" -__version__ = "0.12.1" +__version__ = "0.13.0" diff --git a/lintunes/gui/conflict_dialog.py b/lintunes/gui/conflict_dialog.py index 7bf884f..27a7684 100644 --- a/lintunes/gui/conflict_dialog.py +++ b/lintunes/gui/conflict_dialog.py @@ -27,7 +27,8 @@ from lintunes.storage.conflict_resolver import ( INTRO = ("LinTunes found changes made on more than one machine and merged them " "(play counts kept highest, newest edits win, nothing removed). Both " "versions were backed up first — restore them if a merge isn't what " - "you wanted.") + "you wanted. The backup folder holds what-changed.txt: the same merge " + "in full, every track named.") # Coarsest first, so the combo reads top-down like a volume knob. LEVEL_CHOICES = [ @@ -70,7 +71,7 @@ class ConflictSummaryDialog(QDialog): layout.addWidget(self._body, 1) buttons = QHBoxLayout() - self._open_btn = QPushButton("Open backup folder") + self._open_btn = QPushButton("Open merge report") self._open_btn.clicked.connect(self._open_backup) self._restore_btn = QPushButton("Restore pre-merge backup") self._restore_btn.clicked.connect(self._restore_backup) diff --git a/lintunes/library_manager.py b/lintunes/library_manager.py index c5067bd..417feef 100644 --- a/lintunes/library_manager.py +++ b/lintunes/library_manager.py @@ -61,6 +61,11 @@ class LibraryManager(QObject): self._dirty_tracks = False self._dirty_metadata = False self._dirty_playlists: set[str] = set() + # Which of those are dirty because their *contents* changed, as opposed + # to a column drag or a sort click. A sync that arrives while a playlist + # is merely cosmetically dirty must adopt the disk copy wholesale, not + # treat this machine as the one holding the edits. + self._dirty_playlist_content: set[str] = set() self._deleted_playlists: set[str] = set() # Play counts live in per-machine journals, not library.json — see @@ -238,6 +243,7 @@ class LibraryManager(QObject): for playlist in playlists: self.library.playlists[playlist.persistent_id] = playlist self._dirty_playlists.add(playlist.persistent_id) + self._dirty_playlist_content.add(playlist.persistent_id) self._deleted_playlists.discard(playlist.persistent_id) self._dirty_metadata = True self._schedule_save() @@ -248,6 +254,7 @@ class LibraryManager(QObject): pid = playlist.persistent_id self.library.playlists.pop(pid, None) self._dirty_playlists.discard(pid) + self._dirty_playlist_content.discard(pid) self._deleted_playlists.add(pid) self._dirty_metadata = True self._schedule_save() @@ -257,14 +264,19 @@ class LibraryManager(QObject): playlist = self.library.playlists.get(pid) if playlist: playlist.name = name - self._mark_playlist(pid) + # A rename is an edit like any other: without a stamp it is + # invisible to every merge, and the other machine's older name wins + # by default forever. + playlist.date_modified = _utc_now_iso() + self._mark_playlist(pid, content=True) self.playlists_changed.emit() def _apply_reparent(self, pid: str, parent_pid: str): playlist = self.library.playlists.get(pid) if playlist: playlist.parent_persistent_id = parent_pid - self._mark_playlist(pid) + playlist.date_modified = _utc_now_iso() + self._mark_playlist(pid, content=True) self.playlists_changed.emit() # ---- playlist contents ---- @@ -326,14 +338,16 @@ class LibraryManager(QObject): return playlist.track_ids = list(ids) # The single funnel for every content change (add / remove / reorder / - # undo), and deliberately the *only* place date_modified moves — a - # column resize must not look like an edit to the merge. Nor may a smart - # recompute: it fires every time a play count moves, and bumping the - # timestamp there made "most recently edited" meaningless for smart - # playlists. See conflict_resolver._playlist_newer. + # undo). date_modified moves only where content genuinely changed: here, + # a rename or reparent, and a reconcile whose union is content neither + # copy had — never for a column resize, which must not look like an edit + # to the merge. Nor for a smart recompute: it fires every time a play + # count moves, and bumping the timestamp there made "most recently + # edited" meaningless for smart playlists. See + # conflict_resolver._playlist_newer. if touch: playlist.date_modified = _utc_now_iso() - self._mark_playlist(pid) + self._mark_playlist(pid, content=True) self.playlist_content_changed.emit(pid) def _push_track_ids(self, label, pid, before, after): @@ -892,8 +906,10 @@ class LibraryManager(QObject): # ---- saving ---- - def _mark_playlist(self, pid: str): + def _mark_playlist(self, pid: str, content: bool = False): self._dirty_playlists.add(pid) + if content: + self._dirty_playlist_content.add(pid) self._dirty_metadata = True self._schedule_save() @@ -921,6 +937,7 @@ class LibraryManager(QObject): json_storage.save_playlist(playlist, self.data_dir) self._record_sig(self.data_dir / "playlists" / f"{pid}.json") self._dirty_playlists.clear() + self._dirty_playlist_content.clear() for pid in list(self._deleted_playlists): json_storage.delete_playlist_file(pid, self.data_dir) self._own_sigs.pop(str(self.data_dir / "playlists" / f"{pid}.json"), None) @@ -985,7 +1002,13 @@ class LibraryManager(QObject): touches the player.""" was_dirty = (self._dirty_tracks or bool(self._dirty_playlists) or self._dirty_metadata) - dirty_pids = set(self._dirty_playlists) + # Only a *content* edit makes this machine's copy the one to reconcile + # against. A playlist whose local dirt is a column width adopts the disk + # copy — otherwise a sync that lands in the three seconds after a sort + # click reverted the other machine's reorder and then flushed the + # revert back to disk. + content_pids = set(self._dirty_playlist_content) + pending_pids = set(self._dirty_playlists) disk = json_storage.load_library(self.data_dir) # Fold the journals onto the disk copy *before* reconciling. The tracks # in memory already carry effective counts, so without this the merge @@ -1007,12 +1030,12 @@ class LibraryManager(QObject): mem_pl = self.library.playlists.get(pid) if mem_pl is None: self.library.playlists[pid] = disk_pl - elif pid in dirty_pids: - _reconcile_playlist(mem_pl, disk_pl) # keep local edits, union in + elif pid in content_pids: + _reconcile_playlist(mem_pl, disk_pl) # merge the two edits else: mem_pl.__dict__.update(disk_pl.__dict__) # adopt disk (identity kept) for pid in list(self.library.playlists): - if pid not in disk.playlists and pid not in dirty_pids: + if pid not in disk.playlists and pid not in pending_pids: self.library.playlists.pop(pid, None) if not self._dirty_metadata: @@ -1064,13 +1087,36 @@ def _reconcile_track(mem_track, disk_track): def _reconcile_playlist(mem_pl, disk_pl): - """Reconcile a playlist we were locally editing with the disk copy: our order - wins (the local edit is the more recent one), and tracks that exist only on - the other machine are merged back into position rather than appended (a merge - never drops tracks).""" + """Reconcile a playlist we were locally editing with the copy that just + synced in. Whichever side recorded the more recent edit sets the order, and + tracks that exist only on the other side are merged back into position + rather than appended (a merge never drops tracks). + + The stamps used to go unread here: the local order simply won, on the + strength of a docstring asserting it was the newer one. It often wasn't — + and the result was flushed straight back to disk, so a reorder made on the + other machine was quietly undone. Same rule as + conflict_resolver._playlist_newer, one layer up.""" from lintunes.storage.conflict_resolver import merge_track_order - mem_pl.track_ids = merge_track_order(list(mem_pl.track_ids), - list(disk_pl.track_ids)) + ours, theirs = mem_pl.date_modified, disk_pl.date_modified + disk_newer = bool(theirs) and (not ours or theirs > ours) + mine, other = list(mem_pl.track_ids), list(disk_pl.track_ids) + merged = (merge_track_order(other, mine) if disk_newer + else merge_track_order(mine, other)) + if disk_newer: + # The disk copy is the edited one, so its name/folder/columns are the + # edited ones too — all four used to be dropped on the floor here. + mem_pl.name = disk_pl.name + mem_pl.parent_persistent_id = disk_pl.parent_persistent_id + mem_pl.settings = disk_pl.settings + if disk_newer and merged == other and theirs: + mem_pl.date_modified = theirs + elif merged != mine: + # A union is content neither copy had, so it is newer than both; saying + # so is what lets the other machine adopt it instead of the two trading + # the same tracks back and forth. + mem_pl.date_modified = _utc_now_iso() + mem_pl.track_ids = merged def _utc_now_iso() -> str: diff --git a/lintunes/storage/conflict_resolver.py b/lintunes/storage/conflict_resolver.py index e6345eb..698854c 100644 --- a/lintunes/storage/conflict_resolver.py +++ b/lintunes/storage/conflict_resolver.py @@ -1,14 +1,24 @@ import re import shutil from dataclasses import dataclass, field -from datetime import datetime +from datetime import datetime, timezone from pathlib import Path +from lintunes import sync_identity from lintunes.storage.json_storage import read_json, write_json # Syncthing conflict file pattern: filename.sync-conflict-20240101-123456-ABCDEFG.json -CONFLICT_PATTERN = re.compile(r"^(.+)\.sync-conflict-(\d{8}-\d{6})-\w+(\.\w+)$") +# The last group is the first seven characters of the device ID that last wrote +# the copy being set aside — the only evidence anywhere about who edited what, +# and it used to be matched by a bare \w+ and thrown away with the file. +CONFLICT_PATTERN = re.compile( + r"^(.+)\.sync-conflict-(\d{8}-\d{6})-(\w+)(\.\w+)$") + +# How many re-inserted tracks to name in the merge window before deferring to +# the full list in the backup report. Long enough to recognize what happened, +# short enough that a 2400-track playlist doesn't fill the screen. +TRACKS_TO_NAME = 6 # How many pre-merge snapshots to keep in /.resolved. Each library.json # merge stores two 15 MB copies, and the folder rides the Syncthing share, so an @@ -44,6 +54,32 @@ class ConflictSummary: lines: list[str] = field(default_factory=list) # what was reconciled backup_dir: str = "" # /.resolved/ level: str = CHANGE # WARNING | CHANGE | INFO + # Everything below is for what-changed.txt in the backup folder, which is + # uncapped: `lines` is what fits in a window, `detail` is the whole story. + detail: list[str] = field(default_factory=list) + conflict_file: str = "" # the real Syncthing filename (then deleted) + device: str = "" # its 7-char device ID prefix + device_label: str = "" # ...decoded, when Syncthing's config says + + +@dataclass +class ConflictFile: + """One ``*.sync-conflict-*`` file and the file it conflicts with. + + ``original_path`` is the copy holding the plain name — the one Syncthing + *kept* — and ``conflict_path`` is the one it *set aside*. Which is which is + Syncthing's call, not a statement about who edited it: it sets the local + copy aside just as readily as a remote one. ``device`` is the only real + authorship evidence there is. + """ + conflict_path: Path + original_path: Path + stamp: str = "" # from the filename, e.g. "20240101-123456" + device: str = "" # 7-char device ID prefix of whoever wrote the set-aside copy + + @property + def device_label(self) -> str: + return sync_identity.label_for(self.device) or "" def resolve_conflicts(data_dir: Path) -> list[ConflictSummary]: @@ -67,7 +103,9 @@ def resolve_conflicts(data_dir: Path) -> list[ConflictSummary]: (backup_dir / "incoming").mkdir(parents=True, exist_ok=True) summaries = [] - for conflict_path, original_path in conflict_files: + label = _Labeler(data_dir) + for item in conflict_files: + conflict_path, original_path = item.conflict_path, item.original_path rel = original_path.relative_to(data_dir) # Back up both sides first (belt and suspenders). if original_path.exists(): @@ -86,7 +124,7 @@ def resolve_conflicts(data_dir: Path) -> list[ConflictSummary]: elif original_path.name == "library_metadata.json": summary = _merge_metadata(original_path, conflict_path) elif original_path.parent.name == "playlists": - summary = _merge_playlist(original_path, conflict_path) + summary = _merge_playlist(original_path, conflict_path, item, label) elif original_path.parent.name == "plays": summary = _merge_play_journal(original_path, conflict_path) else: @@ -99,14 +137,51 @@ def resolve_conflicts(data_dir: Path) -> list[ConflictSummary]: level=WARNING) summary.backup_dir = str(backup_dir) + # The conflict filename is the only record of which device wrote the + # copy that lost its name, and it is about to be deleted. + summary.conflict_file = conflict_path.name + summary.device = item.device + summary.device_label = item.device_label summaries.append(summary) # Remove the conflict file from the data dir (its copy is in the backup) # so it isn't reprocessed and Syncthing stops flagging it. conflict_path.unlink(missing_ok=True) + _write_backup_report(backup_dir, summaries) return summaries +def _write_backup_report(backup_dir: Path, summaries: list[ConflictSummary]): + """Leave a readable account of the merge beside the two JSON snapshots. + + Opening the backup folder used to show `original/` and `incoming/`, each + holding one playlist file named after a hex id — enough to restore from, + useless for deciding whether you want to. This is the same merge in words, + uncapped: every re-inserted track, not the first handful.""" + out = [f"LinTunes merge — {datetime.now().strftime('%A %B %-d, %Y at %-I:%M %p')}", + "", + "Pre-merge copies of every file below are in original/ (the copy that " + "was in place) and", + "incoming/ (the copy Syncthing had set aside). The “Restore pre-merge " + "backup” button in the", + "merge window puts original/ back.", + ""] + for summary in summaries: + out.append("=" * 72) + out.append(summary.file) + if summary.conflict_file: + who = f" — written on {summary.device_label}" if summary.device_label \ + else f" — device {summary.device}" if summary.device else "" + out.append(f" conflict file: {summary.conflict_file}{who}") + out.append("") + out.extend(f" {line}" for line in (summary.detail or summary.lines)) + out.append("") + try: + (backup_dir / "what-changed.txt").write_text("\n".join(out) + "\n") + except OSError: + pass # a report we couldn't write must never break the merge + + def restore_backup(backup_dir: Path, data_dir: Path) -> list[str]: """Undo a merge: copy the pre-merge ``original/`` snapshot back over the current files. Returns the list of restored relative paths.""" @@ -138,19 +213,71 @@ def _prune_backups(data_dir: Path, keep: int = BACKUPS_TO_KEEP): shutil.rmtree(stale, ignore_errors=True) -def _find_conflict_files(data_dir: Path) -> list[tuple[Path, Path]]: +def _find_conflict_files(data_dir: Path) -> list["ConflictFile"]: results = [] for path in data_dir.rglob("*.sync-conflict-*"): if ".resolved" in path.parts: # don't reprocess our own backups continue match = CONFLICT_PATTERN.match(path.name) if match: - original_name = match.group(1) + match.group(3) - original_path = path.parent / original_name - results.append((path, original_path)) + original_name = match.group(1) + match.group(4) + results.append(ConflictFile( + conflict_path=path, + original_path=path.parent / original_name, + stamp=match.group(2), + device=match.group(3).upper()[:7])) return results +def _track_labels(data_dir: Path) -> dict[str, str]: + """{track id: "Artist — Title"} straight out of library.json. + + Read lazily and at most once per resolve_conflicts() call: it is the 15 MB + file, and most merges (a play journal, a column width) never need a name. + """ + labels = {} + try: + for tid, track in read_json(data_dir / "library.json").items(): + title = track.get("name") or f"track {tid}" + artist = track.get("artist") or track.get("album_artist") + labels[str(tid)] = f"{artist} — {title}" if artist else title + except Exception: + pass # no library yet, or an unreadable one: fall back to bare ids + return labels + + +class _Labeler: + """Names tracks for the report, loading library.json only if asked.""" + + def __init__(self, data_dir: Path): + self._data_dir = data_dir + self._labels: dict[str, str] | None = None + + def __call__(self, tid) -> str: + if self._labels is None: + self._labels = _track_labels(self._data_dir) + return self._labels.get(str(tid)) or f"track {tid}" + + +def _local_time(stamp: str | None) -> str: + """A naive-UTC ISO stamp as local wall-clock, the way the window shows it.""" + if not stamp: + return "" + try: + when = datetime.fromisoformat(stamp) + except ValueError: + return "" + if when.tzinfo is None: + when = when.replace(tzinfo=timezone.utc) + return when.astimezone().strftime("%b %-d, %-I:%M %p") + + +def _utc_now_iso() -> str: + """Must match library_manager._utc_now_iso — the stamps are compared as + strings across machines, so both writers have to agree on naive UTC.""" + return datetime.now(timezone.utc).replace(tzinfo=None).isoformat() + + def _merge_library(original_path: Path, conflict_path: Path) -> ConflictSummary: original = read_json(original_path) conflict = read_json(conflict_path) @@ -288,27 +415,87 @@ def merge_track_order(primary: list[int], secondary: list[int]) -> list[int]: def _playlist_newer(original: dict, conflict: dict, - original_path: Path, conflict_path: Path) -> bool: - """Is the incoming copy the more recently *edited* one? + original_path: Path, conflict_path: Path) -> tuple[bool, str]: + """Is the set-aside copy the more recently *edited* one? With the reason. Prefer the playlist's own ``date_modified``, which only moves when its contents change. File mtime is a lie: the last column is stretch-sized, so resizing the window rewrites the open playlist's JSON — "most recently - edited" used to mean "most recently resized". Falls back to mtime for - playlists written before ``date_modified`` existed. + edited" used to mean "most recently resized". Worse, a merge rewrites the + file it keeps, so every mtime comparison was a little more biased toward + whichever copy happened to be in place last time. + + A stamp only exists once LinTunes has recorded an edit to that playlist, so + a stamped copy against an unstamped one is real evidence — the unstamped + side has *never* been edited — and it must not fall through to mtime. That + half-stamped case is the common one: every playlist imported from iTunes and + never reordered on this machine is unstamped here and stamped wherever it + was edited. """ ours, theirs = original.get("date_modified"), conflict.get("date_modified") if ours and theirs: - return theirs > ours - return conflict_path.stat().st_mtime > original_path.stat().st_mtime + return theirs > ours, "" + if theirs or ours: + return bool(theirs), ("Only one of the two has ever recorded an edit, " + "so that is the one that was edited.") + return (conflict_path.stat().st_mtime > original_path.stat().st_mtime, + "Neither copy has ever recorded an edit, so this went by the file " + "timestamps instead.") -def _merge_playlist(original_path: Path, conflict_path: Path) -> ConflictSummary: +def _positions(merged: list, tids, label) -> list[str]: + """"Artist — Title → position 12, after “…”" for each of `tids`.""" + index = {} + for i, tid in enumerate(merged): + index.setdefault(tid, i) + out = [] + for tid in sorted(tids, key=lambda t: index.get(t, len(merged))): + i = index.get(tid) + if i is None: + out.append(f"{label(tid)}") + elif i == 0: + out.append(f"{label(tid)} → position 1, at the top") + else: + out.append(f"{label(tid)} → position {i + 1}, " + f"after “{label(merged[i - 1])}”") + return out + + +def _count(n: int, noun: str = "track") -> str: + return f"{n} {noun}" if n == 1 else f"{n} {noun}s" + + +def _capped(entries: list[str]) -> list[str]: + """Bullet the first few and point at the backup report for the rest.""" + lines = [f" • {entry}" for entry in entries[:TRACKS_TO_NAME]] + if len(entries) > TRACKS_TO_NAME: + lines.append(f" …and {len(entries) - TRACKS_TO_NAME} more — the full " + "list is in what-changed.txt in the backup folder.") + return lines + + +def _merge_playlist(original_path: Path, conflict_path: Path, + item: "ConflictFile" = None, label=str) -> ConflictSummary: original = read_json(original_path) conflict = read_json(conflict_path) name = original.get("name") or conflict.get("name") or original_path.stem + device = (item.device_label if item else "") or "" - conflict_newer = _playlist_newer(original, conflict, original_path, conflict_path) + conflict_newer, why = _playlist_newer( + original, conflict, original_path, conflict_path) + + # Which copy is which. "this machine" used to be inferred from which file + # held the plain name, and Syncthing does not work that way — it sets the + # local copy aside as readily as a remote one, so that label was a coin + # flip presented as a fact. Only the device token in the conflict filename + # says anything about authorship, and it only speaks for the copy that was + # set aside. + set_aside = "the copy Syncthing set aside" + in_place = "the copy that was already here" + winner, loser = ((set_aside, in_place) if conflict_newer + else (in_place, set_aside)) + won_at = _local_time((conflict if conflict_newer else original).get("date_modified")) + lost_at = _local_time((original if conflict_newer else conflict).get("date_modified")) # Smart playlists: membership is derived, so keep the newer criteria and let # it recompute (unioning derived track_ids would resurrect non-matches). @@ -326,11 +513,13 @@ def _merge_playlist(original_path: Path, conflict_path: Path) -> ConflictSummary ["Rules are identical on both machines — only the auto-updated " "membership differed. Rebuilt from the rules."], level=INFO) - src = "the other machine" if conflict_newer else "this machine" - return ConflictSummary( - name, "playlist", - [f"Rules taken from {src} (edited more recently)."], - level=CHANGE) + lines = [f"Rules taken from {winner}" + + (f", edited {won_at}." if won_at else ".")] + if device: + lines.append(f"That set-aside copy was last written on {device}.") + if why: + lines.append(why) + return ConflictSummary(name, "playlist", lines, level=CHANGE) orig_ids = list(original.get("track_ids", [])) conf_ids = list(conflict.get("track_ids", [])) @@ -339,21 +528,47 @@ def _merge_playlist(original_path: Path, conflict_path: Path) -> ConflictSummary if conflict_newer: # The other copy was edited last, so it sets the order; anything only we # had is re-inserted at its anchor rather than dumped at the tail. - original["track_ids"] = merge_track_order(conf_ids, orig_ids) + merged = merge_track_order(conf_ids, orig_ids) + else: + merged = merge_track_order(orig_ids, conf_ids) + original["track_ids"] = merged + + renamed_from = "" + if conflict_newer: if "settings" in conflict: original["settings"] = conflict["settings"] - if conflict.get("date_modified"): - original["date_modified"] = conflict["date_modified"] - order_src = "the other machine" - moved_in = len(conf_set - orig_set) + # A rename or a move into a folder made on the winning copy used to be + # discarded by every merge: only track_ids and settings were adopted. + if conflict.get("name") and conflict["name"] != original.get("name"): + renamed_from, original["name"] = original.get("name", ""), conflict["name"] + name = conflict["name"] + if "parent_persistent_id" in conflict: + original["parent_persistent_id"] = conflict["parent_persistent_id"] + + # Whose stamp the merged file carries. A union is content *neither* copy + # had, so it is genuinely newer than both — stamping it is what lets the + # other machine adopt this result next time instead of the two of them + # trading the same tracks back and forth all evening. + if merged == orig_ids: + pass # our content is unchanged; leave the stamp where it was + elif conflict_newer and merged == conf_ids and conflict.get("date_modified"): + original["date_modified"] = conflict["date_modified"] else: - original["track_ids"] = merge_track_order(orig_ids, conf_ids) - order_src = "this machine" - moved_in = len(orig_set - conf_set) + # Anything else is content this file did not have, so it is newer than + # what this file's stamp claims. Note the `conflict_newer` guard above: + # taking the stamp off a copy that *lost* would walk the timestamp + # backwards and hand the next merge a file that lies about its age. + original["date_modified"] = _utc_now_iso() - write_json(original_path, original) + if original != read_json(original_path): + # Only write when something actually changed. An unconditional write + # reset this file's mtime on every merge, and since the mtime fallback + # compares against a conflict file that keeps its origin's mtime, each + # merge made the copy in place harder to beat on the next one. + write_json(original_path, original) - added = len(set(original["track_ids"]) - orig_set) + added = [tid for tid in conf_set - orig_set] + only_here = [tid for tid in orig_set - conf_set] if orig_ids == conf_ids: # The two copies held the same tracks in the same order, so they differed # only in column widths or sort order — a window resize, not an edit. @@ -364,11 +579,50 @@ def _merge_playlist(original_path: Path, conflict_path: Path) -> ConflictSummary name, "playlist", ["Only the column layout differed; track order unchanged."], level=INFO) - lines = [f"Order kept from {order_src} (most recently edited)."] + + lines = [f"Kept the order from {winner}" + + (f", edited {won_at}." if won_at else ".")] + lines.append(f"{loser[0].upper() + loser[1:]} was edited {lost_at}." + if lost_at else + f"{loser[0].upper() + loser[1:]} has never recorded an edit.") + if device: + lines.append(f"That set-aside copy was last written on {device}.") + if why: + lines.append(why) + if renamed_from: + lines.append(f"Renamed from “{renamed_from}” — that rename was made on " + "the copy that won.") + + detail = list(lines) if added: - lines.append(f"{added} track(s) that were only in the other copy were kept, " - "back in position (nothing is removed on a merge).") - return ConflictSummary(name, "playlist", lines, level=CHANGE) + entries = _positions(merged, added, label) + headline = (f"{_count(len(added))} only the other copy had " + f"{'was' if len(added) == 1 else 'were'} put back where " + f"{'it' if len(added) == 1 else 'they'} had been:") + lines.append(headline) + lines.extend(_capped(entries)) + detail.append(headline) + detail.extend(f" • {entry}" for entry in entries) + if only_here: + entries = _positions(merged, only_here, label) + # Deliberately not "the other copy deleted these": a track only this + # copy has was either added here or removed there, and two lists give + # no way to tell which. Both readings are offered rather than guessed. + headline = (f"{_count(len(only_here))} here that the other copy did " + f"not have {'was' if len(only_here) == 1 else 'were'} " + "kept — a merge never removes:") + it, they, them = (("it", "it", "it") if len(only_here) == 1 + else ("them", "they", "them")) + note = (f"If you added {it} here, that's all this is. If you deleted " + f"{it} on the other machine, delete {them} here too or " + f"{they} will come back on the next merge.") + lines.append(headline) + lines.extend(_capped(entries)) + lines.append(note) + detail.append(headline) + detail.extend(f" • {entry}" for entry in entries) + detail.append(note) + return ConflictSummary(name, "playlist", lines, level=CHANGE, detail=detail) def _merge_play_journal(original_path: Path, conflict_path: Path) -> ConflictSummary: @@ -447,14 +701,23 @@ def _merge_metadata(original_path: Path, conflict_path: Path) -> ConflictSummary # machine can silently revert a folder change made on the other. if before is not None and other is not None: kept = read_json(original_path) - loser = other if kept.get("music_folder_set_at") == before.get( - "music_folder_set_at") else before - if _newer_stamp(loser.get("music_folder_set_at"), + # Decide the folder from the two *inputs*, never from whichever copy the + # mtime pick above landed on: when both files are written in the same + # instant that pick is a coin flip, and it was deciding a deliberate + # setting (and making this file's own test pass half the time). + source = other if _newer_stamp( + other.get("music_folder_set_at"), + before.get("music_folder_set_at")) else before + if _newer_stamp(source.get("music_folder_set_at"), kept.get("music_folder_set_at")): for key in _MUSIC_KEYS: - if key in loser: - kept[key] = loser[key] + if key in source: + kept[key] = source[key] write_json(original_path, kept) + # CHANGE when the folder we end up with is not the one we started with — + # the event the user cares about, whether we adopted it here or the + # whole-file pick had already taken that copy. + if any(kept.get(key) != before.get(key) for key in _MUSIC_KEYS): summary.lines.append( "Music folder taken from the copy that set it most recently.") summary.level = CHANGE diff --git a/lintunes/sync_identity.py b/lintunes/sync_identity.py new file mode 100644 index 0000000..6d96f80 --- /dev/null +++ b/lintunes/sync_identity.py @@ -0,0 +1,111 @@ +"""Who wrote the copy Syncthing set aside. + +A conflict file is named ``.sync-conflict--