diff options
| -rw-r--r-- | .pre-commit-config.yaml | 14 | ||||
| -rw-r--r-- | CHANGELOG.md | 37 | ||||
| -rw-r--r-- | pre_commit/clientlib.py | 45 | ||||
| -rw-r--r-- | pre_commit/color.py | 10 | ||||
| -rw-r--r-- | pre_commit/commands/init_templatedir.py | 9 | ||||
| -rw-r--r-- | pre_commit/commands/install_uninstall.py | 2 | ||||
| -rw-r--r-- | pre_commit/commands/run.py | 24 | ||||
| -rw-r--r-- | pre_commit/error_handler.py | 13 | ||||
| -rw-r--r-- | pre_commit/file_lock.py | 6 | ||||
| -rw-r--r-- | pre_commit/languages/docker.py | 19 | ||||
| -rw-r--r-- | pre_commit/languages/docker_image.py | 2 | ||||
| -rw-r--r-- | pre_commit/languages/python.py | 3 | ||||
| -rw-r--r-- | pre_commit/main.py | 42 | ||||
| -rw-r--r-- | pre_commit/repository.py | 8 | ||||
| -rw-r--r-- | pre_commit/store.py | 8 | ||||
| -rw-r--r-- | setup.cfg | 2 | ||||
| -rw-r--r-- | testing/util.py | 12 | ||||
| -rw-r--r-- | tests/clientlib_test.py | 9 | ||||
| -rw-r--r-- | tests/commands/init_templatedir_test.py | 48 | ||||
| -rw-r--r-- | tests/error_handler_test.py | 29 | ||||
| -rw-r--r-- | tests/languages/docker_test.py | 9 | ||||
| -rw-r--r-- | tests/languages/python_test.py | 24 | ||||
| -rw-r--r-- | tests/main_test.py | 21 | ||||
| -rw-r--r-- | tests/store_test.py | 26 |
24 files changed, 323 insertions, 99 deletions
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 36d73c7..e9cf739 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/CHANGELOG.md b/CHANGELOG.md index c487acf..a92a6b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,40 @@ +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 +================== + +### 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/pre_commit/clientlib.py b/pre_commit/clientlib.py index 56ec0dd..8dfa947 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 eb906b7..4ddfdf5 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/commands/init_templatedir.py b/pre_commit/commands/init_templatedir.py index f676fb1..5f17d9c 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/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index c8b7633..85fa53c 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/commands/run.py b/pre_commit/commands/run.py index 567b7cd..1f28c8c 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' diff --git a/pre_commit/error_handler.py b/pre_commit/error_handler.py index b2321ae..13d78cb 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/pre_commit/file_lock.py b/pre_commit/file_lock.py index ff0dc5e..5e7a058 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 diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index 4091492..9c13119 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 0c51df6..311d127 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/pre_commit/languages/python.py b/pre_commit/languages/python.py index 6f7c900..7a68580 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/pre_commit/main.py b/pre_commit/main.py index 874eb53..8647960 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,17 +231,23 @@ 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.', ) + 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( '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', @@ -280,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.', @@ -322,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) @@ -383,6 +380,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/pre_commit/repository.py b/pre_commit/repository.py index 77734ee..46e96c1 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: @@ -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)) diff --git a/pre_commit/store.py b/pre_commit/store.py index 6d8c40a..e5522ec 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) @@ -75,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]: @@ -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): @@ -1,6 +1,6 @@ [metadata] name = pre_commit -version = 2.6.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 diff --git a/testing/util.py b/testing/util.py index 4edb7a9..f556a8d 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/clientlib_test.py b/tests/clientlib_test.py index c48adbd..2e2f738 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' diff --git a/tests/commands/init_templatedir_test.py b/tests/commands/init_templatedir_test.py index d14a171..4e131df 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/error_handler_test.py b/tests/error_handler_test.py index 833bb8f..d066e57 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 diff --git a/tests/languages/docker_test.py b/tests/languages/docker_test.py index b65b223..3bed4bf 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(): diff --git a/tests/languages/python_test.py b/tests/languages/python_test.py index c419ad6..29c5a9b 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 diff --git a/tests/main_test.py b/tests/main_test.py index c472476..f7abeeb 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( diff --git a/tests/store_test.py b/tests/store_test.py index 6a4e900..0947144 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() == [] |