From 0163d1900c94bef491d8cbcb3ac5bcfff4c5f1bd Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 11 Apr 2024 12:02:13 +0200 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20update=20relative=20symlinks=20?= =?UTF-8?q?unnecessarily?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 4 +++ beetsplug/alternatives.py | 12 ++++++-- test/cli_test.py | 60 ++++++++++++++++----------------------- 3 files changed, 39 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63f36e6..6f86425 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ Change Log ========== +## Upcominfg +* Fix an issue where items in a symlink collection with relative links were + always unnecessarily updated. + ## v0.11.1 - 2024-04-24 * Add `--all` flag to update command which will update all configured collections. diff --git a/beetsplug/alternatives.py b/beetsplug/alternatives.py index d1ca4d5..4663112 100644 --- a/beetsplug/alternatives.py +++ b/beetsplug/alternatives.py @@ -420,10 +420,18 @@ def item_change_actions(self, item: Item, path: bytes, dest: bytes): external collection, but might require metadata updates. """ - if path != dest or not util.samefile(os.readlink(path), item.path): + if path != dest: return [Action.MOVE] - else: + + try: + link_target_correct = os.path.samefile(path, item.path) + except FileNotFoundError: + link_target_correct = False + + if link_target_correct: return [] + else: + return [Action.MOVE] @override def update(self, create=None): diff --git a/test/cli_test.py b/test/cli_test.py index 4f2d035..7ec5153 100644 --- a/test/cli_test.py +++ b/test/cli_test.py @@ -117,14 +117,9 @@ def _symlink_view(self): } self.alt_config = self.config["alternatives"]["by-year"] - def test_add_move_remove_album(self, absolute=True): - """Test the symlinks are created and deleted - * An album is added - * The path of the alternative collection is changed - * The query of the alternative collection is changed such that the - album does not match it anymore. - * The links are absolute - """ + def _test_add_move_remove_album(self, *, absolute: bool): + """Test that symlinks are created, moved and deleted.""" + self.add_album( artist="Michael Jackson", album="Thriller", @@ -132,46 +127,37 @@ def test_add_move_remove_album(self, absolute=True): original_year="1982", ) + # Symlink is created self.runcli("alt", "update", "by-year") + alt_path = self.libdir / "by-year/1990/Thriller/track 1.mp3" + library_path = self.libdir / "Michael Jackson/Thriller/track 1.mp3" + assert_symlink(alt_path, library_path, absolute) - by_year_path = self.libdir / "by-year/1990/Thriller/track 1.mp3" - target_path = self.libdir / "Michael Jackson/Thriller/track 1.mp3" - assert_symlink(by_year_path, target_path, absolute) + # Alternative is not updated + assert self.runcli("alt", "update", "by-year") == "" + # Symlink location is updated when path config changes self.alt_config["paths"]["default"] = "$original_year/$album/$title" self.runcli("alt", "update", "by-year") + alt_path = self.libdir / "by-year/1982/Thriller/track 1.mp3" + assert_symlink(alt_path, library_path, absolute) - by_orig_year_path = self.libdir / "by-year/1982/Thriller/track 1.mp3" - assert_symlink(by_orig_year_path, target_path, absolute) - + # Symlink is removed self.alt_config["query"] = "some_field::foobar" self.runcli("alt", "update", "by-year") - - assert_is_not_file(by_orig_year_path) + assert_is_not_file(alt_path) def test_add_move_remove_album_absolute(self): - """Test the absolute symlinks are created and deleted - * Config link type is absolute - * An album is added - * The path of the alternative collection is changed - * The query of the alternative collection is changed such that the - album does not match it anymore. - * The links are absolute - """ + """Test that absolute symlinks are created, moved and deleted.""" + self.alt_config["link_type"] = "absolute" - self.test_add_move_remove_album(absolute=True) + self._test_add_move_remove_album(absolute=True) def test_add_move_remove_album_relative(self): - """Test the relative symlinks are created and deleted - * Config link type is relative - * An album is added - * The path of the alternative collection is changed - * The query of the alternative collection is changed such that the - album does not match it anymore. - * The links are relative - """ + """Test that relative symlinks are created, moved and deleted.""" + self.alt_config["link_type"] = "relative" - self.test_add_move_remove_album(absolute=False) + self._test_add_move_remove_album(absolute=False) def test_update_link_target(self, tmp_path: Path): """Link targets are updated when the item has moved in the library""" @@ -187,11 +173,15 @@ def test_update_link_target(self, tmp_path: Path): absolute=True, ) + # Moving a library item breaks the symlink new_libdir = tmp_path / "newlib" new_libdir.mkdir() self.runcli("move", "-a", "-d", str(new_libdir), "Thriller") - self.runcli("alt", "update", "by-year") + assert alt_path.is_symlink() + assert not alt_path.is_file() + # Updating the alternative fixes the symlink + self.runcli("alt", "update", "by-year") assert_symlink( link=alt_path, target=new_libdir / "Michael Jackson/Thriller/track 1.mp3",