Cover art: detect read-only from the actual write, not statvfs (Sokhi false-positive)

Sokhi got a read-only error from the cover-art filler with NO ':ro' in his
compose. Root cause: my earlier Tim fix added a statvfs pre-flight that bailed
when f_flag & ST_RDONLY — but union/FUSE/network filesystems (mergerfs,
rclone, NFS), ubiquitous in self-hosted setups, misreport those mount flags.
A perfectly writable library could be flagged read-only and blocked. statvfs
is a guess; the only honest test is whether an actual write raises EROFS.

- Removed the statvfs pre-flight entirely. Read-only is now detected solely
  from a real EROFS on the embed write, which also fast-fails the remaining
  files (so no statvfs needed for the fast-fail Tim wanted either).
- Broadened the user message: a genuine read-only mount isn't always ':ro' —
  could be a read-only host/NFS/SMB mount or a mergerfs read-only branch.

Tests: writable FS succeeds even when statvfs would claim read-only (the
regression), real-EROFS-on-write still flagged + bails the rest, EACCES still
not conflated with EROFS. Dropped the now-moot Windows-statvfs test (statvfs
is no longer referenced). 445 art/cover/repair tests pass.

core/metadata/art_apply.py

@@ -113,8 +113,15 @@ def apply_art_to_album_files(
 
     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.
+    (a real EROFS from an actual write — a ':ro' volume, a read-only host/NFS/
+    SMB mount, or a read-only underlying fs) so callers can tell the user the
+    real cause instead of a generic failure.
+
+    NOTE: read-only is detected from an ACTUAL write raising EROFS, never from
+    statvfs/mount flags — union/FUSE/network filesystems (mergerfs, rclone,
+    NFS) common in self-hosted setups misreport those flags, which would
+    false-block a perfectly writable library (Sokhi: read-only error with no
+    ':ro' in compose). The write itself is the only honest test.
     """
     result = {"embedded": 0, "failed": 0, "skipped": 0, "cover_written": False,
               "read_only_fs": False}
@@ -123,26 +130,6 @@ def apply_art_to_album_files(
     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.
-    # POSIX-only — Windows has no os.statvfs (and no ':ro' bind mounts);
-    # there we skip straight to the per-file path, same as before this fix.
-    probe_dir = folder or (os.path.dirname(paths[0]) if paths else None)
-    _statvfs = getattr(os, "statvfs", None)
-    if probe_dir and _statvfs is not None:
-        try:
-            if _statvfs(probe_dir).f_flag & getattr(os, "ST_RDONLY", 1):
-                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
@@ -169,8 +156,13 @@ def apply_art_to_album_files(
                 result["failed"] += 1
         except Exception as exc:
             # Read-only mounts / permission errors land here — skip, don't crash.
+            # A real EROFS = the mount is read-only; flag it and stop trying the
+            # rest (fast-fail without the unreliable statvfs guess).
             if getattr(exc, "errno", None) == errno.EROFS:
                 result["read_only_fs"] = True
+                logger.warning("Could not embed art into %s: read-only filesystem", fp)
+                result["failed"] += len(paths) - paths.index(fp)  # remaining all fail too
+                break
             logger.warning("Could not embed art into %s: %s", fp, exc)
             result["failed"] += 1
 

core/repair_worker.py

@@ -1394,16 +1394,17 @@ class RepairWorker:
 
         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.
+            # The music folder is genuinely read-only at the OS level (the
+            # write raised EROFS). Most common cause is a docker ':ro' volume,
+            # but it can also be a read-only host mount (NFS/SMB exported ro),
+            # a mergerfs/union read-only branch, or the library mounted from
+            # another container as read-only — chmod can't change any of these.
             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.)'),
+                    'error': ('Your music folder is READ-ONLY — the container cannot '
+                              'write to it (chmod cannot change this). Check that the '
+                              "volume isn't mapped ':ro', and that the underlying host "
+                              'mount (NFS/SMB/mergerfs) is read-write, then 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'):

tests/test_art_apply.py

@@ -150,31 +150,36 @@ def test_apply_counts_failures_without_raising(tmp_path, monkeypatch):
 # ── 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')."""
+def test_apply_does_not_trust_statvfs_writable_fs_succeeds(tmp_path, monkeypatch):
+    """Regression (Sokhi): a WRITABLE union/FUSE/NFS mount can misreport
+    ST_RDONLY in statvfs. The apply must NOT use statvfs — it writes anyway,
+    and only the actual write decides. Here the embed succeeds even though
+    statvfs (if it were consulted) would claim read-only."""
     f = tmp_path / 'a.mp3'
     f.write_bytes(b'')
-    audio = SimpleNamespace(pictures=[], tags=None)
+    saved = []
+    audio = SimpleNamespace(pictures=[], tags=None, add_tags=lambda: None,
+                            save=lambda: saved.append(True))
     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)
+    # Even if statvfs lies and says read-only, the write still happens.
     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))
+                        lambda p: SimpleNamespace(f_flag=getattr(aa.os, 'ST_RDONLY', 1)),
+                        raising=False)
 
     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
+    assert res['embedded'] == 1 and saved == [True]
+    assert res['read_only_fs'] is False      # statvfs ignored; write succeeded
 
 
-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."""
+def test_apply_flags_erofs_from_actual_write(tmp_path, monkeypatch):
+    """read-only is detected from a real EROFS on write (the only honest test),
+    and it fast-fails the remaining files."""
     import errno as _errno
-    f = tmp_path / 'a.mp3'
-    f.write_bytes(b'')
+    f1 = tmp_path / 'a.mp3'; f1.write_bytes(b'')
+    f2 = tmp_path / 'b.mp3'; f2.write_bytes(b'')
 
     def _boom():
         raise OSError(_errno.EROFS, 'Read-only file system')
@@ -183,10 +188,10 @@ def test_apply_flags_erofs_from_save(tmp_path, monkeypatch):
     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))
+    res = aa.apply_art_to_album_files([str(f1), str(f2)], {}, {}, folder=str(tmp_path))
 
     assert res['read_only_fs'] is True
-    assert res['failed'] == 1
+    assert res['failed'] == 2                 # first EROFS fails it + bails the rest
 
 
 def test_apply_normal_failure_not_flagged_read_only(tmp_path, monkeypatch):
@@ -205,20 +210,3 @@ def test_apply_normal_failure_not_flagged_read_only(tmp_path, monkeypatch):
     assert res['failed'] == 1
 
 
-def test_apply_works_without_statvfs_windows(tmp_path, monkeypatch):
-    """Windows has no os.statvfs — the pre-flight must vanish gracefully,
-    not crash the apply (Boulder runs on Windows)."""
-    f = tmp_path / 'a.mp3'
-    f.write_bytes(b'')
-    saved = []
-    audio = SimpleNamespace(pictures=[], tags=None, add_tags=lambda: None,
-                            save=lambda: saved.append(True))
-    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)
-    monkeypatch.delattr(aa.os, 'statvfs', raising=False)
-
-    res = aa.apply_art_to_album_files([str(f)], {}, {}, folder=str(tmp_path))
-
-    assert res['embedded'] == 1 and saved == [True]
-    assert res['read_only_fs'] is False