From 7d87da8acdd3b8817d941b9a172be6282722bf5e Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Thu, 11 Jan 2018 21:41:48 -0800 Subject: [PATCH] Move PrefixedCommandRunner -> Prefix --- pre_commit/languages/all.py | 14 ++- pre_commit/languages/docker.py | 28 +++--- pre_commit/languages/docker_image.py | 2 +- pre_commit/languages/golang.py | 22 ++--- pre_commit/languages/helpers.py | 8 +- pre_commit/languages/node.py | 27 +++--- pre_commit/languages/pcre.py | 2 +- pre_commit/languages/pygrep.py | 2 +- pre_commit/languages/python.py | 29 +++--- pre_commit/languages/ruby.py | 51 +++++----- pre_commit/languages/script.py | 4 +- pre_commit/languages/swift.py | 19 ++-- pre_commit/languages/system.py | 2 +- pre_commit/prefix.py | 20 ++++ pre_commit/prefixed_command_runner.py | 50 ---------- pre_commit/repository.py | 62 ++++++------ tests/conftest.py | 6 -- tests/languages/all_test.py | 6 +- tests/languages/ruby_test.py | 39 ++++---- tests/prefix_test.py | 57 +++++++++++ tests/prefixed_command_runner_test.py | 133 -------------------------- tests/repository_test.py | 30 +++--- tests/util_test.py | 29 ++++++ 23 files changed, 270 insertions(+), 372 deletions(-) create mode 100644 pre_commit/prefix.py delete mode 100644 pre_commit/prefixed_command_runner.py create mode 100644 tests/prefix_test.py delete mode 100644 tests/prefixed_command_runner_test.py diff --git a/pre_commit/languages/all.py b/pre_commit/languages/all.py index 514ba611..a56f7e79 100644 --- a/pre_commit/languages/all.py +++ b/pre_commit/languages/all.py @@ -23,25 +23,25 @@ from pre_commit.languages import system # return 'default' if there is no better option. # """ # -# def healthy(repo_cmd_runner, language_version): +# def healthy(prefix, language_version): # """Return whether or not the environment is considered functional.""" # -# def install_environment(repo_cmd_runner, version, additional_dependencies): +# def install_environment(prefix, version, additional_dependencies): # """Installs a repository in the given repository. Note that the current # working directory will already be inside the repository. # # Args: -# repo_cmd_runner - `PrefixedCommandRunner` bound to the repository. +# prefix - `Prefix` bound to the repository. # version - A version specified in the hook configuration or # 'default'. # """ # -# def run_hook(repo_cmd_runner, hook, file_args): +# def run_hook(prefix, hook, file_args): # """Runs a hook and returns the returncode and output of running that # hook. # # Args: -# repo_cmd_runner - `PrefixedCommandRunner` bound to the repository. +# prefix - `Prefix` bound to the repository. # hook - Hook dictionary # file_args - The files to be run # @@ -62,6 +62,4 @@ languages = { 'swift': swift, 'system': system, } - - -all_languages = languages.keys() +all_languages = sorted(languages) diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index f5eed752..f3c46a33 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -22,10 +22,9 @@ def md5(s): # pragma: windows no cover return hashlib.md5(five.to_bytes(s)).hexdigest() -def docker_tag(repo_cmd_runner): # pragma: windows no cover - return 'pre-commit-{}'.format( - md5(os.path.basename(repo_cmd_runner.path())), - ).lower() +def docker_tag(prefix): # pragma: windows no cover + md5sum = md5(os.path.basename(prefix.prefix_dir)).lower() + return 'pre-commit-{}'.format(md5sum) def docker_is_running(): # pragma: windows no cover @@ -41,39 +40,36 @@ def assert_docker_available(): # pragma: windows no cover ) -def build_docker_image(repo_cmd_runner, **kwargs): # pragma: windows no cover +def build_docker_image(prefix, **kwargs): # pragma: windows no cover pull = kwargs.pop('pull') assert not kwargs, kwargs cmd = ( 'docker', 'build', - '--tag', docker_tag(repo_cmd_runner), + '--tag', docker_tag(prefix), '--label', PRE_COMMIT_LABEL, ) if pull: cmd += ('--pull',) # This must come last for old versions of docker. See #477 cmd += ('.',) - helpers.run_setup_cmd(repo_cmd_runner, cmd) + helpers.run_setup_cmd(prefix, cmd) def install_environment( - repo_cmd_runner, version, additional_dependencies, + prefix, version, additional_dependencies, ): # pragma: windows no cover - assert repo_cmd_runner.exists('Dockerfile'), ( - 'No Dockerfile was found in the hook repository' - ) helpers.assert_version_default('docker', version) helpers.assert_no_additional_deps('docker', additional_dependencies) assert_docker_available() - directory = repo_cmd_runner.path( + directory = prefix.path( helpers.environment_dir(ENVIRONMENT_DIR, 'default'), ) # Docker doesn't really have relevant disk environment, but pre-commit # still needs to cleanup it's state files on failure with clean_path_on_failure(directory): - build_docker_image(repo_cmd_runner, pull=True) + build_docker_image(prefix, pull=True) os.mkdir(directory) @@ -90,15 +86,15 @@ def docker_cmd(): ) -def run_hook(repo_cmd_runner, hook, file_args): # pragma: windows no cover +def run_hook(prefix, hook, file_args): # pragma: windows no cover assert_docker_available() # Rebuild the docker image in case it has gone missing, as many people do # automated cleanup of docker images. - build_docker_image(repo_cmd_runner, pull=False) + build_docker_image(prefix, pull=False) hook_cmd = helpers.to_cmd(hook) entry_exe, cmd_rest = hook_cmd[0], hook_cmd[1:] - entry_tag = ('--entrypoint', entry_exe, docker_tag(repo_cmd_runner)) + entry_tag = ('--entrypoint', entry_exe, docker_tag(prefix)) cmd = docker_cmd() + entry_tag + cmd_rest return xargs(cmd, file_args) diff --git a/pre_commit/languages/docker_image.py b/pre_commit/languages/docker_image.py index a6f89e3f..6301970c 100644 --- a/pre_commit/languages/docker_image.py +++ b/pre_commit/languages/docker_image.py @@ -13,7 +13,7 @@ healthy = helpers.basic_healthy install_environment = helpers.no_install -def run_hook(repo_cmd_runner, hook, file_args): # pragma: windows no cover +def run_hook(prefix, hook, file_args): # pragma: windows no cover assert_docker_available() cmd = docker_cmd() + helpers.to_cmd(hook) return xargs(cmd, file_args) diff --git a/pre_commit/languages/golang.py b/pre_commit/languages/golang.py index cad7dfc6..35cfa2ad 100644 --- a/pre_commit/languages/golang.py +++ b/pre_commit/languages/golang.py @@ -26,8 +26,8 @@ def get_env_patch(venv): @contextlib.contextmanager -def in_env(repo_cmd_runner): - envdir = repo_cmd_runner.path( +def in_env(prefix): + envdir = prefix.path( helpers.environment_dir(ENVIRONMENT_DIR, 'default'), ) with envcontext(get_env_patch(envdir)): @@ -50,20 +50,18 @@ def guess_go_dir(remote_url): return 'unknown_src_dir' -def install_environment(repo_cmd_runner, version, additional_dependencies): +def install_environment(prefix, version, additional_dependencies): helpers.assert_version_default('golang', version) - directory = repo_cmd_runner.path( + directory = prefix.path( helpers.environment_dir(ENVIRONMENT_DIR, 'default'), ) with clean_path_on_failure(directory): - remote = git.get_remote_url(repo_cmd_runner.path()) + remote = git.get_remote_url(prefix.prefix_dir) repo_src_dir = os.path.join(directory, 'src', guess_go_dir(remote)) # Clone into the goenv we'll create - helpers.run_setup_cmd( - repo_cmd_runner, ('git', 'clone', '.', repo_src_dir), - ) + helpers.run_setup_cmd(prefix, ('git', 'clone', '.', repo_src_dir)) if sys.platform == 'cygwin': # pragma: no cover _, gopath, _ = cmd_output('cygpath', '-w', directory) @@ -75,10 +73,10 @@ def install_environment(repo_cmd_runner, version, additional_dependencies): for dependency in additional_dependencies: cmd_output('go', 'get', dependency, cwd=repo_src_dir, env=env) # Same some disk space, we don't need these after installation - rmtree(repo_cmd_runner.path(directory, 'src')) - rmtree(repo_cmd_runner.path(directory, 'pkg')) + rmtree(prefix.path(directory, 'src')) + rmtree(prefix.path(directory, 'pkg')) -def run_hook(repo_cmd_runner, hook, file_args): - with in_env(repo_cmd_runner): +def run_hook(prefix, hook, file_args): + with in_env(prefix): return xargs(helpers.to_cmd(hook), file_args) diff --git a/pre_commit/languages/helpers.py b/pre_commit/languages/helpers.py index 30082d6b..ddbe2e80 100644 --- a/pre_commit/languages/helpers.py +++ b/pre_commit/languages/helpers.py @@ -5,8 +5,8 @@ import shlex from pre_commit.util import cmd_output -def run_setup_cmd(runner, cmd): - cmd_output(*cmd, cwd=runner.prefix_dir, encoding=None) +def run_setup_cmd(prefix, cmd): + cmd_output(*cmd, cwd=prefix.prefix_dir, encoding=None) def environment_dir(ENVIRONMENT_DIR, language_version): @@ -39,9 +39,9 @@ def basic_get_default_version(): return 'default' -def basic_healthy(repo_cmd_runner, language_version): +def basic_healthy(prefix, language_version): return True -def no_install(repo_cmd_runner, version, additional_dependencies): +def no_install(prefix, version, additional_dependencies): raise AssertionError('This type is not installable') diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py index aca3c410..5f5d9fdd 100644 --- a/pre_commit/languages/node.py +++ b/pre_commit/languages/node.py @@ -33,8 +33,8 @@ def get_env_patch(venv): # pragma: windows no cover @contextlib.contextmanager -def in_env(repo_cmd_runner, language_version): # pragma: windows no cover - envdir = repo_cmd_runner.path( +def in_env(prefix, language_version): # pragma: windows no cover + envdir = prefix.path( helpers.environment_dir(ENVIRONMENT_DIR, language_version), ) with envcontext(get_env_patch(envdir)): @@ -42,31 +42,26 @@ def in_env(repo_cmd_runner, language_version): # pragma: windows no cover def install_environment( - repo_cmd_runner, version, additional_dependencies, + prefix, version, additional_dependencies, ): # pragma: windows no cover additional_dependencies = tuple(additional_dependencies) - assert repo_cmd_runner.exists('package.json') + assert prefix.exists('package.json') directory = helpers.environment_dir(ENVIRONMENT_DIR, version) - env_dir = repo_cmd_runner.path(directory) + env_dir = prefix.path(directory) with clean_path_on_failure(env_dir): - cmd = [ - sys.executable, '-m', 'nodeenv', '--prebuilt', - '{{prefix}}{}'.format(directory), - ] - + cmd = [sys.executable, '-m', 'nodeenv', '--prebuilt', env_dir] if version != 'default': cmd.extend(['-n', version]) + cmd_output(*cmd) - repo_cmd_runner.run(cmd) - - with in_env(repo_cmd_runner, version): + with in_env(prefix, version): helpers.run_setup_cmd( - repo_cmd_runner, + prefix, ('npm', 'install', '-g', '.') + additional_dependencies, ) -def run_hook(repo_cmd_runner, hook, file_args): # pragma: windows no cover - with in_env(repo_cmd_runner, hook['language_version']): +def run_hook(prefix, hook, file_args): # pragma: windows no cover + with in_env(prefix, hook['language_version']): return xargs(helpers.to_cmd(hook), file_args) diff --git a/pre_commit/languages/pcre.py b/pre_commit/languages/pcre.py index eaacc110..fb078ab7 100644 --- a/pre_commit/languages/pcre.py +++ b/pre_commit/languages/pcre.py @@ -13,7 +13,7 @@ healthy = helpers.basic_healthy install_environment = helpers.no_install -def run_hook(repo_cmd_runner, hook, file_args): +def run_hook(prefix, hook, file_args): # For PCRE the entry is the regular expression to match cmd = (GREP, '-H', '-n', '-P') + tuple(hook['args']) + (hook['entry'],) diff --git a/pre_commit/languages/pygrep.py b/pre_commit/languages/pygrep.py index 4914fd66..878f57d0 100644 --- a/pre_commit/languages/pygrep.py +++ b/pre_commit/languages/pygrep.py @@ -27,7 +27,7 @@ def _process_filename_by_line(pattern, filename): return retv -def run_hook(repo_cmd_runner, hook, file_args): +def run_hook(prefix, hook, file_args): exe = (sys.executable, '-m', __name__) exe += tuple(hook['args']) + (hook['entry'],) return xargs(exe, file_args) diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index 8d891aa3..7fc5443e 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -33,8 +33,8 @@ def get_env_patch(venv): @contextlib.contextmanager -def in_env(repo_cmd_runner, language_version): - envdir = repo_cmd_runner.path( +def in_env(prefix, language_version): + envdir = prefix.path( helpers.environment_dir(ENVIRONMENT_DIR, language_version), ) with envcontext(get_env_patch(envdir)): @@ -98,8 +98,8 @@ def get_default_version(): return get_default_version() -def healthy(repo_cmd_runner, language_version): - with in_env(repo_cmd_runner, language_version): +def healthy(prefix, language_version): + with in_env(prefix, language_version): retcode, _, _ = cmd_output( 'python', '-c', 'import ctypes, datetime, io, os, ssl, weakref', retcode=None, @@ -127,29 +127,26 @@ def norm_version(version): return os.path.expanduser(version) -def install_environment(repo_cmd_runner, version, additional_dependencies): +def install_environment(prefix, version, additional_dependencies): additional_dependencies = tuple(additional_dependencies) directory = helpers.environment_dir(ENVIRONMENT_DIR, version) # Install a virtualenv - with clean_path_on_failure(repo_cmd_runner.path(directory)): - venv_cmd = [ - sys.executable, '-m', 'virtualenv', - '{{prefix}}{}'.format(directory), - ] + env_dir = prefix.path(directory) + with clean_path_on_failure(env_dir): + venv_cmd = [sys.executable, '-m', 'virtualenv', env_dir] if version != 'default': venv_cmd.extend(['-p', norm_version(version)]) else: venv_cmd.extend(['-p', os.path.realpath(sys.executable)]) venv_env = dict(os.environ, VIRTUALENV_NO_DOWNLOAD='1') - repo_cmd_runner.run(venv_cmd, cwd='/', env=venv_env) - with in_env(repo_cmd_runner, version): + cmd_output(*venv_cmd, cwd='/', env=venv_env) + with in_env(prefix, version): helpers.run_setup_cmd( - repo_cmd_runner, - ('pip', 'install', '.') + additional_dependencies, + prefix, ('pip', 'install', '.') + additional_dependencies, ) -def run_hook(repo_cmd_runner, hook, file_args): - with in_env(repo_cmd_runner, hook['language_version']): +def run_hook(prefix, hook, file_args): + with in_env(prefix, hook['language_version']): return xargs(helpers.to_cmd(hook), file_args) diff --git a/pre_commit/languages/ruby.py b/pre_commit/languages/ruby.py index e7e0c328..3bd7130d 100644 --- a/pre_commit/languages/ruby.py +++ b/pre_commit/languages/ruby.py @@ -39,36 +39,32 @@ def get_env_patch(venv, language_version): # pragma: windows no cover @contextlib.contextmanager -def in_env(repo_cmd_runner, language_version): # pragma: windows no cover - envdir = repo_cmd_runner.path( +def in_env(prefix, language_version): # pragma: windows no cover + envdir = prefix.path( helpers.environment_dir(ENVIRONMENT_DIR, language_version), ) with envcontext(get_env_patch(envdir, language_version)): yield -def _install_rbenv( - repo_cmd_runner, version='default', -): # pragma: windows no cover +def _install_rbenv(prefix, version='default'): # pragma: windows no cover directory = helpers.environment_dir(ENVIRONMENT_DIR, version) with tarfile.open(resource_filename('rbenv.tar.gz')) as tf: - tf.extractall(repo_cmd_runner.path('.')) - shutil.move( - repo_cmd_runner.path('rbenv'), repo_cmd_runner.path(directory), - ) + tf.extractall(prefix.path('.')) + shutil.move(prefix.path('rbenv'), prefix.path(directory)) # Only install ruby-build if the version is specified if version != 'default': # ruby-download with tarfile.open(resource_filename('ruby-download.tar.gz')) as tf: - tf.extractall(repo_cmd_runner.path(directory, 'plugins')) + tf.extractall(prefix.path(directory, 'plugins')) # ruby-build with tarfile.open(resource_filename('ruby-build.tar.gz')) as tf: - tf.extractall(repo_cmd_runner.path(directory, 'plugins')) + tf.extractall(prefix.path(directory, 'plugins')) - activate_path = repo_cmd_runner.path(directory, 'bin', 'activate') + activate_path = prefix.path(directory, 'bin', 'activate') with io.open(activate_path, 'w') as activate_file: # This is similar to how you would install rbenv to your home directory # However we do a couple things to make the executables exposed and @@ -84,7 +80,7 @@ def _install_rbenv( # directory "export GEM_HOME='{directory}/gems'\n" 'export PATH="$GEM_HOME/bin:$PATH"\n' - '\n'.format(directory=repo_cmd_runner.path(directory)), + '\n'.format(directory=prefix.path(directory)), ) # If we aren't using the system ruby, add a version here @@ -101,35 +97,32 @@ def _install_ruby(runner, version): # pragma: windows no cover def install_environment( - repo_cmd_runner, version, additional_dependencies, + prefix, version, additional_dependencies, ): # pragma: windows no cover additional_dependencies = tuple(additional_dependencies) directory = helpers.environment_dir(ENVIRONMENT_DIR, version) - with clean_path_on_failure(repo_cmd_runner.path(directory)): + with clean_path_on_failure(prefix.path(directory)): # TODO: this currently will fail if there's no version specified and # there's no system ruby installed. Is this ok? - _install_rbenv(repo_cmd_runner, version=version) - with in_env(repo_cmd_runner, version): + _install_rbenv(prefix, version=version) + with in_env(prefix, version): # Need to call this before installing so rbenv's directories are # set up - helpers.run_setup_cmd(repo_cmd_runner, ('rbenv', 'init', '-')) + helpers.run_setup_cmd(prefix, ('rbenv', 'init', '-')) if version != 'default': - _install_ruby(repo_cmd_runner, version) + _install_ruby(prefix, version) # Need to call this after installing to set up the shims - helpers.run_setup_cmd(repo_cmd_runner, ('rbenv', 'rehash')) + helpers.run_setup_cmd(prefix, ('rbenv', 'rehash')) helpers.run_setup_cmd( - repo_cmd_runner, - ('gem', 'build') + repo_cmd_runner.star('.gemspec'), + prefix, ('gem', 'build') + prefix.star('.gemspec'), ) helpers.run_setup_cmd( - repo_cmd_runner, - ( - ('gem', 'install', '--no-ri', '--no-rdoc') + - repo_cmd_runner.star('.gem') + additional_dependencies - ), + prefix, + ('gem', 'install', '--no-ri', '--no-rdoc') + + prefix.star('.gem') + additional_dependencies, ) -def run_hook(repo_cmd_runner, hook, file_args): # pragma: windows no cover - with in_env(repo_cmd_runner, hook['language_version']): +def run_hook(prefix, hook, file_args): # pragma: windows no cover + with in_env(prefix, hook['language_version']): return xargs(helpers.to_cmd(hook), file_args) diff --git a/pre_commit/languages/script.py b/pre_commit/languages/script.py index 8c3b0c56..8186e77a 100644 --- a/pre_commit/languages/script.py +++ b/pre_commit/languages/script.py @@ -10,7 +10,7 @@ healthy = helpers.basic_healthy install_environment = helpers.no_install -def run_hook(repo_cmd_runner, hook, file_args): +def run_hook(prefix, hook, file_args): cmd = helpers.to_cmd(hook) - cmd = (repo_cmd_runner.prefix_dir + cmd[0],) + cmd[1:] + cmd = (prefix.prefix_dir + cmd[0],) + cmd[1:] return xargs(cmd, file_args) diff --git a/pre_commit/languages/swift.py b/pre_commit/languages/swift.py index f4d1eb5a..2863fbee 100644 --- a/pre_commit/languages/swift.py +++ b/pre_commit/languages/swift.py @@ -7,6 +7,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.xargs import xargs ENVIRONMENT_DIR = 'swift_env' @@ -22,8 +23,8 @@ def get_env_patch(venv): # pragma: windows no cover @contextlib.contextmanager -def in_env(repo_cmd_runner): # pragma: windows no cover - envdir = repo_cmd_runner.path( +def in_env(prefix): # pragma: windows no cover + envdir = prefix.path( helpers.environment_dir(ENVIRONMENT_DIR, 'default'), ) with envcontext(get_env_patch(envdir)): @@ -31,25 +32,25 @@ def in_env(repo_cmd_runner): # pragma: windows no cover def install_environment( - repo_cmd_runner, version, additional_dependencies, + prefix, version, additional_dependencies, ): # pragma: windows no cover helpers.assert_version_default('swift', version) helpers.assert_no_additional_deps('swift', additional_dependencies) - directory = repo_cmd_runner.path( + directory = prefix.path( helpers.environment_dir(ENVIRONMENT_DIR, 'default'), ) # Build the swift package with clean_path_on_failure(directory): os.mkdir(directory) - repo_cmd_runner.run(( + cmd_output( 'swift', 'build', - '-C', '{prefix}', + '-C', prefix.prefix_dir, '-c', BUILD_CONFIG, '--build-path', os.path.join(directory, BUILD_DIR), - )) + ) -def run_hook(repo_cmd_runner, hook, file_args): # pragma: windows no cover - with in_env(repo_cmd_runner): +def run_hook(prefix, hook, file_args): # pragma: windows no cover + with in_env(prefix): return xargs(helpers.to_cmd(hook), file_args) diff --git a/pre_commit/languages/system.py b/pre_commit/languages/system.py index 693a1601..84cd1fe4 100644 --- a/pre_commit/languages/system.py +++ b/pre_commit/languages/system.py @@ -10,5 +10,5 @@ healthy = helpers.basic_healthy install_environment = helpers.no_install -def run_hook(repo_cmd_runner, hook, file_args): +def run_hook(prefix, hook, file_args): return xargs(helpers.to_cmd(hook), file_args) diff --git a/pre_commit/prefix.py b/pre_commit/prefix.py new file mode 100644 index 00000000..128bd861 --- /dev/null +++ b/pre_commit/prefix.py @@ -0,0 +1,20 @@ +from __future__ import unicode_literals + +import os.path + + +class Prefix(object): + def __init__(self, prefix_dir): + self.prefix_dir = prefix_dir.rstrip(os.sep) + os.sep + + def path(self, *parts): + path = os.path.join(self.prefix_dir, *parts) + return os.path.normpath(path) + + def exists(self, *parts): + return os.path.exists(self.path(*parts)) + + def star(self, end): + return tuple( + path for path in os.listdir(self.prefix_dir) if path.endswith(end) + ) diff --git a/pre_commit/prefixed_command_runner.py b/pre_commit/prefixed_command_runner.py deleted file mode 100644 index c2de526b..00000000 --- a/pre_commit/prefixed_command_runner.py +++ /dev/null @@ -1,50 +0,0 @@ -from __future__ import unicode_literals - -import os.path -import subprocess - -from pre_commit.util import cmd_output - - -class PrefixedCommandRunner(object): - """A PrefixedCommandRunner allows you to run subprocess commands with - comand substitution. - - For instance: - PrefixedCommandRunner('/tmp/foo').run(['{prefix}foo.sh', 'bar', 'baz']) - - will run ['/tmp/foo/foo.sh', 'bar', 'baz'] - """ - - def __init__( - self, - prefix_dir, - popen=subprocess.Popen, - makedirs=os.makedirs, - ): - self.prefix_dir = prefix_dir.rstrip(os.sep) + os.sep - self.__popen = popen - self.__makedirs = makedirs - - def _create_path_if_not_exists(self): - if not os.path.exists(self.prefix_dir): - self.__makedirs(self.prefix_dir) - - def run(self, cmd, **kwargs): - self._create_path_if_not_exists() - replaced_cmd = [ - part.replace('{prefix}', self.prefix_dir) for part in cmd - ] - return cmd_output(*replaced_cmd, __popen=self.__popen, **kwargs) - - def path(self, *parts): - path = os.path.join(self.prefix_dir, *parts) - return os.path.normpath(path) - - def exists(self, *parts): - return os.path.exists(self.path(*parts)) - - def star(self, end): - return tuple( - path for path in os.listdir(self.prefix_dir) if path.endswith(end) - ) diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 5c11921c..e01b3d1d 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -21,7 +21,7 @@ from pre_commit.clientlib import load_manifest from pre_commit.clientlib import MANIFEST_HOOK_DICT from pre_commit.languages.all import languages from pre_commit.languages.helpers import environment_dir -from pre_commit.prefixed_command_runner import PrefixedCommandRunner +from pre_commit.prefix import Prefix from pre_commit.schema import apply_defaults from pre_commit.schema import validate @@ -33,22 +33,22 @@ def _state(additional_deps): return {'additional_dependencies': sorted(additional_deps)} -def _state_filename(cmd_runner, venv): - return cmd_runner.path( +def _state_filename(prefix, venv): + return prefix.path( venv, '.install_state_v' + C.INSTALLED_STATE_VERSION, ) -def _read_state(cmd_runner, venv): - filename = _state_filename(cmd_runner, venv) +def _read_state(prefix, venv): + filename = _state_filename(prefix, venv) if not os.path.exists(filename): return None else: return json.loads(io.open(filename).read()) -def _write_state(cmd_runner, venv, state): - state_filename = _state_filename(cmd_runner, venv) +def _write_state(prefix, venv, state): + state_filename = _state_filename(prefix, venv) staging = state_filename + 'staging' with io.open(staging, 'w') as state_file: state_file.write(five.to_text(json.dumps(state))) @@ -56,24 +56,24 @@ def _write_state(cmd_runner, venv, state): os.rename(staging, state_filename) -def _installed(cmd_runner, language_name, language_version, additional_deps): +def _installed(prefix, language_name, language_version, additional_deps): language = languages[language_name] venv = environment_dir(language.ENVIRONMENT_DIR, language_version) return ( venv is None or ( - _read_state(cmd_runner, venv) == _state(additional_deps) and - language.healthy(cmd_runner, language_version) + _read_state(prefix, venv) == _state(additional_deps) and + language.healthy(prefix, language_version) ) ) def _install_all(venvs, repo_url, store): - """Tuple of (cmd_runner, language, version, deps)""" + """Tuple of (prefix, language, version, deps)""" def _need_installed(): return tuple( - (cmd_runner, language_name, version, deps) - for cmd_runner, language_name, version, deps in venvs - if not _installed(cmd_runner, language_name, version, deps) + (prefix, language_name, version, deps) + for prefix, language_name, version, deps in venvs + if not _installed(prefix, language_name, version, deps) ) if not _need_installed(): @@ -90,19 +90,19 @@ def _install_all(venvs, repo_url, store): logger.info('Once installed this environment will be reused.') logger.info('This may take a few minutes...') - for cmd_runner, language_name, version, deps in need_installed: + for prefix, language_name, version, deps in need_installed: language = languages[language_name] venv = environment_dir(language.ENVIRONMENT_DIR, version) # There's potentially incomplete cleanup from previous runs # Clean it up! - if cmd_runner.exists(venv): - shutil.rmtree(cmd_runner.path(venv)) + if prefix.exists(venv): + shutil.rmtree(prefix.path(venv)) - language.install_environment(cmd_runner, version, deps) + language.install_environment(prefix, version, deps) # Write our state to indicate we're installed state = _state(deps) - _write_state(cmd_runner, venv, state) + _write_state(prefix, venv, state) def _hook(*hook_dicts): @@ -156,11 +156,11 @@ class Repository(object): ) @cached_property - def _cmd_runner(self): - return PrefixedCommandRunner(self._repo_path) + def _prefix(self): + return Prefix(self._repo_path) - def _cmd_runner_from_deps(self, language_name, deps): - return self._cmd_runner + def _prefix_from_deps(self, language_name, deps): + return self._prefix @cached_property def manifest_hooks(self): @@ -194,7 +194,7 @@ class Repository(object): ) ret = [] for (language, version), deps in deps_dict.items(): - ret.append((self._cmd_runner, language, version, deps)) + ret.append((self._prefix, language, version, deps)) return tuple(ret) def require_installed(self): @@ -211,20 +211,20 @@ class Repository(object): self.require_installed() language_name = hook['language'] deps = hook['additional_dependencies'] - cmd_runner = self._cmd_runner_from_deps(language_name, deps) - return languages[language_name].run_hook(cmd_runner, hook, file_args) + prefix = self._prefix_from_deps(language_name, deps) + return languages[language_name].run_hook(prefix, hook, file_args) class LocalRepository(Repository): - def _cmd_runner_from_deps(self, language_name, deps): - """local repositories have a cmd runner per hook""" + def _prefix_from_deps(self, language_name, deps): + """local repositories have a prefix per hook""" language = languages[language_name] # pcre / pygrep / script / system / docker_image do not have # environments so they work out of the current directory if language.ENVIRONMENT_DIR is None: - return PrefixedCommandRunner(git.get_root()) + return Prefix(git.get_root()) else: - return PrefixedCommandRunner(self.store.make_local(deps)) + return Prefix(self.store.make_local(deps)) @cached_property def manifest(self): @@ -245,7 +245,7 @@ class LocalRepository(Repository): version = hook['language_version'] deps = hook['additional_dependencies'] ret.append(( - self._cmd_runner_from_deps(language, deps), + self._prefix_from_deps(language, deps), language, version, deps, )) return tuple(ret) diff --git a/tests/conftest.py b/tests/conftest.py index 36743d88..fe710e65 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,7 +14,6 @@ import six import pre_commit.constants as C from pre_commit import output from pre_commit.logging_handler import add_logging_handler -from pre_commit.prefixed_command_runner import PrefixedCommandRunner from pre_commit.runner import Runner from pre_commit.store import Store from pre_commit.util import cmd_output @@ -155,11 +154,6 @@ def store(tempdir_factory): yield Store(os.path.join(tempdir_factory.get(), '.pre-commit')) -@pytest.yield_fixture -def cmd_runner(tempdir_factory): - yield PrefixedCommandRunner(tempdir_factory.get()) - - @pytest.yield_fixture def runner_with_mocked_store(mock_out_store_directory): yield Runner('/', C.CONFIG_FILE) diff --git a/tests/languages/all_test.py b/tests/languages/all_test.py index 95cec104..6e3ab662 100644 --- a/tests/languages/all_test.py +++ b/tests/languages/all_test.py @@ -11,7 +11,7 @@ from pre_commit.languages.all import languages @pytest.mark.parametrize('language', all_languages) def test_install_environment_argspec(language): expected_argspec = inspect.ArgSpec( - args=['repo_cmd_runner', 'version', 'additional_dependencies'], + args=['prefix', 'version', 'additional_dependencies'], varargs=None, keywords=None, defaults=None, ) argspec = inspect.getargspec(languages[language].install_environment) @@ -26,7 +26,7 @@ def test_ENVIRONMENT_DIR(language): @pytest.mark.parametrize('language', all_languages) def test_run_hook_argpsec(language): expected_argspec = inspect.ArgSpec( - args=['repo_cmd_runner', 'hook', 'file_args'], + args=['prefix', 'hook', 'file_args'], varargs=None, keywords=None, defaults=None, ) argspec = inspect.getargspec(languages[language].run_hook) @@ -45,7 +45,7 @@ def test_get_default_version_argspec(language): @pytest.mark.parametrize('language', all_languages) def test_healthy_argspec(language): expected_argspec = inspect.ArgSpec( - args=['repo_cmd_runner', 'language_version'], + args=['prefix', 'language_version'], varargs=None, keywords=None, defaults=None, ) argspec = inspect.getargspec(languages[language].healthy) diff --git a/tests/languages/ruby_test.py b/tests/languages/ruby_test.py index 1eddea1d..bcaf0986 100644 --- a/tests/languages/ruby_test.py +++ b/tests/languages/ruby_test.py @@ -1,39 +1,42 @@ from __future__ import unicode_literals import os.path +import pipes from pre_commit.languages.ruby import _install_rbenv +from pre_commit.prefix import Prefix +from pre_commit.util import cmd_output from testing.util import xfailif_windows_no_ruby @xfailif_windows_no_ruby -def test_install_rbenv(cmd_runner): - _install_rbenv(cmd_runner) +def test_install_rbenv(tempdir_factory): + prefix = Prefix(tempdir_factory.get()) + _install_rbenv(prefix) # Should have created rbenv directory - assert os.path.exists(cmd_runner.path('rbenv-default')) + assert os.path.exists(prefix.path('rbenv-default')) # We should have created our `activate` script - activate_path = cmd_runner.path('rbenv-default', 'bin', 'activate') + activate_path = prefix.path('rbenv-default', 'bin', 'activate') assert os.path.exists(activate_path) # Should be able to activate using our script and access rbenv - cmd_runner.run( - [ - 'bash', - '-c', - ". '{prefix}rbenv-default/bin/activate' && rbenv --help", - ], + cmd_output( + 'bash', '-c', + '. {} && rbenv --help'.format(pipes.quote(prefix.path( + 'rbenv-default', 'bin', 'activate', + ))), ) @xfailif_windows_no_ruby -def test_install_rbenv_with_version(cmd_runner): - _install_rbenv(cmd_runner, version='1.9.3p547') +def test_install_rbenv_with_version(tempdir_factory): + prefix = Prefix(tempdir_factory.get()) + _install_rbenv(prefix, version='1.9.3p547') # Should be able to activate and use rbenv install - cmd_runner.run( - [ - 'bash', - '-c', - ". '{prefix}rbenv-1.9.3p547/bin/activate' && rbenv install --help", - ], + cmd_output( + 'bash', '-c', + '. {} && rbenv install --help'.format(pipes.quote(prefix.path( + 'rbenv-1.9.3p547', 'bin', 'activate', + ))), ) diff --git a/tests/prefix_test.py b/tests/prefix_test.py new file mode 100644 index 00000000..05f3f8a4 --- /dev/null +++ b/tests/prefix_test.py @@ -0,0 +1,57 @@ +from __future__ import unicode_literals + +import os + +import pytest + +from pre_commit.prefix import Prefix + + +def norm_slash(*args): + return tuple(x.replace('/', os.sep) for x in args) + + +@pytest.mark.parametrize( + ('input', 'expected_prefix'), ( + norm_slash('.', './'), + norm_slash('foo', 'foo/'), + norm_slash('bar/', 'bar/'), + norm_slash('foo/bar', 'foo/bar/'), + norm_slash('foo/bar/', 'foo/bar/'), + ), +) +def test_init_normalizes_path_endings(input, expected_prefix): + instance = Prefix(input) + assert instance.prefix_dir == expected_prefix + + +PATH_TESTS = ( + norm_slash('foo', '', 'foo'), + norm_slash('foo', 'bar', 'foo/bar'), + norm_slash('foo/bar', '../baz', 'foo/baz'), + norm_slash('./', 'bar', 'bar'), + norm_slash('./', '', '.'), + norm_slash('/tmp/foo', '/tmp/bar', '/tmp/bar'), +) + + +@pytest.mark.parametrize(('prefix', 'path_end', 'expected_output'), PATH_TESTS) +def test_path(prefix, path_end, expected_output): + instance = Prefix(prefix) + ret = instance.path(path_end) + assert ret == expected_output + + +def test_path_multiple_args(): + instance = Prefix('foo') + ret = instance.path('bar', 'baz') + assert ret == os.path.join('foo', 'bar', 'baz') + + +def test_exists_does_not_exist(tmpdir): + assert not Prefix(str(tmpdir)).exists('foo') + + +def test_exists_does_exist(tmpdir): + tmpdir.ensure('foo') + assert Prefix(str(tmpdir)).exists('foo') diff --git a/tests/prefixed_command_runner_test.py b/tests/prefixed_command_runner_test.py deleted file mode 100644 index c928dc8a..00000000 --- a/tests/prefixed_command_runner_test.py +++ /dev/null @@ -1,133 +0,0 @@ -from __future__ import unicode_literals - -import os -import subprocess - -import mock -import pytest - -from pre_commit.prefixed_command_runner import PrefixedCommandRunner -from pre_commit.util import CalledProcessError - - -def norm_slash(input_tup): - return tuple(x.replace('/', os.sep) for x in input_tup) - - -def test_CalledProcessError_str(): - error = CalledProcessError( - 1, [str('git'), str('status')], 0, (str('stdout'), str('stderr')), - ) - assert str(error) == ( - "Command: ['git', 'status']\n" - "Return code: 1\n" - "Expected return code: 0\n" - "Output: \n" - " stdout\n" - "Errors: \n" - " stderr\n" - ) - - -def test_CalledProcessError_str_nooutput(): - error = CalledProcessError( - 1, [str('git'), str('status')], 0, (str(''), str('')), - ) - assert str(error) == ( - "Command: ['git', 'status']\n" - "Return code: 1\n" - "Expected return code: 0\n" - "Output: (none)\n" - "Errors: (none)\n" - ) - - -@pytest.fixture -def popen_mock(): - popen = mock.Mock(spec=subprocess.Popen) - popen.return_value.communicate.return_value = (b'stdout', b'stderr') - return popen - - -@pytest.fixture -def makedirs_mock(): - return mock.Mock(spec=os.makedirs) - - -@pytest.mark.parametrize( - ('input', 'expected_prefix'), ( - norm_slash(('.', './')), - norm_slash(('foo', 'foo/')), - norm_slash(('bar/', 'bar/')), - norm_slash(('foo/bar', 'foo/bar/')), - norm_slash(('foo/bar/', 'foo/bar/')), - ), -) -def test_init_normalizes_path_endings(input, expected_prefix): - input = input.replace('/', os.sep) - expected_prefix = expected_prefix.replace('/', os.sep) - instance = PrefixedCommandRunner(input) - assert instance.prefix_dir == expected_prefix - - -def test_run_substitutes_prefix(popen_mock, makedirs_mock): - instance = PrefixedCommandRunner( - 'prefix', popen=popen_mock, makedirs=makedirs_mock, - ) - ret = instance.run(['{prefix}bar', 'baz'], retcode=None) - popen_mock.assert_called_once_with( - (str(os.path.join('prefix', 'bar')), str('baz')), - env=None, - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - ) - assert ret == (popen_mock.return_value.returncode, 'stdout', 'stderr') - - -PATH_TESTS = ( - norm_slash(('foo', '', 'foo')), - norm_slash(('foo', 'bar', 'foo/bar')), - norm_slash(('foo/bar', '../baz', 'foo/baz')), - norm_slash(('./', 'bar', 'bar')), - norm_slash(('./', '', '.')), - norm_slash(('/tmp/foo', '/tmp/bar', '/tmp/bar')), -) - - -@pytest.mark.parametrize(('prefix', 'path_end', 'expected_output'), PATH_TESTS) -def test_path(prefix, path_end, expected_output): - instance = PrefixedCommandRunner(prefix) - ret = instance.path(path_end) - assert ret == expected_output - - -def test_path_multiple_args(): - instance = PrefixedCommandRunner('foo') - ret = instance.path('bar', 'baz') - assert ret == os.path.join('foo', 'bar', 'baz') - - -def test_create_path_if_not_exists(in_tmpdir): - instance = PrefixedCommandRunner('foo') - assert not os.path.exists('foo') - instance._create_path_if_not_exists() - assert os.path.exists('foo') - - -def test_exists_does_not_exist(in_tmpdir): - assert not PrefixedCommandRunner('.').exists('foo') - - -def test_exists_does_exist(in_tmpdir): - os.mkdir('foo') - assert PrefixedCommandRunner('.').exists('foo') - - -def test_raises_on_error(popen_mock, makedirs_mock): - popen_mock.return_value.returncode = 1 - with pytest.raises(CalledProcessError): - instance = PrefixedCommandRunner( - '.', popen=popen_mock, makedirs=makedirs_mock, - ) - instance.run(['echo']) diff --git a/tests/repository_test.py b/tests/repository_test.py index 1d38d246..1c518eba 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -191,7 +191,7 @@ def test_run_a_docker_image_hook(tempdir_factory, store, hook_id): def test_run_a_node_hook(tempdir_factory, store): _test_hook_repo( tempdir_factory, store, 'node_hooks_repo', - 'foo', ['/dev/null'], b'Hello World\n', + 'foo', [os.devnull], b'Hello World\n', ) @@ -200,7 +200,7 @@ def test_run_a_node_hook(tempdir_factory, store): def test_run_versioned_node_hook(tempdir_factory, store): _test_hook_repo( tempdir_factory, store, 'node_0_11_8_hooks_repo', - 'node-11-8-hook', ['/dev/null'], b'v0.11.8\nHello World\n', + 'node-11-8-hook', [os.devnull], b'v0.11.8\nHello World\n', ) @@ -209,7 +209,7 @@ def test_run_versioned_node_hook(tempdir_factory, store): def test_run_a_ruby_hook(tempdir_factory, store): _test_hook_repo( tempdir_factory, store, 'ruby_hooks_repo', - 'ruby_hook', ['/dev/null'], b'Hello world from a ruby hook\n', + 'ruby_hook', [os.devnull], b'Hello world from a ruby hook\n', ) @@ -219,7 +219,7 @@ def test_run_versioned_ruby_hook(tempdir_factory, store): _test_hook_repo( tempdir_factory, store, 'ruby_versioned_hooks_repo', 'ruby_hook', - ['/dev/null'], + [os.devnull], b'2.1.5\nHello world from a ruby hook\n', ) @@ -242,7 +242,7 @@ def test_run_ruby_hook_with_disable_shared_gems( _test_hook_repo( tempdir_factory, store, 'ruby_versioned_hooks_repo', 'ruby_hook', - ['/dev/null'], + [os.devnull], b'2.1.5\nHello world from a ruby hook\n', ) @@ -251,7 +251,7 @@ def test_run_ruby_hook_with_disable_shared_gems( def test_system_hook_with_spaces(tempdir_factory, store): _test_hook_repo( tempdir_factory, store, 'system_hook_with_spaces_repo', - 'system-hook-with-spaces', ['/dev/null'], b'Hello World\n', + 'system-hook-with-spaces', [os.devnull], b'Hello World\n', ) @@ -276,7 +276,7 @@ def test_golang_hook(tempdir_factory, store): def test_missing_executable(tempdir_factory, store): _test_hook_repo( tempdir_factory, store, 'not_found_exe', - 'not-found-exe', ['/dev/null'], + 'not-found-exe', [os.devnull], b'Executable `i-dont-exist-lol` not found', expected_return_code=1, ) @@ -424,7 +424,7 @@ def test_cwd_of_hook(tempdir_factory, store): def test_lots_of_files(tempdir_factory, store): _test_hook_repo( tempdir_factory, store, 'script_hooks_repo', - 'bash_hook', ['/dev/null'] * 15000, mock.ANY, + 'bash_hook', [os.devnull] * 15000, mock.ANY, ) @@ -467,7 +467,7 @@ def test_additional_python_dependencies_installed(tempdir_factory, store): config['hooks'][0]['additional_dependencies'] = ['mccabe'] repo = Repository.create(config, store) repo.require_installed() - with python.in_env(repo._cmd_runner, 'default'): + with python.in_env(repo._prefix, 'default'): output = cmd_output('pip', 'freeze', '-l')[1] assert 'mccabe' in output @@ -484,7 +484,7 @@ def test_additional_dependencies_roll_forward(tempdir_factory, store): repo = Repository.create(config, store) repo.require_installed() # We should see our additional dependency installed - with python.in_env(repo._cmd_runner, 'default'): + with python.in_env(repo._prefix, 'default'): output = cmd_output('pip', 'freeze', '-l')[1] assert 'mccabe' in output @@ -499,7 +499,7 @@ def test_additional_ruby_dependencies_installed( config['hooks'][0]['additional_dependencies'] = ['thread_safe', 'tins'] repo = Repository.create(config, store) repo.require_installed() - with ruby.in_env(repo._cmd_runner, 'default'): + with ruby.in_env(repo._prefix, 'default'): output = cmd_output('gem', 'list', '--local')[1] assert 'thread_safe' in output assert 'tins' in output @@ -516,7 +516,7 @@ def test_additional_node_dependencies_installed( config['hooks'][0]['additional_dependencies'] = ['lodash'] repo = Repository.create(config, store) repo.require_installed() - with node.in_env(repo._cmd_runner, 'default'): + with node.in_env(repo._prefix, 'default'): cmd_output('npm', 'config', 'set', 'global', 'true') output = cmd_output('npm', 'ls')[1] assert 'lodash' in output @@ -533,7 +533,7 @@ def test_additional_golang_dependencies_installed( config['hooks'][0]['additional_dependencies'] = deps repo = Repository.create(config, store) repo.require_installed() - binaries = os.listdir(repo._cmd_runner.path( + binaries = os.listdir(repo._prefix.path( helpers.environment_dir(golang.ENVIRONMENT_DIR, 'default'), 'bin', )) # normalize for windows @@ -600,7 +600,7 @@ def test_control_c_control_c_on_install(tempdir_factory, store): # Should have made an environment, however this environment is broken! envdir = 'py_env-{}'.format(python.get_default_version()) - assert repo._cmd_runner.exists(envdir) + assert repo._prefix.exists(envdir) # However, it should be perfectly runnable (reinstall after botched # install) @@ -618,7 +618,7 @@ def test_invalidated_virtualenv(tempdir_factory, store): # Simulate breaking of the virtualenv repo.require_installed() version = python.get_default_version() - libdir = repo._cmd_runner.path('py_env-{}'.format(version), 'lib', version) + libdir = repo._prefix.path('py_env-{}'.format(version), 'lib', version) paths = [ os.path.join(libdir, p) for p in ('site.py', 'site.pyc', '__pycache__') ] diff --git a/tests/util_test.py b/tests/util_test.py index ba2b4a82..156148d5 100644 --- a/tests/util_test.py +++ b/tests/util_test.py @@ -5,6 +5,7 @@ import random import pytest +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 cwd @@ -12,6 +13,34 @@ from pre_commit.util import memoize_by_cwd from pre_commit.util import tmpdir +def test_CalledProcessError_str(): + error = CalledProcessError( + 1, [str('git'), str('status')], 0, (str('stdout'), str('stderr')), + ) + assert str(error) == ( + "Command: ['git', 'status']\n" + "Return code: 1\n" + "Expected return code: 0\n" + "Output: \n" + " stdout\n" + "Errors: \n" + " stderr\n" + ) + + +def test_CalledProcessError_str_nooutput(): + error = CalledProcessError( + 1, [str('git'), str('status')], 0, (str(''), str('')), + ) + assert str(error) == ( + "Command: ['git', 'status']\n" + "Return code: 1\n" + "Expected return code: 0\n" + "Output: (none)\n" + "Errors: (none)\n" + ) + + @pytest.fixture def memoized_by_cwd(): @memoize_by_cwd