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.
This commit is contained in:
Zsin Skri 2018-10-18 20:34:58 +02:00
parent f39cff71d3
commit 30395911e2
9 changed files with 31 additions and 21 deletions

View file

@ -24,7 +24,7 @@ import re
import shutil import shutil
import fnmatch import fnmatch
import functools import functools
from collections import Counter from collections import Counter, namedtuple
from multiprocessing.pool import ThreadPool from multiprocessing.pool import ThreadPool
import traceback import traceback
import subprocess import subprocess
@ -763,7 +763,11 @@ def cpu_count():
num = 0 num = 0
elif sys.platform == 'darwin': elif sys.platform == 'darwin':
try: 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): except (ValueError, OSError, subprocess.CalledProcessError):
num = 0 num = 0
else: else:
@ -794,9 +798,15 @@ def convert_command_args(args):
return [convert(a) for a in args] return [convert(a) for a in args]
# stdout and stderr as bytes
CommandOutput = namedtuple("CommandOutput", ("stdout", "stderr"))
def command_output(cmd, shell=False): def command_output(cmd, shell=False):
"""Runs the command and returns its output after it has exited. """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 ``cmd`` is a list of arguments starting with the command names. The
arguments are bytes on Unix and strings on Windows. arguments are bytes on Unix and strings on Windows.
If ``shell`` is true, ``cmd`` is assumed to be a string and passed to a 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), cmd=' '.join(cmd),
output=stdout + stderr, output=stdout + stderr,
) )
return stdout return CommandOutput(stdout, stderr)
def max_filename_length(path, limit=MAX_FILENAME_LENGTH): def max_filename_length(path, limit=MAX_FILENAME_LENGTH):

View file

@ -129,7 +129,7 @@ def im_getsize(path_in):
['-format', '%w %h', util.syspath(path_in, prefix=False)] ['-format', '%w %h', util.syspath(path_in, prefix=False)]
try: try:
out = util.command_output(cmd) out = util.command_output(cmd).stdout
except subprocess.CalledProcessError as exc: except subprocess.CalledProcessError as exc:
log.warning(u'ImageMagick size query failed') log.warning(u'ImageMagick size query failed')
log.debug( log.debug(
@ -265,7 +265,7 @@ def get_im_version():
cmd = cmd_name + ['--version'] cmd = cmd_name + ['--version']
try: try:
out = util.command_output(cmd) out = util.command_output(cmd).stdout
except (subprocess.CalledProcessError, OSError) as exc: except (subprocess.CalledProcessError, OSError) as exc:
log.debug(u'ImageMagick version check failed: {}', exc) log.debug(u'ImageMagick version check failed: {}', exc)
else: else:

View file

@ -46,7 +46,7 @@ def call(args):
Raise a AnalysisABSubmitError on failure. Raise a AnalysisABSubmitError on failure.
""" """
try: try:
return util.command_output(args) return util.command_output(args).stdout
except subprocess.CalledProcessError as e: except subprocess.CalledProcessError as e:
raise ABSubmitError( raise ABSubmitError(
u'{0} exited with status {1}'.format(args[0], e.returncode) u'{0} exited with status {1}'.format(args[0], e.returncode)

View file

@ -205,7 +205,7 @@ class DuplicatesPlugin(BeetsPlugin):
u'computing checksum', u'computing checksum',
key, displayable_path(item.path)) key, displayable_path(item.path))
try: try:
checksum = command_output(args) checksum = command_output(args).stdout
setattr(item, key, checksum) setattr(item, key, checksum)
item.store() item.store()
self._log.debug(u'computed checksum for {0} using {1}', self._log.debug(u'computed checksum for {0} using {1}',

View file

@ -123,7 +123,7 @@ class IPFSPlugin(BeetsPlugin):
cmd = "ipfs add -q -r".split() cmd = "ipfs add -q -r".split()
cmd.append(album_dir) cmd.append(album_dir)
try: try:
output = util.command_output(cmd).split() output = util.command_output(cmd).stdout.split()
except (OSError, subprocess.CalledProcessError) as exc: except (OSError, subprocess.CalledProcessError) as exc:
self._log.error(u'Failed to add {0}, error: {1}', album_dir, exc) self._log.error(u'Failed to add {0}, error: {1}', album_dir, exc)
return False return False
@ -183,7 +183,7 @@ class IPFSPlugin(BeetsPlugin):
else: else:
cmd = "ipfs add -q ".split() cmd = "ipfs add -q ".split()
cmd.append(tmp.name) cmd.append(tmp.name)
output = util.command_output(cmd) output = util.command_output(cmd).stdout
except (OSError, subprocess.CalledProcessError) as err: except (OSError, subprocess.CalledProcessError) as err:
msg = "Failed to publish library. Error: {0}".format(err) msg = "Failed to publish library. Error: {0}".format(err)
self._log.error(msg) self._log.error(msg)

View file

@ -60,7 +60,7 @@ class KeyFinderPlugin(BeetsPlugin):
try: try:
output = util.command_output([bin, '-f', output = util.command_output([bin, '-f',
util.syspath(item.path)]) util.syspath(item.path)]).stdout
except (subprocess.CalledProcessError, OSError) as exc: except (subprocess.CalledProcessError, OSError) as exc:
self._log.error(u'execution failed: {0}', exc) self._log.error(u'execution failed: {0}', exc)
continue continue

View file

@ -47,12 +47,12 @@ class FatalGstreamerPluginReplayGainError(FatalReplayGainError):
loading the required plugins.""" loading the required plugins."""
def call(args): def call(args, **kwargs):
"""Execute the command and return its output or raise a """Execute the command and return its output or raise a
ReplayGainError on failure. ReplayGainError on failure.
""" """
try: try:
return command_output(args) return command_output(args, **kwargs)
except subprocess.CalledProcessError as e: except subprocess.CalledProcessError as e:
raise ReplayGainError( raise ReplayGainError(
u"{0} exited with status {1}".format(args[0], e.returncode) u"{0} exited with status {1}".format(args[0], e.returncode)
@ -206,7 +206,7 @@ class Bs1770gainBackend(Backend):
self._log.debug( self._log.debug(
u'executing {0}', u' '.join(map(displayable_path, args)) u'executing {0}', u' '.join(map(displayable_path, args))
) )
output = call(args) output = call(args).stdout
self._log.debug(u'analysis finished: {0}', output) self._log.debug(u'analysis finished: {0}', output)
results = self.parse_tool_output(output, path_list, is_album) results = self.parse_tool_output(output, path_list, is_album)
@ -378,7 +378,7 @@ class CommandBackend(Backend):
self._log.debug(u'analyzing {0} files', len(items)) self._log.debug(u'analyzing {0} files', len(items))
self._log.debug(u"executing {0}", " ".join(map(displayable_path, cmd))) self._log.debug(u"executing {0}", " ".join(map(displayable_path, cmd)))
output = call(cmd) output = call(cmd).stdout
self._log.debug(u'analysis finished') self._log.debug(u'analysis finished')
return self.parse_tool_output(output, return self.parse_tool_output(output,
len(items) + (1 if is_album else 0)) len(items) + (1 if is_album else 0))

View file

@ -38,7 +38,7 @@ class KeyFinderTest(unittest.TestCase, TestHelper):
item = Item(path='/file') item = Item(path='/file')
item.add(self.lib) item.add(self.lib)
command_output.return_value = 'dbm' command_output.return_value = util.CommandOutput(b"dbm", b"")
self.run_command('keyfinder') self.run_command('keyfinder')
item.load() item.load()
@ -47,7 +47,7 @@ class KeyFinderTest(unittest.TestCase, TestHelper):
['KeyFinder', '-f', util.syspath(item.path)]) ['KeyFinder', '-f', util.syspath(item.path)])
def test_add_key_on_import(self, command_output): 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 = self.create_importer()
importer.run() importer.run()
@ -60,7 +60,7 @@ class KeyFinderTest(unittest.TestCase, TestHelper):
item = Item(path='/file', initial_key='F') item = Item(path='/file', initial_key='F')
item.add(self.lib) item.add(self.lib)
command_output.return_value = 'C#m' command_output.return_value = util.CommandOutput(b"C#m", b"")
self.run_command('keyfinder') self.run_command('keyfinder')
item.load() item.load()
@ -70,7 +70,7 @@ class KeyFinderTest(unittest.TestCase, TestHelper):
item = Item(path='/file', initial_key='F') item = Item(path='/file', initial_key='F')
item.add(self.lib) item.add(self.lib)
command_output.return_value = 'dbm' command_output.return_value = util.CommandOutput(b"dbm", b"")
self.run_command('keyfinder') self.run_command('keyfinder')
item.load() item.load()

View file

@ -23,6 +23,7 @@ from mock import patch
from test.helper import TestHelper, capture_log, has_program from test.helper import TestHelper, capture_log, has_program
from beets import config from beets import config
from beets.util import CommandOutput
from mediafile import MediaFile from mediafile import MediaFile
from beetsplug.replaygain import (FatalGstreamerPluginReplayGainError, from beetsplug.replaygain import (FatalGstreamerPluginReplayGainError,
GStreamerBackend) GStreamerBackend)
@ -169,7 +170,6 @@ class ReplayGainLdnsCliTest(ReplayGainCliTestBase, unittest.TestCase):
class ReplayGainLdnsCliMalformedTest(TestHelper, unittest.TestCase): class ReplayGainLdnsCliMalformedTest(TestHelper, unittest.TestCase):
@patch('beetsplug.replaygain.call') @patch('beetsplug.replaygain.call')
def setUp(self, call_patch): def setUp(self, call_patch):
self.setup_beets() self.setup_beets()
@ -186,14 +186,14 @@ class ReplayGainLdnsCliMalformedTest(TestHelper, unittest.TestCase):
@patch('beetsplug.replaygain.call') @patch('beetsplug.replaygain.call')
def test_malformed_output(self, call_patch): def test_malformed_output(self, call_patch):
# Return malformed XML (the ampersand should be &) # Return malformed XML (the ampersand should be &)
call_patch.return_value = """ call_patch.return_value = CommandOutput(stdout="""
<album> <album>
<track total="1" number="1" file="&"> <track total="1" number="1" file="&">
<integrated lufs="0" lu="0" /> <integrated lufs="0" lu="0" />
<sample-peak spfs="0" factor="0" /> <sample-peak spfs="0" factor="0" />
</track> </track>
</album> </album>
""" """, stderr="")
with capture_log('beets.replaygain') as logs: with capture_log('beets.replaygain') as logs:
self.run_command('replaygain') self.run_command('replaygain')