mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-05 08:21:11 +01:00
Open up hardlink requirement (#961)
This commit is contained in:
@@ -587,7 +587,7 @@ def atomic_hardlink(source_path: Path, dest_path: Path, max_attempts: int = 100)
|
||||
error=e,
|
||||
)
|
||||
if permission_error or _hardlink_not_supported(e):
|
||||
logger.debug(
|
||||
logger.warning(
|
||||
"Hardlink failed (%s), falling back to copy: %s -> %s",
|
||||
e,
|
||||
source_path,
|
||||
|
||||
@@ -13,7 +13,6 @@ from shelfmark.core.naming import (
|
||||
build_library_path,
|
||||
derive_primary_title,
|
||||
parse_naming_template,
|
||||
same_filesystem,
|
||||
sanitize_filename,
|
||||
)
|
||||
from shelfmark.core.utils import is_audiobook as check_audiobook
|
||||
@@ -39,10 +38,7 @@ _TRANSFER_PROCESS_ERRORS = (AttributeError, KeyError, OSError, RuntimeError, Typ
|
||||
|
||||
|
||||
def should_hardlink(task: DownloadTask) -> bool:
|
||||
"""Check if hardlinking is enabled for this task (Prowlarr torrents only)."""
|
||||
if task.source != "prowlarr":
|
||||
return False
|
||||
|
||||
"""Check if hardlinking is enabled for this torrent-backed task."""
|
||||
if not task.original_download_path:
|
||||
return False
|
||||
|
||||
@@ -96,21 +92,21 @@ def resolve_hardlink_source(
|
||||
if hardlink_enabled and task.original_download_path:
|
||||
hardlink_source = Path(task.original_download_path)
|
||||
hardlink_source_exists = run_blocking_io(hardlink_source.exists)
|
||||
if (
|
||||
destination
|
||||
and hardlink_source_exists
|
||||
and run_blocking_io(same_filesystem, hardlink_source, destination)
|
||||
):
|
||||
if hardlink_source_exists:
|
||||
use_hardlink = True
|
||||
source_path = hardlink_source
|
||||
elif hardlink_source_exists:
|
||||
logger.warning(
|
||||
"Cannot hardlink: %s and %s are on different filesystems. Falling back to copy. To fix: ensure torrent client downloads to same filesystem as destination.",
|
||||
logger.info(
|
||||
"Hardlink enabled for task %s; attempting link from %s to %s",
|
||||
task.task_id,
|
||||
hardlink_source,
|
||||
destination,
|
||||
)
|
||||
if status_callback:
|
||||
status_callback("resolving", "Cannot hardlink (different filesystems), using copy")
|
||||
else:
|
||||
logger.warning(
|
||||
"Hardlink enabled for task %s, but source path does not exist: %s",
|
||||
task.task_id,
|
||||
hardlink_source,
|
||||
)
|
||||
|
||||
return TransferPlan(
|
||||
source_path=source_path,
|
||||
|
||||
@@ -28,7 +28,6 @@ def _run_organize_post_process(
|
||||
task,
|
||||
library: Path,
|
||||
hardlink_enabled: bool = True,
|
||||
same_fs: bool = True,
|
||||
):
|
||||
from shelfmark.download.postprocess.router import (
|
||||
post_process_download as _post_process_download,
|
||||
@@ -37,10 +36,7 @@ def _run_organize_post_process(
|
||||
status_cb = MagicMock()
|
||||
cancel_flag = Event()
|
||||
|
||||
with (
|
||||
patch("shelfmark.core.config.config") as mock_config,
|
||||
patch("shelfmark.download.postprocess.transfer.same_filesystem", return_value=same_fs),
|
||||
):
|
||||
with patch("shelfmark.core.config.config") as mock_config:
|
||||
mock_config.CUSTOM_SCRIPT = None
|
||||
mock_config.get = MagicMock(
|
||||
side_effect=lambda key, default=None, **_kwargs: {
|
||||
@@ -722,7 +718,6 @@ class TestHardlinkDecisionLogic:
|
||||
task=sample_task,
|
||||
library=library,
|
||||
hardlink_enabled=True,
|
||||
same_fs=True,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
@@ -776,6 +771,29 @@ class TestHardlinkDecisionLogic:
|
||||
assert result is not None
|
||||
assert not staged.exists()
|
||||
|
||||
def test_non_prowlarr_torrent_with_original_path_can_hardlink(self, tmp_path, sample_task):
|
||||
"""Torrent-backed sources such as AudiobookBay can hardlink client files."""
|
||||
library = tmp_path / "library"
|
||||
library.mkdir()
|
||||
source = tmp_path / "downloads" / "book.m4b"
|
||||
source.parent.mkdir()
|
||||
source.write_bytes(b"content")
|
||||
|
||||
sample_task.source = "audiobookbay"
|
||||
sample_task.content_type = "audiobook"
|
||||
sample_task.format = "m4b"
|
||||
sample_task.original_download_path = str(source)
|
||||
|
||||
result, _ = _run_organize_post_process(
|
||||
temp_file=source,
|
||||
task=sample_task,
|
||||
library=library,
|
||||
hardlink_enabled=True,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
assert Path(result).stat().st_ino == source.stat().st_ino
|
||||
|
||||
|
||||
class TestHardlinkInodeVerification:
|
||||
"""Tests that verify hardlinks share the same inode."""
|
||||
@@ -1347,7 +1365,7 @@ class TestTorrentSourceCleanupProtection:
|
||||
from shelfmark.download.postprocess.pipeline import transfer_file_to_library
|
||||
|
||||
# Simulate by directly calling transfer_file_to_library with use_hardlink=False
|
||||
# (this is what happens after same_filesystem check fails)
|
||||
# (this is what happens when hardlinking is disabled before transfer)
|
||||
downloads = tmp_path / "downloads"
|
||||
downloads.mkdir()
|
||||
torrent_file = downloads / "book.epub"
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
"""Integration tests for real filesystem processing flows."""
|
||||
|
||||
import errno
|
||||
import os
|
||||
import zipfile
|
||||
from contextlib import nullcontext
|
||||
from pathlib import Path
|
||||
from threading import Event
|
||||
from unittest.mock import MagicMock, patch
|
||||
@@ -46,6 +48,15 @@ def _sync_config(mock_config, mock_core):
|
||||
mock_core.CUSTOM_SCRIPT = mock_config.CUSTOM_SCRIPT
|
||||
|
||||
|
||||
def _hardlink_support_patch(supported: bool):
|
||||
if supported:
|
||||
return nullcontext()
|
||||
return patch(
|
||||
"shelfmark.download.fs.os.link",
|
||||
side_effect=OSError(errno.EXDEV, "Invalid cross-device link"),
|
||||
)
|
||||
|
||||
|
||||
def test_direct_download_rename_moves_file(tmp_path):
|
||||
from shelfmark.download.postprocess.router import (
|
||||
post_process_download as _post_process_download,
|
||||
@@ -366,7 +377,7 @@ def test_torrent_hardlink_enabled_copy_fallback_does_not_extract_archives(tmp_pa
|
||||
with (
|
||||
patch("shelfmark.core.config.config") as mock_config,
|
||||
patch("shelfmark.config.env.TMP_DIR", staging),
|
||||
patch("shelfmark.download.postprocess.transfer.same_filesystem", return_value=False),
|
||||
_hardlink_support_patch(False),
|
||||
):
|
||||
mock_config.get = _build_config(ingest, organization="none", hardlink=True)
|
||||
mock_config.CUSTOM_SCRIPT = None
|
||||
@@ -385,7 +396,7 @@ def test_torrent_hardlink_enabled_copy_fallback_does_not_extract_archives(tmp_pa
|
||||
# Most importantly: hardlink-setting-enabled fallback to copy should NOT extract.
|
||||
assert list(ingest.glob("*.epub")) == []
|
||||
|
||||
assert any(msg.startswith("Copying") for _, msg in statuses)
|
||||
assert any(msg.startswith("Hardlinking") for _, msg in statuses)
|
||||
|
||||
|
||||
def test_torrent_hardlink_enabled_copy_fallback_directory_archive_kept_when_zip_supported(tmp_path):
|
||||
@@ -422,7 +433,7 @@ def test_torrent_hardlink_enabled_copy_fallback_directory_archive_kept_when_zip_
|
||||
with (
|
||||
patch("shelfmark.core.config.config") as mock_config,
|
||||
patch("shelfmark.config.env.TMP_DIR", staging),
|
||||
patch("shelfmark.download.postprocess.transfer.same_filesystem", return_value=False),
|
||||
_hardlink_support_patch(False),
|
||||
):
|
||||
mock_config.get = _build_config(
|
||||
ingest,
|
||||
@@ -1028,20 +1039,20 @@ def test_postprocess_folder_blackbox_matrix(
|
||||
@pytest.mark.parametrize("content_kind", ["book", "audiobook"])
|
||||
@pytest.mark.parametrize("organization", ["none", "organize"])
|
||||
@pytest.mark.parametrize("hardlink_enabled", [False, True])
|
||||
@pytest.mark.parametrize("same_filesystem", [True, False])
|
||||
@pytest.mark.parametrize("hardlink_supported", [True, False])
|
||||
def test_postprocess_torrent_blackbox_matrix(
|
||||
tmp_path,
|
||||
input_kind: str,
|
||||
content_kind: str,
|
||||
organization: str,
|
||||
hardlink_enabled: bool,
|
||||
same_filesystem: bool,
|
||||
hardlink_supported: bool,
|
||||
):
|
||||
"""Torrent-like (original_download_path set) black-box test matrix.
|
||||
|
||||
This exercises:
|
||||
- hardlink enabled/disabled
|
||||
- same-filesystem hardlink vs copy fallback
|
||||
- successful hardlink vs copy fallback
|
||||
- content type differences (book vs audiobook)
|
||||
|
||||
Assertions focus on invariants:
|
||||
@@ -1087,7 +1098,7 @@ def test_postprocess_torrent_blackbox_matrix(
|
||||
source_file.write_text("content")
|
||||
|
||||
task = DownloadTask(
|
||||
task_id=f"torrent-matrix-{input_kind}-{content_kind}-{organization}-{hardlink_enabled}-{same_filesystem}",
|
||||
task_id=f"torrent-matrix-{input_kind}-{content_kind}-{organization}-{hardlink_enabled}-{hardlink_supported}",
|
||||
source="prowlarr",
|
||||
title=title,
|
||||
author=author,
|
||||
@@ -1100,9 +1111,7 @@ def test_postprocess_torrent_blackbox_matrix(
|
||||
with (
|
||||
patch("shelfmark.core.config.config") as mock_config,
|
||||
patch("shelfmark.config.env.TMP_DIR", staging),
|
||||
patch(
|
||||
"shelfmark.download.postprocess.transfer.same_filesystem", return_value=same_filesystem
|
||||
),
|
||||
_hardlink_support_patch(hardlink_supported),
|
||||
):
|
||||
mock_config.get = _build_config(
|
||||
ingest,
|
||||
@@ -1131,8 +1140,8 @@ def test_postprocess_torrent_blackbox_matrix(
|
||||
assert result_path.parent == ingest
|
||||
assert result_path.name == f"random.{extension}"
|
||||
|
||||
# Hardlink only when enabled and same filesystem.
|
||||
if hardlink_enabled and same_filesystem:
|
||||
# Hardlink only when enabled and supported by the filesystem.
|
||||
if hardlink_enabled and hardlink_supported:
|
||||
assert os.stat(source_file).st_ino == os.stat(result_path).st_ino
|
||||
else:
|
||||
assert os.stat(source_file).st_ino != os.stat(result_path).st_ino
|
||||
|
||||
Reference in New Issue
Block a user