mirror of
https://github.com/beetbox/beets.git
synced 2026-07-16 17:48:54 -04:00
fix(importfeeds): keep import going when a symlink can't be created
When `formats` includes `link`, importfeeds creates a symlink per imported item. A failed symlink (lacking privilege on Windows, a read-only directory, or a filesystem without symlink support) raised beets.util.FilesystemError out of the import pipeline and aborted the whole `beet import` run, even though the tracks were already imported. Catch FilesystemError around the link() call, log a per-item warning, and continue with the remaining items. Add regression tests for the warn-and-continue behaviour, that only FilesystemError is caught, and that a successful link still creates the symlink. Add a changelog entry. Fixes #840.
This commit is contained in:
@@ -9,7 +9,14 @@ import re
|
||||
|
||||
from beets import config
|
||||
from beets.plugins import BeetsPlugin
|
||||
from beets.util import bytestring_path, link, mkdirall, normpath, syspath
|
||||
from beets.util import (
|
||||
FilesystemError,
|
||||
bytestring_path,
|
||||
link,
|
||||
mkdirall,
|
||||
normpath,
|
||||
syspath,
|
||||
)
|
||||
|
||||
M3U_DEFAULT_NAME = "imported.m3u"
|
||||
|
||||
@@ -113,7 +120,12 @@ class ImportFeedsPlugin(BeetsPlugin):
|
||||
for path in paths:
|
||||
dest = os.path.join(feedsdir, os.path.basename(path))
|
||||
if not os.path.exists(syspath(dest)):
|
||||
link(path, dest)
|
||||
try:
|
||||
link(path, dest)
|
||||
except FilesystemError as exc:
|
||||
self._log.warning(
|
||||
"could not create symlink for {}: {}", path, exc
|
||||
)
|
||||
|
||||
if "echo" in formats:
|
||||
self._log.info("Location of imported music:")
|
||||
|
||||
@@ -28,6 +28,9 @@ New features
|
||||
Bug fixes
|
||||
~~~~~~~~~
|
||||
|
||||
- :doc:`plugins/importfeeds`: ``beet import`` no longer aborts the whole run
|
||||
when a symlink cannot be created (e.g. on Windows or a read-only directory);
|
||||
the failure is logged and the import continues. :bug:`840`
|
||||
- Album ``store`` no longer copies ``artpath`` onto its items as an absolute
|
||||
path, which broke relative-path portability. A database migration removes any
|
||||
such stale ``artpath`` attributes left on items by earlier versions.
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
import datetime
|
||||
import os
|
||||
from unittest import mock
|
||||
|
||||
import pytest
|
||||
|
||||
from beets.library import Album, Item
|
||||
from beets.test.helper import PluginTestCase
|
||||
from beets.util import FilesystemError
|
||||
from beetsplug.importfeeds import ImportFeedsPlugin
|
||||
|
||||
|
||||
@@ -62,3 +66,60 @@ class ImportFeedsTest(PluginTestCase):
|
||||
assert os.path.isfile(playlist)
|
||||
with open(playlist) as playlist_contents:
|
||||
assert item_path in playlist_contents.read()
|
||||
|
||||
def test_link_failure_warns_and_continues(self):
|
||||
"""A symlink that can't be created is logged as a warning and
|
||||
skipped; remaining items are still processed (#840)."""
|
||||
self.config["importfeeds"]["formats"] = "link"
|
||||
album = Album(album="album/name", id=1)
|
||||
item1 = Item(title="one", album_id=1, path=os.path.join("path", "one"))
|
||||
item2 = Item(title="two", album_id=1, path=os.path.join("path", "two"))
|
||||
self.lib.add(album)
|
||||
self.lib.add(item1)
|
||||
self.lib.add(item2)
|
||||
|
||||
with (
|
||||
mock.patch(
|
||||
"beetsplug.importfeeds.link",
|
||||
side_effect=[FilesystemError("x", "link", (b"a", b"b")), None],
|
||||
) as mock_link,
|
||||
mock.patch.object(self.importfeeds, "_log") as log,
|
||||
):
|
||||
self.importfeeds.album_imported(self.lib, album)
|
||||
|
||||
log.warning.assert_called_once()
|
||||
assert mock_link.call_count == 2
|
||||
|
||||
def test_non_filesystem_error_is_not_swallowed(self):
|
||||
"""Only FilesystemError is caught; other errors still propagate."""
|
||||
self.config["importfeeds"]["formats"] = "link"
|
||||
album = Album(album="album/name", id=1)
|
||||
item = Item(
|
||||
title="song", album_id=1, path=os.path.join("path", "to", "item")
|
||||
)
|
||||
self.lib.add(album)
|
||||
self.lib.add(item)
|
||||
|
||||
with (
|
||||
mock.patch(
|
||||
"beetsplug.importfeeds.link",
|
||||
side_effect=ValueError("unexpected"),
|
||||
),
|
||||
pytest.raises(ValueError, match="unexpected"),
|
||||
):
|
||||
self.importfeeds.album_imported(self.lib, album)
|
||||
|
||||
def test_link_creates_symlink(self):
|
||||
"""The happy path still creates the symlink when it succeeds."""
|
||||
self.config["importfeeds"]["formats"] = "link"
|
||||
self.feeds_dir.mkdir(parents=True, exist_ok=True)
|
||||
album = Album(album="album/name", id=1)
|
||||
item_path = os.path.join("path", "to", "item")
|
||||
item = Item(title="song", album_id=1, path=item_path)
|
||||
self.lib.add(album)
|
||||
self.lib.add(item)
|
||||
|
||||
self.importfeeds.album_imported(self.lib, album)
|
||||
|
||||
linked = self.feeds_dir / item.filepath.name
|
||||
assert os.path.islink(linked)
|
||||
|
||||
Reference in New Issue
Block a user