Merge pull request #1160 from pre-commit/cmd_output_b

Split out cmd_output_b

pre_commit/commands/autoupdate.py

@@ -21,6 +21,7 @@ from pre_commit.clientlib import META
 from pre_commit.commands.migrate_config import migrate_config
 from pre_commit.util import CalledProcessError
 from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 from pre_commit.util import tmpdir
 
 
@@ -38,7 +39,7 @@ def _update_repo(repo_config, store, tags_only):
     """
     with tmpdir() as repo_path:
         git.init_repo(repo_path, repo_config['repo'])
-        cmd_output('git', 'fetch', 'origin', 'HEAD', '--tags', cwd=repo_path)
+        cmd_output_b('git', 'fetch', 'origin', 'HEAD', '--tags', cwd=repo_path)
 
         tag_cmd = ('git', 'describe', 'FETCH_HEAD', '--tags')
         if tags_only:

pre_commit/commands/install_uninstall.py

@@ -13,7 +13,6 @@ from pre_commit import output
 from pre_commit.clientlib import load_config
 from pre_commit.repository import all_hooks
 from pre_commit.repository import install_hook_envs
-from pre_commit.util import cmd_output
 from pre_commit.util import make_executable
 from pre_commit.util import mkdirp
 from pre_commit.util import resource_text
@@ -117,7 +116,7 @@ def install(
         overwrite=False, hooks=False,
         skip_on_missing_config=False, git_dir=None,
 ):
-    if cmd_output('git', 'config', 'core.hooksPath', retcode=None)[1].strip():
+    if git.has_core_hookpaths_set():
         logger.error(
             'Cowardly refusing to install hooks with `core.hooksPath` set.\n'
             'hint: `git config --unset-all core.hooksPath`',

pre_commit/commands/run.py

@@ -16,7 +16,7 @@ from pre_commit.output import get_hook_message
 from pre_commit.repository import all_hooks
 from pre_commit.repository import install_hook_envs
 from pre_commit.staged_files_only import staged_files_only
-from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 from pre_commit.util import noop_context
 
 
@@ -117,15 +117,11 @@ def _run_single_hook(classifier, hook, args, skips, cols):
     )
     sys.stdout.flush()
 
-    diff_before = cmd_output(
+    diff_before = cmd_output_b('git', 'diff', '--no-ext-diff', retcode=None)
-        'git', 'diff', '--no-ext-diff', retcode=None, encoding=None,
-    )
     retcode, stdout, stderr = hook.run(
         tuple(filenames) if hook.pass_filenames else (),
     )
-    diff_after = cmd_output(
+    diff_after = cmd_output_b('git', 'diff', '--no-ext-diff', retcode=None)
-        'git', 'diff', '--no-ext-diff', retcode=None, encoding=None,
-    )
 
     file_modifications = diff_before != diff_after
 
@@ -235,12 +231,12 @@ def _run_hooks(config, hooks, args, environ):
 
 
 def _has_unmerged_paths():
-    _, stdout, _ = cmd_output('git', 'ls-files', '--unmerged')
+    _, stdout, _ = cmd_output_b('git', 'ls-files', '--unmerged')
     return bool(stdout.strip())
 
 
 def _has_unstaged_config(config_file):
-    retcode, _, _ = cmd_output(
+    retcode, _, _ = cmd_output_b(
         'git', 'diff', '--no-ext-diff', '--exit-code', config_file,
         retcode=None,
     )

pre_commit/commands/try_repo.py

@@ -13,7 +13,7 @@ from pre_commit import output
 from pre_commit.clientlib import load_manifest
 from pre_commit.commands.run import run
 from pre_commit.store import Store
-from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 from pre_commit.util import tmpdir
 from pre_commit.xargs import xargs
 
@@ -31,8 +31,8 @@ def _repo_ref(tmpdir, repo, ref):
         logger.warning('Creating temporary repo with uncommitted changes...')
 
         shadow = os.path.join(tmpdir, 'shadow-repo')
-        cmd_output('git', 'clone', repo, shadow)
+        cmd_output_b('git', 'clone', repo, shadow)
-        cmd_output('git', 'checkout', ref, '-b', '_pc_tmp', cwd=shadow)
+        cmd_output_b('git', 'checkout', ref, '-b', '_pc_tmp', cwd=shadow)
 
         idx = git.git_path('index', repo=shadow)
         objs = git.git_path('objects', repo=shadow)
@@ -42,7 +42,7 @@ def _repo_ref(tmpdir, repo, ref):
         if staged_files:
             xargs(('git', 'add', '--'), staged_files, cwd=repo, env=env)
 
-        cmd_output('git', 'add', '-u', cwd=repo, env=env)
+        cmd_output_b('git', 'add', '-u', cwd=repo, env=env)
         git.commit(repo=shadow)
 
         return shadow, git.head_rev(shadow)

pre_commit/git.py

@@ -5,6 +5,7 @@ import os.path
 import sys
 
 from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 
 
 logger = logging.getLogger(__name__)
@@ -50,8 +51,8 @@ def get_git_dir(git_root='.'):
 
 
 def get_remote_url(git_root):
-    ret = cmd_output('git', 'config', 'remote.origin.url', cwd=git_root)[1]
+    _, out, _ = cmd_output('git', 'config', 'remote.origin.url', cwd=git_root)
-    return ret.strip()
+    return out.strip()
 
 
 def is_in_merge_conflict():
@@ -105,8 +106,8 @@ def get_staged_files(cwd=None):
 
 
 def intent_to_add_files():
-    _, stdout_binary, _ = cmd_output('git', 'status', '--porcelain', '-z')
+    _, stdout, _ = cmd_output('git', 'status', '--porcelain', '-z')
-    parts = list(reversed(zsplit(stdout_binary)))
+    parts = list(reversed(zsplit(stdout)))
     intent_to_add = []
     while parts:
         line = parts.pop()
@@ -140,7 +141,12 @@ def has_diff(*args, **kwargs):
     repo = kwargs.pop('repo', '.')
     assert not kwargs, kwargs
     cmd = ('git', 'diff', '--quiet', '--no-ext-diff') + args
-    return cmd_output(*cmd, cwd=repo, retcode=None)[0]
+    return cmd_output_b(*cmd, cwd=repo, retcode=None)[0]
+
+
+def has_core_hookpaths_set():
+    _, out, _ = cmd_output_b('git', 'config', 'core.hooksPath', retcode=None)
+    return bool(out.strip())
 
 
 def init_repo(path, remote):
@@ -148,8 +154,8 @@ def init_repo(path, remote):
         remote = os.path.abspath(remote)
 
     env = no_git_env()
-    cmd_output('git', 'init', path, env=env)
+    cmd_output_b('git', 'init', path, env=env)
-    cmd_output('git', 'remote', 'add', 'origin', remote, cwd=path, env=env)
+    cmd_output_b('git', 'remote', 'add', 'origin', remote, cwd=path, env=env)
 
 
 def commit(repo='.'):
@@ -158,7 +164,7 @@ def commit(repo='.'):
     env['GIT_AUTHOR_NAME'] = env['GIT_COMMITTER_NAME'] = name
     env['GIT_AUTHOR_EMAIL'] = env['GIT_COMMITTER_EMAIL'] = email
     cmd = ('git', 'commit', '--no-edit', '--no-gpg-sign', '-n', '-minit')
-    cmd_output(*cmd, cwd=repo, env=env)
+    cmd_output_b(*cmd, cwd=repo, env=env)
 
 
 def git_path(name, repo='.'):

pre_commit/languages/docker.py

@@ -9,7 +9,7 @@ from pre_commit import five
 from pre_commit.languages import helpers
 from pre_commit.util import CalledProcessError
 from pre_commit.util import clean_path_on_failure
-from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 
 
 ENVIRONMENT_DIR = 'docker'
@@ -29,9 +29,11 @@ def docker_tag(prefix):  # pragma: windows no cover
 
 def docker_is_running():  # pragma: windows no cover
     try:
-        return cmd_output('docker', 'ps')[0] == 0
+        cmd_output_b('docker', 'ps')
     except CalledProcessError:
         return False
+    else:
+        return True
 
 
 def assert_docker_available():  # pragma: windows no cover

pre_commit/languages/golang.py

@@ -11,6 +11,7 @@ from pre_commit.envcontext import Var
 from pre_commit.languages import helpers
 from pre_commit.util import clean_path_on_failure
 from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 from pre_commit.util import rmtree
 
 
@@ -70,9 +71,9 @@ def install_environment(prefix, version, additional_dependencies):
             gopath = directory
         env = dict(os.environ, GOPATH=gopath)
         env.pop('GOBIN', None)
-        cmd_output('go', 'get', './...', cwd=repo_src_dir, env=env)
+        cmd_output_b('go', 'get', './...', cwd=repo_src_dir, env=env)
         for dependency in additional_dependencies:
-            cmd_output('go', 'get', dependency, cwd=repo_src_dir, env=env)
+            cmd_output_b('go', 'get', dependency, cwd=repo_src_dir, env=env)
         # Same some disk space, we don't need these after installation
         rmtree(prefix.path(directory, 'src'))
         pkgdir = prefix.path(directory, 'pkg')

pre_commit/languages/helpers.py

@@ -8,14 +8,14 @@ import shlex
 import six
 
 import pre_commit.constants as C
-from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 from pre_commit.xargs import xargs
 
 FIXED_RANDOM_SEED = 1542676186
 
 
 def run_setup_cmd(prefix, cmd):
-    cmd_output(*cmd, cwd=prefix.prefix_dir, encoding=None)
+    cmd_output_b(*cmd, cwd=prefix.prefix_dir)
 
 
 def environment_dir(ENVIRONMENT_DIR, language_version):

pre_commit/languages/node.py

@@ -11,6 +11,7 @@ from pre_commit.languages import helpers
 from pre_commit.languages.python import bin_dir
 from pre_commit.util import clean_path_on_failure
 from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 
 
 ENVIRONMENT_DIR = 'node_env'
@@ -65,7 +66,7 @@ def install_environment(
         ]
         if version != C.DEFAULT:
             cmd.extend(['-n', version])
-        cmd_output(*cmd)
+        cmd_output_b(*cmd)
 
         with in_env(prefix, version):
             # https://npm.community/t/npm-install-g-git-vs-git-clone-cd-npm-install-g/5449

pre_commit/languages/python.py

@@ -13,6 +13,7 @@ from pre_commit.parse_shebang import find_executable
 from pre_commit.util import CalledProcessError
 from pre_commit.util import clean_path_on_failure
 from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 
 
 ENVIRONMENT_DIR = 'py_env'
@@ -143,11 +144,10 @@ def py_interface(_dir, _make_venv):
 
     def healthy(prefix, language_version):
         with in_env(prefix, language_version):
-            retcode, _, _ = cmd_output(
+            retcode, _, _ = cmd_output_b(
                 'python', '-c',
                 'import ctypes, datetime, io, os, ssl, weakref',
                 retcode=None,
-                encoding=None,
             )
         return retcode == 0
 
@@ -177,7 +177,7 @@ def py_interface(_dir, _make_venv):
 def make_venv(envdir, python):
     env = dict(os.environ, VIRTUALENV_NO_DOWNLOAD='1')
     cmd = (sys.executable, '-mvirtualenv', envdir, '-p', python)
-    cmd_output(*cmd, env=env, cwd='/')
+    cmd_output_b(*cmd, env=env, cwd='/')
 
 
 _interface = py_interface(ENVIRONMENT_DIR, make_venv)

pre_commit/languages/python_venv.py

@@ -6,6 +6,7 @@ import sys
 from pre_commit.languages import python
 from pre_commit.util import CalledProcessError
 from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 
 
 ENVIRONMENT_DIR = 'py_venv'
@@ -48,7 +49,7 @@ def orig_py_exe(exe):  # pragma: no cover (platform specific)
 
 
 def make_venv(envdir, python):
-    cmd_output(orig_py_exe(python), '-mvenv', envdir, cwd='/')
+    cmd_output_b(orig_py_exe(python), '-mvenv', envdir, cwd='/')
 
 
 _interface = python.py_interface(ENVIRONMENT_DIR, make_venv)

pre_commit/languages/rust.py

@@ -10,7 +10,7 @@ from pre_commit.envcontext import envcontext
 from pre_commit.envcontext import Var
 from pre_commit.languages import helpers
 from pre_commit.util import clean_path_on_failure
-from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 
 
 ENVIRONMENT_DIR = 'rustenv'
@@ -83,7 +83,7 @@ def install_environment(prefix, version, additional_dependencies):
                 packages_to_install.add((package,))
 
         for package in packages_to_install:
-            cmd_output(
+            cmd_output_b(
                 'cargo', 'install', '--bins', '--root', directory, *package,
                 cwd=prefix.prefix_dir
             )

pre_commit/languages/swift.py

@@ -8,7 +8,7 @@ from pre_commit.envcontext import envcontext
 from pre_commit.envcontext import Var
 from pre_commit.languages import helpers
 from pre_commit.util import clean_path_on_failure
-from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 
 ENVIRONMENT_DIR = 'swift_env'
 get_default_version = helpers.basic_get_default_version
@@ -43,7 +43,7 @@ def install_environment(
     # Build the swift package
     with clean_path_on_failure(directory):
         os.mkdir(directory)
-        cmd_output(
+        cmd_output_b(
             'swift', 'build',
             '-C', prefix.prefix_dir,
             '-c', BUILD_CONFIG,

pre_commit/make_archives.py

@@ -7,7 +7,7 @@ import os.path
 import tarfile
 
 from pre_commit import output
-from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 from pre_commit.util import rmtree
 from pre_commit.util import tmpdir
 
@@ -39,8 +39,8 @@ def make_archive(name, repo, ref, destdir):
     output_path = os.path.join(destdir, name + '.tar.gz')
     with tmpdir() as tempdir:
         # Clone the repository to the temporary directory
-        cmd_output('git', 'clone', repo, tempdir)
+        cmd_output_b('git', 'clone', repo, tempdir)
-        cmd_output('git', 'checkout', ref, cwd=tempdir)
+        cmd_output_b('git', 'checkout', ref, cwd=tempdir)
 
         # We don't want the '.git' directory
         # It adds a bunch of size to the archive and we don't use it at

pre_commit/staged_files_only.py

@@ -9,6 +9,7 @@ import time
 from pre_commit import git
 from pre_commit.util import CalledProcessError
 from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 from pre_commit.util import mkdirp
 from pre_commit.xargs import xargs
 
@@ -19,10 +20,10 @@ logger = logging.getLogger('pre_commit')
 def _git_apply(patch):
     args = ('apply', '--whitespace=nowarn', patch)
     try:
-        cmd_output('git', *args, encoding=None)
+        cmd_output_b('git', *args)
     except CalledProcessError:
         # Retry with autocrlf=false -- see #570
-        cmd_output('git', '-c', 'core.autocrlf=false', *args, encoding=None)
+        cmd_output_b('git', '-c', 'core.autocrlf=false', *args)
 
 
 @contextlib.contextmanager
@@ -43,11 +44,10 @@ def _intent_to_add_cleared():
 @contextlib.contextmanager
 def _unstaged_changes_cleared(patch_dir):
     tree = cmd_output('git', 'write-tree')[1].strip()
-    retcode, diff_stdout_binary, _ = cmd_output(
+    retcode, diff_stdout_binary, _ = cmd_output_b(
         'git', 'diff-index', '--ignore-submodules', '--binary',
         '--exit-code', '--no-color', '--no-ext-diff', tree, '--',
         retcode=None,
-        encoding=None,
     )
     if retcode and diff_stdout_binary.strip():
         patch_filename = 'patch{}'.format(int(time.time()))
@@ -62,7 +62,7 @@ def _unstaged_changes_cleared(patch_dir):
             patch_file.write(diff_stdout_binary)
 
         # Clear the working directory of unstaged changes
-        cmd_output('git', 'checkout', '--', '.')
+        cmd_output_b('git', 'checkout', '--', '.')
         try:
             yield
         finally:
@@ -77,7 +77,7 @@ def _unstaged_changes_cleared(patch_dir):
                 # We failed to apply the patch, presumably due to fixes made
                 # by hooks.
                 # Roll back the changes made by hooks.
-                cmd_output('git', 'checkout', '--', '.')
+                cmd_output_b('git', 'checkout', '--', '.')
                 _git_apply(patch_filename)
             logger.info('Restored changes from {}.'.format(patch_filename))
     else:

pre_commit/store.py

@@ -12,7 +12,7 @@ from pre_commit import file_lock
 from pre_commit import git
 from pre_commit.util import CalledProcessError
 from pre_commit.util import clean_path_on_failure
-from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 from pre_commit.util import resource_text
 from pre_commit.util import rmtree
 
@@ -161,7 +161,7 @@ class Store(object):
             env = git.no_git_env()
 
             def _git_cmd(*args):
-                cmd_output('git', *args, cwd=directory, env=env)
+                cmd_output_b('git', *args, cwd=directory, env=env)
 
             try:
                 self._shallow_clone(ref, _git_cmd)
@@ -186,7 +186,7 @@ class Store(object):
 
             # initialize the git repository so it looks more like cloned repos
             def _git_cmd(*args):
-                cmd_output('git', *args, cwd=directory, env=env)
+                cmd_output_b('git', *args, cwd=directory, env=env)
 
             git.init_repo(directory, '<<unknown>>')
             _git_cmd('add', '.')

pre_commit/util.py

@@ -117,9 +117,8 @@ class CalledProcessError(RuntimeError):
         __str__ = to_text
 
 
-def cmd_output(*cmd, **kwargs):
+def cmd_output_b(*cmd, **kwargs):
     retcode = kwargs.pop('retcode', 0)
-    encoding = kwargs.pop('encoding', 'UTF-8')
 
     popen_kwargs = {
         'stdin': subprocess.PIPE,
@@ -133,26 +132,29 @@ def cmd_output(*cmd, **kwargs):
         five.n(key): five.n(value)
         for key, value in kwargs.pop('env', {}).items()
     } or None
+    popen_kwargs.update(kwargs)
 
     try:
         cmd = parse_shebang.normalize_cmd(cmd)
     except parse_shebang.ExecutableNotFoundError as e:
-        returncode, stdout, stderr = e.to_output()
+        returncode, stdout_b, stderr_b = e.to_output()
     else:
-        popen_kwargs.update(kwargs)
         proc = subprocess.Popen(cmd, **popen_kwargs)
-        stdout, stderr = proc.communicate()
+        stdout_b, stderr_b = proc.communicate()
         returncode = proc.returncode
-    if encoding is not None and stdout is not None:
-        stdout = stdout.decode(encoding)
-    if encoding is not None and stderr is not None:
-        stderr = stderr.decode(encoding)
 
     if retcode is not None and retcode != returncode:
         raise CalledProcessError(
-            returncode, cmd, retcode, output=(stdout, stderr),
+            returncode, cmd, retcode, output=(stdout_b, stderr_b),
         )
 
+    return returncode, stdout_b, stderr_b
+
+
+def cmd_output(*cmd, **kwargs):
+    returncode, stdout_b, stderr_b = cmd_output_b(*cmd, **kwargs)
+    stdout = stdout_b.decode('UTF-8') if stdout_b is not None else None
+    stderr = stderr_b.decode('UTF-8') if stderr_b is not None else None
     return returncode, stdout, stderr
 
 

pre_commit/xargs.py

@@ -11,7 +11,7 @@ import sys
 import six
 
 from pre_commit import parse_shebang
-from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 
 
 def _environ_size(_env=None):
@@ -122,7 +122,7 @@ def xargs(cmd, varargs, **kwargs):
     partitions = partition(cmd, varargs, target_concurrency, max_length)
 
     def run_cmd_partition(run_cmd):
-        return cmd_output(*run_cmd, encoding=None, retcode=None, **kwargs)
+        return cmd_output_b(*run_cmd, retcode=None, **kwargs)
 
     threads = min(len(partitions), target_concurrency)
     with _thread_mapper(threads) as thread_map:

tests/languages/docker_test.py

@@ -9,7 +9,7 @@ from pre_commit.util import CalledProcessError
 
 def test_docker_is_running_process_error():
     with mock.patch(
-        'pre_commit.languages.docker.cmd_output',
+        'pre_commit.languages.docker.cmd_output_b',
         side_effect=CalledProcessError(*(None,) * 4),
     ):
         assert docker.docker_is_running() is False

tests/repository_test.py

@@ -28,6 +28,7 @@ from pre_commit.repository import all_hooks
 from pre_commit.repository import Hook
 from pre_commit.repository import install_hook_envs
 from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_b
 from testing.fixtures import make_config_from_repo
 from testing.fixtures import make_repo
 from testing.fixtures import modify_manifest
@@ -380,7 +381,7 @@ def _make_grep_repo(language, entry, store, args=()):
 @pytest.fixture
 def greppable_files(tmpdir):
     with tmpdir.as_cwd():
-        cmd_output('git', 'init', '.')
+        cmd_output_b('git', 'init', '.')
         tmpdir.join('f1').write_binary(b"hello'hi\nworld\n")
         tmpdir.join('f2').write_binary(b'foo\nbar\nbaz\n')
         tmpdir.join('f3').write_binary(b'[WARN] hi\n')
@@ -439,9 +440,8 @@ class TestPCRE(TestPygrep):
 def _norm_pwd(path):
     # Under windows bash's temp and windows temp is different.
     # This normalizes to the bash /tmp
-    return cmd_output(
+    return cmd_output_b(
         'bash', '-c', "cd '{}' && pwd".format(path),
-        encoding=None,
     )[1].strip()
 
 
@@ -654,7 +654,7 @@ def test_invalidated_virtualenv(tempdir_factory, store):
     paths = [
         os.path.join(libdir, p) for p in ('site.py', 'site.pyc', '__pycache__')
     ]
-    cmd_output('rm', '-rf', *paths)
+    cmd_output_b('rm', '-rf', *paths)
 
     # pre-commit should rebuild the virtualenv and it should be runnable
     retv, stdout, stderr = _get_hook(config, store, 'foo').run(())
@@ -664,7 +664,7 @@ def test_invalidated_virtualenv(tempdir_factory, store):
 def test_really_long_file_paths(tempdir_factory, store):
     base_path = tempdir_factory.get()
     really_long_path = os.path.join(base_path, 'really_long' * 10)
-    cmd_output('git', 'init', really_long_path)
+    cmd_output_b('git', 'init', really_long_path)
 
     path = make_repo(tempdir_factory, 'python_hooks_repo')
     config = make_config_from_repo(path)
@@ -687,7 +687,7 @@ def test_config_overrides_repo_specifics(tempdir_factory, store):
 
 def _create_repo_with_tags(tempdir_factory, src, tag):
     path = make_repo(tempdir_factory, src)
-    cmd_output('git', 'tag', tag, cwd=path)
+    cmd_output_b('git', 'tag', tag, cwd=path)
     return path