From 1a263ad18faea35b52ecf261b6d0d681908ba1a7 Mon Sep 17 00:00:00 2001 From: Byoungchan Lee Date: Sun, 10 May 2026 09:09:44 +0900 Subject: [PATCH 1/4] Add locked python install helper --- pre_commit/languages/python.py | 26 ++++++++++++++++++++++++++ tests/languages/python_test.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index 88ececce6..2786bb79d 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -226,3 +226,29 @@ def install_environment( cmd_output_b(*venv_cmd, cwd='/') with in_env(prefix, version): lang_base.setup_cmd(prefix, install_cmd) + + +def install_environment_locked( + prefix: Prefix, + version: str, + lockfile: str, +) -> None: + envdir = lang_base.environment_dir(prefix, ENVIRONMENT_DIR, version) + venv_cmd = [sys.executable, '-mvirtualenv', envdir] + python = norm_version(version) + if python is not None: + venv_cmd.extend(('-p', python)) + + cmd_output_b(*venv_cmd, cwd='/') + with in_env(prefix, version): + lang_base.setup_cmd( + prefix, + ('python', '-mpip', 'install', '--require-hashes', '-r', lockfile), + ) + lang_base.setup_cmd( + prefix, + ( + 'python', '-mpip', 'install', + '--no-deps', '--no-build-isolation', '.', + ), + ) diff --git a/tests/languages/python_test.py b/tests/languages/python_test.py index 593634b79..845478e8e 100644 --- a/tests/languages/python_test.py +++ b/tests/languages/python_test.py @@ -7,6 +7,7 @@ import pytest import pre_commit.constants as C +from pre_commit import lang_base from pre_commit.envcontext import envcontext from pre_commit.languages import python from pre_commit.prefix import Prefix @@ -180,6 +181,34 @@ def test_healthy_venv_creator(python_dir): assert python.health_check(prefix, C.DEFAULT) is None +def test_install_environment_locked_uses_pip(python_dir): + prefix, _ = python_dir + lockfile = '/path/to/requirements.txt' + + with ( + mock.patch.object(python, 'cmd_output_b') as cmd_output_b, + mock.patch.object(lang_base, 'setup_cmd') as setup_cmd, + ): + python.install_environment_locked(prefix, C.DEFAULT, lockfile) + + cmd_output_b.assert_called_once_with( + sys.executable, '-mvirtualenv', prefix.path('py_env-default'), cwd='/', + ) + setup_cmd.assert_has_calls(( + mock.call( + prefix, + ('python', '-mpip', 'install', '--require-hashes', '-r', lockfile), + ), + mock.call( + prefix, + ( + 'python', '-mpip', 'install', + '--no-deps', '--no-build-isolation', '.', + ), + ), + )) + + def test_unhealthy_python_goes_missing(python_dir): prefix, tmpdir = python_dir From 571d434c3eb4f7bf8504e1cc10e04e75995b00f5 Mon Sep 17 00:00:00 2001 From: Byoungchan Lee Date: Sun, 10 May 2026 09:10:33 +0900 Subject: [PATCH 2/4] Key python hook environments by lockfile --- pre_commit/clientlib.py | 35 +++++++++ pre_commit/hook.py | 5 +- pre_commit/repository.py | 155 +++++++++++++++++++++++++++++++++------ pre_commit/store.py | 40 ++++++++-- tests/clientlib_test.py | 47 ++++++++++++ tests/repository_test.py | 58 +++++++++++++++ tests/store_test.py | 3 + 7 files changed, 311 insertions(+), 32 deletions(-) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index 51f14d26e..29c1048ef 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -404,6 +404,39 @@ def check(self, dct: dict[str, Any]) -> None: raise cfgv.ValidationError(f'{self.key!r} cannot be overridden') +class PythonLockfile(NamedTuple): + key: str + default: str | None + + def check(self, dct: dict[str, Any]) -> None: + if self.key not in dct: + return + + with cfgv.validate_context(f'At key: {self.key}'): + cfgv.check_string(dct[self.key]) + + if dct.get('additional_dependencies'): + raise cfgv.ValidationError( + f'{self.key!r} cannot be combined with ' + f"'additional_dependencies'", + ) + + if ( + 'language' in dct and + _translate_language(dct['language']) != 'python' + ): + raise cfgv.ValidationError( + f'{self.key!r} is only supported for language: python', + ) + + def apply_default(self, dct: dict[str, Any]) -> None: + if self.default is not None: + dct.setdefault(self.key, self.default) + + def remove_default(self, dct: dict[str, Any]) -> None: + raise NotImplementedError + + _COMMON_HOOK_WARNINGS = ( OptionalSensibleRegexAtHook('files', cfgv.check_string), OptionalSensibleRegexAtHook('exclude', cfgv.check_string), @@ -454,12 +487,14 @@ def check(self, dct: dict[str, Any]) -> None: ), StagesMigrationNoDefault('stages', []), LanguageMigration('language', cfgv.check_one_of(language_names)), # remove + PythonLockfile('python_lockfile', None), *_COMMON_HOOK_WARNINGS, ) LOCAL_HOOK_DICT = cfgv.Map( 'Hook', 'id', *MANIFEST_HOOK_DICT.items, + PythonLockfile('python_lockfile', ''), *_COMMON_HOOK_WARNINGS, ) CONFIG_REPO_DICT = cfgv.Map( diff --git a/pre_commit/hook.py b/pre_commit/hook.py index 309cd5be3..e51dfbd08 100644 --- a/pre_commit/hook.py +++ b/pre_commit/hook.py @@ -24,6 +24,8 @@ class Hook(NamedTuple): types_or: Sequence[str] exclude_types: Sequence[str] additional_dependencies: Sequence[str] + python_lockfile: str + python_lockfile_sha256: str args: Sequence[str] always_run: bool fail_fast: bool @@ -37,12 +39,13 @@ class Hook(NamedTuple): verbose: bool @property - def install_key(self) -> tuple[Prefix, str, str, tuple[str, ...]]: + def install_key(self) -> tuple[Prefix, str, str, tuple[str, ...], str]: return ( self.prefix, self.language, self.language_version, tuple(self.additional_dependencies), + self.python_lockfile_sha256, ) @classmethod diff --git a/pre_commit/repository.py b/pre_commit/repository.py index a9461ab63..a9d26d708 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -1,16 +1,19 @@ from __future__ import annotations +import hashlib import json import logging import os from collections.abc import Sequence from typing import Any +from typing import cast import pre_commit.constants as C from pre_commit.all_languages import languages from pre_commit.clientlib import load_manifest from pre_commit.clientlib import LOCAL from pre_commit.clientlib import META +from pre_commit.errors import FatalError from pre_commit.hook import Hook from pre_commit.lang_base import environment_dir from pre_commit.prefix import Prefix @@ -30,8 +33,15 @@ def _state_filename_v2(venv: str) -> str: return os.path.join(venv, '.install_state_v2') -def _state(additional_deps: Sequence[str]) -> object: - return {'additional_dependencies': additional_deps} +def _state(hook: Hook) -> object: + return { + 'additional_dependencies': hook.additional_dependencies, + 'python_lockfile_sha256': hook.python_lockfile_sha256, + } + + +def _state_v1_legacy(hook: Hook) -> object: + return {'additional_dependencies': hook.additional_dependencies} def _read_state(venv: str) -> object | None: @@ -56,7 +66,7 @@ def _hook_installed(hook: Hook) -> bool: return ( ( os.path.exists(_state_filename_v2(venv)) or - _read_state(venv) == _state(hook.additional_dependencies) + _read_state(venv) in (_state(hook), _state_v1_legacy(hook)) ) and not lang.health_check(hook.prefix, hook.language_version) ) @@ -82,9 +92,15 @@ def _hook_install(hook: Hook) -> None: rmtree(venv) with clean_path_on_failure(venv): - lang.install_environment( - hook.prefix, hook.language_version, hook.additional_dependencies, - ) + if hook.python_lockfile: + cast(Any, lang).install_environment_locked( + hook.prefix, hook.language_version, hook.python_lockfile, + ) + else: + lang.install_environment( + hook.prefix, hook.language_version, + hook.additional_dependencies, + ) health_error = lang.health_check(hook.prefix, hook.language_version) if health_error: raise AssertionError( @@ -99,16 +115,55 @@ def _hook_install(hook: Hook) -> None: state_filename = _state_filename_v1(venv) staging = f'{state_filename}staging' with open(staging, 'w') as state_file: - state_file.write(json.dumps(_state(hook.additional_dependencies))) + state_file.write(json.dumps(_state(hook))) # Move the file into place atomically to indicate we've installed os.replace(staging, state_filename) open(_state_filename_v2(venv), 'a+').close() +def _lockfile_sha256(hook_id: str, lockfile: str) -> str: + try: + with open(lockfile, 'rb') as f: + digest = hashlib.sha256() + for chunk in iter(lambda: f.read(1024 * 1024), b''): + digest.update(chunk) + return digest.hexdigest() + except OSError as e: + raise FatalError( + f'Could not read python_lockfile for hook `{hook_id}`: ' + f'{lockfile}: {e.strerror}', + ) + + +def _lockfile_path(config_file: str, lockfile: str) -> str: + if os.path.isabs(lockfile): + return os.path.normpath(lockfile) + else: + return os.path.normpath( + os.path.join( + os.path.dirname(os.path.abspath(config_file)), + lockfile, + ), + ) + + +def install_key_for_hook( + hook: dict[str, Any], + config_file: str, +) -> tuple[Sequence[str], str]: + lockfile = hook.get('python_lockfile', '') + if not lockfile: + return hook.get('additional_dependencies', ()), '' + + lockfile = _lockfile_path(config_file, lockfile) + return (), _lockfile_sha256(hook['id'], lockfile) + + def _hook( *hook_dicts: dict[str, Any], root_config: dict[str, Any], + config_file: str, ) -> dict[str, Any]: ret, rest = dict(hook_dicts[0]), hook_dicts[1:] for dct in rest: @@ -123,6 +178,31 @@ def _hook( if not ret['stages']: ret['stages'] = root_config['default_stages'] + ret.setdefault('python_lockfile', '') + ret['python_lockfile_sha256'] = '' + if ret['python_lockfile']: + if lang != 'python': + logger.error( + f'The hook `{ret["id"]}` specifies `python_lockfile` but is ' + f'using language `{lang}`. `python_lockfile` is only ' + f'supported for language `python`.', + ) + exit(1) + if ret['additional_dependencies']: + logger.error( + f'The hook `{ret["id"]}` specifies `python_lockfile` and ' + f'`additional_dependencies`. These options are mutually ' + f'exclusive.', + ) + exit(1) + + ret['python_lockfile'] = _lockfile_path( + config_file, ret['python_lockfile'], + ) + ret['python_lockfile_sha256'] = _lockfile_sha256( + ret['id'], ret['python_lockfile'], + ) + if languages[lang].ENVIRONMENT_DIR is None: if ret['language_version'] != C.DEFAULT: logger.error( @@ -148,30 +228,42 @@ def _non_cloned_repository_hooks( repo_config: dict[str, Any], store: Store, root_config: dict[str, Any], + config_file: str, ) -> tuple[Hook, ...]: - def _prefix(language_name: str, deps: Sequence[str]) -> Prefix: + def _prefix( + language_name: str, + deps: Sequence[str], + python_lockfile_sha256: str, + ) -> Prefix: language = languages[language_name] # pygrep / script / system / docker_image do not have # environments so they work out of the current directory if language.ENVIRONMENT_DIR is None: return Prefix(os.getcwd()) else: - return Prefix(store.make_local(deps)) + return Prefix(store.make_local(deps, python_lockfile_sha256)) - return tuple( - Hook.create( - repo_config['repo'], - _prefix(hook['language'], hook['additional_dependencies']), - _hook(hook, root_config=root_config), + ret = [] + for hook in repo_config['hooks']: + hook = _hook(hook, root_config=root_config, config_file=config_file) + ret.append( + Hook.create( + repo_config['repo'], + _prefix( + hook['language'], hook['additional_dependencies'], + hook['python_lockfile_sha256'], + ), + hook, + ), ) - for hook in repo_config['hooks'] - ) + return tuple(ret) def _cloned_repository_hooks( repo_config: dict[str, Any], store: Store, root_config: dict[str, Any], + config_file: str, ) -> tuple[Hook, ...]: repo, rev = repo_config['repo'], repo_config['rev'] manifest_path = os.path.join(store.clone(repo, rev), C.MANIFEST_FILE) @@ -187,13 +279,21 @@ def _cloned_repository_hooks( exit(1) hook_dcts = [ - _hook(by_id[hook['id']], hook, root_config=root_config) + _hook( + by_id[hook['id']], hook, root_config=root_config, + config_file=config_file, + ) for hook in repo_config['hooks'] ] return tuple( Hook.create( repo_config['repo'], - Prefix(store.clone(repo, rev, hook['additional_dependencies'])), + Prefix( + store.clone( + repo, rev, hook['additional_dependencies'], + hook['python_lockfile_sha256'], + ), + ), hook, ) for hook in hook_dcts @@ -204,16 +304,21 @@ def _repository_hooks( repo_config: dict[str, Any], store: Store, root_config: dict[str, Any], + config_file: str, ) -> tuple[Hook, ...]: if repo_config['repo'] in {LOCAL, META}: - return _non_cloned_repository_hooks(repo_config, store, root_config) + return _non_cloned_repository_hooks( + repo_config, store, root_config, config_file, + ) else: - return _cloned_repository_hooks(repo_config, store, root_config) + return _cloned_repository_hooks( + repo_config, store, root_config, config_file, + ) def install_hook_envs(hooks: Sequence[Hook], store: Store) -> None: def _need_installed() -> list[Hook]: - seen: set[tuple[Prefix, str, str, tuple[str, ...]]] = set() + seen: set[tuple[Prefix, str, str, tuple[str, ...], str]] = set() ret = [] for hook in hooks: if hook.install_key not in seen and not _hook_installed(hook): @@ -229,9 +334,13 @@ def _need_installed() -> list[Hook]: _hook_install(hook) -def all_hooks(root_config: dict[str, Any], store: Store) -> tuple[Hook, ...]: +def all_hooks( + root_config: dict[str, Any], + store: Store, + config_file: str = C.CONFIG_FILE, +) -> tuple[Hook, ...]: return tuple( hook for repo in root_config['repos'] - for hook in _repository_hooks(repo, store, root_config) + for hook in _repository_hooks(repo, store, root_config, config_file) ) diff --git a/pre_commit/store.py b/pre_commit/store.py index dc90c0519..a3ef27e9c 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -123,21 +123,32 @@ def connect( yield db @classmethod - def db_repo_name(cls, repo: str, deps: Sequence[str]) -> str: + def db_repo_name( + cls, + repo: str, + deps: Sequence[str], + python_lockfile_sha256: str = '', + ) -> str: if deps: - return f'{repo}:{",".join(deps)}' + ret = f'{repo}:{",".join(deps)}' else: - return repo + ret = repo + + if python_lockfile_sha256: + return f'{ret}:python_lockfile_sha256={python_lockfile_sha256}' + + return ret def _new_repo( self, repo: str, ref: str, deps: Sequence[str], + python_lockfile_sha256: str, make_strategy: Callable[[str], None], ) -> str: original_repo = repo - repo = self.db_repo_name(repo, deps) + repo = self.db_repo_name(repo, deps, python_lockfile_sha256) def _get_result() -> str | None: # Check if we already exist @@ -192,7 +203,13 @@ def _shallow_clone(self, ref: str, git_cmd: Callable[..., None]) -> None: '--depth=1', ) - def clone(self, repo: str, ref: str, deps: Sequence[str] = ()) -> str: + def clone( + self, + repo: str, + ref: str, + deps: Sequence[str] = (), + python_lockfile_sha256: str = '', + ) -> str: """Clone the given url and checkout the specific ref.""" def clone_strategy(directory: str) -> None: @@ -207,11 +224,18 @@ def _git_cmd(*args: str) -> None: except CalledProcessError: self._complete_clone(ref, _git_cmd) - return self._new_repo(repo, ref, deps, clone_strategy) + return self._new_repo( + repo, ref, deps, python_lockfile_sha256, clone_strategy, + ) - def make_local(self, deps: Sequence[str]) -> str: + def make_local( + self, + deps: Sequence[str], + python_lockfile_sha256: str = '', + ) -> str: return self._new_repo( - 'local', C.LOCAL_REPO_VERSION, deps, _make_local_repo, + 'local', C.LOCAL_REPO_VERSION, deps, python_lockfile_sha256, + _make_local_repo, ) def _create_configs_table(self, db: sqlite3.Connection) -> None: diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index 2c42b80cf..5dbe7bceb 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -435,6 +435,53 @@ def test_valid_manifests(manifest_obj): assert is_valid_according_to_schema(manifest_obj, MANIFEST_SCHEMA) +def test_python_lockfile_valid_for_local_python_hook(): + cfg = sample_local_config() + cfg['hooks'][0].update({ + 'entry': 'python hook.py', + 'language': 'python', + 'python_lockfile': 'requirements.txt', + }) + + cfgv.validate(cfg, CONFIG_REPO_DICT) + + +def test_python_lockfile_valid_for_config_hook(): + cfg = { + 'repo': 'https://example.invalid/repo', + 'rev': 'v1.0.0', + 'hooks': [{'id': 'hook', 'python_lockfile': 'requirements.txt'}], + } + + cfgv.validate(cfg, CONFIG_REPO_DICT) + + +@pytest.mark.parametrize('language', ('system', 'node')) +def test_python_lockfile_invalid_for_non_python_local_hook(language): + cfg = sample_local_config() + cfg['hooks'][0].update({ + 'entry': 'echo hi', + 'language': language, + 'python_lockfile': 'requirements.txt', + }) + + with pytest.raises(cfgv.ValidationError): + cfgv.validate(cfg, CONFIG_REPO_DICT) + + +def test_python_lockfile_invalid_with_additional_dependencies(): + cfg = sample_local_config() + cfg['hooks'][0].update({ + 'entry': 'python hook.py', + 'language': 'python', + 'additional_dependencies': ['astpretty'], + 'python_lockfile': 'requirements.txt', + }) + + with pytest.raises(cfgv.ValidationError): + cfgv.validate(cfg, CONFIG_REPO_DICT) + + @pytest.mark.parametrize( 'config_repo', ( diff --git a/tests/repository_test.py b/tests/repository_test.py index 5d71c3e4c..b171dbb63 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -15,6 +15,7 @@ from pre_commit.all_languages import languages from pre_commit.clientlib import CONFIG_SCHEMA from pre_commit.clientlib import load_manifest +from pre_commit.errors import FatalError from pre_commit.hook import Hook from pre_commit.languages import python from pre_commit.languages import unsupported @@ -55,12 +56,67 @@ def _get_hook_no_install(repo_config, store, hook_id): return hook +def _config_with_repo(repo_config): + config = {'repos': [repo_config]} + config = cfgv.validate(config, CONFIG_SCHEMA) + return cfgv.apply_defaults(config, CONFIG_SCHEMA) + + def _get_hook(repo_config, store, hook_id): hook = _get_hook_no_install(repo_config, store, hook_id) install_hook_envs([hook], store) return hook +def test_python_lockfile_changes_store_key(tempdir_factory, store, tmp_path): + path = make_repo(tempdir_factory, 'python_hooks_repo') + config_file = tmp_path.joinpath(C.CONFIG_FILE) + lockfile = tmp_path.joinpath('requirements.txt') + lockfile.write_text('identify==2.6.10\n') + config_file.write_text('') + + config = make_config_from_repo(path) + config['hooks'][0]['python_lockfile'] = 'requirements.txt' + hook1, = all_hooks(_config_with_repo(config), store, str(config_file)) + + lockfile.write_text('identify==2.6.11\n') + hook2, = all_hooks(_config_with_repo(config), store, str(config_file)) + + assert hook1.python_lockfile_sha256 != hook2.python_lockfile_sha256 + assert hook1.prefix != hook2.prefix + + +def test_python_lockfiles_do_not_collide(tempdir_factory, store, tmp_path): + path = make_repo(tempdir_factory, 'python_hooks_repo') + config_file = tmp_path.joinpath(C.CONFIG_FILE) + config_file.write_text('') + tmp_path.joinpath('requirements-1.txt').write_text('identify==2.6.10\n') + tmp_path.joinpath('requirements-2.txt').write_text('identify==2.6.11\n') + + config = make_config_from_repo(path) + config['hooks'].append(dict(config['hooks'][0])) + config['hooks'][0]['python_lockfile'] = 'requirements-1.txt' + config['hooks'][1]['python_lockfile'] = 'requirements-2.txt' + hooks = all_hooks(_config_with_repo(config), store, str(config_file)) + + assert hooks[0].prefix != hooks[1].prefix + + +def test_python_lockfile_missing_fails(tempdir_factory, store, tmp_path): + path = make_repo(tempdir_factory, 'python_hooks_repo') + config_file = tmp_path.joinpath(C.CONFIG_FILE) + config_file.write_text('') + + config = make_config_from_repo(path) + config['hooks'][0]['python_lockfile'] = 'requirements.txt' + + with pytest.raises(FatalError) as excinfo: + all_hooks(_config_with_repo(config), store, str(config_file)) + + msg = str(excinfo.value) + assert 'Could not read python_lockfile for hook `foo`' in msg + + def _test_hook_repo( tempdir_factory, store, @@ -415,6 +471,8 @@ def test_manifest_hooks(tempdir_factory, store): src=f'file://{path}', prefix=Prefix(mock.ANY), additional_dependencies=[], + python_lockfile='', + python_lockfile_sha256='', alias='', always_run=False, args=[], diff --git a/tests/store_test.py b/tests/store_test.py index 13f198ea2..58a1be778 100644 --- a/tests/store_test.py +++ b/tests/store_test.py @@ -261,6 +261,9 @@ def test_create_when_store_already_exists(store): def test_db_repo_name(store): assert store.db_repo_name('repo', ()) == 'repo' assert store.db_repo_name('repo', ('b', 'a', 'c')) == 'repo:b,a,c' + assert store.db_repo_name( + 'repo', (), 'a' * 64, + ) == f'repo:python_lockfile_sha256={"a" * 64}' def test_local_resources_reflects_reality(): From f63e51f57cf35d8357009408193c792edfe8b2fb Mon Sep 17 00:00:00 2001 From: Byoungchan Lee Date: Sun, 10 May 2026 09:11:07 +0900 Subject: [PATCH 3/4] Resolve hook lockfiles from config paths --- pre_commit/commands/install_uninstall.py | 4 +++- pre_commit/commands/run.py | 2 +- pre_commit/meta_hooks/check_hooks_apply.py | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index d19e0d47e..67a78e87f 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -142,7 +142,9 @@ def install( def install_hooks(config_file: str, store: Store) -> int: - install_hook_envs(all_hooks(load_config(config_file), store), store) + install_hook_envs( + all_hooks(load_config(config_file), store, config_file), store, + ) return 0 diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 8ab505ffb..1a8416cb3 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -423,7 +423,7 @@ def run( config = load_config(config_file) hooks = [ hook - for hook in all_hooks(config, store) + for hook in all_hooks(config, store, config_file) if not args.hook or hook.id == args.hook or hook.alias == args.hook if args.hook_stage in hook.stages ] diff --git a/pre_commit/meta_hooks/check_hooks_apply.py b/pre_commit/meta_hooks/check_hooks_apply.py index 84c142b45..2858b9dc7 100644 --- a/pre_commit/meta_hooks/check_hooks_apply.py +++ b/pre_commit/meta_hooks/check_hooks_apply.py @@ -18,7 +18,7 @@ def check_all_hooks_match_files(config_file: str) -> int: ) retv = 0 - for hook in all_hooks(config, Store()): + for hook in all_hooks(config, Store(), config_file): if hook.always_run or hook.language == 'fail': continue elif not any(classifier.filenames_for_hook(hook)): From 309f76104c44fa29e014884bd3a35bebf3bef324 Mon Sep 17 00:00:00 2001 From: Byoungchan Lee Date: Sun, 10 May 2026 09:11:38 +0900 Subject: [PATCH 4/4] Preserve locked python repos during gc --- pre_commit/commands/gc.py | 27 +++++++++++++++++++-------- tests/commands/gc_test.py | 27 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/pre_commit/commands/gc.py b/pre_commit/commands/gc.py index 975d5e4c1..d04a45172 100644 --- a/pre_commit/commands/gc.py +++ b/pre_commit/commands/gc.py @@ -11,6 +11,8 @@ from pre_commit.clientlib import load_manifest from pre_commit.clientlib import LOCAL from pre_commit.clientlib import META +from pre_commit.errors import FatalError +from pre_commit.repository import install_key_for_hook from pre_commit.store import Store from pre_commit.util import rmtree @@ -20,14 +22,19 @@ def _mark_used_repos( all_repos: dict[tuple[str, str], str], unused_repos: set[tuple[str, str]], repo: dict[str, Any], + config_path: str, ) -> None: if repo['repo'] == META: return elif repo['repo'] == LOCAL: for hook in repo['hooks']: - deps = hook.get('additional_dependencies') + deps, python_lockfile_sha256 = install_key_for_hook( + hook, config_path, + ) unused_repos.discard(( - store.db_repo_name(repo['repo'], deps), + store.db_repo_name( + repo['repo'], deps, python_lockfile_sha256, + ), C.LOCAL_REPO_VERSION, )) else: @@ -49,12 +56,14 @@ def _mark_used_repos( if hook['id'] not in by_id: continue - deps = hook.get( - 'additional_dependencies', - by_id[hook['id']]['additional_dependencies'], + hook_dct = dict(by_id[hook['id']]) + hook_dct.update(hook) + deps, python_lockfile_sha256 = install_key_for_hook( + hook_dct, config_path, ) unused_repos.discard(( - store.db_repo_name(repo['repo'], deps), repo['rev'], + store.db_repo_name(repo['repo'], deps, python_lockfile_sha256), + repo['rev'], )) @@ -73,12 +82,14 @@ def _gc(store: Store) -> int: for config_path in configs: try: config = load_config(config_path) - except InvalidConfigError: + except (InvalidConfigError, FatalError): dead_configs.append(config_path) continue else: for repo in config['repos']: - _mark_used_repos(store, all_repos, unused_repos, repo) + _mark_used_repos( + store, all_repos, unused_repos, repo, config_path, + ) paths = [(path,) for path in dead_configs] db.executemany('DELETE FROM configs WHERE path = ?', paths) diff --git a/tests/commands/gc_test.py b/tests/commands/gc_test.py index 992b02f3b..856964d17 100644 --- a/tests/commands/gc_test.py +++ b/tests/commands/gc_test.py @@ -111,6 +111,33 @@ def test_gc_unused_local_repo_with_env(store, in_git_dir, cap_out): _remove_config_assert_cleared(store, cap_out) +def test_gc_preserves_python_lockfile_repo( + tempdir_factory, store, in_git_dir, cap_out, +): + path = make_repo(tempdir_factory, 'python_hooks_repo') + with open('requirements.txt', 'w') as f: + f.write('identify==2.6.10\n') + config = make_config_from_repo(path) + config['hooks'][0]['python_lockfile'] = 'requirements.txt' + write_config('.', config) + store.mark_config_used(C.CONFIG_FILE) + + all_hooks(load_config(C.CONFIG_FILE), store) + + assert _config_count(store) == 1 + assert _repo_count(store) == 2 + assert not gc(store) + assert _config_count(store) == 1 + assert _repo_count(store) == 2 + assert cap_out.get().splitlines()[-1] == '0 repo(s) removed.' + + os.remove(C.CONFIG_FILE) + assert not gc(store) + assert _config_count(store) == 0 + assert _repo_count(store) == 0 + assert cap_out.get().splitlines()[-1] == '2 repo(s) removed.' + + def test_gc_config_with_missing_hook( tempdir_factory, store, in_git_dir, cap_out, ):