diff --git a/core/metadata/art_apply.py b/core/metadata/art_apply.py index e267a3e5..fe79d266 100644 --- a/core/metadata/art_apply.py +++ b/core/metadata/art_apply.py @@ -15,6 +15,7 @@ Two jobs, both reusing the post-processing standard so the user's from __future__ import annotations import contextlib +import errno import os from typing import Iterable @@ -111,13 +112,34 @@ def apply_art_to_album_files( musicbrainz_release_id). Existing tags are preserved — only art is added. Returns counts; never raises (unwritable/read-only files are skipped). + ``read_only_fs`` is True when the target filesystem itself rejects writes + (EROFS — a docker ``:ro`` volume mount; chmod can't fix that) so callers + can tell the user the actual cure instead of a generic failure. """ - result = {"embedded": 0, "failed": 0, "skipped": 0, "cover_written": False} + result = {"embedded": 0, "failed": 0, "skipped": 0, "cover_written": False, + "read_only_fs": False} symbols = get_mutagen_symbols() paths = [p for p in (file_paths or []) if p] if not symbols: return result + # Pre-flight: if the mount is read-only, every save below would fail with + # EROFS one by one (Tim's report: a wall of per-file warnings and a 777 + # chmod that couldn't help). statvfs asks the kernel without writing. + probe_dir = folder or (os.path.dirname(paths[0]) if paths else None) + if probe_dir: + try: + if os.statvfs(probe_dir).f_flag & os.ST_RDONLY: + logger.warning( + "Art apply skipped: %s is on a READ-ONLY filesystem " + "(docker ':ro' volume mount — chmod cannot fix this)", + probe_dir) + result["read_only_fs"] = True + result["failed"] = len(paths) + return result + except OSError: + pass # statvfs unavailable/odd fs — fall through to per-file handling + for fp in paths: if not os.path.isfile(fp): result["skipped"] += 1 @@ -144,6 +166,8 @@ def apply_art_to_album_files( result["failed"] += 1 except Exception as exc: # Read-only mounts / permission errors land here — skip, don't crash. + if getattr(exc, "errno", None) == errno.EROFS: + result["read_only_fs"] = True logger.warning("Could not embed art into %s: %s", fp, exc) result["failed"] += 1 @@ -153,5 +177,7 @@ def apply_art_to_album_files( download_cover_art(album_info, target_dir, context) result["cover_written"] = folder_has_cover_sidecar(target_dir) except Exception as exc: + if getattr(exc, "errno", None) == errno.EROFS: + result["read_only_fs"] = True logger.warning("cover.jpg write failed for %s: %s", target_dir, exc) return result diff --git a/core/repair_worker.py b/core/repair_worker.py index 74d3a634..88d698d0 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -1349,11 +1349,23 @@ class RepairWorker: art_result = apply_art_to_album_files(resolved, metadata, album_info, folder=folder) embedded = art_result.get('embedded', 0) + if art_result.get('read_only_fs'): + # Tim's report: a docker ':ro' volume — every write fails EROFS, + # the user chmod 777's in vain, and a soft "(read-only?)" hint + # wasn't loud enough. Fail the fix with the actual cure. + return {'success': False, 'action': 'applied_cover_art', + 'error': ('Your music folder is mounted READ-ONLY — the container ' + 'cannot write to it, and chmod cannot change that. ' + "Remove ':ro' from the volume mapping in your docker " + "compose/run (e.g. '/music:/music:ro' → '/music:/music') " + 'and recreate the container. (Database thumbnail was ' + 'still updated.)'), + 'art_result': art_result} msg = f'Applied cover art: embedded into {embedded}/{len(resolved)} file(s)' if art_result.get('cover_written'): msg += ' + wrote cover.jpg' if embedded == 0 and not art_result.get('cover_written'): - # DB updated but nothing reached disk (e.g. read-only mount). + # DB updated but nothing reached disk (e.g. permissions). msg = 'Updated database thumbnail, but could not write art to files (read-only?)' return {'success': True, 'action': 'applied_cover_art', 'message': msg, 'art_result': art_result} diff --git a/tests/test_art_apply.py b/tests/test_art_apply.py index 8e240e41..a7661ef3 100644 --- a/tests/test_art_apply.py +++ b/tests/test_art_apply.py @@ -145,3 +145,61 @@ def test_apply_counts_failures_without_raising(tmp_path, monkeypatch): res = aa.apply_art_to_album_files([str(f1)], {'album': 'B'}, {'album_name': 'B'}, folder=str(tmp_path)) assert res['embedded'] == 0 assert res['failed'] == 1 # save() raised (read-only) — counted, not crashed + + +# ── read-only filesystem detection (Tim: docker ':ro' mount, Errno 30) ── + + +def test_apply_short_circuits_on_read_only_mount(tmp_path, monkeypatch): + """statvfs says the mount is RO -> bail before touching any file, flag + read_only_fs so the caller can name the actual cure (remove ':ro').""" + f = tmp_path / 'a.mp3' + f.write_bytes(b'') + audio = SimpleNamespace(pictures=[], tags=None) + monkeypatch.setattr(aa, 'get_mutagen_symbols', lambda: _fake_symbols(audio)) + monkeypatch.setattr(aa.os, 'statvfs', + lambda p: SimpleNamespace(f_flag=aa.os.ST_RDONLY)) + called = [] + monkeypatch.setattr(aa, 'download_cover_art', lambda *a, **k: called.append(a)) + + res = aa.apply_art_to_album_files([str(f)], {}, {}, folder=str(tmp_path)) + + assert res['read_only_fs'] is True + assert res['embedded'] == 0 and res['failed'] == 1 + assert called == [] # cover.jpg write never attempted + + +def test_apply_flags_erofs_from_save(tmp_path, monkeypatch): + """statvfs passes (overlay quirk) but the save itself raises EROFS -> + per-file detection still sets read_only_fs.""" + import errno as _errno + f = tmp_path / 'a.mp3' + f.write_bytes(b'') + + def _boom(): + raise OSError(_errno.EROFS, 'Read-only file system') + audio = SimpleNamespace(pictures=[], tags={'ok': 1}, save=_boom) + monkeypatch.setattr(aa, 'get_mutagen_symbols', lambda: _fake_symbols(audio)) + monkeypatch.setattr(aa, 'embed_album_art_metadata', lambda *a, **k: True) + monkeypatch.setattr(aa, 'download_cover_art', lambda *a, **k: None) + + res = aa.apply_art_to_album_files([str(f)], {}, {}, folder=str(tmp_path)) + + assert res['read_only_fs'] is True + assert res['failed'] == 1 + + +def test_apply_normal_failure_not_flagged_read_only(tmp_path, monkeypatch): + def _boom(): + raise PermissionError(13, 'Permission denied') + f = tmp_path / 'a.mp3' + f.write_bytes(b'') + audio = SimpleNamespace(pictures=[], tags={'ok': 1}, save=_boom) + monkeypatch.setattr(aa, 'get_mutagen_symbols', lambda: _fake_symbols(audio)) + monkeypatch.setattr(aa, 'embed_album_art_metadata', lambda *a, **k: True) + monkeypatch.setattr(aa, 'download_cover_art', lambda *a, **k: None) + + res = aa.apply_art_to_album_files([str(f)], {}, {}, folder=str(tmp_path)) + + assert res['read_only_fs'] is False # EACCES is chmod-able, EROFS is not + assert res['failed'] == 1