FIX: Dereference symlinks before hardlinking (#5684)

When creating a hardlink, either during import or `beet convert`, if the
origin of the hardlink was a symlink, that symlink used to be directly
copied. This could create broken symlinks if the origin symlink was
relative, and in either case, probably wasn't the user's desired
behavior.

This change de-references all symlinks before creating a hardlink, such
that the end result is a normal file with the same inode as the original
file. See #5676 for more discussion about the original issue.

Fixes #5676

beets/util/__init__.py

@@ -577,10 +577,14 @@ def hardlink(path: bytes, dest: bytes, replace: bool = False):
     if samefile(path, dest):
         return
 
-    if os.path.exists(syspath(dest)) and not replace:
+    # Dereference symlinks, expand "~", and convert relative paths to absolute
+    origin_path = Path(os.fsdecode(path)).expanduser().resolve()
+    dest_path = Path(os.fsdecode(dest)).expanduser().resolve()
+
+    if dest_path.exists() and not replace:
         raise FilesystemError("file exists", "rename", (path, dest))
     try:
-        os.link(syspath(path), syspath(dest))
+        dest_path.hardlink_to(origin_path)
     except NotImplementedError:
         raise FilesystemError(
             "OS does not support hard links.link",

docs/changelog.rst

@@ -27,6 +27,9 @@ New features:
 
 Bug fixes:
 
+- When hardlinking from a symlink (e.g. importing a symlink with hardlinking
+  enabled), dereference the symlink then hardlink, rather than creating a new
+  (potentially broken) symlink :bug:`5676`
 - :doc:`/plugins/spotify`: The plugin now gracefully handles audio-features API
   deprecation (HTTP 403 errors). When a 403 error is encountered from the
   audio-features endpoint, the plugin logs a warning once and skips audio

test/test_files.py

@@ -36,7 +36,8 @@ class MoveTest(BeetsTestCase):
         super().setUp()
 
         # make a temporary file
-        self.path = self.temp_dir_path / "temp.mp3"
+        self.temp_music_file_name = "temp.mp3"
+        self.path = self.temp_dir_path / self.temp_music_file_name
         shutil.copy(self.resource_path, self.path)
 
         # add it to a temporary library
@@ -197,6 +198,21 @@ class MoveTest(BeetsTestCase):
         self.i.move(operation=MoveOperation.HARDLINK)
         assert self.i.path == util.normpath(self.dest)
 
+    @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks")
+    def test_hardlink_from_symlink(self):
+        link_path = join(self.temp_dir, b"temp_link.mp3")
+        link_source = join("./", self.temp_music_file_name)
+        os.symlink(syspath(link_source), syspath(link_path))
+        self.i.path = link_path
+        self.i.move(operation=MoveOperation.HARDLINK)
+
+        s1 = os.stat(syspath(self.path))
+        s2 = os.stat(syspath(self.dest))
+        assert (s1[stat.ST_INO], s1[stat.ST_DEV]) == (
+            s2[stat.ST_INO],
+            s2[stat.ST_DEV],
+        )
+
 
 class HelperTest(unittest.TestCase):
     def test_ancestry_works_on_file(self):