Merge branch 'master' into ignored_alias_types

beets/util/__init__.py

@@ -17,7 +17,6 @@
 import os
 import sys
 import errno
-import locale
 import re
 import tempfile
 import shutil
@@ -332,12 +331,7 @@ def arg_encoding():
     """Get the encoding for command-line arguments (and other OS
     locale-sensitive strings).
     """
-    try:
-        return locale.getdefaultlocale()[1] or 'utf-8'
-    except ValueError:
-        # Invalid locale environment variable setting. To avoid
-        # failing entirely for no good reason, assume UTF-8.
-        return 'utf-8'
+    return sys.getfilesystemencoding()
 
 
 def _fsencoding():
@@ -837,13 +831,14 @@ def cpu_count():
 
 
 def convert_command_args(args):
-    """Convert command arguments to bytestrings on Python 2 and
-    surrogate-escaped strings on Python 3."""
+    """Convert command arguments, which may either be `bytes` or `str`
+    objects, to uniformly surrogate-escaped strings.
+    """
     assert isinstance(args, list)
 
     def convert(arg):
         if isinstance(arg, bytes):
-            arg = arg.decode(arg_encoding(), 'surrogateescape')
+            return os.fsdecode(arg)
         return arg
 
     return [convert(a) for a in args]
@@ -1092,21 +1087,3 @@ def lazy_property(func):
         return value
 
     return wrapper
-
-
-def decode_commandline_path(path):
-    """Prepare a path for substitution into commandline template.
-
-    On Python 3, we need to construct the subprocess commands to invoke as a
-    Unicode string. On Unix, this is a little unfortunate---the OS is
-    expecting bytes---so we use surrogate escaping and decode with the
-    argument encoding, which is the same encoding that will then be
-    *reversed* to recover the same bytes before invoking the OS. On
-    Windows, we want to preserve the Unicode filename "as is."
-    """
-    # On Python 3, the template is a Unicode string, which only supports
-    # substitution of Unicode variables.
-    if platform.system() == 'Windows':
-        return path.decode(_fsencoding())
-    else:
-        return path.decode(arg_encoding(), 'surrogateescape')

beetsplug/convert.py

@@ -14,7 +14,7 @@
 
 """Converts tracks or albums to external directory
 """
-from beets.util import par_map, decode_commandline_path, arg_encoding
+from beets.util import par_map, arg_encoding
 
 import os
 import threading
@@ -214,8 +214,8 @@ class ConvertPlugin(BeetsPlugin):
             self._log.info('Encoding {0}', util.displayable_path(source))
 
         command = command.decode(arg_encoding(), 'surrogateescape')
-        source = decode_commandline_path(source)
-        dest = decode_commandline_path(dest)
+        source = os.fsdecode(source)
+        dest = os.fsdecode(dest)
 
         # Substitute $source and $dest in the argument list.
         args = shlex.split(command)

beetsplug/duplicates.py

@@ -16,11 +16,12 @@
 """
 
 import shlex
+import os
 
 from beets.plugins import BeetsPlugin
 from beets.ui import decargs, print_, Subcommand, UserError
 from beets.util import command_output, displayable_path, subprocess, \
-    bytestring_path, MoveOperation, decode_commandline_path
+    bytestring_path, MoveOperation
 from beets.library import Item, Album
 
 
@@ -200,7 +201,7 @@ class DuplicatesPlugin(BeetsPlugin):
         output as flexattr on a key that is the name of the program, and
         return the key, checksum tuple.
         """
-        args = [p.format(file=decode_commandline_path(item.path))
+        args = [p.format(file=os.fsdecode(item.path))
                 for p in shlex.split(prog)]
         key = args[0]
         checksum = getattr(item, key, False)

beetsplug/replaygain.py

@@ -49,13 +49,14 @@ class FatalGstreamerPluginReplayGainError(FatalReplayGainError):
     loading the required plugins."""
 
 
-def call(args, **kwargs):
+def call(args, log, **kwargs):
     """Execute the command and return its output or raise a
     ReplayGainError on failure.
     """
     try:
         return command_output(args, **kwargs)
     except subprocess.CalledProcessError as e:
+        log.debug(e.output.decode('utf8', 'ignore'))
         raise ReplayGainError(
             "{} exited with status {}".format(args[0], e.returncode)
         )
@@ -261,7 +262,7 @@ class FfmpegBackend(Backend):
 
         # check that ffmpeg is installed
         try:
-            ffmpeg_version_out = call([self._ffmpeg_path, "-version"])
+            ffmpeg_version_out = call([self._ffmpeg_path, "-version"], log)
         except OSError:
             raise FatalReplayGainError(
                 f"could not find ffmpeg at {self._ffmpeg_path}"
@@ -394,7 +395,7 @@ class FfmpegBackend(Backend):
         self._log.debug(
             'executing {0}', ' '.join(map(displayable_path, cmd))
         )
-        output = call(cmd).stderr.splitlines()
+        output = call(cmd, self._log).stderr.splitlines()
 
         # parse output
 
@@ -525,7 +526,7 @@ class CommandBackend(Backend):
             # Check whether the program is in $PATH.
             for cmd in ('mp3gain', 'aacgain'):
                 try:
-                    call([cmd, '-v'])
+                    call([cmd, '-v'], self._log)
                     self.command = cmd
                 except OSError:
                     pass
@@ -605,7 +606,7 @@ class CommandBackend(Backend):
 
         self._log.debug('analyzing {0} files', len(items))
         self._log.debug("executing {0}", " ".join(map(displayable_path, cmd)))
-        output = call(cmd).stdout
+        output = call(cmd, self._log).stdout
         self._log.debug('analysis finished')
         return self.parse_tool_output(output,
                                       len(items) + (1 if is_album else 0))
@@ -1085,7 +1086,7 @@ class ExceptionWatcher(Thread):
             try:
                 exc = self._queue.get_nowait()
                 self._callback()
-                raise exc[1].with_traceback(exc[2])
+                raise exc
             except queue.Empty:
                 # No exceptions yet, loop back to check
                 #  whether `_stopevent` is set
@@ -1338,23 +1339,16 @@ class ReplayGainPlugin(BeetsPlugin):
 
     def _apply(self, func, args, kwds, callback):
         if self._has_pool():
-            def catch_exc(func, exc_queue, log):
-                """Wrapper to catch raised exceptions in threads
+            def handle_exc(exc):
+                """Handle exceptions in the async work.
                 """
-                def wfunc(*args, **kwargs):
-                    try:
-                        return func(*args, **kwargs)
-                    except ReplayGainError as e:
-                        log.info(e.args[0])  # log non-fatal exceptions
-                    except Exception:
-                        exc_queue.put(sys.exc_info())
-                return wfunc
+                if isinstance(exc, ReplayGainError):
+                    self._log.info(exc.args[0])  # Log non-fatal exceptions.
+                else:
+                    self.exc_queue.put(exc)
 
-            # Wrap function and callback to catch exceptions
-            func = catch_exc(func, self.exc_queue, self._log)
-            callback = catch_exc(callback, self.exc_queue, self._log)
-
-            self.pool.apply_async(func, args, kwds, callback)
+            self.pool.apply_async(func, args, kwds, callback,
+                                  error_callback=handle_exc)
         else:
             callback(func(*args, **kwds))
 

docs/changelog.rst

@@ -50,6 +50,13 @@ New features:
 
 Bug fixes:
 
+* :doc:`/plugins/replaygain`: Avoid a crash when errors occur in the analysis
+  backend.
+  :bug:`4506`
+* We now use Python's defaults for command-line argument encoding, which
+  should reduce the chance for errors and "file not found" failures when
+  invoking other command-line tools, especially on Windows.
+  :bug:`4507`
 * We now respect the Spotify API's rate limiting, which avoids crashing when the API reports code 429 (too many requests).
   :bug:`4370`
 * Fix implicit paths OR queries (e.g. ``beet list /path/ , /other-path/``)

setup.py

@@ -85,7 +85,7 @@ setup(
     },
 
     install_requires=[
-        'unidecode',
+        'unidecode>=1.3.6',
         'musicbrainzngs>=0.4',
         'pyyaml',
         'mediafile>=0.9.0',

test/test_library.py

@@ -441,7 +441,7 @@ class DestinationTest(_common.TestCase):
         self.lib.directory = b'lib'
         self.lib.path_formats = [('default', '$title')]
         self.i.title = 'ab\xa2\xbdd'
-        self.assertEqual(self.i.destination(), np('lib/abC_ 1_2 d'))
+        self.assertEqual(self.i.destination(), np('lib/abC_ 1_2d'))
 
     def test_destination_with_replacements(self):
         self.lib.directory = b'base'
@@ -637,7 +637,7 @@ class DestinationFunctionTest(_common.TestCase, PathFormattingMixin):
 
     def test_asciify_variable(self):
         self._setf('%asciify{ab\xa2\xbdd}')
-        self._assert_dest(b'/base/abC_ 1_2 d')
+        self._assert_dest(b'/base/abC_ 1_2d')
 
     def test_left_variable(self):
         self._setf('%left{$title, 3}')

test/test_util.py

@@ -101,6 +101,7 @@ class UtilTest(unittest.TestCase):
             ])
         self.assertEqual(p, 'foo/_/bar')
 
+    @unittest.skipIf(sys.platform == 'win32', 'win32')
     def test_convert_command_args_keeps_undecodeable_bytes(self):
         arg = b'\x82'  # non-ascii bytes
         cmd_args = util.convert_command_args([arg])