v0.2.2: device-sync guards — quit warning, sleep/shutdown inhibition
Quitting mid-transfer now asks (Keep Syncing / Quit Anyway) from closeEvent, which every quit path hits — including the self-update restart, which previously bypassed closeEvent via a bare quit() and now routes through close(). A running sync holds its own GNOME inhibitor (suspend + logout, "Syncing a playlist to a device") so the machine won't sleep or shutdown/restart under a copy. Along the way: the Inhibit D-Bus call marshaled Python ints as signed against GNOME's (susu) signature, so every call was rejected and the playback sleep inhibitor had silently never worked — _uint fixes both holders (verified live: cookies taken, IsInhibited flips, releases clean). Also investigated trav's mid-copy Syncthing question: the transfer works from a click-time snapshot and never reads live library state, so remote changes can't corrupt it — no state freeze needed. Hardened the one real gap: a source file relocated under the queue (remote metadata edit moving files) is now skipped, dropped from the m3u, and reported, instead of aborting the whole sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -24,6 +24,23 @@ When a round closes, move its finished items to `tasks-done.md`.
|
||||
|
||||
# old
|
||||
|
||||
## Round 27 — device-sync guards (v0.2.2)
|
||||
|
||||
- [x] Quit warning while a transfer is copying (closeEvent; covers X button,
|
||||
Ctrl+Q, MPRIS Quit, and the self-update restart, which used to bypass
|
||||
closeEvent entirely).
|
||||
- [x] Machine can't sleep or shutdown/restart mid-transfer: second
|
||||
SleepInhibitor (suspend+logout flags, its own reason text for GNOME's dialog).
|
||||
- [x] Found & fixed: the GNOME Inhibit D-Bus call sent signed ints against a
|
||||
(susu) signature, so the *playback* sleep inhibitor had silently never
|
||||
worked — `_uint` marshalling repairs both.
|
||||
- [x] Investigated Syncthing changes landing mid-copy: safe by design (the
|
||||
SyncPlan is a click-time snapshot; the worker never reads live library
|
||||
state). Hardened the one gap: a source file relocated under the queue is
|
||||
skipped + dropped from the m3u + reported, instead of aborting the sync.
|
||||
|
||||
Tests in `tests/test_round27.py`. Fix round → patch bump **0.2.2**.
|
||||
|
||||
## Round 26 — Device menu: sync playlist to Rabbit R1 (v0.2.0)
|
||||
|
||||
- [x] Device menu (left of Track) with "Sync Playlist to Rabbit"; grayed out on
|
||||
|
||||
@@ -1,3 +1,3 @@
|
||||
"""LinTunes — iTunes-style music library manager and player for Linux."""
|
||||
|
||||
__version__ = "0.2.1"
|
||||
__version__ = "0.2.2"
|
||||
|
||||
+20
-3
@@ -238,12 +238,25 @@ class DeviceSyncWorker(QObject):
|
||||
pass # one stubborn file shouldn't kill the sync
|
||||
total_kib = max(plan.bytes_to_copy // 1024, 1)
|
||||
done = 0
|
||||
vanished: set[str] = set()
|
||||
for i, item in enumerate(plan.copies, start=1):
|
||||
label = f"{i}/{len(plan.copies)} {Path(item.dest_name).stem}"
|
||||
# A source gone since planning means Syncthing moved/deleted
|
||||
# it under us (e.g. a metadata edit on the other machine
|
||||
# relocated the file) — skip it, don't abort the transfer.
|
||||
# Once open, the fd pins the inode, so the read stays
|
||||
# consistent even if the file is moved mid-copy.
|
||||
try:
|
||||
fsrc = open(item.src, "rb")
|
||||
except FileNotFoundError:
|
||||
vanished.add(item.dest_name)
|
||||
done += item.size
|
||||
self.progress.emit(done // 1024, total_kib, label)
|
||||
continue
|
||||
dest = plan.dest_dir / item.dest_name
|
||||
# Manual chunked copy: byte-accurate progress, and no
|
||||
# copystat (gvfs-MTP rejects it).
|
||||
with open(item.src, "rb") as fsrc, open(dest, "wb") as fdst:
|
||||
with fsrc, open(dest, "wb") as fdst:
|
||||
while True:
|
||||
chunk = fsrc.read(CHUNK)
|
||||
if not chunk:
|
||||
@@ -253,13 +266,17 @@ class DeviceSyncWorker(QObject):
|
||||
self.progress.emit(done // 1024, total_kib, label)
|
||||
dest = None
|
||||
# Written last so an interrupted sync leaves the old m3u intact.
|
||||
# Vanished tracks are left out so the m3u only lists files that
|
||||
# are really there; the next sync picks up their new locations.
|
||||
entries = [e for e in plan.entries if e[0] not in vanished]
|
||||
(plan.dest_dir / plan.m3u_name).write_text(
|
||||
build_m3u(plan.entries), encoding="utf-8")
|
||||
build_m3u(entries), encoding="utf-8")
|
||||
device_name = plan.device.name if plan.device else "device"
|
||||
self.finished.emit({
|
||||
"playlist": plan.playlist_name, "device": device_name,
|
||||
"copied": len(plan.copies), "kept": plan.kept,
|
||||
"copied": len(plan.copies) - len(vanished), "kept": plan.kept,
|
||||
"removed": removed, "skipped": plan.skipped,
|
||||
"vanished": len(vanished),
|
||||
})
|
||||
except OSError as e:
|
||||
if dest is not None:
|
||||
|
||||
@@ -12,7 +12,7 @@ from PyQt6.QtGui import QAction, QKeySequence
|
||||
from lintunes import device_sync, mpris, tagging, theme
|
||||
from lintunes.art_search import AlbumArtFetcher
|
||||
from lintunes.eventlog import log_control
|
||||
from lintunes.inhibit import SleepInhibitor
|
||||
from lintunes.inhibit import INHIBIT_LOGOUT, INHIBIT_SUSPEND, SleepInhibitor
|
||||
from lintunes.player import Player
|
||||
from lintunes.importers import file_importer
|
||||
from lintunes.gui.album_art_dialog import AlbumArtDialog
|
||||
@@ -116,6 +116,12 @@ class MainWindow(QMainWindow):
|
||||
self.statusBar().addPermanentWidget(self._sync_progress)
|
||||
self._sync_worker = None
|
||||
self._sync_device_name = ""
|
||||
# Separate instance from the playback inhibitor: pausing music
|
||||
# mid-sync must not drop the sync's hold. Logout flag included so
|
||||
# GNOME's shutdown/restart dialog names the transfer as the blocker.
|
||||
self._sync_inhibitor = SleepInhibitor(
|
||||
reason="Syncing a playlist to a device",
|
||||
flags=INHIBIT_LOGOUT | INHIBIT_SUSPEND)
|
||||
|
||||
# Wiring
|
||||
self._transport.play_clicked.connect(self.play_pause)
|
||||
@@ -158,10 +164,14 @@ class MainWindow(QMainWindow):
|
||||
self._update_totals()
|
||||
|
||||
def _restart_for_update(self):
|
||||
# The pull already succeeded; quit through the normal path (flushes
|
||||
# the library, tears down the player) and let main() re-exec us.
|
||||
# The pull already succeeded. Route through close() — not a bare
|
||||
# quit() — so closeEvent runs: it flushes the library, tears down the
|
||||
# player, and can veto the restart while a device sync is copying.
|
||||
self.restart_requested = True
|
||||
QApplication.instance().quit()
|
||||
if self.close():
|
||||
QApplication.instance().quit()
|
||||
else:
|
||||
self.restart_requested = False
|
||||
|
||||
# ---- status bar totals ----
|
||||
|
||||
@@ -322,6 +332,7 @@ class MainWindow(QMainWindow):
|
||||
self._sync_progress.setValue(0)
|
||||
self._sync_label.show()
|
||||
self._sync_progress.show()
|
||||
self._sync_inhibitor.inhibit()
|
||||
self._sync_worker.start()
|
||||
|
||||
def _on_sync_progress(self, done_kib, total_kib, label):
|
||||
@@ -333,6 +344,7 @@ class MainWindow(QMainWindow):
|
||||
f"{device_sync.format_bytes(total_kib * 1024)}")
|
||||
|
||||
def _on_sync_finished(self, summary):
|
||||
self._sync_inhibitor.release()
|
||||
self._sync_label.hide()
|
||||
self._sync_progress.hide()
|
||||
msg = (f"Synced “{summary['playlist']}” to the {summary['device']}: "
|
||||
@@ -340,9 +352,13 @@ class MainWindow(QMainWindow):
|
||||
f"{summary['removed']} removed")
|
||||
if summary["skipped"]:
|
||||
msg += f", {summary['skipped']} skipped (no local file)"
|
||||
if summary.get("vanished"):
|
||||
msg += (f", {summary['vanished']} changed under us "
|
||||
"(re-sync to pick them up)")
|
||||
self.statusBar().showMessage(msg, 8000)
|
||||
|
||||
def _on_sync_failed(self, message):
|
||||
self._sync_inhibitor.release()
|
||||
self._sync_label.hide()
|
||||
self._sync_progress.hide()
|
||||
QMessageBox.warning(self, "Sync failed", message)
|
||||
@@ -682,7 +698,26 @@ class MainWindow(QMainWindow):
|
||||
self._inhibitor.release()
|
||||
|
||||
def closeEvent(self, event):
|
||||
# Quitting mid-transfer abandons a partial file on the device (it
|
||||
# self-heals next sync, but silently) — make it a deliberate choice.
|
||||
if self._sync_worker is not None and self._sync_worker.busy():
|
||||
box = QMessageBox(self)
|
||||
box.setWindowTitle("Sync in progress")
|
||||
box.setText(
|
||||
f"Still copying to the {self._sync_device_name}.\n"
|
||||
"If you quit now, the unfinished track will be re-copied "
|
||||
"on the next sync.")
|
||||
quit_button = box.addButton(
|
||||
"Quit Anyway", QMessageBox.ButtonRole.DestructiveRole)
|
||||
keep = box.addButton(
|
||||
"Keep Syncing", QMessageBox.ButtonRole.RejectRole)
|
||||
box.setDefaultButton(keep)
|
||||
box.exec()
|
||||
if box.clickedButton() is not quit_button:
|
||||
event.ignore()
|
||||
return
|
||||
self._inhibitor.release()
|
||||
self._sync_inhibitor.release()
|
||||
self.player.shutdown()
|
||||
self._manager.flush()
|
||||
super().closeEvent(event)
|
||||
|
||||
+28
-11
@@ -1,26 +1,43 @@
|
||||
"""Keep the machine awake while music is playing.
|
||||
"""Keep the machine awake while music plays or a device sync runs.
|
||||
|
||||
Uses the GNOME SessionManager D-Bus interface (the same session bus PyQt6's
|
||||
QtDBus already talks to for MPRIS) to register a *suspend* inhibitor. GNOME's
|
||||
critical-battery action overrides this inhibitor, so a near-dead laptop still
|
||||
suspends as expected — we only block idle/automatic suspend while playing.
|
||||
QtDBus already talks to for MPRIS) to register an inhibitor. Playback holds a
|
||||
*suspend* inhibitor; a device sync additionally holds *logout* (which covers
|
||||
shutdown/reboot from the session — GNOME's power dialog names the holder and
|
||||
its reason). GNOME's critical-battery action overrides these, so a near-dead
|
||||
laptop still suspends as expected.
|
||||
|
||||
On non-GNOME desktops the interface won't be available; every method then
|
||||
no-ops silently rather than failing.
|
||||
Instances are independent (one D-Bus cookie each), so playback and sync can
|
||||
hold inhibitors simultaneously without stepping on each other. On non-GNOME
|
||||
desktops the interface won't be available; every method then no-ops silently
|
||||
rather than failing.
|
||||
"""
|
||||
from PyQt6.QtDBus import QDBusConnection, QDBusInterface, QDBusReply
|
||||
from PyQt6.QtCore import QMetaType
|
||||
from PyQt6.QtDBus import (QDBusArgument, QDBusConnection, QDBusInterface,
|
||||
QDBusReply)
|
||||
|
||||
|
||||
SERVICE = "org.gnome.SessionManager"
|
||||
PATH = "/org/gnome/SessionManager"
|
||||
INHIBIT_LOGOUT = 1 # GsmInhibitorFlag: inhibit logout/shutdown/reboot
|
||||
INHIBIT_SUSPEND = 4 # GsmInhibitorFlag: inhibit suspending the session/computer
|
||||
APP_ID = "org.lintunes.LinTunes"
|
||||
REASON = "Playing music"
|
||||
|
||||
|
||||
def _uint(value: int) -> QDBusArgument:
|
||||
"""Marshal as D-Bus uint32. Inhibit's signature is (susu) — a plain
|
||||
Python int marshals as *signed* int32 and GNOME rejects the whole call
|
||||
with a type error (which QDBusReply reported merely as "invalid", so
|
||||
the inhibitor silently never took effect)."""
|
||||
return QDBusArgument(value, QMetaType.Type.UInt.value)
|
||||
|
||||
|
||||
class SleepInhibitor:
|
||||
def __init__(self):
|
||||
def __init__(self, reason: str = REASON, flags: int = INHIBIT_SUSPEND):
|
||||
self._cookie: int | None = None
|
||||
self._reason = reason
|
||||
self._flags = flags
|
||||
bus = QDBusConnection.sessionBus()
|
||||
self._iface = QDBusInterface(SERVICE, PATH, SERVICE, bus)
|
||||
|
||||
@@ -28,11 +45,11 @@ class SleepInhibitor:
|
||||
return self._iface.isValid()
|
||||
|
||||
def inhibit(self):
|
||||
"""Block automatic suspend. No-op if already inhibiting or unavailable."""
|
||||
"""Take the inhibitor. No-op if already inhibiting or unavailable."""
|
||||
if self._cookie is not None or not self._available():
|
||||
return
|
||||
reply = QDBusReply(self._iface.call(
|
||||
"Inhibit", APP_ID, 0, REASON, INHIBIT_SUSPEND))
|
||||
"Inhibit", APP_ID, _uint(0), self._reason, _uint(self._flags)))
|
||||
if reply.isValid():
|
||||
self._cookie = int(reply.value())
|
||||
|
||||
@@ -41,5 +58,5 @@ class SleepInhibitor:
|
||||
if self._cookie is None:
|
||||
return
|
||||
if self._available():
|
||||
self._iface.call("Uninhibit", self._cookie)
|
||||
self._iface.call("Uninhibit", _uint(self._cookie))
|
||||
self._cookie = None
|
||||
|
||||
@@ -177,15 +177,16 @@ class TestWorker:
|
||||
assert seen[-1][0] == seen[-1][1] == 2 * device_sync.CHUNK // 1024
|
||||
assert "1/1" in seen[-1][2]
|
||||
|
||||
def test_source_vanishing_reports_failure(self, qapp, tmp_path):
|
||||
def test_device_side_error_reports_failure(self, qapp, tmp_path):
|
||||
# A file squatting on the dest-dir path stands in for a device-side
|
||||
# write error (unplug mid-sync). Vanished *sources* are tolerated
|
||||
# (round 27); device errors still abort.
|
||||
a = _audio(tmp_path, "a.mp3")
|
||||
plan = plan_sync("Mix", [_track(1, "A", "X", a)], tmp_path / "Music")
|
||||
a.unlink() # unplugged/deleted between plan and run
|
||||
(tmp_path / "Music").mkdir()
|
||||
(tmp_path / "Music" / "Mix").write_bytes(b"not a dir")
|
||||
result = self._synced(qapp, plan)
|
||||
assert "Sync failed" in result["error"]
|
||||
# No half-copied file left behind at full-looking size.
|
||||
leftover = plan.dest_dir / "X - A.mp3"
|
||||
assert not leftover.exists() or leftover.stat().st_size != plan.copies[0].size
|
||||
|
||||
def test_empty_playlist_clears_folder(self, qapp, tmp_path):
|
||||
dest = tmp_path / "Music" / "Mix"
|
||||
|
||||
@@ -0,0 +1,140 @@
|
||||
"""Round 27: device-sync guards.
|
||||
|
||||
A running transfer now (a) holds a GNOME suspend+logout inhibitor so the
|
||||
machine won't sleep or restart under it, (b) is protected by a quit-warning
|
||||
in MainWindow.closeEvent (not testable headless — no MainWindow tests exist),
|
||||
and (c) tolerates a source file vanishing mid-queue: Syncthing applying a
|
||||
remote metadata edit can relocate a music file between planning and copying,
|
||||
so the worker skips it, drops it from the m3u, and reports it instead of
|
||||
aborting the whole transfer. Device-side errors still abort.
|
||||
"""
|
||||
|
||||
from lintunes.device_sync import DeviceSyncWorker, plan_sync
|
||||
from lintunes.inhibit import INHIBIT_LOGOUT, INHIBIT_SUSPEND, SleepInhibitor
|
||||
from lintunes.models import Track
|
||||
|
||||
|
||||
def _track(tid, name, artist, path):
|
||||
return Track(track_id=tid, name=name, artist=artist,
|
||||
location=str(path), total_time=180_000)
|
||||
|
||||
|
||||
def _audio(tmp_path, filename, data=b"x" * 100):
|
||||
path = tmp_path / "local" / filename
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_bytes(data)
|
||||
return path
|
||||
|
||||
|
||||
class _FakeIface:
|
||||
"""Stands in for the QDBusInterface: records calls, hands out cookies."""
|
||||
|
||||
def __init__(self):
|
||||
self.calls = []
|
||||
self._next_cookie = 41
|
||||
|
||||
def isValid(self):
|
||||
return True
|
||||
|
||||
def call(self, method, *args):
|
||||
from PyQt6.QtDBus import QDBusMessage
|
||||
self.calls.append((method, *args))
|
||||
stub = QDBusMessage.createMethodCall("a.b", "/", "a.b", method)
|
||||
if method == "Inhibit":
|
||||
self._next_cookie += 1
|
||||
return stub.createReply([self._next_cookie])
|
||||
return stub.createReply([])
|
||||
|
||||
|
||||
class TestSleepInhibitor:
|
||||
def _patched(self, monkeypatch, **kwargs):
|
||||
# _uint wraps ints in write-only QDBusArguments; identity here so the
|
||||
# fake can record and assert on the raw values.
|
||||
from lintunes import inhibit as inhibit_module
|
||||
monkeypatch.setattr(inhibit_module, "_uint", lambda v: v)
|
||||
inhibitor = SleepInhibitor(**kwargs)
|
||||
inhibitor._iface = _FakeIface()
|
||||
return inhibitor
|
||||
|
||||
def test_custom_reason_and_flags_reach_dbus(self, qapp, monkeypatch):
|
||||
inhibitor = self._patched(
|
||||
monkeypatch, reason="Syncing a playlist to a device",
|
||||
flags=INHIBIT_LOGOUT | INHIBIT_SUSPEND)
|
||||
inhibitor.inhibit()
|
||||
method, app_id, xid, reason, flags = inhibitor._iface.calls[0]
|
||||
assert method == "Inhibit"
|
||||
assert reason == "Syncing a playlist to a device"
|
||||
assert flags == 5 # logout (1) + suspend (4)
|
||||
|
||||
def test_defaults_unchanged_for_playback(self, qapp, monkeypatch):
|
||||
inhibitor = self._patched(monkeypatch)
|
||||
inhibitor.inhibit()
|
||||
method, app_id, xid, reason, flags = inhibitor._iface.calls[0]
|
||||
assert reason == "Playing music"
|
||||
assert flags == INHIBIT_SUSPEND == 4
|
||||
|
||||
def test_release_uninhibits_with_cookie(self, qapp, monkeypatch):
|
||||
inhibitor = self._patched(monkeypatch)
|
||||
inhibitor.inhibit()
|
||||
cookie = inhibitor._cookie
|
||||
inhibitor.release()
|
||||
assert ("Uninhibit", cookie) in inhibitor._iface.calls
|
||||
assert inhibitor._cookie is None
|
||||
|
||||
def test_second_inhibit_while_held_is_noop(self, qapp, monkeypatch):
|
||||
inhibitor = self._patched(monkeypatch)
|
||||
inhibitor.inhibit()
|
||||
inhibitor.inhibit()
|
||||
assert len([c for c in inhibitor._iface.calls
|
||||
if c[0] == "Inhibit"]) == 1
|
||||
|
||||
def test_release_without_hold_is_noop(self, qapp, monkeypatch):
|
||||
inhibitor = self._patched(monkeypatch)
|
||||
inhibitor.release()
|
||||
assert inhibitor._iface.calls == []
|
||||
|
||||
|
||||
class TestVanishedSource:
|
||||
def _run(self, plan):
|
||||
worker = DeviceSyncWorker(plan)
|
||||
results = {}
|
||||
worker.finished.connect(lambda s: results.update(s))
|
||||
worker.failed.connect(lambda m: results.update(error=m))
|
||||
worker._run()
|
||||
return results
|
||||
|
||||
def test_vanished_source_is_skipped_not_fatal(self, qapp, tmp_path):
|
||||
a = _audio(tmp_path, "a.mp3", b"a" * 50)
|
||||
b = _audio(tmp_path, "b.mp3", b"b" * 70)
|
||||
tracks = [_track(1, "A", "X", a), _track(2, "B", "Y", b)]
|
||||
plan = plan_sync("Mix", tracks, tmp_path / "Music")
|
||||
a.unlink() # Syncthing relocated it between plan and copy
|
||||
|
||||
summary = self._run(plan)
|
||||
|
||||
assert "error" not in summary
|
||||
assert summary["vanished"] == 1
|
||||
assert summary["copied"] == 1
|
||||
assert not (plan.dest_dir / "X - A.mp3").exists()
|
||||
assert (plan.dest_dir / "Y - B.mp3").read_bytes() == b"b" * 70
|
||||
|
||||
def test_vanished_track_left_out_of_m3u(self, qapp, tmp_path):
|
||||
a = _audio(tmp_path, "a.mp3")
|
||||
b = _audio(tmp_path, "b.mp3")
|
||||
tracks = [_track(1, "A", "X", a), _track(2, "B", "Y", b)]
|
||||
plan = plan_sync("Mix", tracks, tmp_path / "Music")
|
||||
a.unlink()
|
||||
|
||||
self._run(plan)
|
||||
|
||||
m3u = (plan.dest_dir / "Mix.m3u").read_text(encoding="utf-8")
|
||||
assert "X - A.mp3" not in m3u
|
||||
assert "Y - B.mp3" in m3u
|
||||
|
||||
def test_all_sources_vanished_still_finishes(self, qapp, tmp_path):
|
||||
a = _audio(tmp_path, "a.mp3")
|
||||
plan = plan_sync("Mix", [_track(1, "A", "X", a)], tmp_path / "Music")
|
||||
a.unlink()
|
||||
summary = self._run(plan)
|
||||
assert summary["vanished"] == 1 and summary["copied"] == 0
|
||||
assert (plan.dest_dir / "Mix.m3u").read_text() == "#EXTM3U\n"
|
||||
Reference in New Issue
Block a user