From 5fb721f7a7415e5ba614710c6538caf30f7aed1c Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 8 Jun 2020 13:29:31 -0700 Subject: [PATCH 01/25] normalize slashes even earlier on windows for filenames --- pre_commit/commands/run.py | 27 ++++++++++++------- pre_commit/meta_hooks/check_hooks_apply.py | 7 +++-- .../meta_hooks/check_useless_excludes.py | 7 +++-- tests/commands/run_test.py | 4 +-- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index f6916c26..567b7cd3 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -72,13 +72,7 @@ def filter_by_include_exclude( class Classifier: - def __init__(self, filenames: Sequence[str]) -> None: - # on windows we normalize all filenames to use forward slashes - # this makes it easier to filter using the `files:` regex - # this also makes improperly quoted shell-based hooks work better - # see #1173 - if os.altsep == '/' and os.sep == '\\': - filenames = [f.replace(os.sep, os.altsep) for f in filenames] + def __init__(self, filenames: Collection[str]) -> None: self.filenames = [f for f in filenames if os.path.lexists(f)] @functools.lru_cache(maxsize=None) @@ -105,6 +99,22 @@ class Classifier: names = self.by_types(names, hook.types, hook.exclude_types) return tuple(names) + @classmethod + def from_config( + cls, + filenames: Collection[str], + include: str, + exclude: str, + ) -> 'Classifier': + # on windows we normalize all filenames to use forward slashes + # this makes it easier to filter using the `files:` regex + # this also makes improperly quoted shell-based hooks work better + # see #1173 + if os.altsep == '/' and os.sep == '\\': + filenames = [f.replace(os.sep, os.altsep) for f in filenames] + filenames = filter_by_include_exclude(filenames, include, exclude) + return Classifier(filenames) + def _get_skips(environ: EnvironT) -> Set[str]: skips = environ.get('SKIP', '') @@ -247,10 +257,9 @@ def _run_hooks( """Actually run the hooks.""" skips = _get_skips(environ) cols = _compute_cols(hooks) - filenames = filter_by_include_exclude( + classifier = Classifier.from_config( _all_filenames(args), config['files'], config['exclude'], ) - classifier = Classifier(filenames) retval = 0 for hook in hooks: retval |= _run_single_hook( diff --git a/pre_commit/meta_hooks/check_hooks_apply.py b/pre_commit/meta_hooks/check_hooks_apply.py index d0244a94..a1e93529 100644 --- a/pre_commit/meta_hooks/check_hooks_apply.py +++ b/pre_commit/meta_hooks/check_hooks_apply.py @@ -11,10 +11,13 @@ from pre_commit.store import Store def check_all_hooks_match_files(config_file: str) -> int: - classifier = Classifier(git.get_all_files()) + config = load_config(config_file) + classifier = Classifier.from_config( + git.get_all_files(), config['files'], config['exclude'], + ) retv = 0 - for hook in all_hooks(load_config(config_file), Store()): + for hook in all_hooks(config, Store()): if hook.always_run or hook.language == 'fail': continue elif not classifier.filenames_for_hook(hook): diff --git a/pre_commit/meta_hooks/check_useless_excludes.py b/pre_commit/meta_hooks/check_useless_excludes.py index 30b8d810..db6865c6 100644 --- a/pre_commit/meta_hooks/check_useless_excludes.py +++ b/pre_commit/meta_hooks/check_useless_excludes.py @@ -28,11 +28,14 @@ def exclude_matches_any( def check_useless_excludes(config_file: str) -> int: config = load_config(config_file) - classifier = Classifier(git.get_all_files()) + filenames = git.get_all_files() + classifier = Classifier.from_config( + filenames, config['files'], config['exclude'], + ) retv = 0 exclude = config['exclude'] - if not exclude_matches_any(classifier.filenames, '', exclude): + if not exclude_matches_any(filenames, '', exclude): print( f'The global exclude pattern {exclude!r} does not match any files', ) diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index cf9794ed..2461ed5b 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -939,7 +939,7 @@ def test_classifier_normalizes_filenames_on_windows_to_forward_slashes(tmpdir): tmpdir.join('a/b/c').ensure() with mock.patch.object(os, 'altsep', '/'): with mock.patch.object(os, 'sep', '\\'): - classifier = Classifier((r'a\b\c',)) + classifier = Classifier.from_config((r'a\b\c',), '', '^$') assert classifier.filenames == ['a/b/c'] @@ -947,7 +947,7 @@ def test_classifier_does_not_normalize_backslashes_non_windows(tmpdir): with mock.patch.object(os.path, 'lexists', return_value=True): with mock.patch.object(os, 'altsep', None): with mock.patch.object(os, 'sep', '/'): - classifier = Classifier((r'a/b\c',)) + classifier = Classifier.from_config((r'a/b\c',), '', '^$') assert classifier.filenames == [r'a/b\c'] From 2f25085d60bf953fea457a3240c91886145dbcc5 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 8 Jun 2020 15:17:13 -0700 Subject: [PATCH 02/25] v2.5.0 --- CHANGELOG.md | 21 +++++++++++++++++++++ setup.cfg | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75abd063..55bbd3ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,24 @@ +2.5.0 - 2020-06-08 +================== + +### Features +- Expose a `PRE_COMMIT=1` environment variable when running hooks + - #1467 PR by @tech-chad. + - #1426 issue by @lorenzwalthert. + +### Fixes +- Fix `UnicodeDecodeError` on windows when using the `py` launcher to detect + executables with non-ascii characters in the path + - #1474 PR by @asottile. + - #1472 issue by DrFobos. +- Fix `DeprecationWarning` on python3.9 for `random.shuffle` method + - #1480 PR by @asottile. + - #1479 issue by @isidentical. +- Normalize slashes earlier such that global `files` / `exclude` use forward + slashes on windows as well. + - #1494 PR by @asottile. + - #1476 issue by @harrybiddle. + 2.4.0 - 2020-05-11 ================== diff --git a/setup.cfg b/setup.cfg index 08aae264..03e31d1e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = pre_commit -version = 2.4.0 +version = 2.5.0 description = A framework for managing and maintaining multi-language pre-commit hooks. long_description = file: README.md long_description_content_type = text/markdown From 6ee9e13b2686f66c0e796947dfcb775201b1c3d6 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Tue, 9 Jun 2020 09:45:40 -0700 Subject: [PATCH 03/25] prevent infinite recursion of post-checkout on clone --- pre_commit/git.py | 3 ++- tests/git_test.py | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pre_commit/git.py b/pre_commit/git.py index 7e757f24..576bef8c 100644 --- a/pre_commit/git.py +++ b/pre_commit/git.py @@ -158,7 +158,8 @@ def init_repo(path: str, remote: str) -> None: remote = os.path.abspath(remote) env = no_git_env() - cmd_output_b('git', 'init', path, env=env) + # avoid the user's template so that hooks do not recurse + cmd_output_b('git', 'init', '--template=', path, env=env) cmd_output_b('git', 'remote', 'add', 'origin', remote, cwd=path, env=env) diff --git a/tests/git_test.py b/tests/git_test.py index e73a6f24..fafd4a6e 100644 --- a/tests/git_test.py +++ b/tests/git_test.py @@ -186,3 +186,8 @@ def test_no_git_env(): 'GIT_SSH': '/usr/bin/ssh', 'GIT_SSH_COMMAND': 'ssh -o', } + + +def test_init_repo_no_hooks(tmpdir): + git.init_repo(str(tmpdir), remote='dne') + assert not tmpdir.join('.git/hooks').exists() From 0e5eb199292d44107a591f8bf886874f18f0a304 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Tue, 9 Jun 2020 14:18:42 -0700 Subject: [PATCH 04/25] v2.5.1 --- CHANGELOG.md | 8 ++++++++ setup.cfg | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55bbd3ce..375a9f3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +2.5.1 - 2020-06-09 +================== + +### Fixes +- Prevent infinite recursion of post-checkout on clone + - #1497 PR by @asottile. + - #1496 issue by @admorgan. + 2.5.0 - 2020-06-08 ================== diff --git a/setup.cfg b/setup.cfg index 03e31d1e..f1ce18d6 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = pre_commit -version = 2.5.0 +version = 2.5.1 description = A framework for managing and maintaining multi-language pre-commit hooks. long_description = file: README.md long_description_content_type = text/markdown From fd53cdea17ed17a1775fc5e23e75d6ecdbdb04b6 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 12 Jun 2020 13:26:29 +0200 Subject: [PATCH 05/25] Add foor missing required dependencies in CONTRIBUTING.md There could still be missing dependencies, I'm not using a fresh environement to test that. Also added the specific required git version see https://github.com/git/git/blob/master/Documentation/RelNotes/2.24.0.txt --- CONTRIBUTING.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d70a89dd..76df4370 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,12 +4,16 @@ - The complete test suite depends on having at least the following installed (possibly not a complete list) - - git (A sufficiently newer version is required to run pre-push tests) + - git (Version 2.24.0 or above is required to run pre-merge-commit tests) - python2 (Required by a test which checks different python versions) - python3 (Required by a test which checks different python versions) - tox (or virtualenv) - ruby + gem - docker + - conda + - cargo (required by tests for rust dependencies) + - go (required by tests for go dependencies) + - swift ### Setting up an environment From e1e6a32c512275e4f7bf7a057bc5d4baa27303fa Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 15 Jun 2020 13:50:47 -0700 Subject: [PATCH 06/25] skip rbenv if ruby and gem are installed with default language_version --- azure-pipelines.yml | 1 + pre_commit/languages/ruby.py | 73 +++++++++++++++++++++++------------- testing/util.py | 4 -- tests/languages/ruby_test.py | 32 ++++++++++++++-- tests/repository_test.py | 7 +--- 5 files changed, 78 insertions(+), 39 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index c21843e1..fb400107 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -19,6 +19,7 @@ jobs: toxenvs: [py37] os: windows pre_test: + - task: UseRubyVersion@0 - powershell: Write-Host "##vso[task.prependpath]$env:CONDA\Scripts" displayName: Add conda to PATH - powershell: | diff --git a/pre_commit/languages/ruby.py b/pre_commit/languages/ruby.py index fe524ec3..73b23cc0 100644 --- a/pre_commit/languages/ruby.py +++ b/pre_commit/languages/ruby.py @@ -1,4 +1,5 @@ import contextlib +import functools import os.path import shutil import tarfile @@ -7,6 +8,7 @@ from typing import Sequence from typing import Tuple import pre_commit.constants as C +from pre_commit import parse_shebang from pre_commit.envcontext import envcontext from pre_commit.envcontext import PatchesT from pre_commit.envcontext import UNSET @@ -19,33 +21,51 @@ from pre_commit.util import clean_path_on_failure from pre_commit.util import resource_bytesio ENVIRONMENT_DIR = 'rbenv' -get_default_version = helpers.basic_get_default_version healthy = helpers.basic_healthy +@functools.lru_cache(maxsize=1) +def get_default_version() -> str: + if all(parse_shebang.find_executable(exe) for exe in ('ruby', 'gem')): + return 'system' + else: + return C.DEFAULT + + def get_env_patch( venv: str, language_version: str, -) -> PatchesT: # pragma: win32 no cover +) -> PatchesT: patches: PatchesT = ( ('GEM_HOME', os.path.join(venv, 'gems')), ('GEM_PATH', UNSET), - ('RBENV_ROOT', venv), ('BUNDLE_IGNORE_CONFIG', '1'), - ( - 'PATH', ( - os.path.join(venv, 'gems', 'bin'), os.pathsep, - os.path.join(venv, 'shims'), os.pathsep, - os.path.join(venv, 'bin'), os.pathsep, Var('PATH'), - ), - ), ) - if language_version != C.DEFAULT: - patches += (('RBENV_VERSION', language_version),) + if language_version == 'system': + patches += ( + ( + 'PATH', ( + os.path.join(venv, 'gems', 'bin'), os.pathsep, + Var('PATH'), + ), + ), + ) + else: # pragma: win32 no cover + patches += ( + ('RBENV_ROOT', venv), + ('RBENV_VERSION', language_version), + ( + 'PATH', ( + os.path.join(venv, 'gems', 'bin'), os.pathsep, + os.path.join(venv, 'shims'), os.pathsep, + os.path.join(venv, 'bin'), os.pathsep, Var('PATH'), + ), + ), + ) return patches -@contextlib.contextmanager # pragma: win32 no cover +@contextlib.contextmanager def in_env( prefix: Prefix, language_version: str, @@ -65,7 +85,7 @@ def _extract_resource(filename: str, dest: str) -> None: def _install_rbenv( prefix: Prefix, - version: str = C.DEFAULT, + version: str, ) -> None: # pragma: win32 no cover directory = helpers.environment_dir(ENVIRONMENT_DIR, version) @@ -92,21 +112,22 @@ def _install_ruby( def install_environment( prefix: Prefix, version: str, additional_dependencies: Sequence[str], -) -> None: # pragma: win32 no cover +) -> None: additional_dependencies = tuple(additional_dependencies) directory = helpers.environment_dir(ENVIRONMENT_DIR, version) 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(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(prefix, ('rbenv', 'init', '-')) - if version != C.DEFAULT: + if version != 'system': # pragma: win32 no cover + _install_rbenv(prefix, version) + with in_env(prefix, version): + # Need to call this before installing so rbenv's directories + # are set up + helpers.run_setup_cmd(prefix, ('rbenv', 'init', '-')) + # XXX: this will *always* fail if `version == C.DEFAULT` _install_ruby(prefix, version) - # Need to call this after installing to set up the shims - helpers.run_setup_cmd(prefix, ('rbenv', 'rehash')) + # Need to call this after installing to set up the shims + helpers.run_setup_cmd(prefix, ('rbenv', 'rehash')) + + with in_env(prefix, version): helpers.run_setup_cmd( prefix, ('gem', 'build', *prefix.star('.gemspec')), ) @@ -123,6 +144,6 @@ def run_hook( hook: Hook, file_args: Sequence[str], color: bool, -) -> Tuple[int, bytes]: # pragma: win32 no cover +) -> Tuple[int, bytes]: with in_env(hook.prefix, hook.language_version): return helpers.run_xargs(hook, hook.cmd, file_args, color=color) diff --git a/testing/util.py b/testing/util.py index bfe14218..4edb7a9e 100644 --- a/testing/util.py +++ b/testing/util.py @@ -38,10 +38,6 @@ skipif_cant_run_swift = pytest.mark.skipif( parse_shebang.find_executable('swift') is None, reason="swift isn't installed or can't be found", ) -xfailif_windows_no_ruby = pytest.mark.xfail( - os.name == 'nt', - reason='Ruby support not yet implemented on windows.', -) xfailif_windows = pytest.mark.xfail(os.name == 'nt', reason='windows') diff --git a/tests/languages/ruby_test.py b/tests/languages/ruby_test.py index 36a029d1..853bb732 100644 --- a/tests/languages/ruby_test.py +++ b/tests/languages/ruby_test.py @@ -1,15 +1,39 @@ import os.path +from unittest import mock +import pytest + +import pre_commit.constants as C +from pre_commit import parse_shebang from pre_commit.languages import ruby from pre_commit.prefix import Prefix from pre_commit.util import cmd_output -from testing.util import xfailif_windows_no_ruby +from testing.util import xfailif_windows -@xfailif_windows_no_ruby +ACTUAL_GET_DEFAULT_VERSION = ruby.get_default_version.__wrapped__ + + +@pytest.fixture +def find_exe_mck(): + with mock.patch.object(parse_shebang, 'find_executable') as mck: + yield mck + + +def test_uses_default_version_when_not_available(find_exe_mck): + find_exe_mck.return_value = None + assert ACTUAL_GET_DEFAULT_VERSION() == C.DEFAULT + + +def test_uses_system_if_both_gem_and_ruby_are_available(find_exe_mck): + find_exe_mck.return_value = '/path/to/exe' + assert ACTUAL_GET_DEFAULT_VERSION() == 'system' + + +@xfailif_windows # pragma: win32 no cover def test_install_rbenv(tempdir_factory): prefix = Prefix(tempdir_factory.get()) - ruby._install_rbenv(prefix) + ruby._install_rbenv(prefix, C.DEFAULT) # Should have created rbenv directory assert os.path.exists(prefix.path('rbenv-default')) @@ -18,7 +42,7 @@ def test_install_rbenv(tempdir_factory): cmd_output('rbenv', '--help') -@xfailif_windows_no_ruby +@xfailif_windows # pragma: win32 no cover def test_install_rbenv_with_version(tempdir_factory): prefix = Prefix(tempdir_factory.get()) ruby._install_rbenv(prefix, version='1.9.3p547') diff --git a/tests/repository_test.py b/tests/repository_test.py index 2ac78863..d3b3dd55 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -34,7 +34,6 @@ from testing.util import get_resource_path from testing.util import skipif_cant_run_docker from testing.util import skipif_cant_run_swift from testing.util import xfailif_windows -from testing.util import xfailif_windows_no_ruby def _norm_out(b): @@ -260,7 +259,6 @@ def test_run_versioned_node_hook(tempdir_factory, store): ) -@xfailif_windows_no_ruby def test_run_a_ruby_hook(tempdir_factory, store): _test_hook_repo( tempdir_factory, store, 'ruby_hooks_repo', @@ -268,7 +266,7 @@ def test_run_a_ruby_hook(tempdir_factory, store): ) -@xfailif_windows_no_ruby +@xfailif_windows # pragma: win32 no cover def test_run_versioned_ruby_hook(tempdir_factory, store): _test_hook_repo( tempdir_factory, store, 'ruby_versioned_hooks_repo', @@ -278,7 +276,7 @@ def test_run_versioned_ruby_hook(tempdir_factory, store): ) -@xfailif_windows_no_ruby +@xfailif_windows # pragma: win32 no cover def test_run_ruby_hook_with_disable_shared_gems( tempdir_factory, store, @@ -524,7 +522,6 @@ def test_additional_dependencies_roll_forward(tempdir_factory, store): assert 'mccabe' not in cmd_output('pip', 'freeze', '-l')[1] -@xfailif_windows_no_ruby # pragma: win32 no cover def test_additional_ruby_dependencies_installed(tempdir_factory, store): path = make_repo(tempdir_factory, 'ruby_hooks_repo') config = make_config_from_repo(path) From 1392471854f6a2bdf57bafd3b56c9da183fad331 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 17 Jun 2020 12:55:30 -0700 Subject: [PATCH 07/25] xfail a flaky node test on windows --- tests/repository_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/repository_test.py b/tests/repository_test.py index d3b3dd55..ee57d992 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -234,6 +234,7 @@ def test_run_a_docker_image_hook(tempdir_factory, store, hook_id): ) +@xfailif_windows # pragma: win32 no cover def test_run_a_node_hook(tempdir_factory, store): _test_hook_repo( tempdir_factory, store, 'node_hooks_repo', From 6ec47ea73640190d86186fd50b7a32fa0a9c4b9b Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Fri, 19 Jun 2020 13:58:14 -0700 Subject: [PATCH 08/25] fix node hooks when NPM_CONFIG_USERCONFIG is set --- pre_commit/languages/node.py | 3 +++ tests/repository_test.py | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py index 26f4919e..d99e6f2c 100644 --- a/pre_commit/languages/node.py +++ b/pre_commit/languages/node.py @@ -10,6 +10,7 @@ import pre_commit.constants as C from pre_commit import parse_shebang from pre_commit.envcontext import envcontext from pre_commit.envcontext import PatchesT +from pre_commit.envcontext import UNSET from pre_commit.envcontext import Var from pre_commit.hook import Hook from pre_commit.languages import helpers @@ -56,6 +57,8 @@ def get_env_patch(venv: str) -> PatchesT: ('NODE_VIRTUAL_ENV', venv), ('NPM_CONFIG_PREFIX', install_prefix), ('npm_config_prefix', install_prefix), + ('NPM_CONFIG_USERCONFIG', UNSET), + ('npm_config_userconfig', UNSET), ('NODE_PATH', os.path.join(venv, lib_dir, 'node_modules')), ('PATH', (bin_dir(venv), os.pathsep, Var('PATH'))), ) diff --git a/tests/repository_test.py b/tests/repository_test.py index ee57d992..84e4da93 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -260,6 +260,14 @@ def test_run_versioned_node_hook(tempdir_factory, store): ) +@xfailif_windows # pragma: win32 no cover +def test_node_hook_with_npm_userconfig_set(tempdir_factory, store, tmpdir): + cfg = tmpdir.join('cfg') + cfg.write('cache=/dne\n') + with mock.patch.dict(os.environ, NPM_CONFIG_USERCONFIG=str(cfg)): + test_run_a_node_hook(tempdir_factory, store) + + def test_run_a_ruby_hook(tempdir_factory, store): _test_hook_repo( tempdir_factory, store, 'ruby_hooks_repo', From 6fe1702ee106f4d7f4a8ad73550db2145208ef24 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 1 Jul 2020 12:39:34 -0700 Subject: [PATCH 09/25] v2.6.0 --- CHANGELOG.md | 15 +++++++++++++++ setup.cfg | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 375a9f3b..c487acf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,18 @@ +2.6.0 - 2020-07-01 +================== + +### Fixes +- Fix node hooks when `NPM_CONFIG_USERCONFIG` is set + - #1521 PR by @asottile. + - #1516 issue by @rkm. + +### Features +- Skip `rbenv` / `ruby-download` if system ruby is available + - #1509 PR by @asottile. +- Partial support for ruby on windows (if system ruby is installed) + - #1509 PR by @asottile. + - #201 issue by @asottile. + 2.5.1 - 2020-06-09 ================== diff --git a/setup.cfg b/setup.cfg index f1ce18d6..0ce58b1a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = pre_commit -version = 2.5.1 +version = 2.6.0 description = A framework for managing and maintaining multi-language pre-commit hooks. long_description = file: README.md long_description_content_type = text/markdown From c9ad2e14515537c1f7f3857804f69bb7fb05b3b1 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 8 Jul 2020 13:55:28 -0700 Subject: [PATCH 10/25] upgrade mypy to get typeshed fixes --- .pre-commit-config.yaml | 14 +++++++------- pre_commit/file_lock.py | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 36d73c7a..e9cf7394 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,6 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v2.5.0 + rev: v3.1.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer @@ -12,20 +12,20 @@ repos: - id: requirements-txt-fixer - id: double-quote-string-fixer - repo: https://gitlab.com/pycqa/flake8 - rev: 3.8.0 + rev: 3.8.3 hooks: - id: flake8 additional_dependencies: [flake8-typing-imports==1.6.0] - repo: https://github.com/pre-commit/mirrors-autopep8 - rev: v1.5.2 + rev: v1.5.3 hooks: - id: autopep8 - repo: https://github.com/pre-commit/pre-commit - rev: v2.4.0 + rev: v2.6.0 hooks: - id: validate_manifest - repo: https://github.com/asottile/pyupgrade - rev: v2.4.1 + rev: v2.6.2 hooks: - id: pyupgrade args: [--py36-plus] @@ -40,11 +40,11 @@ repos: - id: add-trailing-comma args: [--py36-plus] - repo: https://github.com/asottile/setup-cfg-fmt - rev: v1.9.0 + rev: v1.10.0 hooks: - id: setup-cfg-fmt - repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.770 + rev: v0.782 hooks: - id: mypy exclude: ^testing/resources/ diff --git a/pre_commit/file_lock.py b/pre_commit/file_lock.py index ff0dc5e6..5e7a0586 100644 --- a/pre_commit/file_lock.py +++ b/pre_commit/file_lock.py @@ -21,13 +21,13 @@ if os.name == 'nt': # pragma: no cover (windows) ) -> Generator[None, None, None]: try: # TODO: https://github.com/python/typeshed/pull/3607 - msvcrt.locking(fileno, msvcrt.LK_NBLCK, _region) # type: ignore + msvcrt.locking(fileno, msvcrt.LK_NBLCK, _region) except OSError: blocked_cb() while True: try: # TODO: https://github.com/python/typeshed/pull/3607 - msvcrt.locking(fileno, msvcrt.LK_LOCK, _region) # type: ignore # noqa: E501 + msvcrt.locking(fileno, msvcrt.LK_LOCK, _region) except OSError as e: # Locking violation. Returned when the _LK_LOCK or _LK_RLCK # flag is specified and the file cannot be locked after 10 @@ -46,7 +46,7 @@ if os.name == 'nt': # pragma: no cover (windows) # "Regions should be locked only briefly and should be unlocked # before closing a file or exiting the program." # TODO: https://github.com/python/typeshed/pull/3607 - msvcrt.locking(fileno, msvcrt.LK_UNLCK, _region) # type: ignore + msvcrt.locking(fileno, msvcrt.LK_UNLCK, _region) else: # pragma: win32 no cover import fcntl From 7da72563dd3c9c0c73292c9a3ab5cc10061132f6 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 15 Jul 2020 21:07:21 -0700 Subject: [PATCH 11/25] require healthy() after installation --- pre_commit/repository.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 77734ee6..91c43055 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -82,6 +82,12 @@ def _hook_install(hook: Hook) -> None: lang.install_environment( hook.prefix, hook.language_version, hook.additional_dependencies, ) + if not lang.healthy(hook.prefix, hook.language_version): + raise AssertionError( + f'BUG: expected environment for {hook.language} to be healthy() ' + f'immediately after install, please open an issue describing ' + f'your environment', + ) # Write our state to indicate we're installed _write_state(hook.prefix, venv, _state(hook.additional_dependencies)) From 1b435f1f1fa7432cbb1b2bef61c3ec0071d036cb Mon Sep 17 00:00:00 2001 From: Greg Singer Date: Sun, 19 Jul 2020 16:37:44 -0500 Subject: [PATCH 12/25] add init-templatedir --no-allow-missing-config Add a `--no-allow-missing-config` option to the `init-templatedir` command. Enable configuration of a Git template that requires newly cloned repos to have a `pre-commit` config. --- pre_commit/commands/init_templatedir.py | 9 +++-- pre_commit/main.py | 7 ++++ tests/commands/init_templatedir_test.py | 48 +++++++++++++++++++++++++ tests/main_test.py | 21 +++++++++++ 4 files changed, 83 insertions(+), 2 deletions(-) diff --git a/pre_commit/commands/init_templatedir.py b/pre_commit/commands/init_templatedir.py index f676fb19..5f17d9c1 100644 --- a/pre_commit/commands/init_templatedir.py +++ b/pre_commit/commands/init_templatedir.py @@ -15,10 +15,15 @@ def init_templatedir( store: Store, directory: str, hook_types: Sequence[str], + skip_on_missing_config: bool = True, ) -> int: install( - config_file, store, hook_types=hook_types, - overwrite=True, skip_on_missing_config=True, git_dir=directory, + config_file, + store, + hook_types=hook_types, + overwrite=True, + skip_on_missing_config=skip_on_missing_config, + git_dir=directory, ) try: _, out, _ = cmd_output('git', 'config', 'init.templateDir') diff --git a/pre_commit/main.py b/pre_commit/main.py index 874eb53a..ffcc2e87 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -245,6 +245,12 @@ def main(argv: Optional[Sequence[str]] = None) -> int: init_templatedir_parser.add_argument( 'directory', help='The directory in which to write the hook script.', ) + init_templatedir_parser.add_argument( + '--no-allow-missing-config', + action='store_false', + dest='allow_missing_config', + help='Assume cloned repos should have a `pre-commit` config.', + ) _add_hook_type_option(init_templatedir_parser) install_parser = subparsers.add_parser( @@ -383,6 +389,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: return init_templatedir( args.config, store, args.directory, hook_types=args.hook_types, + skip_on_missing_config=args.allow_missing_config, ) elif args.command == 'install-hooks': return install_hooks(args.config, store) diff --git a/tests/commands/init_templatedir_test.py b/tests/commands/init_templatedir_test.py index d14a171f..4e131dff 100644 --- a/tests/commands/init_templatedir_test.py +++ b/tests/commands/init_templatedir_test.py @@ -1,6 +1,8 @@ import os.path from unittest import mock +import pytest + import pre_commit.constants as C from pre_commit.commands.init_templatedir import init_templatedir from pre_commit.envcontext import envcontext @@ -90,3 +92,49 @@ def test_init_templatedir_hookspath_set(tmpdir, tempdir_factory, store): C.CONFIG_FILE, store, target, hook_types=['pre-commit'], ) assert target.join('hooks/pre-commit').exists() + + +@pytest.mark.parametrize( + ('skip', 'commit_retcode', 'commit_output_snippet'), + ( + (True, 0, 'Skipping `pre-commit`.'), + (False, 1, f'No {C.CONFIG_FILE} file was found'), + ), +) +def test_init_templatedir_skip_on_missing_config( + tmpdir, + tempdir_factory, + store, + cap_out, + skip, + commit_retcode, + commit_output_snippet, +): + target = str(tmpdir.join('tmpl')) + init_git_dir = git_dir(tempdir_factory) + with cwd(init_git_dir): + cmd_output('git', 'config', 'init.templateDir', target) + init_templatedir( + C.CONFIG_FILE, + store, + target, + hook_types=['pre-commit'], + skip_on_missing_config=skip, + ) + + lines = cap_out.get().splitlines() + assert len(lines) == 1 + assert lines[0].startswith('pre-commit installed at') + + with envcontext((('GIT_TEMPLATE_DIR', target),)): + verify_git_dir = git_dir(tempdir_factory) + + with cwd(verify_git_dir): + retcode, output = git_commit( + fn=cmd_output_mocked_pre_commit_home, + tempdir_factory=tempdir_factory, + retcode=None, + ) + + assert retcode == commit_retcode + assert commit_output_snippet in output diff --git a/tests/main_test.py b/tests/main_test.py index c4724768..f7abeeb4 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -159,7 +159,28 @@ def test_try_repo(mock_store_dir): def test_init_templatedir(mock_store_dir): with mock.patch.object(main, 'init_templatedir') as patch: main.main(('init-templatedir', 'tdir')) + assert patch.call_count == 1 + assert 'tdir' in patch.call_args[0] + assert patch.call_args[1]['hook_types'] == ['pre-commit'] + assert patch.call_args[1]['skip_on_missing_config'] is True + + +def test_init_templatedir_options(mock_store_dir): + args = ( + 'init-templatedir', + 'tdir', + '--hook-type', + 'commit-msg', + '--no-allow-missing-config', + ) + with mock.patch.object(main, 'init_templatedir') as patch: + main.main(args) + + assert patch.call_count == 1 + assert 'tdir' in patch.call_args[0] + assert patch.call_args[1]['hook_types'] == ['commit-msg'] + assert patch.call_args[1]['skip_on_missing_config'] is False def test_help_cmd_in_empty_directory( From cee834bb5e643b80128e929dc9fb873e7d88c1e8 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Fri, 24 Jul 2020 15:56:35 -0700 Subject: [PATCH 13/25] better error handling when Store is readonly --- pre_commit/error_handler.py | 13 ++++++++++--- tests/error_handler_test.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/pre_commit/error_handler.py b/pre_commit/error_handler.py index b2321ae0..13d78cbb 100644 --- a/pre_commit/error_handler.py +++ b/pre_commit/error_handler.py @@ -18,10 +18,17 @@ class FatalError(RuntimeError): def _log_and_exit(msg: str, exc: BaseException, formatted: str) -> None: error_msg = f'{msg}: {type(exc).__name__}: '.encode() + force_bytes(exc) output.write_line_b(error_msg) - log_path = os.path.join(Store().directory, 'pre-commit.log') - output.write_line(f'Check the log at {log_path}') - with open(log_path, 'wb') as log: + storedir = Store().directory + log_path = os.path.join(storedir, 'pre-commit.log') + with contextlib.ExitStack() as ctx: + if os.access(storedir, os.W_OK): + output.write_line(f'Check the log at {log_path}') + log = ctx.enter_context(open(log_path, 'wb')) + else: # pragma: win32 no cover + output.write_line(f'Failed to write to log at {log_path}') + log = sys.stdout.buffer + _log_line = functools.partial(output.write_line, stream=log) _log_line_b = functools.partial(output.write_line_b, stream=log) diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py index 833bb8f8..d066e572 100644 --- a/tests/error_handler_test.py +++ b/tests/error_handler_test.py @@ -1,13 +1,16 @@ import os.path import re +import stat import sys from unittest import mock import pytest from pre_commit import error_handler +from pre_commit.store import Store from pre_commit.util import CalledProcessError from testing.util import cmd_output_mocked_pre_commit_home +from testing.util import xfailif_windows @pytest.fixture @@ -168,3 +171,29 @@ def test_error_handler_no_tty(tempdir_factory): out_lines = out.splitlines() assert out_lines[-2] == 'An unexpected error has occurred: ValueError: ☃' assert out_lines[-1] == f'Check the log at {log_file}' + + +@xfailif_windows # pragma: win32 no cover +def test_error_handler_read_only_filesystem(mock_store_dir, cap_out, capsys): + # a better scenario would be if even the Store crash would be handled + # but realistically we're only targetting systems where the Store has + # already been set up + Store() + + write = (stat.S_IWGRP | stat.S_IWOTH | stat.S_IWUSR) + os.chmod(mock_store_dir, os.stat(mock_store_dir).st_mode & ~write) + + with pytest.raises(SystemExit): + with error_handler.error_handler(): + raise ValueError('ohai') + + output = cap_out.get() + assert output.startswith( + 'An unexpected error has occurred: ValueError: ohai\n' + 'Failed to write to log at ', + ) + + # our cap_out mock is imperfect so the rest of the output goes to capsys + out, _ = capsys.readouterr() + # the things that normally go to the log file will end up here + assert '### version information' in out From 68510596d31963c078bcec94e2b28b8b03b795c3 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Fri, 24 Jul 2020 15:30:36 -0700 Subject: [PATCH 14/25] warn on old list-style configuration --- pre_commit/clientlib.py | 45 +++++++++++++++++++++++++---------------- pre_commit/color.py | 10 +++++++++ pre_commit/main.py | 35 ++++++++++++-------------------- tests/clientlib_test.py | 9 ++++++++- 4 files changed, 59 insertions(+), 40 deletions(-) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index 56ec0dd1..8dfa9473 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -12,8 +12,10 @@ import cfgv from identify.identify import ALL_TAGS import pre_commit.constants as C +from pre_commit.color import add_color_option from pre_commit.error_handler import FatalError from pre_commit.languages.all import all_languages +from pre_commit.logging_handler import logging_handler from pre_commit.util import parse_version from pre_commit.util import yaml_load @@ -43,6 +45,7 @@ def _make_argparser(filenames_help: str) -> argparse.ArgumentParser: parser = argparse.ArgumentParser() parser.add_argument('filenames', nargs='*', help=filenames_help) parser.add_argument('-V', '--version', action='version', version=C.VERSION) + add_color_option(parser) return parser @@ -92,14 +95,16 @@ load_manifest = functools.partial( def validate_manifest_main(argv: Optional[Sequence[str]] = None) -> int: parser = _make_argparser('Manifest filenames.') args = parser.parse_args(argv) - ret = 0 - for filename in args.filenames: - try: - load_manifest(filename) - except InvalidManifestError as e: - print(e) - ret = 1 - return ret + + with logging_handler(args.color): + ret = 0 + for filename in args.filenames: + try: + load_manifest(filename) + except InvalidManifestError as e: + print(e) + ret = 1 + return ret LOCAL = 'local' @@ -290,7 +295,11 @@ class InvalidConfigError(FatalError): def ordered_load_normalize_legacy_config(contents: str) -> Dict[str, Any]: data = yaml_load(contents) if isinstance(data, list): - # TODO: Once happy, issue a deprecation warning and instructions + logger.warning( + 'normalizing pre-commit configuration to a top-level map. ' + 'support for top level list will be removed in a future version. ' + 'run: `pre-commit migrate-config` to automatically fix this.', + ) return {'repos': data} else: return data @@ -307,11 +316,13 @@ load_config = functools.partial( def validate_config_main(argv: Optional[Sequence[str]] = None) -> int: parser = _make_argparser('Config filenames.') args = parser.parse_args(argv) - ret = 0 - for filename in args.filenames: - try: - load_config(filename) - except InvalidConfigError as e: - print(e) - ret = 1 - return ret + + with logging_handler(args.color): + ret = 0 + for filename in args.filenames: + try: + load_config(filename) + except InvalidConfigError as e: + print(e) + ret = 1 + return ret diff --git a/pre_commit/color.py b/pre_commit/color.py index eb906b78..4ddfdf5b 100644 --- a/pre_commit/color.py +++ b/pre_commit/color.py @@ -1,3 +1,4 @@ +import argparse import os import sys @@ -95,3 +96,12 @@ def use_color(setting: str) -> bool: os.getenv('TERM') != 'dumb' ) ) + + +def add_color_option(parser: argparse.ArgumentParser) -> None: + parser.add_argument( + '--color', default=os.environ.get('PRE_COMMIT_COLOR', 'auto'), + type=use_color, + metavar='{' + ','.join(COLOR_CHOICES) + '}', + help='Whether to use color in output. Defaults to `%(default)s`.', + ) diff --git a/pre_commit/main.py b/pre_commit/main.py index ffcc2e87..86479607 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -8,8 +8,8 @@ from typing import Sequence from typing import Union import pre_commit.constants as C -from pre_commit import color from pre_commit import git +from pre_commit.color import add_color_option from pre_commit.commands.autoupdate import autoupdate from pre_commit.commands.clean import clean from pre_commit.commands.gc import gc @@ -41,15 +41,6 @@ os.environ.pop('__PYVENV_LAUNCHER__', None) COMMANDS_NO_GIT = {'clean', 'gc', 'init-templatedir', 'sample-config'} -def _add_color_option(parser: argparse.ArgumentParser) -> None: - parser.add_argument( - '--color', default=os.environ.get('PRE_COMMIT_COLOR', 'auto'), - type=color.use_color, - metavar='{' + ','.join(color.COLOR_CHOICES) + '}', - help='Whether to use color in output. Defaults to `%(default)s`.', - ) - - def _add_config_option(parser: argparse.ArgumentParser) -> None: parser.add_argument( '-c', '--config', default=C.CONFIG_FILE, @@ -195,7 +186,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: 'autoupdate', help="Auto-update pre-commit config to the latest repos' versions.", ) - _add_color_option(autoupdate_parser) + add_color_option(autoupdate_parser) _add_config_option(autoupdate_parser) autoupdate_parser.add_argument( '--bleeding-edge', action='store_true', @@ -216,11 +207,11 @@ def main(argv: Optional[Sequence[str]] = None) -> int: clean_parser = subparsers.add_parser( 'clean', help='Clean out pre-commit files.', ) - _add_color_option(clean_parser) + add_color_option(clean_parser) _add_config_option(clean_parser) hook_impl_parser = subparsers.add_parser('hook-impl') - _add_color_option(hook_impl_parser) + add_color_option(hook_impl_parser) _add_config_option(hook_impl_parser) hook_impl_parser.add_argument('--hook-type') hook_impl_parser.add_argument('--hook-dir') @@ -230,7 +221,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: hook_impl_parser.add_argument(dest='rest', nargs=argparse.REMAINDER) gc_parser = subparsers.add_parser('gc', help='Clean unused cached repos.') - _add_color_option(gc_parser) + add_color_option(gc_parser) _add_config_option(gc_parser) init_templatedir_parser = subparsers.add_parser( @@ -240,7 +231,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: '`git config init.templateDir`.' ), ) - _add_color_option(init_templatedir_parser) + add_color_option(init_templatedir_parser) _add_config_option(init_templatedir_parser) init_templatedir_parser.add_argument( 'directory', help='The directory in which to write the hook script.', @@ -256,7 +247,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: install_parser = subparsers.add_parser( 'install', help='Install the pre-commit script.', ) - _add_color_option(install_parser) + add_color_option(install_parser) _add_config_option(install_parser) install_parser.add_argument( '-f', '--overwrite', action='store_true', @@ -286,32 +277,32 @@ def main(argv: Optional[Sequence[str]] = None) -> int: 'useful.' ), ) - _add_color_option(install_hooks_parser) + add_color_option(install_hooks_parser) _add_config_option(install_hooks_parser) migrate_config_parser = subparsers.add_parser( 'migrate-config', help='Migrate list configuration to new map configuration.', ) - _add_color_option(migrate_config_parser) + add_color_option(migrate_config_parser) _add_config_option(migrate_config_parser) run_parser = subparsers.add_parser('run', help='Run hooks.') - _add_color_option(run_parser) + add_color_option(run_parser) _add_config_option(run_parser) _add_run_options(run_parser) sample_config_parser = subparsers.add_parser( 'sample-config', help=f'Produce a sample {C.CONFIG_FILE} file', ) - _add_color_option(sample_config_parser) + add_color_option(sample_config_parser) _add_config_option(sample_config_parser) try_repo_parser = subparsers.add_parser( 'try-repo', help='Try the hooks in a repository, useful for developing new hooks.', ) - _add_color_option(try_repo_parser) + add_color_option(try_repo_parser) _add_config_option(try_repo_parser) try_repo_parser.add_argument( 'repo', help='Repository to source hooks from.', @@ -328,7 +319,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: uninstall_parser = subparsers.add_parser( 'uninstall', help='Uninstall the pre-commit script.', ) - _add_color_option(uninstall_parser) + add_color_option(uninstall_parser) _add_config_option(uninstall_parser) _add_hook_type_option(uninstall_parser) diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index c48adbde..2e2f738c 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -30,6 +30,10 @@ def test_check_type_tag_failures(value): check_type_tag(value) +def test_check_type_tag_success(): + check_type_tag('file') + + @pytest.mark.parametrize( ('config_obj', 'expected'), ( ( @@ -110,15 +114,18 @@ def test_validate_config_main_ok(): assert not validate_config_main(('.pre-commit-config.yaml',)) -def test_validate_config_old_list_format_ok(tmpdir): +def test_validate_config_old_list_format_ok(tmpdir, cap_out): f = tmpdir.join('cfg.yaml') f.write('- {repo: meta, hooks: [{id: identity}]}') assert not validate_config_main((f.strpath,)) + start = '[WARNING] normalizing pre-commit configuration to a top-level map' + assert cap_out.get().startswith(start) def test_validate_warn_on_unknown_keys_at_repo_level(tmpdir, caplog): f = tmpdir.join('cfg.yaml') f.write( + 'repos:\n' '- repo: https://gitlab.com/pycqa/flake8\n' ' rev: 3.7.7\n' ' hooks:\n' From 4063730925f19c860c2f402bf86b733ba97c2630 Mon Sep 17 00:00:00 2001 From: Johan Henkens Date: Fri, 21 Aug 2020 20:40:59 -0700 Subject: [PATCH 15/25] Save diff between hook executions --- pre_commit/commands/run.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 567b7cd3..1f28c8c7 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -134,9 +134,10 @@ def _run_single_hook( hook: Hook, skips: Set[str], cols: int, + diff_before: bytes, verbose: bool, use_color: bool, -) -> bool: +) -> Tuple[bool, bytes]: filenames = classifier.filenames_for_hook(hook) if hook.id in skips or hook.alias in skips: @@ -151,6 +152,7 @@ def _run_single_hook( ) duration = None retcode = 0 + diff_after = diff_before files_modified = False out = b'' elif not filenames and not hook.always_run: @@ -166,21 +168,20 @@ def _run_single_hook( ) duration = None retcode = 0 + diff_after = diff_before files_modified = False out = b'' else: # print hook and dots first in case the hook takes a while to run output.write(_start_msg(start=hook.name, end_len=6, cols=cols)) - diff_cmd = ('git', 'diff', '--no-ext-diff') - diff_before = cmd_output_b(*diff_cmd, retcode=None) if not hook.pass_filenames: filenames = () time_before = time.time() language = languages[hook.language] retcode, out = language.run_hook(hook, filenames, use_color) duration = round(time.time() - time_before, 2) or 0 - diff_after = cmd_output_b(*diff_cmd, retcode=None) + diff_after = _get_diff() # if the hook makes changes, fail the commit files_modified = diff_before != diff_after @@ -212,7 +213,7 @@ def _run_single_hook( output.write_line_b(out.strip(), logfile_name=hook.log_file) output.write_line() - return files_modified or bool(retcode) + return files_modified or bool(retcode), diff_after def _compute_cols(hooks: Sequence[Hook]) -> int: @@ -248,6 +249,11 @@ def _all_filenames(args: argparse.Namespace) -> Collection[str]: return git.get_staged_files() +def _get_diff() -> bytes: + _, out, _ = cmd_output_b('git', 'diff', '--no-ext-diff', retcode=None) + return out + + def _run_hooks( config: Dict[str, Any], hooks: Sequence[Hook], @@ -261,14 +267,16 @@ def _run_hooks( _all_filenames(args), config['files'], config['exclude'], ) retval = 0 + prior_diff = _get_diff() for hook in hooks: - retval |= _run_single_hook( - classifier, hook, skips, cols, + current_retval, prior_diff = _run_single_hook( + classifier, hook, skips, cols, prior_diff, verbose=args.verbose, use_color=args.color, ) + retval |= current_retval if retval and config['fail_fast']: break - if retval and args.show_diff_on_failure and git.has_diff(): + if retval and args.show_diff_on_failure and prior_diff: if args.all_files: output.write_line( 'pre-commit hook(s) made changes.\n' From bf33f4c91c2e73bac36ec37a3dcf92bef8f0492c Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Fri, 21 Aug 2020 23:11:30 -0700 Subject: [PATCH 16/25] allow pre-commit to succeed on a readonly store directory --- pre_commit/store.py | 6 ++++++ tests/store_test.py | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/pre_commit/store.py b/pre_commit/store.py index 6d8c40a9..809a6f4d 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -43,6 +43,10 @@ class Store: def __init__(self, directory: Optional[str] = None) -> None: self.directory = directory or Store.get_default_directory() self.db_path = os.path.join(self.directory, 'db.db') + self.readonly = ( + os.path.exists(self.directory) and + not os.access(self.directory, os.W_OK) + ) if not os.path.exists(self.directory): os.makedirs(self.directory, exist_ok=True) @@ -218,6 +222,8 @@ class Store: ) def mark_config_used(self, path: str) -> None: + if self.readonly: # pragma: win32 no cover + return path = os.path.realpath(path) # don't insert config files that do not exist if not os.path.exists(path): diff --git a/tests/store_test.py b/tests/store_test.py index 6a4e900c..0947144e 100644 --- a/tests/store_test.py +++ b/tests/store_test.py @@ -1,5 +1,6 @@ import os.path import sqlite3 +import stat from unittest import mock import pytest @@ -12,6 +13,7 @@ from pre_commit.util import cmd_output from testing.fixtures import git_dir from testing.util import cwd from testing.util import git_commit +from testing.util import xfailif_windows def test_our_session_fixture_works(): @@ -217,3 +219,27 @@ def test_select_all_configs_roll_forward(store): def test_mark_config_as_used_roll_forward(store, tmpdir): _simulate_pre_1_14_0(store) test_mark_config_as_used(store, tmpdir) + + +@xfailif_windows # pragma: win32 no cover +def test_mark_config_as_used_readonly(tmpdir): + cfg = tmpdir.join('f').ensure() + store_dir = tmpdir.join('store') + # make a store, then we'll convert its directory to be readonly + assert not Store(str(store_dir)).readonly # directory didn't exist + assert not Store(str(store_dir)).readonly # directory did exist + + def _chmod_minus_w(p): + st = os.stat(p) + os.chmod(p, st.st_mode & ~(stat.S_IWUSR | stat.S_IWOTH | stat.S_IWGRP)) + + _chmod_minus_w(store_dir) + for fname in os.listdir(store_dir): + assert not os.path.isdir(fname) + _chmod_minus_w(os.path.join(store_dir, fname)) + + store = Store(str(store_dir)) + assert store.readonly + # should be skipped due to readonly + store.mark_config_used(str(cfg)) + assert store.select_all_configs() == [] From f1de792877f904b7349d3ae163a3694f2854ade1 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 22 Aug 2020 13:31:12 -0700 Subject: [PATCH 17/25] v2.7.0 --- CHANGELOG.md | 23 +++++++++++++++++++++++ setup.cfg | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c487acf6..e692f3dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,26 @@ +2.7.0 - 2020-08-22 +================== + +### Features +- Produce error message if an environment is immediately unhealthy + - #1535 PR by @asottile. +- Add --no-allow-missing-config option to init-templatedir + - #1539 PR by @singergr. +- Add warning for old list-style configuration + - #1544 PR by @asottile. +- Allow pre-commit to succeed on a readonly store. + - #1570 PR by @asottile. + - #1536 issue by @asottile. + +### Fixes +- Fix error messaging when the store directory is readonly + - #1546 PR by @asottile. + - #1536 issue by @asottile. +- Improve `diff` performance with many hooks + - #1566 PR by @jhenkens. + - #1564 issue by @jhenkens. + + 2.6.0 - 2020-07-01 ================== diff --git a/setup.cfg b/setup.cfg index 0ce58b1a..c9d7f82e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = pre_commit -version = 2.6.0 +version = 2.7.0 description = A framework for managing and maintaining multi-language pre-commit hooks. long_description = file: README.md long_description_content_type = text/markdown From eb8b48aeb439ee69610a7c08a3a1de75fbbfe572 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod Date: Sun, 23 Aug 2020 00:14:10 +0000 Subject: [PATCH 18/25] remove docker_is_running check from source Moved to testing.util so it can be used for the skipif_cant_run_docker test hooks. --- pre_commit/languages/docker.py | 19 ------------------- pre_commit/languages/docker_image.py | 2 -- testing/util.py | 12 +++++++++++- tests/languages/docker_test.py | 9 --------- 4 files changed, 11 insertions(+), 31 deletions(-) diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index 4091492c..9c131198 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -7,9 +7,7 @@ import pre_commit.constants as C from pre_commit.hook import Hook from pre_commit.languages import helpers from pre_commit.prefix import Prefix -from pre_commit.util import CalledProcessError from pre_commit.util import clean_path_on_failure -from pre_commit.util import cmd_output_b ENVIRONMENT_DIR = 'docker' PRE_COMMIT_LABEL = 'PRE_COMMIT' @@ -26,21 +24,6 @@ def docker_tag(prefix: Prefix) -> str: # pragma: win32 no cover return f'pre-commit-{md5sum}' -def docker_is_running() -> bool: # pragma: win32 no cover - try: - cmd_output_b('docker', 'ps') - except CalledProcessError: - return False - else: - return True - - -def assert_docker_available() -> None: # pragma: win32 no cover - assert docker_is_running(), ( - 'Docker is either not running or not configured in this environment' - ) - - def build_docker_image( prefix: Prefix, *, @@ -63,7 +46,6 @@ def install_environment( ) -> None: # pragma: win32 no cover helpers.assert_version_default('docker', version) helpers.assert_no_additional_deps('docker', additional_dependencies) - assert_docker_available() directory = prefix.path( helpers.environment_dir(ENVIRONMENT_DIR, C.DEFAULT), @@ -101,7 +83,6 @@ def run_hook( file_args: Sequence[str], color: bool, ) -> Tuple[int, bytes]: # pragma: win32 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(hook.prefix, pull=False) diff --git a/pre_commit/languages/docker_image.py b/pre_commit/languages/docker_image.py index 0c51df62..311d1277 100644 --- a/pre_commit/languages/docker_image.py +++ b/pre_commit/languages/docker_image.py @@ -3,7 +3,6 @@ from typing import Tuple from pre_commit.hook import Hook from pre_commit.languages import helpers -from pre_commit.languages.docker import assert_docker_available from pre_commit.languages.docker import docker_cmd ENVIRONMENT_DIR = None @@ -17,6 +16,5 @@ def run_hook( file_args: Sequence[str], color: bool, ) -> Tuple[int, bytes]: # pragma: win32 no cover - assert_docker_available() cmd = docker_cmd() + hook.cmd return helpers.run_xargs(hook, cmd, file_args, color=color) diff --git a/testing/util.py b/testing/util.py index 4edb7a9e..f556a8dd 100644 --- a/testing/util.py +++ b/testing/util.py @@ -5,14 +5,24 @@ import subprocess import pytest from pre_commit import parse_shebang -from pre_commit.languages.docker import docker_is_running +from pre_commit.util import CalledProcessError from pre_commit.util import cmd_output +from pre_commit.util import cmd_output_b from testing.auto_namedtuple import auto_namedtuple TESTING_DIR = os.path.abspath(os.path.dirname(__file__)) +def docker_is_running() -> bool: # pragma: win32 no cover + try: + cmd_output_b('docker', 'ps') + except CalledProcessError: # pragma: no cover + return False + else: + return True + + def get_resource_path(path): return os.path.join(TESTING_DIR, 'resources', path) diff --git a/tests/languages/docker_test.py b/tests/languages/docker_test.py index b65b2235..3bed4bfa 100644 --- a/tests/languages/docker_test.py +++ b/tests/languages/docker_test.py @@ -1,15 +1,6 @@ from unittest import mock from pre_commit.languages import docker -from pre_commit.util import CalledProcessError - - -def test_docker_is_running_process_error(): - with mock.patch( - 'pre_commit.languages.docker.cmd_output_b', - side_effect=CalledProcessError(1, (), 0, b'', None), - ): - assert docker.docker_is_running() is False def test_docker_fallback_user(): From b63b37ac36caf89de55a7ae45bb57d981b1b1e36 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 23 Aug 2020 09:55:29 -0700 Subject: [PATCH 19/25] fix cache of invalidated unhealthy environment version info --- pre_commit/languages/python.py | 3 ++- tests/languages/python_test.py | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index 6f7c9005..7a685808 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -191,7 +191,8 @@ def healthy(prefix: Prefix, language_version: str) -> bool: return ( 'version_info' in cfg and - _version_info(py_exe) == cfg['version_info'] and ( + # always use uncached lookup here in case we replaced an unhealthy env + _version_info.__wrapped__(py_exe) == cfg['version_info'] and ( 'base-executable' not in cfg or _version_info(cfg['base-executable']) == cfg['version_info'] ) diff --git a/tests/languages/python_test.py b/tests/languages/python_test.py index c419ad62..29c5a9bf 100644 --- a/tests/languages/python_test.py +++ b/tests/languages/python_test.py @@ -8,6 +8,7 @@ import pre_commit.constants as C from pre_commit.envcontext import envcontext from pre_commit.languages import python from pre_commit.prefix import Prefix +from pre_commit.util import make_executable def test_read_pyvenv_cfg(tmpdir): @@ -141,3 +142,26 @@ def test_unhealthy_old_virtualenv(python_dir): os.remove(prefix.path('py_env-default/pyvenv.cfg')) assert python.healthy(prefix, C.DEFAULT) is False + + +def test_unhealthy_then_replaced(python_dir): + prefix, tmpdir = python_dir + + python.install_environment(prefix, C.DEFAULT, ()) + + # simulate an exe which returns an old version + exe_name = 'python.exe' if sys.platform == 'win32' else 'python' + py_exe = prefix.path(python.bin_dir('py_env-default'), exe_name) + os.rename(py_exe, f'{py_exe}.tmp') + + with open(py_exe, 'w') as f: + f.write('#!/usr/bin/env bash\necho 1.2.3\n') + make_executable(py_exe) + + # should be unhealthy due to version mismatch + assert python.healthy(prefix, C.DEFAULT) is False + + # now put the exe back and it should be healthy again + os.replace(f'{py_exe}.tmp', py_exe) + + assert python.healthy(prefix, C.DEFAULT) is True From 79b098c409460166d810c51a3048ba427a0e9e80 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 23 Aug 2020 10:18:59 -0700 Subject: [PATCH 20/25] fix atomic file replace on windows --- pre_commit/commands/install_uninstall.py | 2 +- pre_commit/repository.py | 2 +- pre_commit/store.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index c8b7633b..85fa53cb 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -165,7 +165,7 @@ def _uninstall_hook_script(hook_type: str) -> None: output.write_line(f'{hook_type} uninstalled') if os.path.exists(legacy_path): - os.rename(legacy_path, hook_path) + os.replace(legacy_path, hook_path) output.write_line(f'Restored previous hooks to {hook_path}') diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 91c43055..46e96c1d 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -48,7 +48,7 @@ def _write_state(prefix: Prefix, venv: str, state: object) -> None: with open(staging, 'w') as state_file: state_file.write(json.dumps(state)) # Move the file into place atomically to indicate we've installed - os.rename(staging, state_filename) + os.replace(staging, state_filename) def _hook_installed(hook: Hook) -> bool: diff --git a/pre_commit/store.py b/pre_commit/store.py index 809a6f4d..e5522ec3 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -79,7 +79,7 @@ class Store: self._create_config_table(db) # Atomic file move - os.rename(tmpfile, self.db_path) + os.replace(tmpfile, self.db_path) @contextlib.contextmanager def exclusive_lock(self) -> Generator[None, None, None]: From f511afe40e3f0ea7474d37f19c69741d3e167876 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 23 Aug 2020 10:53:21 -0700 Subject: [PATCH 21/25] v2.7.1 --- CHANGELOG.md | 14 ++++++++++++++ setup.cfg | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e692f3dc..a92a6b36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,17 @@ +2.7.1 - 2020-08-23 +================== + +### Fixes +- Improve performance of docker hooks by removing slow `ps` call + - #1572 PR by @rkm. + - #1569 issue by @asottile. +- Fix un-`healthy()` invalidation followed by install being reported as + un-`healthy()`. + - #1576 PR by @asottile. + - #1575 issue by @jab. +- Fix rare file race condition on windows with `os.replace()` + - #1577 PR by @asottile. + 2.7.0 - 2020-08-22 ================== diff --git a/setup.cfg b/setup.cfg index c9d7f82e..4153d765 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = pre_commit -version = 2.7.0 +version = 2.7.1 description = A framework for managing and maintaining multi-language pre-commit hooks. long_description = file: README.md long_description_content_type = text/markdown From b149c7a344a407fb3c9c8c99b9647c3c95f1a998 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 7 Sep 2020 13:23:02 -0700 Subject: [PATCH 22/25] fix for node healthy() when system executable moves --- pre_commit/languages/node.py | 7 ++++++- tests/languages/node_test.py | 37 ++++++++++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py index d99e6f2c..dccbb7ca 100644 --- a/pre_commit/languages/node.py +++ b/pre_commit/languages/node.py @@ -21,7 +21,6 @@ from pre_commit.util import cmd_output from pre_commit.util import cmd_output_b ENVIRONMENT_DIR = 'node_env' -healthy = helpers.basic_healthy @functools.lru_cache(maxsize=1) @@ -73,6 +72,12 @@ def in_env( yield +def healthy(prefix: Prefix, language_version: str) -> bool: + with in_env(prefix, language_version): + retcode, _, _ = cmd_output_b('node', '--version', retcode=None) + return retcode == 0 + + def install_environment( prefix: Prefix, version: str, additional_dependencies: Sequence[str], ) -> None: diff --git a/tests/languages/node_test.py b/tests/languages/node_test.py index fd300469..c8e2d47d 100644 --- a/tests/languages/node_test.py +++ b/tests/languages/node_test.py @@ -1,14 +1,19 @@ +import os +import shutil import sys from unittest import mock import pytest import pre_commit.constants as C +from pre_commit import envcontext from pre_commit import parse_shebang -from pre_commit.languages.node import get_default_version +from pre_commit.languages import node +from pre_commit.prefix import Prefix +from testing.util import xfailif_windows -ACTUAL_GET_DEFAULT_VERSION = get_default_version.__wrapped__ +ACTUAL_GET_DEFAULT_VERSION = node.get_default_version.__wrapped__ @pytest.fixture @@ -45,3 +50,31 @@ def test_uses_default_when_node_and_npm_are_not_available(find_exe_mck): def test_sets_default_on_windows(find_exe_mck): find_exe_mck.return_value = '/path/to/exe' assert ACTUAL_GET_DEFAULT_VERSION() == C.DEFAULT + + +@xfailif_windows # pragma: win32 no cover +def test_healthy_system_node(tmpdir): + tmpdir.join('package.json').write('{"name": "t", "version": "1.0.0"}') + + prefix = Prefix(str(tmpdir)) + node.install_environment(prefix, 'system', ()) + assert node.healthy(prefix, 'system') + + +@xfailif_windows # pragma: win32 no cover +def test_unhealthy_if_system_node_goes_missing(tmpdir): + bin_dir = tmpdir.join('bin').ensure_dir() + node_bin = bin_dir.join('node') + node_bin.mksymlinkto(shutil.which('node')) + + prefix_dir = tmpdir.join('prefix').ensure_dir() + prefix_dir.join('package.json').write('{"name": "t", "version": "1.0.0"}') + + path = ('PATH', (str(bin_dir), os.pathsep, envcontext.Var('PATH'))) + with envcontext.envcontext((path,)): + prefix = Prefix(str(prefix_dir)) + node.install_environment(prefix, 'system', ()) + assert node.healthy(prefix, 'system') + + node_bin.remove() + assert not node.healthy(prefix, 'system') From 3a0406847b16e0f8950f90f894a6fce5dcd72813 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 7 Sep 2020 15:01:50 -0700 Subject: [PATCH 23/25] fix excess whitespace in traceback in error --- pre_commit/error_handler.py | 2 +- requirements-dev.txt | 1 + tests/commands/install_uninstall_test.py | 50 ++++++++++++------------ tests/commands/try_repo_test.py | 14 ++++--- tests/error_handler_test.py | 34 +++++++++------- tests/repository_test.py | 6 +-- 6 files changed, 59 insertions(+), 48 deletions(-) diff --git a/pre_commit/error_handler.py b/pre_commit/error_handler.py index 13d78cbb..009f6d9c 100644 --- a/pre_commit/error_handler.py +++ b/pre_commit/error_handler.py @@ -52,7 +52,7 @@ def _log_and_exit(msg: str, exc: BaseException, formatted: str) -> None: _log_line('```') _log_line() _log_line('```') - _log_line(formatted) + _log_line(formatted.rstrip()) _log_line('```') raise SystemExit(1) diff --git a/requirements-dev.txt b/requirements-dev.txt index d6a13dc4..14ada96e 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -2,3 +2,4 @@ covdefaults coverage pytest pytest-env +re-assert diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 5809a3f2..481a7279 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -3,6 +3,8 @@ import re import sys from unittest import mock +import re_assert + import pre_commit.constants as C from pre_commit import git from pre_commit.commands import install_uninstall @@ -143,7 +145,7 @@ FILES_CHANGED = ( ) -NORMAL_PRE_COMMIT_RUN = re.compile( +NORMAL_PRE_COMMIT_RUN = re_assert.Matches( fr'^\[INFO\] Initializing environment for .+\.\n' fr'Bash hook\.+Passed\n' fr'\[master [a-f0-9]{{7}}\] commit!\n' @@ -159,7 +161,7 @@ def test_install_pre_commit_and_run(tempdir_factory, store): ret, output = _get_commit_output(tempdir_factory) assert ret == 0 - assert NORMAL_PRE_COMMIT_RUN.match(output) + NORMAL_PRE_COMMIT_RUN.assert_matches(output) def test_install_pre_commit_and_run_custom_path(tempdir_factory, store): @@ -171,7 +173,7 @@ def test_install_pre_commit_and_run_custom_path(tempdir_factory, store): ret, output = _get_commit_output(tempdir_factory) assert ret == 0 - assert NORMAL_PRE_COMMIT_RUN.match(output) + NORMAL_PRE_COMMIT_RUN.assert_matches(output) def test_install_in_submodule_and_run(tempdir_factory, store): @@ -185,7 +187,7 @@ def test_install_in_submodule_and_run(tempdir_factory, store): assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 - assert NORMAL_PRE_COMMIT_RUN.match(output) + NORMAL_PRE_COMMIT_RUN.assert_matches(output) def test_install_in_worktree_and_run(tempdir_factory, store): @@ -198,7 +200,7 @@ def test_install_in_worktree_and_run(tempdir_factory, store): assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0 ret, output = _get_commit_output(tempdir_factory) assert ret == 0 - assert NORMAL_PRE_COMMIT_RUN.match(output) + NORMAL_PRE_COMMIT_RUN.assert_matches(output) def test_commit_am(tempdir_factory, store): @@ -243,7 +245,7 @@ def test_install_idempotent(tempdir_factory, store): ret, output = _get_commit_output(tempdir_factory) assert ret == 0 - assert NORMAL_PRE_COMMIT_RUN.match(output) + NORMAL_PRE_COMMIT_RUN.assert_matches(output) def _path_without_us(): @@ -297,7 +299,7 @@ def test_environment_not_sourced(tempdir_factory, store): ) -FAILING_PRE_COMMIT_RUN = re.compile( +FAILING_PRE_COMMIT_RUN = re_assert.Matches( r'^\[INFO\] Initializing environment for .+\.\n' r'Failing hook\.+Failed\n' r'- hook id: failing_hook\n' @@ -316,10 +318,10 @@ def test_failing_hooks_returns_nonzero(tempdir_factory, store): ret, output = _get_commit_output(tempdir_factory) assert ret == 1 - assert FAILING_PRE_COMMIT_RUN.match(output) + FAILING_PRE_COMMIT_RUN.assert_matches(output) -EXISTING_COMMIT_RUN = re.compile( +EXISTING_COMMIT_RUN = re_assert.Matches( fr'^legacy hook\n' fr'\[master [a-f0-9]{{7}}\] commit!\n' fr'{FILES_CHANGED}' @@ -342,7 +344,7 @@ def test_install_existing_hooks_no_overwrite(tempdir_factory, store): # Make sure we installed the "old" hook correctly ret, output = _get_commit_output(tempdir_factory, touch_file='baz') assert ret == 0 - assert EXISTING_COMMIT_RUN.match(output) + EXISTING_COMMIT_RUN.assert_matches(output) # Now install pre-commit (no-overwrite) assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0 @@ -351,7 +353,7 @@ def test_install_existing_hooks_no_overwrite(tempdir_factory, store): ret, output = _get_commit_output(tempdir_factory) assert ret == 0 assert output.startswith('legacy hook\n') - assert NORMAL_PRE_COMMIT_RUN.match(output[len('legacy hook\n'):]) + NORMAL_PRE_COMMIT_RUN.assert_matches(output[len('legacy hook\n'):]) def test_legacy_overwriting_legacy_hook(tempdir_factory, store): @@ -377,10 +379,10 @@ def test_install_existing_hook_no_overwrite_idempotent(tempdir_factory, store): ret, output = _get_commit_output(tempdir_factory) assert ret == 0 assert output.startswith('legacy hook\n') - assert NORMAL_PRE_COMMIT_RUN.match(output[len('legacy hook\n'):]) + NORMAL_PRE_COMMIT_RUN.assert_matches(output[len('legacy hook\n'):]) -FAIL_OLD_HOOK = re.compile( +FAIL_OLD_HOOK = re_assert.Matches( r'fail!\n' r'\[INFO\] Initializing environment for .+\.\n' r'Bash hook\.+Passed\n', @@ -401,7 +403,7 @@ def test_failing_existing_hook_returns_1(tempdir_factory, store): # We should get a failure from the legacy hook ret, output = _get_commit_output(tempdir_factory) assert ret == 1 - assert FAIL_OLD_HOOK.match(output) + FAIL_OLD_HOOK.assert_matches(output) def test_install_overwrite_no_existing_hooks(tempdir_factory, store): @@ -413,7 +415,7 @@ def test_install_overwrite_no_existing_hooks(tempdir_factory, store): ret, output = _get_commit_output(tempdir_factory) assert ret == 0 - assert NORMAL_PRE_COMMIT_RUN.match(output) + NORMAL_PRE_COMMIT_RUN.assert_matches(output) def test_install_overwrite(tempdir_factory, store): @@ -426,7 +428,7 @@ def test_install_overwrite(tempdir_factory, store): ret, output = _get_commit_output(tempdir_factory) assert ret == 0 - assert NORMAL_PRE_COMMIT_RUN.match(output) + NORMAL_PRE_COMMIT_RUN.assert_matches(output) def test_uninstall_restores_legacy_hooks(tempdir_factory, store): @@ -441,7 +443,7 @@ def test_uninstall_restores_legacy_hooks(tempdir_factory, store): # Make sure we installed the "old" hook correctly ret, output = _get_commit_output(tempdir_factory, touch_file='baz') assert ret == 0 - assert EXISTING_COMMIT_RUN.match(output) + EXISTING_COMMIT_RUN.assert_matches(output) def test_replace_old_commit_script(tempdir_factory, store): @@ -463,7 +465,7 @@ def test_replace_old_commit_script(tempdir_factory, store): ret, output = _get_commit_output(tempdir_factory) assert ret == 0 - assert NORMAL_PRE_COMMIT_RUN.match(output) + NORMAL_PRE_COMMIT_RUN.assert_matches(output) def test_uninstall_doesnt_remove_not_our_hooks(in_git_dir): @@ -476,7 +478,7 @@ def test_uninstall_doesnt_remove_not_our_hooks(in_git_dir): assert pre_commit.exists() -PRE_INSTALLED = re.compile( +PRE_INSTALLED = re_assert.Matches( fr'Bash hook\.+Passed\n' fr'\[master [a-f0-9]{{7}}\] commit!\n' fr'{FILES_CHANGED}' @@ -493,7 +495,7 @@ def test_installs_hooks_with_hooks_True(tempdir_factory, store): ) assert ret == 0 - assert PRE_INSTALLED.match(output) + PRE_INSTALLED.assert_matches(output) def test_install_hooks_command(tempdir_factory, store): @@ -506,7 +508,7 @@ def test_install_hooks_command(tempdir_factory, store): ) assert ret == 0 - assert PRE_INSTALLED.match(output) + PRE_INSTALLED.assert_matches(output) def test_installed_from_venv(tempdir_factory, store): @@ -533,7 +535,7 @@ def test_installed_from_venv(tempdir_factory, store): }, ) assert ret == 0 - assert NORMAL_PRE_COMMIT_RUN.match(output) + NORMAL_PRE_COMMIT_RUN.assert_matches(output) def _get_push_output(tempdir_factory, remote='origin', opts=()): @@ -880,7 +882,7 @@ def test_prepare_commit_msg_legacy( def test_pre_merge_commit_integration(tempdir_factory, store): - expected = re.compile( + output_pattern = re_assert.Matches( r'^\[INFO\] Initializing environment for .+\n' r'Bash hook\.+Passed\n' r"Merge made by the 'recursive' strategy.\n" @@ -902,7 +904,7 @@ def test_pre_merge_commit_integration(tempdir_factory, store): tempdir_factory=tempdir_factory, ) assert ret == 0 - assert expected.match(output) + output_pattern.assert_matches(output) def test_install_disallow_missing_config(tempdir_factory, store): diff --git a/tests/commands/try_repo_test.py b/tests/commands/try_repo_test.py index d3ec3fda..a157d163 100644 --- a/tests/commands/try_repo_test.py +++ b/tests/commands/try_repo_test.py @@ -3,6 +3,8 @@ import re import time from unittest import mock +import re_assert + from pre_commit import git from pre_commit.commands.try_repo import try_repo from pre_commit.util import cmd_output @@ -43,7 +45,7 @@ def test_try_repo_repo_only(cap_out, tempdir_factory): _run_try_repo(tempdir_factory, verbose=True) start, config, rest = _get_out(cap_out) assert start == '' - assert re.match( + config_pattern = re_assert.Matches( '^repos:\n' '- repo: .+\n' ' rev: .+\n' @@ -51,8 +53,8 @@ def test_try_repo_repo_only(cap_out, tempdir_factory): ' - id: bash_hook\n' ' - id: bash_hook2\n' ' - id: bash_hook3\n$', - config, ) + config_pattern.assert_matches(config) assert rest == '''\ Bash hook............................................(no files to check)Skipped - hook id: bash_hook @@ -71,14 +73,14 @@ def test_try_repo_with_specific_hook(cap_out, tempdir_factory): _run_try_repo(tempdir_factory, hook='bash_hook', verbose=True) start, config, rest = _get_out(cap_out) assert start == '' - assert re.match( + config_pattern = re_assert.Matches( '^repos:\n' '- repo: .+\n' ' rev: .+\n' ' hooks:\n' ' - id: bash_hook\n$', - config, ) + config_pattern.assert_matches(config) assert rest == '''\ Bash hook............................................(no files to check)Skipped - hook id: bash_hook @@ -128,14 +130,14 @@ def test_try_repo_uncommitted_changes(cap_out, tempdir_factory): start, config, rest = _get_out(cap_out) assert start == '[WARNING] Creating temporary repo with uncommitted changes...\n' # noqa: E501 - assert re.match( + config_pattern = re_assert.Matches( '^repos:\n' '- repo: .+shadow-repo\n' ' rev: .+\n' ' hooks:\n' ' - id: bash_hook\n$', - config, ) + config_pattern.assert_matches(config) assert rest == 'modified name!...........................................................Passed\n' # noqa: E501 diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py index d066e572..5dc08505 100644 --- a/tests/error_handler_test.py +++ b/tests/error_handler_test.py @@ -1,10 +1,10 @@ import os.path -import re import stat import sys from unittest import mock import pytest +import re_assert from pre_commit import error_handler from pre_commit.store import Store @@ -37,7 +37,7 @@ def test_error_handler_fatal_error(mocked_log_and_exit): mock.ANY, ) - assert re.match( + pattern = re_assert.Matches( r'Traceback \(most recent call last\):\n' r' File ".+pre_commit.error_handler.py", line \d+, in error_handler\n' r' yield\n' @@ -45,8 +45,8 @@ def test_error_handler_fatal_error(mocked_log_and_exit): r'in test_error_handler_fatal_error\n' r' raise exc\n' r'(pre_commit\.error_handler\.)?FatalError: just a test\n', - mocked_log_and_exit.call_args[0][2], ) + pattern.assert_matches(mocked_log_and_exit.call_args[0][2]) def test_error_handler_uncaught_error(mocked_log_and_exit): @@ -60,7 +60,7 @@ def test_error_handler_uncaught_error(mocked_log_and_exit): # Tested below mock.ANY, ) - assert re.match( + pattern = re_assert.Matches( r'Traceback \(most recent call last\):\n' r' File ".+pre_commit.error_handler.py", line \d+, in error_handler\n' r' yield\n' @@ -68,8 +68,8 @@ def test_error_handler_uncaught_error(mocked_log_and_exit): r'in test_error_handler_uncaught_error\n' r' raise exc\n' r'ValueError: another test\n', - mocked_log_and_exit.call_args[0][2], ) + pattern.assert_matches(mocked_log_and_exit.call_args[0][2]) def test_error_handler_keyboardinterrupt(mocked_log_and_exit): @@ -83,7 +83,7 @@ def test_error_handler_keyboardinterrupt(mocked_log_and_exit): # Tested below mock.ANY, ) - assert re.match( + pattern = re_assert.Matches( r'Traceback \(most recent call last\):\n' r' File ".+pre_commit.error_handler.py", line \d+, in error_handler\n' r' yield\n' @@ -91,15 +91,19 @@ def test_error_handler_keyboardinterrupt(mocked_log_and_exit): r'in test_error_handler_keyboardinterrupt\n' r' raise exc\n' r'KeyboardInterrupt\n', - mocked_log_and_exit.call_args[0][2], ) + pattern.assert_matches(mocked_log_and_exit.call_args[0][2]) def test_log_and_exit(cap_out, mock_store_dir): + tb = ( + 'Traceback (most recent call last):\n' + ' File "", line 2, in \n' + 'pre_commit.error_handler.FatalError: hai\n' + ) + with pytest.raises(SystemExit): - error_handler._log_and_exit( - 'msg', error_handler.FatalError('hai'), "I'm a stacktrace", - ) + error_handler._log_and_exit('msg', error_handler.FatalError('hai'), tb) printed = cap_out.get() log_file = os.path.join(mock_store_dir, 'pre-commit.log') @@ -108,7 +112,7 @@ def test_log_and_exit(cap_out, mock_store_dir): assert os.path.exists(log_file) with open(log_file) as f: logged = f.read() - expected = ( + pattern = re_assert.Matches( r'^### version information\n' r'\n' r'```\n' @@ -127,10 +131,12 @@ def test_log_and_exit(cap_out, mock_store_dir): r'```\n' r'\n' r'```\n' - r"I'm a stacktrace\n" - r'```\n' + r'Traceback \(most recent call last\):\n' + r' File "", line 2, in \n' + r'pre_commit\.error_handler\.FatalError: hai\n' + r'```\n', ) - assert re.match(expected, logged) + pattern.assert_matches(logged) def test_error_handler_non_ascii_exception(mock_store_dir): diff --git a/tests/repository_test.py b/tests/repository_test.py index 84e4da93..035b02a6 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -1,5 +1,4 @@ import os.path -import re import shutil import sys from typing import Any @@ -8,6 +7,7 @@ from unittest import mock import cfgv import pytest +import re_assert import pre_commit.constants as C from pre_commit.clientlib import CONFIG_SCHEMA @@ -843,12 +843,12 @@ def test_too_new_version(tempdir_factory, store, fake_log_handler): with pytest.raises(SystemExit): _get_hook(config, store, 'bash_hook') msg = fake_log_handler.handle.call_args[0][0].msg - assert re.match( + pattern = re_assert.Matches( r'^The hook `bash_hook` requires pre-commit version 999\.0\.0 but ' r'version \d+\.\d+\.\d+ is installed. ' r'Perhaps run `pip install --upgrade pre-commit`\.$', - msg, ) + pattern.assert_matches(msg) @pytest.mark.parametrize('version', ('0.1.0', C.VERSION)) From 273326b89b3eb17656a46f63f094fd1c0a55af84 Mon Sep 17 00:00:00 2001 From: Celeborn2BeAlive Date: Wed, 9 Sep 2020 09:32:44 +0200 Subject: [PATCH 24/25] drop python.exe extension on windows on shebang --- pre_commit/commands/install_uninstall.py | 2 +- tests/commands/install_uninstall_test.py | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index 85fa53cb..684b5980 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -55,7 +55,7 @@ def is_our_script(filename: str) -> bool: def shebang() -> str: if sys.platform == 'win32': - py = SYS_EXE + py, _ = os.path.splitext(SYS_EXE) else: exe_choices = [ f'python{sys.version_info[0]}.{sys.version_info[1]}', diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 481a7279..7a4b9063 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -56,8 +56,13 @@ def patch_sys_exe(exe): def test_shebang_windows(): + with patch_platform('win32'), patch_sys_exe('python'): + assert shebang() == '#!/usr/bin/env python' + + +def test_shebang_windows_drop_ext(): with patch_platform('win32'), patch_sys_exe('python.exe'): - assert shebang() == '#!/usr/bin/env python.exe' + assert shebang() == '#!/usr/bin/env python' def test_shebang_posix_not_on_path(): From 48886449907f4db13a8f1994340c9948623cd832 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Tue, 15 Sep 2020 12:04:25 -0700 Subject: [PATCH 25/25] remove hardcoded python location --- pre_commit/languages/python.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index 7a685808..afa093d5 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -114,11 +114,6 @@ def get_default_version() -> str: # pragma: no cover (platform dependent) if _find_by_py_launcher(exe): return exe - # Give a best-effort try for windows - default_folder_name = exe.replace('.', '') - if os.path.exists(fr'C:\{default_folder_name}\python.exe'): - return exe - # We tried! return C.DEFAULT @@ -155,12 +150,6 @@ def norm_version(version: str) -> str: if version_exec and version_exec != version: return version_exec - # If it is in the form pythonx.x search in the default - # place on windows - if version.startswith('python'): - default_folder_name = version.replace('.', '') - return fr'C:\{default_folder_name}\python.exe' - # Otherwise assume it is a path return os.path.expanduser(version)