Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util.command_output: return stderr, too #3329

Merged
merged 2 commits into from
Jul 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions beets/util/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -794,9 +798,16 @@ 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. 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.
If ``shell`` is true, ``cmd`` is assumed to be a string and passed to a
Expand Down Expand Up @@ -831,7 +842,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):
Expand Down
4 changes: 2 additions & 2 deletions beets/util/artresizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion beetsplug/absubmit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion beetsplug/duplicates.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}',
Expand Down
4 changes: 2 additions & 2 deletions beetsplug/ipfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion beetsplug/keyfinder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions beetsplug/replaygain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand Down
3 changes: 3 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
8 changes: 4 additions & 4 deletions test/test_keyfinder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()

Expand All @@ -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()
Expand All @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions test/test_replaygain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand All @@ -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="""
<album>
<track total="1" number="1" file="&">
<integrated lufs="0" lu="0" />
<sample-peak spfs="0" factor="0" />
</track>
</album>
"""
""", stderr="")

with capture_log('beets.replaygain') as logs:
self.run_command('replaygain')
Expand Down