From cff98a634dc8405e985cb04fe7230c6fe6171b47 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 19 Jul 2017 14:23:39 -0700 Subject: [PATCH] Recover from invalid python virtualenvs --- pre_commit/languages/all.py | 3 +++ pre_commit/languages/docker.py | 1 + pre_commit/languages/golang.py | 1 + pre_commit/languages/helpers.py | 4 ++++ pre_commit/languages/node.py | 1 + pre_commit/languages/pcre.py | 1 + pre_commit/languages/python.py | 10 +++++++++- pre_commit/languages/ruby.py | 1 + pre_commit/languages/script.py | 1 + pre_commit/languages/swift.py | 1 + pre_commit/languages/system.py | 1 + pre_commit/repository.py | 12 +++++++----- tests/languages/all_test.py | 10 ++++++++++ tests/languages/helpers_test.py | 12 ++++++++++++ tests/repository_test.py | 23 +++++++++++++++++++++++ 15 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 tests/languages/helpers_test.py diff --git a/pre_commit/languages/all.py b/pre_commit/languages/all.py index 5546025d..5de57fb8 100644 --- a/pre_commit/languages/all.py +++ b/pre_commit/languages/all.py @@ -21,6 +21,9 @@ from pre_commit.languages import system # return 'default' if there is no better option. # """ # +# def healthy(repo_cmd_runner, language_version): +# """Return whether or not the environment is considered functional.""" +# # def install_environment(repo_cmd_runner, version, additional_dependencies): # """Installs a repository in the given repository. Note that the current # working directory will already be inside the repository. diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index 8404ac84..a9a0d342 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -15,6 +15,7 @@ from pre_commit.xargs import xargs ENVIRONMENT_DIR = 'docker' PRE_COMMIT_LABEL = 'PRE_COMMIT' get_default_version = helpers.basic_get_default_version +healthy = helpers.basic_healthy def md5(s): # pragma: windows no cover diff --git a/pre_commit/languages/golang.py b/pre_commit/languages/golang.py index ee04ca79..c091bacf 100644 --- a/pre_commit/languages/golang.py +++ b/pre_commit/languages/golang.py @@ -15,6 +15,7 @@ from pre_commit.xargs import xargs ENVIRONMENT_DIR = 'golangenv' get_default_version = helpers.basic_get_default_version +healthy = helpers.basic_healthy def get_env_patch(venv): diff --git a/pre_commit/languages/helpers.py b/pre_commit/languages/helpers.py index 6af77e30..930a0755 100644 --- a/pre_commit/languages/helpers.py +++ b/pre_commit/languages/helpers.py @@ -37,3 +37,7 @@ def assert_no_additional_deps(lang, additional_deps): def basic_get_default_version(): return 'default' + + +def basic_healthy(repo_cmd_runner, language_version): + return True diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py index b5f7c56e..69378b06 100644 --- a/pre_commit/languages/node.py +++ b/pre_commit/languages/node.py @@ -13,6 +13,7 @@ from pre_commit.xargs import xargs ENVIRONMENT_DIR = 'node_env' get_default_version = helpers.basic_get_default_version +healthy = helpers.basic_healthy def get_env_patch(venv): # pragma: windows no cover diff --git a/pre_commit/languages/pcre.py b/pre_commit/languages/pcre.py index faba5395..6ef373f0 100644 --- a/pre_commit/languages/pcre.py +++ b/pre_commit/languages/pcre.py @@ -9,6 +9,7 @@ from pre_commit.xargs import xargs ENVIRONMENT_DIR = None GREP = 'ggrep' if sys.platform == 'darwin' else 'grep' get_default_version = helpers.basic_get_default_version +healthy = helpers.basic_healthy def install_environment(repo_cmd_runner, version, additional_dependencies): diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index 11f37765..7800e17a 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -10,11 +10,11 @@ from pre_commit.envcontext import Var from pre_commit.languages import helpers from pre_commit.parse_shebang import find_executable from pre_commit.util import clean_path_on_failure +from pre_commit.util import cmd_output from pre_commit.xargs import xargs ENVIRONMENT_DIR = 'py_env' -get_default_version = helpers.basic_get_default_version def bin_dir(venv): @@ -83,6 +83,14 @@ def get_default_version(): return get_default_version() +def healthy(repo_cmd_runner, language_version): + with in_env(repo_cmd_runner, language_version): + retcode, _, _ = cmd_output( + 'python', '-c', 'import datetime, io, os, weakref', retcode=None, + ) + return retcode == 0 + + def norm_version(version): if os.name == 'nt': # pragma: no cover (windows) # Try looking up by name diff --git a/pre_commit/languages/ruby.py b/pre_commit/languages/ruby.py index 41c13a87..e7e0c328 100644 --- a/pre_commit/languages/ruby.py +++ b/pre_commit/languages/ruby.py @@ -17,6 +17,7 @@ from pre_commit.xargs import xargs ENVIRONMENT_DIR = 'rbenv' get_default_version = helpers.basic_get_default_version +healthy = helpers.basic_healthy def get_env_patch(venv, language_version): # pragma: windows no cover diff --git a/pre_commit/languages/script.py b/pre_commit/languages/script.py index c4b6593d..0bbb3091 100644 --- a/pre_commit/languages/script.py +++ b/pre_commit/languages/script.py @@ -6,6 +6,7 @@ from pre_commit.xargs import xargs ENVIRONMENT_DIR = None get_default_version = helpers.basic_get_default_version +healthy = helpers.basic_healthy def install_environment(repo_cmd_runner, version, additional_dependencies): diff --git a/pre_commit/languages/swift.py b/pre_commit/languages/swift.py index a27dfac2..f4d1eb5a 100644 --- a/pre_commit/languages/swift.py +++ b/pre_commit/languages/swift.py @@ -11,6 +11,7 @@ from pre_commit.xargs import xargs ENVIRONMENT_DIR = 'swift_env' get_default_version = helpers.basic_get_default_version +healthy = helpers.basic_healthy BUILD_DIR = '.build' BUILD_CONFIG = 'release' diff --git a/pre_commit/languages/system.py b/pre_commit/languages/system.py index 31480792..1f1688d8 100644 --- a/pre_commit/languages/system.py +++ b/pre_commit/languages/system.py @@ -6,6 +6,7 @@ from pre_commit.xargs import xargs ENVIRONMENT_DIR = None get_default_version = helpers.basic_get_default_version +healthy = helpers.basic_healthy def install_environment(repo_cmd_runner, version, additional_dependencies): diff --git a/pre_commit/repository.py b/pre_commit/repository.py index fcc79bd6..d2d30dfc 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -36,7 +36,7 @@ def _state_filename(cmd_runner, venv): ) -def _read_installed_state(cmd_runner, venv): +def _read_state(cmd_runner, venv): filename = _state_filename(cmd_runner, venv) if not os.path.exists(filename): return None @@ -44,7 +44,7 @@ def _read_installed_state(cmd_runner, venv): return json.loads(io.open(filename).read()) -def _write_installed_state(cmd_runner, venv, state): +def _write_state(cmd_runner, venv, state): state_filename = _state_filename(cmd_runner, venv) staging = state_filename + 'staging' with io.open(staging, 'w') as state_file: @@ -57,8 +57,10 @@ def _installed(cmd_runner, language_name, language_version, additional_deps): language = languages[language_name] venv = environment_dir(language.ENVIRONMENT_DIR, language_version) return ( - venv is None or - _read_installed_state(cmd_runner, venv) == _state(additional_deps) + venv is None or ( + _read_state(cmd_runner, venv) == _state(additional_deps) and + language.healthy(cmd_runner, language_version) + ) ) @@ -89,7 +91,7 @@ def _install_all(venvs, repo_url): language.install_environment(cmd_runner, version, deps) # Write our state to indicate we're installed state = _state(deps) - _write_installed_state(cmd_runner, venv, state) + _write_state(cmd_runner, venv, state) def _validate_minimum_version(hook): diff --git a/tests/languages/all_test.py b/tests/languages/all_test.py index dd1ed27b..95cec104 100644 --- a/tests/languages/all_test.py +++ b/tests/languages/all_test.py @@ -40,3 +40,13 @@ def test_get_default_version_argspec(language): ) argspec = inspect.getargspec(languages[language].get_default_version) assert argspec == expected_argspec + + +@pytest.mark.parametrize('language', all_languages) +def test_healthy_argspec(language): + expected_argspec = inspect.ArgSpec( + args=['repo_cmd_runner', 'language_version'], + varargs=None, keywords=None, defaults=None, + ) + argspec = inspect.getargspec(languages[language].healthy) + assert argspec == expected_argspec diff --git a/tests/languages/helpers_test.py b/tests/languages/helpers_test.py new file mode 100644 index 00000000..7019e260 --- /dev/null +++ b/tests/languages/helpers_test.py @@ -0,0 +1,12 @@ +from __future__ import absolute_import +from __future__ import unicode_literals + +from pre_commit.languages import helpers + + +def test_basic_get_default_version(): + assert helpers.basic_get_default_version() == 'default' + + +def test_basic_healthy(): + assert helpers.basic_healthy(None, None) is True diff --git a/tests/repository_test.py b/tests/repository_test.py index 9096161e..80b4ccf7 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -600,6 +600,29 @@ def test_control_c_control_c_on_install(tempdir_factory, store): assert retv == 0 +def test_invalidated_virtualenv(tempdir_factory, store): + # A cached virtualenv may become invalidated if the system python upgrades + # This should not cause every hook in that virtualenv to fail. + path = make_repo(tempdir_factory, 'python_hooks_repo') + config = make_config_from_repo(path) + repo = Repository.create(config, 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) + paths = [ + os.path.join(libdir, p) for p in ('site.py', 'site.pyc', '__pycache__') + ] + cmd_output('rm', '-rf', *paths) + + # pre-commit should rebuild the virtualenv and it should be runnable + repo = Repository.create(config, store) + hook = repo.hooks[0][1] + retv, stdout, stderr = repo.run_hook(hook, []) + assert retv == 0 + + @pytest.mark.integration def test_really_long_file_paths(tempdir_factory, store): base_path = tempdir_factory.get()