From 5264b9093876a595d9c69892c5124f2fc55804e6 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Fri, 1 Mar 2019 10:35:15 +0000 Subject: Let subprocess decode stdout based on locale Subprocesses can return decoded strings if we give them a "universal_newlines=True" argument. We can therefore offload that to them, and not explicitly decode output ourselves. This also fixes multiple bugs where we would not be respecting the locale used by the user, and in some cases force it to "ascii". --- buildstream/_platform/linux.py | 3 +-- buildstream/utils.py | 6 +----- tests/sources/git.py | 20 ++++++++++++-------- tests/testutils/repo/bzr.py | 5 ++--- tests/testutils/repo/git.py | 14 ++++++++++---- tests/testutils/repo/ostree.py | 5 ++--- 6 files changed, 28 insertions(+), 25 deletions(-) diff --git a/buildstream/_platform/linux.py b/buildstream/_platform/linux.py index 702059a5d..85e810c26 100644 --- a/buildstream/_platform/linux.py +++ b/buildstream/_platform/linux.py @@ -143,8 +143,7 @@ class Linux(Platform): '--unshare-user', '--uid', '0', '--gid', '0', whoami, - ]) - output = output.decode('UTF-8').strip() + ], universal_newlines=True).strip() except subprocess.CalledProcessError: output = '' diff --git a/buildstream/utils.py b/buildstream/utils.py index a4e161ed4..2960348e9 100644 --- a/buildstream/utils.py +++ b/buildstream/utils.py @@ -1122,14 +1122,10 @@ def _call(*popenargs, terminate=False, **kwargs): with _signals.suspendable(suspend_proc, resume_proc), _signals.terminator(kill_proc): process = subprocess.Popen( # pylint: disable=subprocess-popen-preexec-fn - *popenargs, preexec_fn=preexec_fn, **kwargs) + *popenargs, preexec_fn=preexec_fn, universal_newlines=True, **kwargs) output, _ = process.communicate() exit_code = process.poll() - # Program output is returned as bytes, we want utf8 strings - if output is not None: - output = output.decode('UTF-8') - return (exit_code, output) diff --git a/tests/sources/git.py b/tests/sources/git.py index 40df30ac6..1f05055e4 100644 --- a/tests/sources/git.py +++ b/tests/sources/git.py @@ -869,15 +869,15 @@ def test_git_describe(cli, tmpdir, datafiles, ref_storage, tag_type): else: options = ['--tags'] describe = subprocess.check_output(['git', 'describe', *options], - cwd=checkout).decode('ascii') + cwd=checkout, universal_newlines=True) assert describe.startswith('tag2-2-') describe_fp = subprocess.check_output(['git', 'describe', '--first-parent', *options], - cwd=checkout).decode('ascii') + cwd=checkout, universal_newlines=True) assert describe_fp.startswith('tag1-2-') tags = subprocess.check_output(['git', 'tag'], - cwd=checkout).decode('ascii') + cwd=checkout, universal_newlines=True) tags = set(tags.splitlines()) assert tags == set(['tag1', 'tag2']) @@ -979,16 +979,18 @@ def test_git_describe_head_is_tagged(cli, tmpdir, datafiles, ref_storage, tag_ty else: options = ['--tags'] describe = subprocess.check_output(['git', 'describe', *options], - cwd=checkout).decode('ascii') + cwd=checkout, universal_newlines=True) assert describe.startswith('tag') tags = subprocess.check_output(['git', 'tag'], - cwd=checkout).decode('ascii') + cwd=checkout, + universal_newlines=True) tags = set(tags.splitlines()) assert tags == set(['tag']) rev_list = subprocess.check_output(['git', 'rev-list', '--all'], - cwd=checkout).decode('ascii') + cwd=checkout, + universal_newlines=True) assert set(rev_list.splitlines()) == set([tagged_ref]) @@ -1066,11 +1068,13 @@ def test_git_describe_relevant_history(cli, tmpdir, datafiles): result.assert_success() describe = subprocess.check_output(['git', 'describe'], - cwd=checkout).decode('ascii') + cwd=checkout, + universal_newlines=True) assert describe.startswith('tag1-2-') rev_list = subprocess.check_output(['git', 'rev-list', '--all'], - cwd=checkout).decode('ascii') + cwd=checkout, + universal_newlines=True) assert set(rev_list.splitlines()) == set([head, tagged_ref, branch_boundary]) diff --git a/tests/testutils/repo/bzr.py b/tests/testutils/repo/bzr.py index e159fa052..0bedb3aa7 100644 --- a/tests/testutils/repo/bzr.py +++ b/tests/testutils/repo/bzr.py @@ -43,9 +43,8 @@ class Bzr(Repo): return config def latest_commit(self): - output = subprocess.check_output([ + return subprocess.check_output([ self.bzr, 'version-info', '--custom', '--template={revno}', os.path.join(self.repo, 'trunk') - ], env=BZR_ENV) - return output.decode('UTF-8').strip() + ], env=BZR_ENV, universal_newlines=True).strip() diff --git a/tests/testutils/repo/git.py b/tests/testutils/repo/git.py index 9f617a763..12384e052 100644 --- a/tests/testutils/repo/git.py +++ b/tests/testutils/repo/git.py @@ -98,8 +98,11 @@ class Git(Repo): return config def latest_commit(self): - output = self._run_git('rev-parse', 'HEAD', stdout=subprocess.PIPE).stdout - return output.decode('UTF-8').strip() + return self._run_git( + 'rev-parse', 'HEAD', + stdout=subprocess.PIPE, + universal_newlines=True, + ).stdout.strip() def branch(self, branch_name): self._run_git('checkout', '-b', branch_name) @@ -115,5 +118,8 @@ class Git(Repo): return self.latest_commit() def rev_parse(self, rev): - output = self._run_git('rev-parse', rev, stdout=subprocess.PIPE).stdout - return output.decode('UTF-8').strip() + return self._run_git( + 'rev-parse', rev, + stdout=subprocess.PIPE, + universal_newlines=True, + ).stdout.strip() diff --git a/tests/testutils/repo/ostree.py b/tests/testutils/repo/ostree.py index 1cd7c8979..d4e50fea0 100644 --- a/tests/testutils/repo/ostree.py +++ b/tests/testutils/repo/ostree.py @@ -40,9 +40,8 @@ class OSTree(Repo): return config def latest_commit(self): - output = subprocess.check_output([ + return subprocess.check_output([ self.ostree, 'rev-parse', '--repo', self.repo, 'master' - ]) - return output.decode('UTF-8').strip() + ], universal_newlines=True).strip() -- cgit v1.2.1