Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update dependencies and do not install reflink on Windows for tests #5407

Merged
merged 6 commits into from
Sep 8, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
run: |
sudo apt update
sudo apt install ffmpeg gobject-introspection libgirepository1.0-dev
poetry install --extras replaygain
poetry install --extras=replaygain --extras=reflink

- name: Install Python dependencies
run: poetry install --only=main,test --extras=autobpm
Expand Down
8 changes: 0 additions & 8 deletions beets/test/_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import os
import sys
import tempfile
import unittest
from contextlib import contextmanager

Expand Down Expand Up @@ -66,13 +65,6 @@
HAVE_SYMLINK = sys.platform != "win32"
HAVE_HARDLINK = sys.platform != "win32"

try:
import reflink

HAVE_REFLINK = reflink.supported_at(tempfile.gettempdir())
except ImportError:
HAVE_REFLINK = False


def item(lib=None):
global _item_ident
Expand Down
16 changes: 15 additions & 1 deletion beets/test/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
from functools import cached_property
from io import StringIO
from pathlib import Path
from tempfile import mkdtemp, mkstemp
from tempfile import gettempdir, mkdtemp, mkstemp
from typing import Any, ClassVar
from unittest.mock import patch

Expand Down Expand Up @@ -147,6 +147,20 @@ def has_program(cmd, args=["--version"]):
return True


def check_reflink_support(path: str) -> bool:
try:
import reflink
except ImportError:
return False

return reflink.supported_at(path)


NEEDS_REFLINK = unittest.skipUnless(
check_reflink_support(gettempdir()), "no reflink support for libdir"
)


class TestHelper(_common.Assertions):
"""Helper mixin for high-level cli and plugin tests.

Expand Down
38 changes: 21 additions & 17 deletions beets/util/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# included in all copies or substantial portions of the Software.

"""Miscellaneous utility functions."""

from __future__ import annotations

import errno
Expand All @@ -29,6 +30,7 @@
from collections import Counter
from contextlib import suppress
from enum import Enum
from importlib import import_module
from logging import Logger
from multiprocessing.pool import ThreadPool
from pathlib import Path
Expand Down Expand Up @@ -166,7 +168,7 @@
"""Provide the canonical form of the path suitable for storing in
the database.
"""
path = syspath(path, prefix=False)

Check failure on line 171 in beets/util/__init__.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Incompatible types in assignment (expression has type "str", variable has type "bytes")
path = os.path.normpath(os.path.abspath(os.path.expanduser(path)))
return bytestring_path(path)

Expand All @@ -180,7 +182,7 @@

The argument should *not* be the result of a call to `syspath`.
"""
out = []

Check failure on line 185 in beets/util/__init__.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Need type annotation for "out" (hint: "out: List[<type>] = ...")
last_path = None
while path:
path = os.path.dirname(path)
Expand All @@ -197,17 +199,17 @@

def sorted_walk(
path: AnyStr,
ignore: Sequence = (),

Check failure on line 202 in beets/util/__init__.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Missing type parameters for generic type "Sequence"
ignore_hidden: bool = False,
logger: Optional[Logger] = None,
) -> Generator[Tuple, None, None]:

Check failure on line 205 in beets/util/__init__.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Missing type parameters for generic type "Tuple"
"""Like `os.walk`, but yields things in case-insensitive sorted,
breadth-first order. Directory and file names matching any glob
pattern in `ignore` are skipped. If `logger` is provided, then
warning messages are logged there when a directory cannot be listed.
"""
# Make sure the paths aren't Unicode strings.
path = bytestring_path(path)

Check failure on line 212 in beets/util/__init__.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Incompatible types in assignment (expression has type "bytes", variable has type "str")
ignore = [bytestring_path(i) for i in ignore]

# Get all the directories and files at this level.
Expand All @@ -224,7 +226,7 @@
dirs = []
files = []
for base in contents:
base = bytestring_path(base)

Check failure on line 229 in beets/util/__init__.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Incompatible types in assignment (expression has type "bytes", variable has type "str")

# Skip ignored filenames.
skip = False
Expand All @@ -240,8 +242,8 @@
continue

# Add to output as either a file or a directory.
cur = os.path.join(path, base)

Check failure on line 245 in beets/util/__init__.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

No overload variant of "join" matches argument types "bytes", "str"
if (ignore_hidden and not hidden.is_hidden(cur)) or not ignore_hidden:

Check failure on line 246 in beets/util/__init__.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Argument 1 to "is_hidden" has incompatible type "str"; expected "Union[bytes, Path]"
if os.path.isdir(syspath(cur)):
dirs.append(base)
else:
Expand Down Expand Up @@ -615,31 +617,33 @@
Raise an `OSError` if `dest` already exists, unless `replace` is
True. If `path` == `dest`, then do nothing.

If reflinking fails and `fallback` is enabled, try copying the file
instead. Otherwise, raise an error without trying a plain copy.

May raise an `ImportError` if the `reflink` module is not available.
If `fallback` is enabled, ignore errors and copy the file instead.
Otherwise, errors are re-raised as FilesystemError with an explanation.
"""
import reflink as pyreflink

if samefile(path, dest):
return

if os.path.exists(syspath(dest)) and not replace:
raise FilesystemError("file exists", "rename", (path, dest))

if fallback:
with suppress(Exception):
return import_module("reflink").reflink(path, dest)
return copy(path, dest, replace)

try:
pyreflink.reflink(path, dest)
except (NotImplementedError, pyreflink.ReflinkImpossibleError):
if fallback:
copy(path, dest, replace)
else:
raise FilesystemError(
"OS/filesystem does not support reflinks.",
"link",
(path, dest),
traceback.format_exc(),
)
import_module("reflink").reflink(path, dest)
except (ImportError, OSError):
raise
except Exception as exc:
msg = {
"EXDEV": "Cannot reflink across devices",
"EOPNOTSUPP": "Device does not support reflinks",
}.get(str(exc), "OS does not support reflinks")

raise FilesystemError(
msg, "reflink", (path, dest), traceback.format_exc()
) from exc


def unique_path(path: bytes) -> bytes:
Expand Down
4 changes: 2 additions & 2 deletions docs/reference/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -600,13 +600,13 @@ Defaults to ``no``.

This kind of clone is only available on certain filesystems: for example,
btrfs and APFS. For more details on filesystem support, see the `pyreflink`_
documentation. Note that you need to install ``pyreflink``, either through
documentation. Note that you need to install ``pyreflink``, either through
``python -m pip install beets[reflink]`` or ``python -m pip install reflink``.

The option is ignored if ``move`` is enabled (i.e., beets can move or
copy files but it doesn't make sense to do both).

.. _file clones: https://blogs.oracle.com/otn/save-disk-space-on-linux-by-cloning-files-on-btrfs-and-ocfs2
.. _file clones: https://en.wikipedia.org/wiki/Copy-on-write
.. _pyreflink: https://reflink.readthedocs.io/en/latest/

resume
Expand Down
1,444 changes: 749 additions & 695 deletions poetry.lock

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ musicbrainzngs = ">=0.4"
pyyaml = "*"
typing_extensions = { version = "*", python = "<=3.10" }
unidecode = ">=1.3.6"

beautifulsoup4 = { version = "*", optional = true }
dbus-python = { version = "*", optional = true }
flask = { version = "*", optional = true }
Expand All @@ -61,7 +62,7 @@ pyxdg = { version = "*", optional = true }
rarfile = { version = "*", optional = true }
reflink = { version = "*", optional = true }
requests = { version = "*", optional = true }
resampy = {version = ">=0.4.3", optional = true}
resampy = { version = ">=0.4.3", optional = true }
requests-oauthlib = { version = ">=0.6.1", optional = true }
soco = { version = "*", optional = true }

Expand All @@ -79,7 +80,6 @@ python3-discogs-client = ">=2.3.15"
py7zr = "*"
pyxdg = "*"
rarfile = "*"
reflink = "*"
requests_oauthlib = "*"
responses = ">=0.3.0"

Expand Down
16 changes: 6 additions & 10 deletions test/test_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
# The above copyright notice and this permission notice shall be
# included in all copies or substantial portions of the Software.

"""Test file manipulation functionality of Item.
"""
"""Test file manipulation functionality of Item."""

import os
import shutil
Expand All @@ -27,7 +26,7 @@
from beets import util
from beets.test import _common
from beets.test._common import item, touch
from beets.test.helper import BeetsTestCase
from beets.test.helper import NEEDS_REFLINK, BeetsTestCase
from beets.util import MoveOperation, bytestring_path, syspath


Expand Down Expand Up @@ -87,22 +86,20 @@ def test_copy_does_not_depart(self):
self.i.move(operation=MoveOperation.COPY)
self.assertExists(self.path)

@unittest.skipUnless(_common.HAVE_REFLINK, "need reflink")
def test_reflink_arrives(self):
self.i.move(operation=MoveOperation.REFLINK_AUTO)
self.assertExists(self.dest)

@unittest.skipUnless(_common.HAVE_REFLINK, "need reflink")
def test_reflink_does_not_depart(self):
self.i.move(operation=MoveOperation.REFLINK_AUTO)
self.assertExists(self.path)

@unittest.skipUnless(_common.HAVE_REFLINK, "need reflink")
@NEEDS_REFLINK
def test_force_reflink_arrives(self):
self.i.move(operation=MoveOperation.REFLINK)
self.assertExists(self.dest)

@unittest.skipUnless(_common.HAVE_REFLINK, "need reflink")
@NEEDS_REFLINK
def test_force_reflink_does_not_depart(self):
self.i.move(operation=MoveOperation.REFLINK)
self.assertExists(self.path)
Expand Down Expand Up @@ -286,7 +283,7 @@ def test_albuminfo_move_copies_file(self):
self.assertExists(oldpath)
self.assertExists(self.i.path)

@unittest.skipUnless(_common.HAVE_REFLINK, "need reflink")
@NEEDS_REFLINK
def test_albuminfo_move_reflinks_file(self):
oldpath = self.i.path
self.ai.album = "newAlbumName"
Expand Down Expand Up @@ -571,7 +568,7 @@ def test_successful_copy(self):
self.assertExists(self.dest)
self.assertExists(self.path)

@unittest.skipUnless(_common.HAVE_REFLINK, "need reflink")
@NEEDS_REFLINK
def test_successful_reflink(self):
util.reflink(self.path, self.dest)
self.assertExists(self.dest)
Expand All @@ -585,7 +582,6 @@ def test_unsuccessful_copy(self):
with pytest.raises(util.FilesystemError):
util.copy(self.path, self.otherpath)

@unittest.skipUnless(_common.HAVE_REFLINK, "need reflink")
snejus marked this conversation as resolved.
Show resolved Hide resolved
def test_unsuccessful_reflink(self):
with pytest.raises(util.FilesystemError):
util.reflink(self.path, self.otherpath)
Expand Down
11 changes: 6 additions & 5 deletions test/test_importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
# included in all copies or substantial portions of the Software.


"""Tests for the general importer functionality.
"""
"""Tests for the general importer functionality."""

import os
import re
import shutil
Expand All @@ -37,6 +37,7 @@
from beets.importer import albums_in_dir
from beets.test import _common
from beets.test.helper import (
NEEDS_REFLINK,
AsIsImporterMixin,
AutotagStub,
BeetsTestCase,
Expand Down Expand Up @@ -209,7 +210,7 @@ def test_import_hardlink_arrives(self):
s2[stat.ST_DEV],
)

@unittest.skipUnless(_common.HAVE_REFLINK, "need reflinks")
@NEEDS_REFLINK
def test_import_reflink_arrives(self):
# Detecting reflinks is currently tricky due to various fs
# implementations, we'll just check the file exists.
Expand Down Expand Up @@ -392,7 +393,7 @@ def test_import_single_files(self):
assert len(self.lib.albums()) == 2

def test_set_fields(self):
genre = "\U0001F3B7 Jazz"
genre = "\U0001f3b7 Jazz"
collection = "To Listen"

config["import"]["set_fields"] = {
Expand Down Expand Up @@ -579,7 +580,7 @@ def test_asis_no_data_source(self):
self.lib.items().get().data_source

def test_set_fields(self):
genre = "\U0001F3B7 Jazz"
genre = "\U0001f3b7 Jazz"
collection = "To Listen"
comments = "managed by beets"

Expand Down
Loading