Skip to content

Commit ea09e8b

Browse files
asottileasottile
committed
add support for top-level map manifest
1 parent 8416413 commit ea09e8b

File tree

10 files changed

+157
-62
lines changed

10 files changed

+157
-62
lines changed

‎pre_commit/clientlib.py‎

Lines changed: 46 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,20 @@ def transform_stage(stage: str) -> str:
7272
return _STAGES.get(stage, stage)
7373

7474

75-
MINIMAL_MANIFEST_SCHEMA = cfgv.Array(
76-
cfgv.Map(
77-
'Hook', 'id',
78-
cfgv.Required('id', cfgv.check_string),
79-
cfgv.Optional('stages', cfgv.check_array(cfgv.check_string), []),
75+
_MINIMAL_MANIFEST_SCHEMA = cfgv.Map(
76+
'Manifest', None,
77+
cfgv.RequiredRecurse(
78+
'hooks',
79+
cfgv.KeyValueMap(
80+
'Hooks',
81+
cfgv.check_string,
82+
cfgv.Map(
83+
'Hook', None,
84+
cfgv.Optional(
85+
'stages', cfgv.check_array(cfgv.check_string), [],
86+
),
87+
),
88+
),
8089
),
8190
)
8291

@@ -85,16 +94,16 @@ def warn_for_stages_on_repo_init(repo: str, directory: str) -> None:
8594
try:
8695
manifest = cfgv.load_from_filename(
8796
os.path.join(directory, C.MANIFEST_FILE),
88-
schema=MINIMAL_MANIFEST_SCHEMA,
89-
load_strategy=yaml_load,
97+
schema=_MINIMAL_MANIFEST_SCHEMA,
98+
load_strategy=_load_manifest_backward_compat,
9099
exc_tp=InvalidManifestError,
91100
)
92101
except InvalidManifestError:
93102
return # they'll get a better error message when it actually loads!
94103

95104
legacy_stages = {} # sorted set
96-
for hook in manifest:
97-
for stage in hook.get('stages', ()):
105+
for hook in manifest['hooks'].values():
106+
for stage in hook['stages']:
98107
if stage in _STAGES:
99108
legacy_stages[stage] = True
100109

@@ -228,7 +237,7 @@ def check(self, dct: dict[str, Any]) -> None:
228237

229238

230239
MANIFEST_HOOK_DICT = cfgv.Map(
231-
'Hook', 'id',
240+
'Hook', None,
232241

233242
# check first in case it uses some newer, incompatible feature
234243
cfgv.Optional(
@@ -237,7 +246,6 @@ def check(self, dct: dict[str, Any]) -> None:
237246
'0',
238247
),
239248

240-
cfgv.Required('id', cfgv.check_string),
241249
cfgv.Required('name', cfgv.check_string),
242250
cfgv.Required('entry', cfgv.check_string),
243251
LanguageMigrationRequired('language', cfgv.check_one_of(language_names)),
@@ -263,26 +271,46 @@ def check(self, dct: dict[str, Any]) -> None:
263271
StagesMigration('stages', []),
264272
cfgv.Optional('verbose', cfgv.check_bool, False),
265273
)
266-
MANIFEST_SCHEMA = cfgv.Array(MANIFEST_HOOK_DICT)
274+
275+
276+
MANIFEST_SCHEMA = cfgv.Map(
277+
'Manifest', None,
278+
279+
# check first in case it uses some newer, incompatible feature
280+
cfgv.Optional(
281+
'minimum_pre_commit_version',
282+
cfgv.check_and(cfgv.check_string, check_min_version),
283+
'0',
284+
),
285+
cfgv.RequiredRecurse(
286+
'hooks',
287+
cfgv.KeyValueMap('Hooks', cfgv.check_string, MANIFEST_HOOK_DICT),
288+
),
289+
)
267290

268291

269292
class InvalidManifestError(FatalError):
270293
pass
271294

272295

273-
def _load_manifest_forward_compat(contents: str) -> object:
296+
_MINIMAL_LEGACY_MANIFEST_SCHEMA = cfgv.Array(
297+
cfgv.Map('Hook', 'id', cfgv.Required('id', cfgv.check_string)),
298+
)
299+
300+
301+
def _load_manifest_backward_compat(contents: str) -> object:
274302
obj = yaml_load(contents)
275-
if isinstance(obj, dict):
276-
check_min_version('5')
277-
raise AssertionError('unreachable')
303+
if isinstance(obj, list):
304+
cfgv.validate(obj, _MINIMAL_LEGACY_MANIFEST_SCHEMA)
305+
return {'hooks': {hook.pop('id'): hook for hook in obj}}
278306
else:
279307
return obj
280308

281309

282310
load_manifest = functools.partial(
283311
cfgv.load_from_filename,
284312
schema=MANIFEST_SCHEMA,
285-
load_strategy=_load_manifest_forward_compat,
313+
load_strategy=_load_manifest_backward_compat,
286314
exc_tp=InvalidManifestError,
287315
)
288316

@@ -448,7 +476,6 @@ def check(self, dct: dict[str, Any]) -> None:
448476
*(
449477
cfgv.OptionalNoDefault(item.key, item.check_fn)
450478
for item in MANIFEST_HOOK_DICT.items
451-
if item.key != 'id'
452479
if item.key != 'stages'
453480
if item.key != 'language' # remove
454481
),
@@ -459,6 +486,7 @@ def check(self, dct: dict[str, Any]) -> None:
459486
LOCAL_HOOK_DICT = cfgv.Map(
460487
'Hook', 'id',
461488

489+
cfgv.Required('id', cfgv.check_string),
462490
*MANIFEST_HOOK_DICT.items,
463491
*_COMMON_HOOK_WARNINGS,
464492
)

‎pre_commit/commands/autoupdate.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ def update(self, tags_only: bool, freeze: bool) -> RevInfo:
7777
except InvalidManifestError as e:
7878
raise RepositoryCannotBeUpdatedError(f'[{self.repo}] {e}')
7979
else:
80-
hook_ids = frozenset(hook['id'] for hook in manifest)
80+
hook_ids = frozenset(hook['id'] for hook in manifest['hooks'])
8181

8282
return self._replace(rev=rev, frozen=frozen, hook_ids=hook_ids)
8383

‎pre_commit/commands/gc.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def _mark_used_repos(
4343
return
4444
else:
4545
unused_repos.discard(key)
46-
by_id = {hook['id']: hook for hook in manifest}
46+
by_id = manifest['hooks']
4747

4848
for hook in repo['hooks']:
4949
if hook['id'] not in by_id:

‎pre_commit/commands/try_repo.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ def try_repo(args: argparse.Namespace) -> int:
5858
else:
5959
repo_path = store.clone(repo, ref)
6060
manifest = load_manifest(os.path.join(repo_path, C.MANIFEST_FILE))
61-
manifest = sorted(manifest, key=lambda hook: hook['id'])
62-
hooks = [{'id': hook['id']} for hook in manifest]
61+
hook_ids = sorted(hook['id'] for hook in manifest['hooks'])
62+
hooks = [{'id': hook_id} for hook_id in hook_ids]
6363

6464
config = {'repos': [{'repo': repo, 'rev': ref, 'hooks': hooks}]}
6565
config_s = yaml_dump(config)

‎pre_commit/repository.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ def _cloned_repository_hooks(
175175
) -> tuple[Hook, ...]:
176176
repo, rev = repo_config['repo'], repo_config['rev']
177177
manifest_path = os.path.join(store.clone(repo, rev), C.MANIFEST_FILE)
178-
by_id = {hook['id']: hook for hook in load_manifest(manifest_path)}
178+
by_id = load_manifest(manifest_path)['hooks']
179179

180180
for hook in repo_config['hooks']:
181181
if hook['id'] not in by_id:

‎setup.cfg‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ classifiers =
1818
[options]
1919
packages = find:
2020
install_requires =
21-
cfgv>=2.0.0
21+
cfgv>=3.5.0
2222
identify>=1.0.0
2323
nodeenv>=0.11.1
2424
pyyaml>=5.1

‎testing/fixtures.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ def make_config_from_repo(repo_path, rev=None, hooks=None, check=True):
101101
config = {
102102
'repo': f'file://{repo_path}',
103103
'rev': rev or git.head_rev(repo_path),
104-
'hooks': hooks or [{'id': hook['id']} for hook in manifest],
104+
'hooks': hooks or [{'id': hook_id} for hook_id in manifest['hooks']],
105105
}
106106

107107
if check:

‎tests/clientlib_test.py‎

Lines changed: 60 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from pre_commit.clientlib import CONFIG_REPO_DICT
1313
from pre_commit.clientlib import CONFIG_SCHEMA
1414
from pre_commit.clientlib import DEFAULT_LANGUAGE_VERSION
15-
from pre_commit.clientlib import InvalidManifestError
1615
from pre_commit.clientlib import load_manifest
1716
from pre_commit.clientlib import MANIFEST_HOOK_DICT
1817
from pre_commit.clientlib import MANIFEST_SCHEMA
@@ -405,30 +404,40 @@ def test_unsupported_language_migration_language_required():
405404
@pytest.mark.parametrize(
406405
'manifest_obj',
407406
(
408-
[{
409-
'id': 'a',
410-
'name': 'b',
411-
'entry': 'c',
412-
'language': 'python',
413-
'files': r'\.py$',
414-
}],
415-
[{
416-
'id': 'a',
417-
'name': 'b',
418-
'entry': 'c',
419-
'language': 'python',
420-
'language_version': 'python3.4',
421-
'files': r'\.py$',
422-
}],
407+
{
408+
'hooks': {
409+
'a': {
410+
'name': 'b',
411+
'entry': 'c',
412+
'language': 'python',
413+
'files': r'\.py$',
414+
},
415+
},
416+
},
417+
{
418+
'hooks': {
419+
'a': {
420+
'name': 'b',
421+
'entry': 'c',
422+
'language': 'python',
423+
'language_version': 'python3.4',
424+
'files': r'\.py$',
425+
},
426+
},
427+
},
423428
# A regression in 0.13.5: always_run and files are permissible
424-
[{
425-
'id': 'a',
426-
'name': 'b',
427-
'entry': 'c',
428-
'language': 'python',
429-
'files': '',
430-
'always_run': True,
431-
}],
429+
{
430+
'hooks': {
431+
'a': {
432+
'id': 'a',
433+
'name': 'b',
434+
'entry': 'c',
435+
'language': 'python',
436+
'files': '',
437+
'always_run': True,
438+
},
439+
},
440+
},
432441
),
433442
)
434443
def test_valid_manifests(manifest_obj):
@@ -592,16 +601,31 @@ def test_config_hook_stages_defaulting():
592601
}
593602

594603

595-
def test_manifest_v5_forward_compat(tmp_path):
604+
def test_manifest_backward_compat(tmp_path):
605+
src = '''\
606+
- id: example
607+
name: example
608+
language: unsupported
609+
entry: echo
610+
'''
596611
manifest = tmp_path.joinpath('.pre-commit-hooks.yaml')
597-
manifest.write_text('hooks: {}')
598-
599-
with pytest.raises(InvalidManifestError) as excinfo:
600-
load_manifest(manifest)
601-
assert str(excinfo.value) == (
602-
f'\n'
603-
f'==> File {manifest}\n'
604-
f'=====> \n'
605-
f'=====> pre-commit version 5 is required but version {C.VERSION} '
606-
f'is installed. Perhaps run `pip install --upgrade pre-commit`.'
607-
)
612+
manifest.write_text(src)
613+
614+
ret = load_manifest(manifest)
615+
616+
# just to make the assertion easier
617+
assert 'id' not in ret['hooks']['example']
618+
for k in tuple(ret['hooks']['example']):
619+
if k not in {'name', 'language', 'entry'}:
620+
ret['hooks']['example'].pop(k)
621+
622+
assert ret == {
623+
'minimum_pre_commit_version': '0',
624+
'hooks': {
625+
'example': {
626+
'name': 'example',
627+
'language': 'unsupported',
628+
'entry': 'echo',
629+
},
630+
},
631+
}

‎tests/repository_test.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,8 @@ def local_python_config():
344344
repo_path = get_resource_path('python_hooks_repo')
345345
manifest = load_manifest(os.path.join(repo_path, C.MANIFEST_FILE))
346346
hooks = [
347-
dict(hook, additional_dependencies=[repo_path]) for hook in manifest
347+
{'id': hook_id, **hook, 'additional_dependencies': [repo_path]}
348+
for hook_id, hook in manifest['hooks'].items()
348349
]
349350
return {'repo': 'local', 'hooks': hooks}
350351

‎tests/store_test.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,48 @@ def test_warning_for_deprecated_stages_on_init(store, tempdir_factory, caplog):
144144
assert caplog.record_tuples == []
145145

146146

147+
def test_warning_for_deprecated_stages_v5_manifest(
148+
store, tempdir_factory, caplog,
149+
):
150+
manifest = '''\
151+
hooks:
152+
hook1:
153+
name: hook1
154+
language: system
155+
entry: echo hook1
156+
stages: [commit, push]
157+
hook2:
158+
name: hook2
159+
language: system
160+
entry: echo hook2
161+
stages: [push, merge-commit]
162+
'''
163+
164+
path = git_dir(tempdir_factory)
165+
with open(os.path.join(path, C.MANIFEST_FILE), 'w') as f:
166+
f.write(manifest)
167+
cmd_output('git', 'add', '.', cwd=path)
168+
git_commit(cwd=path)
169+
rev = git.head_rev(path)
170+
171+
store.clone(path, rev)
172+
assert caplog.record_tuples[1] == (
173+
'pre_commit',
174+
logging.WARNING,
175+
f'repo `{path}` uses deprecated stage names '
176+
f'(commit, push, merge-commit) which will be removed in a future '
177+
f'version. '
178+
f'Hint: often `pre-commit autoupdate --repo {shlex.quote(path)}` '
179+
f'will fix this. '
180+
f'if it does not -- consider reporting an issue to that repo.',
181+
)
182+
183+
# should not re-warn
184+
caplog.clear()
185+
store.clone(path, rev)
186+
assert caplog.record_tuples == []
187+
188+
147189
def test_no_warning_for_non_deprecated_stages_on_init(
148190
store, tempdir_factory, caplog,
149191
):

0 commit comments

Comments
 (0)