pre-commit · asottile · Oct 7, 2019 · Oct 6, 2019
diff --git a/pre_commit/commands/autoupdate.py b/pre_commit/commands/autoupdate.py
@@ -21,6 +21,7 @@
 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:

diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py
@@ -13,7 +13,6 @@
 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`',

diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py
@@ -16,7 +16,7 @@
 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(
-        'git', 'diff', '--no-ext-diff', retcode=None, encoding=None,
-    )
+    diff_before = cmd_output_b('git', 'diff', '--no-ext-diff', retcode=None)
     retcode, stdout, stderr = hook.run(
         tuple(filenames) if hook.pass_filenames else (),
     )
-    diff_after = cmd_output(
-        'git', 'diff', '--no-ext-diff', retcode=None, encoding=None,
-    )
+    diff_after = cmd_output_b('git', 'diff', '--no-ext-diff', retcode=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,
     )

diff --git a/pre_commit/commands/try_repo.py b/pre_commit/commands/try_repo.py
@@ -13,7 +13,7 @@
 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('git', 'checkout', ref, '-b', '_pc_tmp', cwd=shadow)
+        cmd_output_b('git', 'clone', repo, 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)

diff --git a/pre_commit/git.py b/pre_commit/git.py
@@ -5,6 +5,7 @@
 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]
-    return ret.strip()
+    _, out, _ = cmd_output('git', 'config', 'remote.origin.url', cwd=git_root)
+    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')
-    parts = list(reversed(zsplit(stdout_binary)))
+    _, stdout, _ = cmd_output('git', 'status', '--porcelain', '-z')
+    parts = list(reversed(zsplit(stdout)))
     intent_to_add = []
     while parts:
         line = parts.pop()
@@ -140,16 +141,21 @@ 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):
     if os.path.isdir(remote):
         remote = os.path.abspath(remote)
 
     env = no_git_env()
-    cmd_output('git', 'init', path, env=env)
-    cmd_output('git', 'remote', 'add', 'origin', remote, cwd=path, env=env)
+    cmd_output_b('git', 'init', 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='.'):

diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py
@@ -9,7 +9,7 @@
 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

diff --git a/pre_commit/languages/golang.py b/pre_commit/languages/golang.py
@@ -11,6 +11,7 @@
 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')

diff --git a/pre_commit/languages/helpers.py b/pre_commit/languages/helpers.py
@@ -8,14 +8,14 @@
 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):

diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py
@@ -11,6 +11,7 @@
 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

diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py
@@ -13,6 +13,7 @@
 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 in_env(prefix, language_version):
 
     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 install_environment(prefix, version, additional_dependencies):
 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)

diff --git a/pre_commit/languages/python_venv.py b/pre_commit/languages/python_venv.py
@@ -6,6 +6,7 @@
 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)

diff --git a/pre_commit/languages/rust.py b/pre_commit/languages/rust.py
@@ -10,7 +10,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
 
 
 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
             )

diff --git a/pre_commit/languages/swift.py b/pre_commit/languages/swift.py
@@ -8,7 +8,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
 
 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,

diff --git a/pre_commit/make_archives.py b/pre_commit/make_archives.py
@@ -7,7 +7,7 @@
 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('git', 'checkout', ref, cwd=tempdir)
+        cmd_output_b('git', 'clone', repo, 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

diff --git a/pre_commit/staged_files_only.py b/pre_commit/staged_files_only.py
@@ -9,6 +9,7 @@
 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 @@
 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: