Merging upstream version 2.7.1.

Signed-off-by: Daniel Baumann <daniel@debian.org>

.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/

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
 ==================
 

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,6 +95,8 @@ load_manifest = functools.partial(
 def validate_manifest_main(argv: Optional[Sequence[str]] = None) -> int:
     parser = _make_argparser('Manifest filenames.')
     args = parser.parse_args(argv)
+
+    with logging_handler(args.color):
         ret = 0
         for filename in args.filenames:
             try:
@@ -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,6 +316,8 @@ load_config = functools.partial(
 def validate_config_main(argv: Optional[Sequence[str]] = None) -> int:
     parser = _make_argparser('Config filenames.')
     args = parser.parse_args(argv)
+
+    with logging_handler(args.color):
         ret = 0
         for filename in args.filenames:
             try:

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`.',
+    )

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,
+        config_file,
-        overwrite=True, skip_on_missing_config=True, git_dir=directory,
+        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')

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}')
 
 

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(
+        current_retval, prior_diff = _run_single_hook(
-            classifier, hook, skips, cols,
+            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'

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)
 

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
 

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)

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)

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']
         )

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)

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))
 

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):

setup.cfg

@@ -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

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)
 

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'

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

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

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():

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

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(

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() == []