From 30395911e24e653146d731a045a7586d1ffc45bb Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Thu, 18 Oct 2018 20:34:58 +0200 Subject: [PATCH 1/2] util.command_output: return stderr, too Return a namedtuple CommandOutput(stdout, stderr) instead of just stdout from util.command_ouput, allowing separate access to stdout and stderr. This change is required by the ffmpeg replaygain backend (GitHub PullRequest #3056) as ffmpeg's ebur128 filter outputs only to stderr. --- beets/util/__init__.py | 16 +++++++++++++--- beets/util/artresizer.py | 4 ++-- beetsplug/absubmit.py | 2 +- beetsplug/duplicates.py | 2 +- beetsplug/ipfs.py | 4 ++-- beetsplug/keyfinder.py | 2 +- beetsplug/replaygain.py | 8 ++++---- test/test_keyfinder.py | 8 ++++---- test/test_replaygain.py | 6 +++--- 9 files changed, 31 insertions(+), 21 deletions(-) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 162502eb16..b23832c6d4 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -24,7 +24,7 @@ import shutil import fnmatch import functools -from collections import Counter +from collections import Counter, namedtuple from multiprocessing.pool import ThreadPool import traceback import subprocess @@ -763,7 +763,11 @@ def cpu_count(): num = 0 elif sys.platform == 'darwin': try: - num = int(command_output(['/usr/sbin/sysctl', '-n', 'hw.ncpu'])) + num = int(command_output([ + '/usr/sbin/sysctl', + '-n', + 'hw.ncpu', + ]).stdout) except (ValueError, OSError, subprocess.CalledProcessError): num = 0 else: @@ -794,9 +798,15 @@ def convert(arg): return [convert(a) for a in args] +# stdout and stderr as bytes +CommandOutput = namedtuple("CommandOutput", ("stdout", "stderr")) + + def command_output(cmd, shell=False): """Runs the command and returns its output after it has exited. + Returns a CommandOutput. + ``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 @@ -831,7 +841,7 @@ def command_output(cmd, shell=False): cmd=' '.join(cmd), output=stdout + stderr, ) - return stdout + return CommandOutput(stdout, stderr) def max_filename_length(path, limit=MAX_FILENAME_LENGTH): diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py index 1ee3e560d0..99e28c0cc3 100644 --- a/beets/util/artresizer.py +++ b/beets/util/artresizer.py @@ -129,7 +129,7 @@ def im_getsize(path_in): ['-format', '%w %h', util.syspath(path_in, prefix=False)] try: - out = util.command_output(cmd) + out = util.command_output(cmd).stdout except subprocess.CalledProcessError as exc: log.warning(u'ImageMagick size query failed') log.debug( @@ -265,7 +265,7 @@ def get_im_version(): cmd = cmd_name + ['--version'] try: - out = util.command_output(cmd) + out = util.command_output(cmd).stdout except (subprocess.CalledProcessError, OSError) as exc: log.debug(u'ImageMagick version check failed: {}', exc) else: diff --git a/beetsplug/absubmit.py b/beetsplug/absubmit.py index 69c4d2a984..7419736a32 100644 --- a/beetsplug/absubmit.py +++ b/beetsplug/absubmit.py @@ -46,7 +46,7 @@ def call(args): Raise a AnalysisABSubmitError on failure. """ try: - return util.command_output(args) + return util.command_output(args).stdout except subprocess.CalledProcessError as e: raise ABSubmitError( u'{0} exited with status {1}'.format(args[0], e.returncode) diff --git a/beetsplug/duplicates.py b/beetsplug/duplicates.py index b316cfda63..4e6e540ea2 100644 --- a/beetsplug/duplicates.py +++ b/beetsplug/duplicates.py @@ -205,7 +205,7 @@ def _checksum(self, item, prog): u'computing checksum', key, displayable_path(item.path)) try: - checksum = command_output(args) + checksum = command_output(args).stdout setattr(item, key, checksum) item.store() self._log.debug(u'computed checksum for {0} using {1}', diff --git a/beetsplug/ipfs.py b/beetsplug/ipfs.py index 90ba5fdd08..f2408c2593 100644 --- a/beetsplug/ipfs.py +++ b/beetsplug/ipfs.py @@ -123,7 +123,7 @@ def ipfs_add(self, album): cmd = "ipfs add -q -r".split() cmd.append(album_dir) try: - output = util.command_output(cmd).split() + output = util.command_output(cmd).stdout.split() except (OSError, subprocess.CalledProcessError) as exc: self._log.error(u'Failed to add {0}, error: {1}', album_dir, exc) return False @@ -183,7 +183,7 @@ def ipfs_publish(self, lib): else: cmd = "ipfs add -q ".split() cmd.append(tmp.name) - output = util.command_output(cmd) + output = util.command_output(cmd).stdout except (OSError, subprocess.CalledProcessError) as err: msg = "Failed to publish library. Error: {0}".format(err) self._log.error(msg) diff --git a/beetsplug/keyfinder.py b/beetsplug/keyfinder.py index 3a738478e6..ea928ef439 100644 --- a/beetsplug/keyfinder.py +++ b/beetsplug/keyfinder.py @@ -60,7 +60,7 @@ def find_key(self, items, write=False): try: output = util.command_output([bin, '-f', - util.syspath(item.path)]) + util.syspath(item.path)]).stdout except (subprocess.CalledProcessError, OSError) as exc: self._log.error(u'execution failed: {0}', exc) continue diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 90d7ee236e..58a4df83cf 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -47,12 +47,12 @@ class FatalGstreamerPluginReplayGainError(FatalReplayGainError): loading the required plugins.""" -def call(args): +def call(args, **kwargs): """Execute the command and return its output or raise a ReplayGainError on failure. """ try: - return command_output(args) + return command_output(args, **kwargs) except subprocess.CalledProcessError as e: raise ReplayGainError( u"{0} exited with status {1}".format(args[0], e.returncode) @@ -206,7 +206,7 @@ def compute_chunk_gain(self, items, is_album): self._log.debug( u'executing {0}', u' '.join(map(displayable_path, args)) ) - output = call(args) + output = call(args).stdout self._log.debug(u'analysis finished: {0}', output) results = self.parse_tool_output(output, path_list, is_album) @@ -378,7 +378,7 @@ def compute_gain(self, items, is_album): self._log.debug(u'analyzing {0} files', len(items)) self._log.debug(u"executing {0}", " ".join(map(displayable_path, cmd))) - output = call(cmd) + output = call(cmd).stdout self._log.debug(u'analysis finished') return self.parse_tool_output(output, len(items) + (1 if is_album else 0)) diff --git a/test/test_keyfinder.py b/test/test_keyfinder.py index c2b4227d7d..a9ac43a274 100644 --- a/test/test_keyfinder.py +++ b/test/test_keyfinder.py @@ -38,7 +38,7 @@ def test_add_key(self, command_output): item = Item(path='/file') item.add(self.lib) - command_output.return_value = 'dbm' + command_output.return_value = util.CommandOutput(b"dbm", b"") self.run_command('keyfinder') item.load() @@ -47,7 +47,7 @@ def test_add_key(self, command_output): ['KeyFinder', '-f', util.syspath(item.path)]) def test_add_key_on_import(self, command_output): - command_output.return_value = 'dbm' + command_output.return_value = util.CommandOutput(b"dbm", b"") importer = self.create_importer() importer.run() @@ -60,7 +60,7 @@ def test_force_overwrite(self, command_output): item = Item(path='/file', initial_key='F') item.add(self.lib) - command_output.return_value = 'C#m' + command_output.return_value = util.CommandOutput(b"C#m", b"") self.run_command('keyfinder') item.load() @@ -70,7 +70,7 @@ def test_do_not_overwrite(self, command_output): item = Item(path='/file', initial_key='F') item.add(self.lib) - command_output.return_value = 'dbm' + command_output.return_value = util.CommandOutput(b"dbm", b"") self.run_command('keyfinder') item.load() diff --git a/test/test_replaygain.py b/test/test_replaygain.py index 9f14374cc9..b482da14e4 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -23,6 +23,7 @@ from test.helper import TestHelper, capture_log, has_program from beets import config +from beets.util import CommandOutput from mediafile import MediaFile from beetsplug.replaygain import (FatalGstreamerPluginReplayGainError, GStreamerBackend) @@ -169,7 +170,6 @@ class ReplayGainLdnsCliTest(ReplayGainCliTestBase, unittest.TestCase): class ReplayGainLdnsCliMalformedTest(TestHelper, unittest.TestCase): - @patch('beetsplug.replaygain.call') def setUp(self, call_patch): self.setup_beets() @@ -186,14 +186,14 @@ def setUp(self, call_patch): @patch('beetsplug.replaygain.call') def test_malformed_output(self, call_patch): # Return malformed XML (the ampersand should be &) - call_patch.return_value = """ + call_patch.return_value = CommandOutput(stdout=""" - """ + """, stderr="") with capture_log('beets.replaygain') as logs: self.run_command('replaygain') From 45aa75a4ef5426aa4961ee0d6a076c42d6ab0cf1 Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Mon, 15 Jul 2019 11:39:30 +0200 Subject: [PATCH 2/2] document util.command_output return value change After commit `30395911 util.command_output: return stderr, too`, `command_output` returns a tuple of stdout and stderr. Document that change by adding a changelog entry and add a usage note to `command_output`'s docstring. --- beets/util/__init__.py | 3 ++- docs/changelog.rst | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index b23832c6d4..29b2a73e7e 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -805,7 +805,8 @@ def convert(arg): def command_output(cmd, shell=False): """Runs the command and returns its output after it has exited. - Returns a CommandOutput. + Returns a CommandOutput. The attributes ``stdout`` and ``stderr`` contain + byte strings of the respective output streams. ``cmd`` is a list of arguments starting with the command names. The arguments are bytes on Unix and strings on Windows. diff --git a/docs/changelog.rst b/docs/changelog.rst index c9fbc5fecc..e0e19101b7 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -74,6 +74,9 @@ For plugin developers: is almost identical apart from the name change. Again, we'll re-export at the old location (with a deprecation warning) for backwards compatibility, but might stop doing this in a future release. +* ``beets.util.command_output`` now returns a named tuple of stdout and stderr + instead of only returning stdout. stdout can be accessed as a simple + attribute. For packagers: