Merge pull request #4848 from wisp3rwind/pr_add_syspath_2

Always use syspath conversions (#3690 split up, part 2)

beets/util/artresizer.py

@@ -460,11 +460,11 @@ def write_metadata(self, file, metadata):
 
         # FIXME: Detect and handle other file types (currently, the only user
         # is the thumbnails plugin, which generates PNG images).
-        im = Image.open(file)
+        im = Image.open(syspath(file))
         meta = PngImagePlugin.PngInfo()
         for k, v in metadata.items():
             meta.add_text(k, v, 0)
-        im.save(file, "PNG", pnginfo=meta)
+        im.save(py3_path(file), "PNG", pnginfo=meta)
 
 
 class Shareable(type):

beetsplug/convert.py

@@ -553,7 +553,7 @@ def _get_art_resize(self, artpath):
     def _cleanup(self, task, session):
         for path in task.old_paths:
             if path in _temp_files:
-                if os.path.isfile(path):
+                if os.path.isfile(util.syspath(path)):
                     util.remove(path)
                 _temp_files.remove(path)
 

beetsplug/embedart.py

@@ -226,8 +226,8 @@ def remove_artfile(self, album):
         appropriate configuration option is enabled).
         """
         if self.config['remove_art_file'] and album.artpath:
-            if os.path.isfile(album.artpath):
+            if os.path.isfile(syspath(album.artpath)):
                 self._log.debug('Removing album art file for {0}', album)
-                os.remove(album.artpath)
+                os.remove(syspath(album.artpath))
                 album.artpath = None
                 album.store()

beetsplug/fetchart.py

@@ -1133,7 +1133,8 @@ def __init__(self):
     def fetch_art(self, session, task):
         """Find art for the album being imported."""
         if task.is_album:  # Only fetch art for full albums.
-            if task.album.artpath and os.path.isfile(task.album.artpath):
+            if (task.album.artpath
+                    and os.path.isfile(syspath(task.album.artpath))):
                 # Album already has art (probably a re-import); skip it.
                 return
             if task.choice_flag == importer.action.ASIS:
@@ -1237,7 +1238,8 @@ def batch_fetch_art(self, lib, albums, force, quiet):
         fetchart CLI command.
         """
         for album in albums:
-            if album.artpath and not force and os.path.isfile(album.artpath):
+            if (album.artpath and not force
+                    and os.path.isfile(syspath(album.artpath))):
                 if not quiet:
                     message = ui.colorize('text_highlight_minor',
                                           'has album art')

beetsplug/ipfs.py

@@ -17,6 +17,7 @@
 
 from beets import ui, util, library, config
 from beets.plugins import BeetsPlugin
+from beets.util import syspath
 
 import subprocess
 import shutil
@@ -172,7 +173,10 @@ def ipfs_get_from_hash(self, lib, _hash):
         imp = ui.commands.TerminalImportSession(lib, loghandler=None,
                                                 query=None, paths=[_hash])
         imp.run()
-        shutil.rmtree(_hash)
+        # This uses a relative path, hence we cannot use util.syspath(_hash,
+        # prefix=True). However, that should be fine since the hash will not
+        # exceed MAX_PATH.
+        shutil.rmtree(syspath(_hash, prefix=False))
 
     def ipfs_publish(self, lib):
         with tempfile.NamedTemporaryFile() as tmp:

beetsplug/metasync/itunes.py

@@ -28,19 +28,20 @@
 from beets import util
 from beets.dbcore import types
 from beets.library import DateType
+from beets.util import bytestring_path, syspath
 from confuse import ConfigValueError
 from beetsplug.metasync import MetaSource
 
 
 @contextmanager
 def create_temporary_copy(path):
-    temp_dir = tempfile.mkdtemp()
-    temp_path = os.path.join(temp_dir, 'temp_itunes_lib')
-    shutil.copyfile(path, temp_path)
+    temp_dir = bytestring_path(tempfile.mkdtemp())
+    temp_path = os.path.join(temp_dir, b'temp_itunes_lib')
+    shutil.copyfile(syspath(path), syspath(temp_path))
     try:
         yield temp_path
     finally:
-        shutil.rmtree(temp_dir)
+        shutil.rmtree(syspath(temp_dir))
 
 
 def _norm_itunes_path(path):

beetsplug/thumbnails.py

@@ -31,12 +31,13 @@
 from beets.plugins import BeetsPlugin
 from beets.ui import Subcommand, decargs
 from beets import util
+from beets.util import bytestring_path, displayable_path, syspath
 from beets.util.artresizer import ArtResizer
 
 
 BASE_DIR = os.path.join(BaseDirectory.xdg_cache_home, "thumbnails")
-NORMAL_DIR = util.bytestring_path(os.path.join(BASE_DIR, "normal"))
-LARGE_DIR = util.bytestring_path(os.path.join(BASE_DIR, "large"))
+NORMAL_DIR = bytestring_path(os.path.join(BASE_DIR, "normal"))
+LARGE_DIR = bytestring_path(os.path.join(BASE_DIR, "large"))
 
 
 class ThumbnailsPlugin(BeetsPlugin):
@@ -85,8 +86,8 @@ def _check_local_ok(self):
             return False
 
         for dir in (NORMAL_DIR, LARGE_DIR):
-            if not os.path.exists(dir):
-                os.makedirs(dir)
+            if not os.path.exists(syspath(dir)):
+                os.makedirs(syspath(dir))
 
         if not ArtResizer.shared.can_write_metadata:
             raise RuntimeError(
@@ -136,19 +137,19 @@ def make_cover_thumbnail(self, album, size, target_dir):
         """
         target = os.path.join(target_dir, self.thumbnail_file_name(album.path))
 
-        if os.path.exists(target) and \
-           os.stat(target).st_mtime > os.stat(album.artpath).st_mtime:
+        if (os.path.exists(syspath(target))
+                and os.stat(syspath(target)).st_mtime
+                > os.stat(syspath(album.artpath)).st_mtime):
             if self.config['force']:
                 self._log.debug("found a suitable {1}x{1} thumbnail for {0}, "
                                 "forcing regeneration", album, size)
             else:
                 self._log.debug("{1}x{1} thumbnail for {0} exists and is "
                                 "recent enough", album, size)
                 return False
-        resized = ArtResizer.shared.resize(size, album.artpath,
-                                           util.syspath(target))
-        self.add_tags(album, util.syspath(resized))
-        shutil.move(resized, target)
+        resized = ArtResizer.shared.resize(size, album.artpath, target)
+        self.add_tags(album, resized)
+        shutil.move(syspath(resized), syspath(target))
         return True
 
     def thumbnail_file_name(self, path):
@@ -157,31 +158,31 @@ def thumbnail_file_name(self, path):
         """
         uri = self.get_uri(path)
         hash = md5(uri.encode('utf-8')).hexdigest()
-        return util.bytestring_path(f"{hash}.png")
+        return bytestring_path(f"{hash}.png")
 
     def add_tags(self, album, image_path):
         """Write required metadata to the thumbnail
         See https://standards.freedesktop.org/thumbnail-spec/latest/x142.html
         """
-        mtime = os.stat(album.artpath).st_mtime
+        mtime = os.stat(syspath(album.artpath)).st_mtime
         metadata = {"Thumb::URI": self.get_uri(album.artpath),
                     "Thumb::MTime": str(mtime)}
         try:
             ArtResizer.shared.write_metadata(image_path, metadata)
         except Exception:
             self._log.exception("could not write metadata to {0}",
-                                util.displayable_path(image_path))
+                                displayable_path(image_path))
 
     def make_dolphin_cover_thumbnail(self, album):
         outfilename = os.path.join(album.path, b".directory")
-        if os.path.exists(outfilename):
+        if os.path.exists(syspath(outfilename)):
             return
         artfile = os.path.split(album.artpath)[1]
-        with open(outfilename, 'w') as f:
+        with open(syspath(outfilename), 'w') as f:
             f.write('[Desktop Entry]\n')
             f.write('Icon=./{}'.format(artfile.decode('utf-8')))
             f.close()
-        self._log.debug("Wrote file {0}", util.displayable_path(outfilename))
+        self._log.debug("Wrote file {0}", displayable_path(outfilename))
 
 
 class URIGetter:
@@ -243,7 +244,7 @@ def uri(self, path):
         g_file_ptr = self.libgio.g_file_new_for_path(path)
         if not g_file_ptr:
             raise RuntimeError("No gfile pointer received for {}".format(
-                util.displayable_path(path)))
+                displayable_path(path)))
 
         try:
             uri_ptr = self.libgio.g_file_get_uri(g_file_ptr)
@@ -252,7 +253,7 @@ def uri(self, path):
         if not uri_ptr:
             self.libgio.g_free(uri_ptr)
             raise RuntimeError("No URI received from the gfile pointer for "
-                               "{}".format(util.displayable_path(path)))
+                               "{}".format(displayable_path(path)))
 
         try:
             uri = copy_c_string(uri_ptr)

test/test_thumbnails.py

@@ -51,8 +51,7 @@ def test_add_tags(self, mock_stat, _, mock_artresizer):
             b"/path/to/thumbnail",
             metadata,
         )
-        # FIXME: Plugin should use syspath
-        mock_stat.assert_called_once_with(album.artpath)
+        mock_stat.assert_called_once_with(syspath(album.artpath))
 
     @patch('beetsplug.thumbnails.os')
     @patch('beetsplug.thumbnails.ArtResizer')
@@ -69,17 +68,14 @@ def test_check_local_ok(self, mock_giouri, mock_artresizer, mock_os):
         mock_artresizer.shared.can_write_metadata = True
 
         def exists(path):
-            # FIXME: Plugin should use syspath
-            if path == NORMAL_DIR:
+            if path == syspath(NORMAL_DIR):
                 return False
-            # FIXME: Plugin should use syspath
-            if path == LARGE_DIR:
+            if path == syspath(LARGE_DIR):
                 return True
             raise ValueError(f"unexpected path {path!r}")
         mock_os.path.exists = exists
         plugin = ThumbnailsPlugin()
-        # FIXME: Plugin should use syspath
-        mock_os.makedirs.assert_called_once_with(NORMAL_DIR)
+        mock_os.makedirs.assert_called_once_with(syspath(NORMAL_DIR))
         self.assertTrue(plugin._check_local_ok())
 
         # test metadata writer function
@@ -125,11 +121,9 @@ def test_make_cover_thumbnail(self, mock_shutils, mock_os, mock_util,
         mock_os.path.exists.return_value = False
 
         def os_stat(target):
-            # FIXME: Plugin should use syspath
-            if target == md5_file:
+            if target == syspath(md5_file):
                 return Mock(st_mtime=1)
-            # FIXME: Plugin should use syspath
-            elif target == path_to_art:
+            elif target == syspath(path_to_art):
                 return Mock(st_mtime=2)
             else:
                 raise ValueError(f"invalid target {target}")
@@ -140,29 +134,25 @@ def os_stat(target):
 
         plugin.make_cover_thumbnail(album, 12345, thumbnail_dir)
 
-        # FIXME: Plugin should use syspath
-        mock_os.path.exists.assert_called_once_with(md5_file)
+        mock_os.path.exists.assert_called_once_with(syspath(md5_file))
         mock_os.stat.has_calls([call(syspath(md5_file)),
                                 call(syspath(path_to_art))],
                                any_order=True)
 
         mock_resize.assert_called_once_with(12345, path_to_art, md5_file)
         plugin.add_tags.assert_called_once_with(album, path_to_resized_art)
-        # FIXME: Plugin should use syspath
-        mock_shutils.move.assert_called_once_with(path_to_resized_art,
-                                                  md5_file)
+        mock_shutils.move.assert_called_once_with(syspath(path_to_resized_art),
+                                                  syspath(md5_file))
 
         # now test with recent thumbnail & with force
         mock_os.path.exists.return_value = True
         plugin.force = False
         mock_resize.reset_mock()
 
         def os_stat(target):
-            # FIXME: Plugin should use syspath
-            if target == md5_file:
+            if target == syspath(md5_file):
                 return Mock(st_mtime=3)
-            # FIXME: Plugin should use syspath
-            elif target == path_to_art:
+            elif target == syspath(path_to_art):
                 return Mock(st_mtime=2)
             else:
                 raise ValueError(f"invalid target {target}")