From c26266cab060b0f7cfc8966f2a67c407604ad8f4 Mon Sep 17 00:00:00 2001
From: Johnny Robeson <johnny@localmomentum.net>
Date: Thu, 4 Aug 2016 23:38:21 -0400
Subject: [PATCH] convert byte args to string on PY3 in command_output

The strings are surrogateescaped to make sure we can get the bytes
representation back
---
 beets/util/__init__.py | 24 ++++++++++++++++++++++--
 test/test_util.py      | 13 +++++++++++--
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/beets/util/__init__.py b/beets/util/__init__.py
index 5732c81b5..c2d5c9334 100644
--- a/beets/util/__init__.py
+++ b/beets/util/__init__.py
@@ -737,10 +737,28 @@ def cpu_count():
         return 1
 
 
+def convert_command_args(args):
+    """Convert command arguments to bytestrings on Python 2 and
+    surrogate-escaped strings on Python 3."""
+    assert isinstance(args, list)
+
+    def convert(arg):
+        if six.PY2:
+            if isinstance(arg, six.text_type):
+                arg = arg.encode(arg_encoding())
+        else:
+            if isinstance(arg, bytes):
+                arg = arg.decode(arg_encoding(), 'surrogateescape')
+        return arg
+
+    return [convert(a) for a in args]
+
+
 def command_output(cmd, shell=False):
     """Runs the command and returns its output after it has exited.
 
-    ``cmd`` is a list of byte string arguments starting with the command names.
+    ``cmd`` is a list of arguments starting with the command names. The
+    arguments are bytes on Unix and strings on Windows.
     If ``shell`` is true, ``cmd`` is assumed to be a string and passed to a
     shell to execute.
 
@@ -751,6 +769,8 @@ def command_output(cmd, shell=False):
     This replaces `subprocess.check_output` which can have problems if lots of
     output is sent to stderr.
     """
+    cmd = convert_command_args(cmd)
+
     proc = subprocess.Popen(
         cmd,
         stdout=subprocess.PIPE,
@@ -762,7 +782,7 @@ def command_output(cmd, shell=False):
     if proc.returncode:
         raise subprocess.CalledProcessError(
             returncode=proc.returncode,
-            cmd=b' '.join(cmd),
+            cmd=' '.join(cmd),
             output=stdout + stderr,
         )
     return stdout
diff --git a/test/test_util.py b/test/test_util.py
index f4c2eca80..490992039 100644
--- a/test/test_util.py
+++ b/test/test_util.py
@@ -104,6 +104,15 @@ class UtilTest(unittest.TestCase):
             ])
         self.assertEqual(p, u'foo/_/bar')
 
+    @unittest.skipIf(six.PY2, 'surrogateescape error handler not available'
+                     'on Python 2')
+    def test_convert_command_args_keeps_undecodeable_bytes(self):
+        arg = b'\x82'  # non-ascii bytes
+        cmd_args = util.convert_command_args([arg])
+
+        self.assertEqual(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):
@@ -113,9 +122,9 @@ class UtilTest(unittest.TestCase):
 
         mock_popen.side_effect = popen_fail
         with self.assertRaises(subprocess.CalledProcessError) as exc_context:
-            util.command_output([b"taga", b"\xc3\xa9"])
+            util.command_output(['taga', '\xc3\xa9'])
         self.assertEqual(exc_context.exception.returncode, 1)
-        self.assertEqual(exc_context.exception.cmd, b"taga \xc3\xa9")
+        self.assertEqual(exc_context.exception.cmd, 'taga \xc3\xa9')
 
 
 class PathConversionTest(_common.TestCase):