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.
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