diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 720ca311a..06e02ee08 100644 --- a/beets/util/__init__.py +++ b/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') diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 95240dc39..17a18e358 100644 --- a/beetsplug/convert.py +++ b/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) diff --git a/beetsplug/duplicates.py b/beetsplug/duplicates.py index fdd5c1750..f655c61f2 100644 --- a/beetsplug/duplicates.py +++ b/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) diff --git a/docs/changelog.rst b/docs/changelog.rst index 9df77d5e7..8ea1c0322 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -51,6 +51,10 @@ 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/``) diff --git a/test/test_util.py b/test/test_util.py index fcaf9f5ce..14ac7f2b2 100644 --- a/test/test_util.py +++ b/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])