diff --git a/beets/test/helper.py b/beets/test/helper.py index c0d785d6e..85ea6bcf7 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -117,20 +117,9 @@ def capture_stdout(): print(capture.getvalue()) -def _convert_args(args): - """Convert args to bytestrings for Python 2 and convert them to strings - on Python 3. - """ - for i, elem in enumerate(args): - if isinstance(elem, bytes): - args[i] = elem.decode(util.arg_encoding()) - - return args - - def has_program(cmd, args=["--version"]): """Returns `True` if `cmd` can be executed.""" - full_cmd = _convert_args([cmd] + args) + full_cmd = [cmd] + args try: with open(os.devnull, "wb") as devnull: subprocess.check_call( @@ -385,7 +374,7 @@ class TestHelper(_common.Assertions, ConfigMixin): if hasattr(self, "lib"): lib = self.lib lib = kwargs.get("lib", lib) - beets.ui._raw_main(_convert_args(list(args)), lib) + beets.ui._raw_main(list(args), lib) def run_with_output(self, *args): with capture_stdout() as out: diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 99aa04f0a..1822c3e7c 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1359,13 +1359,8 @@ def import_func(lib, opts, args): # what we need. On Python 3, we need to undo the "helpful" # conversion to Unicode strings to get the real bytestring # filename. - paths = [ - p.encode(util.arg_encoding(), "surrogateescape") for p in paths - ] - paths_from_logfiles = [ - p.encode(util.arg_encoding(), "surrogateescape") - for p in paths_from_logfiles - ] + paths = [os.fsencode(p) for p in paths] + paths_from_logfiles = [os.fsencode(p) for p in paths_from_logfiles] # Check the user-specified directories. for path in paths: diff --git a/beets/util/__init__.py b/beets/util/__init__.py index fea864ce8..68dbaee65 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -369,13 +369,6 @@ def components(path: AnyStr) -> list[AnyStr]: return comps -def arg_encoding() -> str: - """Get the encoding for command-line arguments (and other OS - locale-sensitive strings). - """ - return sys.getfilesystemencoding() - - def bytestring_path(path: PathLike) -> bytes: """Given a path, which is either a bytes or a unicode, returns a str path (ensuring that we never deal with Unicode pathnames). Path should be @@ -832,19 +825,6 @@ def plurality(objs: Sequence[T]) -> tuple[T, int]: return c.most_common(1)[0] -def convert_command_args(args: list[BytesOrStr]) -> list[str]: - """Convert command arguments, which may either be `bytes` or `str` - objects, to uniformly surrogate-escaped strings.""" - assert isinstance(args, list) - - def convert(arg) -> str: - if isinstance(arg, bytes): - return os.fsdecode(arg) - return arg - - return [convert(a) for a in args] - - # stdout and stderr as bytes class CommandOutput(NamedTuple): stdout: bytes @@ -869,7 +849,7 @@ def command_output(cmd: list[BytesOrStr], shell: bool = False) -> CommandOutput: This replaces `subprocess.check_output` which can have problems if lots of output is sent to stderr. """ - converted_cmd = convert_command_args(cmd) + converted_cmd = [os.fsdecode(a) for a in cmd] devnull = subprocess.DEVNULL diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 0c50fc73f..a8c32e955 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -28,7 +28,7 @@ from confuse import ConfigTypeError, Optional from beets import art, config, plugins, ui, util from beets.library import Item, parse_query_string from beets.plugins import BeetsPlugin -from beets.util import arg_encoding, par_map +from beets.util import par_map from beets.util.artresizer import ArtResizer from beets.util.m3u import M3UFile @@ -284,7 +284,7 @@ class ConvertPlugin(BeetsPlugin): if not quiet and not pretend: self._log.info("Encoding {0}", util.displayable_path(source)) - command = command.decode(arg_encoding(), "surrogateescape") + command = os.fsdecode(command) source = os.fsdecode(source) dest = os.fsdecode(dest) @@ -298,7 +298,7 @@ class ConvertPlugin(BeetsPlugin): "dest": dest, } ) - encode_cmd.append(args[i].encode(util.arg_encoding())) + encode_cmd.append(os.fsdecode(args[i])) if pretend: self._log.info("{0}", " ".join(ui.decargs(args))) diff --git a/beetsplug/hook.py b/beetsplug/hook.py index c7f8091cb..5ce5ef828 100644 --- a/beetsplug/hook.py +++ b/beetsplug/hook.py @@ -17,9 +17,9 @@ import shlex import string import subprocess +import sys from beets.plugins import BeetsPlugin -from beets.util import arg_encoding class CodingFormatter(string.Formatter): @@ -73,7 +73,7 @@ class HookPlugin(BeetsPlugin): # For backwards compatibility, use a string formatter that decodes # bytes (in particular, paths) to unicode strings. - formatter = CodingFormatter(arg_encoding()) + formatter = CodingFormatter(sys.getfilesystemencoding()) command_pieces = [ formatter.format(piece, event=event, **kwargs) for piece in shlex.split(command) diff --git a/test/plugins/test_convert.py b/test/plugins/test_convert.py index 2fd1d177b..a2b4eaf67 100644 --- a/test/plugins/test_convert.py +++ b/test/plugins/test_convert.py @@ -143,18 +143,13 @@ class ConvertCommand: in tests. """ - def run_convert_path(self, path, *args): + def run_convert_path(self, item, *args): """Run the `convert` command on a given path.""" - # The path is currently a filesystem bytestring. Convert it to - # an argument bytestring. - path = os.fsencode(os.fsdecode(path)) - - args = args + (b"path:" + path,) - return self.run_command("convert", *args) + return self.run_command("convert", *args, f"path:{item.filepath}") def run_convert(self, *args): """Run the `convert` command on `self.item`.""" - return self.run_convert_path(self.item.path, *args) + return self.run_convert_path(self.item, *args) @_common.slow_test() @@ -320,7 +315,7 @@ class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand): def test_transcode_from_lossless(self): [item] = self.add_item_fixtures(ext="flac") with control_stdin("y"): - self.run_convert_path(item.path) + self.run_convert_path(item) converted = os.path.join(self.convert_dest, b"converted.mp3") self.assertFileTag(converted, "mp3") @@ -328,14 +323,14 @@ class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand): self.config["convert"]["never_convert_lossy_files"] = False [item] = self.add_item_fixtures(ext="ogg") with control_stdin("y"): - self.run_convert_path(item.path) + self.run_convert_path(item) converted = os.path.join(self.convert_dest, b"converted.mp3") self.assertFileTag(converted, "mp3") def test_transcode_from_lossy_prevented(self): [item] = self.add_item_fixtures(ext="ogg") with control_stdin("y"): - self.run_convert_path(item.path) + self.run_convert_path(item) converted = os.path.join(self.convert_dest, b"converted.ogg") self.assertNoFileTag(converted, "mp3") diff --git a/test/plugins/test_thumbnails.py b/test/plugins/test_thumbnails.py index 00cd545d4..bd3e22714 100644 --- a/test/plugins/test_thumbnails.py +++ b/test/plugins/test_thumbnails.py @@ -100,12 +100,10 @@ class ThumbnailsTest(BeetsTestCase): @patch("beetsplug.thumbnails.ThumbnailsPlugin._check_local_ok", Mock()) @patch("beetsplug.thumbnails.ArtResizer") - @patch("beetsplug.thumbnails.util") + @patch("beets.util.syspath", Mock(side_effect=lambda x: x)) @patch("beetsplug.thumbnails.os") @patch("beetsplug.thumbnails.shutil") - def test_make_cover_thumbnail( - self, mock_shutils, mock_os, mock_util, mock_artresizer - ): + def test_make_cover_thumbnail(self, mock_shutils, mock_os, mock_artresizer): thumbnail_dir = os.path.normpath(b"/thumbnail/dir") md5_file = os.path.join(thumbnail_dir, b"md5") path_to_art = os.path.normpath(b"/path/to/art") @@ -116,7 +114,6 @@ class ThumbnailsTest(BeetsTestCase): plugin.add_tags = Mock() album = Mock(artpath=path_to_art) - mock_util.syspath.side_effect = lambda x: x plugin.thumbnail_file_name = Mock(return_value=b"md5") mock_os.path.exists.return_value = False diff --git a/test/rsrc/convert_stub.py b/test/rsrc/convert_stub.py index cba693c4e..aec4ba572 100755 --- a/test/rsrc/convert_stub.py +++ b/test/rsrc/convert_stub.py @@ -4,18 +4,9 @@ a specified text tag. """ -import locale import sys -# From `beets.util`. -def arg_encoding(): - try: - return locale.getdefaultlocale()[1] or "utf-8" - except ValueError: - return "utf-8" - - def convert(in_file, out_file, tag): """Copy `in_file` to `out_file` and append the string `tag`.""" if not isinstance(tag, bytes): diff --git a/test/test_ui.py b/test/test_ui.py index 6c23b321f..e9588dbc6 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -865,6 +865,9 @@ class ConfigTest(TestPluginTestCase): # Custom BEETSDIR self.beetsdir = os.path.join(self.temp_dir, b"beetsdir") + self.cli_config_path = os.path.join( + os.fsdecode(self.temp_dir), "config.yaml" + ) os.makedirs(syspath(self.beetsdir)) self.env_patcher = patch( "os.environ", @@ -952,20 +955,18 @@ class ConfigTest(TestPluginTestCase): assert repls == [("[xy]", "z"), ("foo", "bar")] def test_cli_config_option(self): - config_path = os.path.join(self.temp_dir, b"config.yaml") - with open(config_path, "w") as file: + with open(self.cli_config_path, "w") as file: file.write("anoption: value") - self.run_command("--config", config_path, "test", lib=None) + self.run_command("--config", self.cli_config_path, "test", lib=None) assert config["anoption"].get() == "value" def test_cli_config_file_overwrites_user_defaults(self): with open(self.user_config_path, "w") as file: file.write("anoption: value") - cli_config_path = os.path.join(self.temp_dir, b"config.yaml") - with open(cli_config_path, "w") as file: + with open(self.cli_config_path, "w") as file: file.write("anoption: cli overwrite") - self.run_command("--config", cli_config_path, "test", lib=None) + self.run_command("--config", self.cli_config_path, "test", lib=None) assert config["anoption"].get() == "cli overwrite" def test_cli_config_file_overwrites_beetsdir_defaults(self): @@ -974,10 +975,9 @@ class ConfigTest(TestPluginTestCase): with open(env_config_path, "w") as file: file.write("anoption: value") - cli_config_path = os.path.join(self.temp_dir, b"config.yaml") - with open(cli_config_path, "w") as file: + with open(self.cli_config_path, "w") as file: file.write("anoption: cli overwrite") - self.run_command("--config", cli_config_path, "test", lib=None) + self.run_command("--config", self.cli_config_path, "test", lib=None) assert config["anoption"].get() == "cli overwrite" # @unittest.skip('Difficult to implement with optparse') @@ -998,29 +998,27 @@ class ConfigTest(TestPluginTestCase): # # @unittest.skip('Difficult to implement with optparse') # def test_multiple_cli_config_overwrite(self): - # cli_config_path = os.path.join(self.temp_dir, b'config.yaml') # cli_overwrite_config_path = os.path.join(self.temp_dir, # b'overwrite_config.yaml') # - # with open(cli_config_path, 'w') as file: + # with open(self.cli_config_path, 'w') as file: # file.write('anoption: value') # # with open(cli_overwrite_config_path, 'w') as file: # file.write('anoption: overwrite') # - # self.run_command('--config', cli_config_path, + # self.run_command('--config', self.cli_config_path, # '--config', cli_overwrite_config_path, 'test') # assert config['anoption'].get() == 'cli overwrite' # FIXME: fails on windows @unittest.skipIf(sys.platform == "win32", "win32") def test_cli_config_paths_resolve_relative_to_user_dir(self): - cli_config_path = os.path.join(self.temp_dir, b"config.yaml") - with open(cli_config_path, "w") as file: + with open(self.cli_config_path, "w") as file: file.write("library: beets.db\n") file.write("statefile: state") - self.run_command("--config", cli_config_path, "test", lib=None) + self.run_command("--config", self.cli_config_path, "test", lib=None) self.assert_equal_path( util.bytestring_path(config["library"].as_filename()), os.path.join(self.user_config_dir, b"beets.db"), @@ -1033,12 +1031,11 @@ class ConfigTest(TestPluginTestCase): def test_cli_config_paths_resolve_relative_to_beetsdir(self): os.environ["BEETSDIR"] = os.fsdecode(self.beetsdir) - cli_config_path = os.path.join(self.temp_dir, b"config.yaml") - with open(cli_config_path, "w") as file: + with open(self.cli_config_path, "w") as file: file.write("library: beets.db\n") file.write("statefile: state") - self.run_command("--config", cli_config_path, "test", lib=None) + self.run_command("--config", self.cli_config_path, "test", lib=None) self.assert_equal_path( util.bytestring_path(config["library"].as_filename()), os.path.join(self.beetsdir, b"beets.db"), @@ -1057,12 +1054,11 @@ class ConfigTest(TestPluginTestCase): ) def test_cli_config_file_loads_plugin_commands(self): - cli_config_path = os.path.join(self.temp_dir, b"config.yaml") - with open(cli_config_path, "w") as file: + with open(self.cli_config_path, "w") as file: file.write("pluginpath: %s\n" % _common.PLUGINPATH) file.write("plugins: test") - self.run_command("--config", cli_config_path, "plugin", lib=None) + self.run_command("--config", self.cli_config_path, "plugin", lib=None) assert plugins.find_plugins()[0].is_test_plugin self.unload_plugins() @@ -1483,9 +1479,7 @@ class CommonOptionsParserCliTest(BeetsTestCase): assert output == "the album artist\n" def test_format_option_unicode(self): - output = self.run_with_output( - b"ls", b"-f", "caf\xe9".encode(util.arg_encoding()) - ) + output = self.run_with_output("ls", "-f", "caf\xe9") assert output == "caf\xe9\n" def test_root_format_option(self): diff --git a/test/test_util.py b/test/test_util.py index 85534949f..f5b4fd102 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -97,13 +97,6 @@ class UtilTest(unittest.TestCase): p = util.sanitize_path("foo//bar", [(re.compile(r"^$"), "_")]) assert 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]) - - assert cmd_args[0] == arg.decode(util.arg_encoding(), "surrogateescape") - @patch("beets.util.subprocess.Popen") def test_command_output(self, mock_popen): def popen_fail(*args, **kwargs):