Skip to content

Commit 757fafa

Browse files
gantoineclaude
andcommitted
feat(fs): hardlink import/export assets when possible, harden sync init
Importer (gamelist/launchbox file:// flows) and exporters (gamelist.xml, metadata.pegasus.txt local exports) now hardlink media assets when source and destination share a filesystem, falling back transparently to a copy on EXDEV / EPERM / EOPNOTSUPP / EMLINK / EACCES (cross-device, FAT32, exFAT, network mounts, etc.). Saves disk space and is effectively instantaneous on large files (videos, manuals, miximages). Covers keep a real copy (allow_link=False) because _store_cover resizes the small cover in place via PIL.Image.save, which would truncate the shared inode and corrupt the user's source image. Also makes FSSyncHandler tolerate a missing/unwritable /romm/sync at startup: an OSError from mkdir now logs a warning instead of crashing the whole app at module-import time. Sync calls still fail at use time if the mount remains broken — the right place to surface the error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c7ecf5d commit 757fafa

9 files changed

Lines changed: 336 additions & 16 deletions

File tree

backend/handler/filesystem/base_handler.py

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@
1616
from starlette.datastructures import UploadFile
1717

1818
from config.config_manager import config_manager as cm
19+
from logger.logger import log
1920
from models.base import FILE_NAME_MAX_LENGTH
20-
from utils.filesystem import iter_directories, iter_files
21+
from utils.filesystem import iter_directories, iter_files, link_or_copy_file
2122

2223
TAG_REGEX = re.compile(r"\(([^)]+)\)|\[([^]]+)\]")
2324
EXTENSION_REGEX = re.compile(r"\.(([a-z]+\.)*\w+)$")
@@ -148,13 +149,22 @@ class Asset(Enum):
148149

149150

150151
class FSHandler:
151-
def __init__(self, base_path: str):
152+
def __init__(self, base_path: str, tolerate_missing_base: bool = False):
152153
self.base_path = Path(base_path).resolve()
153154
self._locks: dict[str, asyncio.Lock] = {}
154155
self._lock_mutex = asyncio.Lock()
155156

156-
# Create base directory synchronously during initialization
157-
self.base_path.mkdir(parents=True, exist_ok=True)
157+
# Create base directory synchronously during initialization.
158+
try:
159+
self.base_path.mkdir(parents=True, exist_ok=True)
160+
except OSError:
161+
if not tolerate_missing_base:
162+
raise
163+
164+
log.warning(
165+
f"Could not create or access {self.base_path}; "
166+
"feature will be unavailable until the directory is writable."
167+
)
158168

159169
async def _get_file_lock(self, file_path: str) -> asyncio.Lock:
160170
"""Get or create a lock for a specific file path."""
@@ -487,13 +497,23 @@ async def stream_file(self, file_path: str):
487497

488498
return await open_file(full_path, "rb")
489499

490-
async def copy_file(self, source_full_path: Path, dest_path: str) -> None:
500+
async def copy_file(
501+
self,
502+
source_full_path: Path,
503+
dest_path: str,
504+
allow_link: bool = True,
505+
) -> None:
491506
"""
492507
Copy a file from source to destination.
493508
494509
Args:
495510
source_full_path: Absolute path to the source file
496511
dest_path: Relative path to the destination file
512+
allow_link: If True (default), try a hardlink first and fall back to
513+
a copy when the link isn't possible (cross-device, unsupported
514+
filesystem, etc.). Pass False when the caller will mutate the
515+
destination in place, since mutating a hardlinked file also
516+
mutates the source — see `_store_cover`'s resize step.
497517
498518
Raises:
499519
FileNotFoundError: If source file does not exist
@@ -518,7 +538,10 @@ async def copy_file(self, source_full_path: Path, dest_path: str) -> None:
518538
# Create destination directory if needed
519539
dest_parent_anyio_path = AnyioPath(str(dest_full_path.parent))
520540
await dest_parent_anyio_path.mkdir(parents=True, exist_ok=True)
521-
shutil.copy2(str(source_full_path), str(dest_full_path))
541+
if allow_link:
542+
link_or_copy_file(source_full_path, dest_full_path)
543+
else:
544+
shutil.copy2(str(source_full_path), str(dest_full_path))
522545

523546
async def move_file_or_folder(self, source_path: str, dest_path: str) -> None:
524547
"""

backend/handler/filesystem/resources_handler.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,10 @@ async def _store_cover(
129129
log.warning(f"Cover file not found: {url_cover}")
130130
return None
131131
dest_path = f"{cover_file}/{size.value}.png"
132-
await self.copy_file(resolved, dest_path)
132+
# allow_link=False: small-size covers get resized in place
133+
# below, which would mutate the user's source image if the
134+
# destination were a hardlink.
135+
await self.copy_file(resolved, dest_path, allow_link=False)
133136

134137
if ENABLE_SCHEDULED_CONVERT_IMAGES_TO_WEBP:
135138
self.image_converter.convert_to_webp(

backend/handler/filesystem/sync_handler.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ class FSSyncHandler(FSHandler):
1212
"""Filesystem handler for sync folder operations (File Transfer mode)."""
1313

1414
def __init__(self) -> None:
15-
super().__init__(base_path=SYNC_BASE_PATH)
15+
super().__init__(base_path=SYNC_BASE_PATH, tolerate_missing_base=True)
1616

1717
def build_incoming_path(
1818
self, device_id: str, platform_slug: str | None = None

backend/tests/handler/filesystem/test_base_handler.py

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import asyncio
2+
import errno
23
import shutil
34
import tempfile
45
from io import BytesIO
@@ -515,3 +516,105 @@ async def create_and_remove_dir(dir_name):
515516
dirs = await handler.list_directories(".")
516517
for i in range(5):
517518
assert f"test_dir_{i}" not in dirs
519+
520+
async def test_copy_file_hardlinks_by_default(
521+
self, handler: FSHandler, sample_file_content
522+
):
523+
"""copy_file defaults to allow_link=True: when source and dest are on
524+
the same filesystem, the destination is a hardlink to the source."""
525+
source_full = handler.base_path / "source.bin"
526+
source_full.write_bytes(sample_file_content)
527+
528+
await handler.copy_file(source_full, "dest.bin")
529+
530+
dest_full = handler.base_path / "dest.bin"
531+
assert dest_full.read_bytes() == sample_file_content
532+
assert source_full.stat().st_ino == dest_full.stat().st_ino
533+
534+
async def test_copy_file_allow_link_false_does_real_copy(
535+
self, handler: FSHandler, sample_file_content
536+
):
537+
"""allow_link=False forces a real copy — content matches but inodes
538+
differ, so mutating dest won't affect source. This is the path
539+
`_store_cover` uses to keep PIL's in-place resize from corrupting the
540+
user's source image."""
541+
source_full = handler.base_path / "source.bin"
542+
source_full.write_bytes(sample_file_content)
543+
544+
await handler.copy_file(source_full, "dest.bin", allow_link=False)
545+
546+
dest_full = handler.base_path / "dest.bin"
547+
assert dest_full.read_bytes() == sample_file_content
548+
assert source_full.stat().st_ino != dest_full.stat().st_ino
549+
550+
async def test_copy_file_allow_link_falls_back_to_copy_on_exdev(
551+
self, handler: FSHandler, sample_file_content
552+
):
553+
"""When os.link raises EXDEV (cross-filesystem), copy_file transparently
554+
falls back to a real copy and the result is still a valid file."""
555+
source_full = handler.base_path / "source.bin"
556+
source_full.write_bytes(sample_file_content)
557+
558+
with patch(
559+
"utils.filesystem.os.link",
560+
side_effect=OSError(errno.EXDEV, "Cross-device link"),
561+
):
562+
await handler.copy_file(source_full, "dest.bin")
563+
564+
dest_full = handler.base_path / "dest.bin"
565+
assert dest_full.read_bytes() == sample_file_content
566+
assert source_full.stat().st_ino != dest_full.stat().st_ino
567+
568+
async def test_copy_file_nonexistent_source(self, handler: FSHandler):
569+
"""Missing source must raise FileNotFoundError regardless of link mode."""
570+
with pytest.raises(FileNotFoundError, match="Source file not found"):
571+
await handler.copy_file(handler.base_path / "missing.bin", "dest.bin")
572+
573+
574+
class TestFSHandlerTolerateMissingBase:
575+
"""Tests for the tolerate_missing_base flag, which lets optional features
576+
(like the sync folder) come up degraded instead of crashing the whole app
577+
when the base path can't be created."""
578+
579+
def test_default_raises_on_mkdir_failure(self):
580+
"""Default behavior: an OSError from mkdir propagates, so misconfigured
581+
critical paths (resources, library) still fail loudly at startup."""
582+
with patch.object(
583+
Path, "mkdir", side_effect=PermissionError(errno.EACCES, "denied")
584+
):
585+
with pytest.raises(PermissionError):
586+
FSHandler("/some/unwritable/path")
587+
588+
def test_tolerate_missing_base_swallows_oserror(self, tmp_path, caplog):
589+
"""With tolerate_missing_base=True, mkdir failure is logged but does
590+
not raise — the handler instance is still constructed so module-level
591+
imports don't crash."""
592+
target = tmp_path / "missing"
593+
594+
with patch.object(
595+
Path, "mkdir", side_effect=PermissionError(errno.EACCES, "denied")
596+
):
597+
handler = FSHandler(str(target), tolerate_missing_base=True)
598+
599+
# Handler exists and base_path is set, even though the directory
600+
# could not be created.
601+
assert handler.base_path == target.resolve()
602+
assert not handler.base_path.exists()
603+
604+
def test_tolerate_missing_base_still_creates_when_possible(self, tmp_path):
605+
"""tolerate_missing_base=True should not change behavior when mkdir
606+
succeeds — the directory must still be created normally."""
607+
target = tmp_path / "new_dir"
608+
assert not target.exists()
609+
610+
handler = FSHandler(str(target), tolerate_missing_base=True)
611+
612+
assert handler.base_path.exists()
613+
assert handler.base_path.is_dir()
614+
615+
def test_tolerate_missing_base_reraises_non_oserror(self, tmp_path):
616+
"""Only OSError gets swallowed; programming errors (e.g. a RuntimeError
617+
from corrupted state) must still surface."""
618+
with patch.object(Path, "mkdir", side_effect=RuntimeError("unexpected")):
619+
with pytest.raises(RuntimeError, match="unexpected"):
620+
FSHandler(str(tmp_path / "x"), tolerate_missing_base=True)

backend/tests/handler/filesystem/test_sync_handler.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
"""Tests for filesystem sync handler."""
22

3+
import errno
34
import os
45
import shutil
56
import tempfile
67
from pathlib import Path
8+
from unittest.mock import patch
79

810
import pytest
911

@@ -133,3 +135,21 @@ def test_remove_incoming_file_outside_base_raises(
133135
def test_remove_incoming_file_nonexistent(self, handler: FSSyncHandler):
134136
# Should not raise for nonexistent files
135137
handler.remove_incoming_file("/nonexistent/path/file.sav")
138+
139+
140+
class TestFSSyncHandlerStartup:
141+
"""Sync is an optional feature: if /romm/sync isn't writable (bad mount,
142+
wrong ownership), the app must still boot. Failures should surface when
143+
sync is actually used, not at module-import time."""
144+
145+
def test_init_does_not_raise_when_base_path_unwritable(self):
146+
"""Regression test for the PermissionError on /romm/sync at boot.
147+
FSSyncHandler must construct successfully even when mkdir fails."""
148+
with patch.object(
149+
Path, "mkdir", side_effect=PermissionError(errno.EACCES, "denied")
150+
):
151+
handler = FSSyncHandler()
152+
153+
assert handler is not None
154+
# base_path is set even though the directory wasn't created.
155+
assert isinstance(handler.base_path, Path)
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
"""Tests for utils.filesystem helpers."""
2+
3+
import errno
4+
import os
5+
from unittest.mock import patch
6+
7+
import pytest
8+
9+
from utils.filesystem import link_or_copy_file
10+
11+
12+
class TestLinkOrCopyFile:
13+
"""Test the hardlink-with-copy-fallback helper used by importers and exporters."""
14+
15+
def test_same_filesystem_creates_hardlink(self, tmp_path):
16+
"""When source and dest are on the same filesystem, the helper creates a
17+
hardlink — source and dest share an inode and st_nlink reflects both."""
18+
source = tmp_path / "source.bin"
19+
source.write_bytes(b"payload")
20+
dest = tmp_path / "dest.bin"
21+
22+
link_or_copy_file(source, dest)
23+
24+
assert dest.read_bytes() == b"payload"
25+
# Hardlink: shared inode, link count >= 2
26+
assert source.stat().st_ino == dest.stat().st_ino
27+
assert source.stat().st_nlink >= 2
28+
29+
def test_falls_back_to_copy_on_exdev(self, tmp_path):
30+
"""When os.link raises EXDEV (cross-device), the helper falls back to
31+
shutil.copy2 — content matches but inodes differ."""
32+
source = tmp_path / "source.bin"
33+
source.write_bytes(b"payload")
34+
dest = tmp_path / "dest.bin"
35+
36+
exdev = OSError(errno.EXDEV, "Cross-device link")
37+
38+
with patch("utils.filesystem.os.link", side_effect=exdev):
39+
link_or_copy_file(source, dest)
40+
41+
assert dest.read_bytes() == b"payload"
42+
# Real copy: separate inodes
43+
assert source.stat().st_ino != dest.stat().st_ino
44+
45+
def test_falls_back_to_copy_on_eperm(self, tmp_path):
46+
"""EPERM (filesystem doesn't permit hardlinks, e.g. FAT32) must also
47+
trigger the copy fallback."""
48+
source = tmp_path / "source.bin"
49+
source.write_bytes(b"payload")
50+
dest = tmp_path / "dest.bin"
51+
52+
eperm = OSError(errno.EPERM, "Operation not permitted")
53+
54+
with patch("utils.filesystem.os.link", side_effect=eperm):
55+
link_or_copy_file(source, dest)
56+
57+
assert dest.read_bytes() == b"payload"
58+
assert source.stat().st_ino != dest.stat().st_ino
59+
60+
def test_reraises_non_fallback_oserror(self, tmp_path):
61+
"""An OSError that isn't in the fallback set (e.g. ENOSPC — disk full)
62+
must propagate; we don't want to mask real disk errors."""
63+
source = tmp_path / "source.bin"
64+
source.write_bytes(b"payload")
65+
dest = tmp_path / "dest.bin"
66+
67+
enospc = OSError(errno.ENOSPC, "No space left on device")
68+
69+
with patch("utils.filesystem.os.link", side_effect=enospc):
70+
with pytest.raises(OSError) as excinfo:
71+
link_or_copy_file(source, dest)
72+
73+
assert excinfo.value.errno == errno.ENOSPC
74+
assert not dest.exists()
75+
76+
def test_mutation_after_link_affects_source(self, tmp_path):
77+
"""Document hardlink semantics: writing through the dest path with
78+
O_TRUNC truncates the shared inode and therefore mutates the source.
79+
This is the exact hazard `_store_cover` avoids with allow_link=False."""
80+
source = tmp_path / "source.bin"
81+
source.write_bytes(b"original")
82+
dest = tmp_path / "dest.bin"
83+
84+
link_or_copy_file(source, dest)
85+
86+
# Truncating-write through dest affects source (same inode).
87+
with open(dest, "wb") as f:
88+
f.write(b"mutated")
89+
90+
assert source.read_bytes() == b"mutated"
91+
92+
def test_mutation_after_copy_does_not_affect_source(self, tmp_path):
93+
"""When the helper falls back to copy, the inodes are independent —
94+
mutation through dest leaves the source untouched."""
95+
source = tmp_path / "source.bin"
96+
source.write_bytes(b"original")
97+
dest = tmp_path / "dest.bin"
98+
99+
with patch(
100+
"utils.filesystem.os.link",
101+
side_effect=OSError(errno.EXDEV, "Cross-device link"),
102+
):
103+
link_or_copy_file(source, dest)
104+
105+
with open(dest, "wb") as f:
106+
f.write(b"mutated")
107+
108+
assert source.read_bytes() == b"original"
109+
110+
def test_dest_already_exists_raises(self, tmp_path):
111+
"""os.link raises EEXIST when dest already exists; that's not in the
112+
fallback set, so it propagates. Callers are responsible for handling
113+
the already-exists case before calling this helper."""
114+
source = tmp_path / "source.bin"
115+
source.write_bytes(b"payload")
116+
dest = tmp_path / "dest.bin"
117+
dest.write_bytes(b"existing")
118+
119+
with pytest.raises(OSError) as excinfo:
120+
link_or_copy_file(source, dest)
121+
122+
assert excinfo.value.errno == errno.EEXIST
123+
124+
def test_helper_uses_os_link_first(self, tmp_path):
125+
"""Sanity-check that the helper calls os.link before any copy logic —
126+
guards against a future refactor regressing to copy-only."""
127+
source = tmp_path / "source.bin"
128+
source.write_bytes(b"payload")
129+
dest = tmp_path / "dest.bin"
130+
131+
with patch("utils.filesystem.os.link", wraps=os.link) as link_spy:
132+
link_or_copy_file(source, dest)
133+
134+
link_spy.assert_called_once_with(source, dest)

0 commit comments

Comments
 (0)