diff options
author | Monty Taylor <mordred@inaugust.com> | 2013-08-15 19:22:33 -0300 |
---|---|---|
committer | Monty Taylor <mordred@inaugust.com> | 2013-09-28 14:15:59 -0400 |
commit | d322c34fec2a7096203eba762d93b478a89cebe6 (patch) | |
tree | 4e3983b5195ed2795d4ae62ee0d8ea2b0444554a | |
parent | 9f04dba0ecc07a33dc4b256bec4f860c30b2b500 (diff) | |
download | pbr-d322c34fec2a7096203eba762d93b478a89cebe6.tar.gz |
Rework run_shell_command
A previous patch for windows support made the simple case of passing a
string to run_shell_command complicated with quoting. Clean that up by
reverting to list, which avoids the crazy quoting we had to do.
Change-Id: Ia3b8ac6a57e7400d0aab0d5265c851e536fb4e87
-rw-r--r-- | pbr/packaging.py | 82 | ||||
-rw-r--r-- | pbr/tests/test_setup.py | 31 |
2 files changed, 48 insertions, 65 deletions
diff --git a/pbr/packaging.py b/pbr/packaging.py index fea9d10..8233dec 100644 --- a/pbr/packaging.py +++ b/pbr/packaging.py @@ -23,7 +23,6 @@ Utilities with minimum-depends for use in setup.py import email import os import re -import shlex import subprocess import sys @@ -82,33 +81,25 @@ def _parse_mailmap(mailmap_info): return mapping -def _wrap_in_quotes(values): - return ["'%s'" % value for value in values] - - -def _make_links_args(links): - return ["-f '%s'" % link for link in links] - - def _pip_install(links, requires, root=None, option_dict=dict()): if get_boolean_option( option_dict, 'skip_pip_install', 'SKIP_PIP_INSTALL'): return - root_cmd = "" + cmd = [sys.executable, '-m', 'pip.__init__', 'install'] if root: - root_cmd = "--root=\"%s\"" % root + cmd.append("--root=%s" % root) + for link in links: + cmd.append("-f") + cmd.append(link) _run_shell_command( - "\"%s\" -m pip.__init__ install %s %s %s" % ( - sys.executable, - root_cmd, - " ".join(links), - " ".join(_wrap_in_quotes(requires))), + cmd + requires, throw_on_error=True, buffer=False) def read_git_mailmap(root_dir=None, mailmap='.mailmap'): if not root_dir: - root_dir = _run_shell_command('git rev-parse --show-toplevel') + root_dir = _run_shell_command( + ['git', 'rev-parse', '--show-toplevel']) mailmap = os.path.join(root_dir, mailmap) if os.path.exists(mailmap): @@ -203,6 +194,13 @@ def parse_dependency_links(requirements_files=None): return dependency_links +def _run_git_command(cmd, git_dir, **kwargs): + if not isinstance(cmd, (list, tuple)): + cmd = [cmd] + return _run_shell_command( + ['git', '--git-dir=%s' % git_dir] + cmd, **kwargs) + + def _run_shell_command(cmd, throw_on_error=False, buffer=True): if buffer: out_location = subprocess.PIPE @@ -211,11 +209,7 @@ def _run_shell_command(cmd, throw_on_error=False, buffer=True): out_location = None err_location = None - # shlex has issues with unicode on older versions - if sys.version_info < (2, 7): - cmd = cmd.encode('ascii') - - output = subprocess.Popen(shlex.split(cmd), + output = subprocess.Popen(cmd, stdout=out_location, stderr=err_location) out = output.communicate() @@ -228,7 +222,7 @@ def _run_shell_command(cmd, throw_on_error=False, buffer=True): def _get_git_directory(): - return _run_shell_command("git rev-parse --git-dir", None) + return _run_shell_command(['git', 'rev-parse', '--git-dir']) def get_boolean_option(option_dict, option_name, env_name): @@ -252,8 +246,7 @@ def write_git_changelog(git_dir=None, dest_dir=os.path.curdir, if git_dir is None: git_dir = _get_git_directory() if git_dir: - git_log_cmd = 'git --git-dir=\"%s\" log' % git_dir - changelog = _run_shell_command(git_log_cmd) + changelog = _run_git_command('log', git_dir) mailmap = read_git_mailmap() with open(new_changelog, "wb") as changelog_file: changelog_file.write(canonicalize_emails( @@ -279,14 +272,12 @@ def generate_authors(git_dir=None, dest_dir='.', option_dict=dict()): authors = [] # don't include jenkins email address in AUTHORS file - git_log_cmd = ("git --git-dir=\"" + git_dir + - "\" log --format='%aN <%aE>'") - authors += _run_shell_command(git_log_cmd).split('\n') + git_log_cmd = ['log', '--format=%aN <%aE>'] + authors += _run_git_command(git_log_cmd, git_dir).split('\n') authors = [a for a in authors if not re.search(ignore_emails, a)] # get all co-authors from commit messages - co_authors_cmd = ("git log --git-dir=\"" + git_dir + "\"") - co_authors_out = _run_shell_command(co_authors_cmd) + co_authors_out = _run_git_command('log', git_dir) co_authors = re.findall('Co-authored-by:.+', co_authors_out, re.MULTILINE) co_authors = [signed.split(":", 1)[1].strip() @@ -319,8 +310,7 @@ def _find_git_files(dirname='', git_dir=None): git_dir = _get_git_directory() if git_dir: log.info("[pbr] In git context, generating filelist from git") - git_ls_cmd = "git --git-dir=\"%s\" ls-files -z" % git_dir - file_list = _run_shell_command(git_ls_cmd) + file_list = _run_git_command(['ls-files', '-z'], git_dir) file_list = file_list.split(b'\x00'.decode('utf-8')) return [f for f in file_list if f] @@ -364,9 +354,9 @@ class LocalInstall(install.install): option_dict = self.distribution.get_option_dict('pbr') if (not self.single_version_externally_managed and self.distribution.install_requires): - links = _make_links_args(self.distribution.dependency_links) _pip_install( - links, self.distribution.install_requires, self.root, + self.distribution.dependency_links, + self.distribution.install_requires, self.root, option_dict=option_dict) return du_install.install.run(self) @@ -397,8 +387,7 @@ class _PipInstallTestRequires(object): def install_test_requirements(self): - links = _make_links_args( - parse_dependency_links(TEST_REQUIREMENTS_FILES)) + links = parse_dependency_links(TEST_REQUIREMENTS_FILES) if self.distribution.tests_require: option_dict = self.distribution.get_option_dict('pbr') _pip_install( @@ -742,14 +731,13 @@ def _get_revno(git_dir): tags then we fall back to counting commits since the beginning of time. """ - describe = _run_shell_command( - "git --git-dir=\"%s\" describe --always" % git_dir) + describe = _run_git_command(['describe', '--always'], git_dir) if "-" in describe: return describe.rsplit("-", 2)[-2] # no tags found - revlist = _run_shell_command( - "git --git-dir=\"%s\" rev-list --abbrev-commit HEAD" % git_dir) + revlist = _run_git_command( + ['rev-list', '--abbrev-commit', 'HEAD'], git_dir) return len(revlist.splitlines()) @@ -763,18 +751,16 @@ def _get_version_from_git(pre_version): if git_dir: if pre_version: try: - return _run_shell_command( - "git --git-dir=\"" + git_dir + "\" describe --exact-match", + return _run_git_command( + ['describe', '--exact-match'], git_dir, throw_on_error=True).replace('-', '.') except Exception: - sha = _run_shell_command( - "git --git-dir=\"" + git_dir + - "\" log -n1 --pretty=format:%h") + sha = _run_git_command( + ['log', '-n1', '--pretty=format:%h'], git_dir) return "%s.a%s.g%s" % (pre_version, _get_revno(git_dir), sha) else: - return _run_shell_command( - "git --git-dir=\"" + git_dir + "\" describe --always").replace( - '-', '.') + return _run_git_command( + ['describe', '--always'], git_dir).replace('-', '.') return None diff --git a/pbr/tests/test_setup.py b/pbr/tests/test_setup.py index d54f4fd..189bd51 100644 --- a/pbr/tests/test_setup.py +++ b/pbr/tests/test_setup.py @@ -170,20 +170,15 @@ class GitLogsTest(tests.BaseTestCase): with open(os.path.join(self.temp_path, "ChangeLog"), "r") as ch_fh: self.assertTrue("email@foo.com" in ch_fh.read()) - def _fake_log_output(self, cmd, mapping): - for (k, v) in mapping.items(): - if cmd.startswith(k): - return v.encode('utf-8') - return b"" - def test_generate_authors(self): - author_old = "Foo Foo <email@foo.com>" - author_new = "Bar Bar <email@bar.com>" - co_author = "Foo Bar <foo@bar.com>" - co_author_by = "Co-authored-by: " + co_author - - git_log_cmd = ("git --git-dir=%s log --format" % self.git_dir) - git_co_log_cmd = ("git log --git-dir=%s" % self.git_dir) + author_old = u"Foo Foo <email@foo.com>" + author_new = u"Bar Bar <email@bar.com>" + co_author = u"Foo Bar <foo@bar.com>" + co_author_by = u"Co-authored-by: " + co_author + + git_log_cmd = ( + "git --git-dir=%s log --format=%%aN <%%aE>" % self.git_dir) + git_co_log_cmd = ("git --git-dir=%s log" % self.git_dir) git_top_level = "git rev-parse --show-toplevel" cmd_map = { git_log_cmd: author_new, @@ -197,10 +192,12 @@ class GitLogsTest(tests.BaseTestCase): "os.path.exists", lambda path: os.path.abspath(path) in exist_files)) - self.useFixture(fixtures.FakePopen(lambda proc_args: { - "stdout": BytesIO( - self._fake_log_output(' '.join(proc_args["args"]), cmd_map)) - })) + def _fake_run_shell_command(cmd, **kwargs): + return cmd_map[" ".join(cmd)] + + self.useFixture(fixtures.MonkeyPatch( + "pbr.packaging._run_shell_command", + _fake_run_shell_command)) with open(os.path.join(self.temp_path, "AUTHORS.in"), "w") as auth_fh: auth_fh.write("%s\n" % author_old) |