Merge pull request #3315 from pre-commit/warn-deprecated-stage-names-…

…on-init add warning for deprecates stages for remote repos on init
pre-commit · Oct 1, 2024 · a2f7b80 · a2f7b80
2 parents 7555e11 + d317223
commit a2f7b80
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 0 deletions.
diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py
@@ -2,6 +2,7 @@
 
 import functools
 import logging
+import os.path
 import re
 import shlex
 import sys
@@ -70,6 +71,43 @@ def transform_stage(stage: str) -> str:
     return _STAGES.get(stage, stage)
 
 
+MINIMAL_MANIFEST_SCHEMA = cfgv.Array(
+    cfgv.Map(
+        'Hook', 'id',
+        cfgv.Required('id', cfgv.check_string),
+        cfgv.Optional('stages', cfgv.check_array(cfgv.check_string), []),
+    ),
+)
+
+
+def warn_for_stages_on_repo_init(repo: str, directory: str) -> None:
+    try:
+        manifest = cfgv.load_from_filename(
+            os.path.join(directory, C.MANIFEST_FILE),
+            schema=MINIMAL_MANIFEST_SCHEMA,
+            load_strategy=yaml_load,
+            exc_tp=InvalidManifestError,
+        )
+    except InvalidManifestError:
+        return  # they'll get a better error message when it actually loads!
+
+    legacy_stages = {}  # sorted set
+    for hook in manifest:
+        for stage in hook.get('stages', ()):
+            if stage in _STAGES:
+                legacy_stages[stage] = True
+
+    if legacy_stages:
+        logger.warning(
+            f'repo `{repo}` uses deprecated stage names '
+            f'({", ".join(legacy_stages)}) which will be removed in a '
+            f'future version.  '
+            f'Hint: often `pre-commit autoupdate --repo {shlex.quote(repo)}` '
+            f'will fix this.  '
+            f'if it does not -- consider reporting an issue to that repo.',
+        )
+
+
 class StagesMigrationNoDefault(NamedTuple):
     key: str
     default: Sequence[str]

diff --git a/pre_commit/store.py b/pre_commit/store.py
@@ -10,6 +10,7 @@
 from typing import Callable
 
 import pre_commit.constants as C
+from pre_commit import clientlib
 from pre_commit import file_lock
 from pre_commit import git
 from pre_commit.util import CalledProcessError
@@ -136,6 +137,7 @@ def _new_repo(
             deps: Sequence[str],
             make_strategy: Callable[[str], None],
     ) -> str:
+        original_repo = repo
         repo = self.db_repo_name(repo, deps)
 
         def _get_result() -> str | None:
@@ -168,6 +170,9 @@ def _get_result() -> str | None:
                     'INSERT INTO repos (repo, ref, path) VALUES (?, ?, ?)',
                     [repo, ref, directory],
                 )
+
+            clientlib.warn_for_stages_on_repo_init(original_repo, directory)
+
         return directory
 
     def _complete_clone(self, ref: str, git_cmd: Callable[..., None]) -> None:

diff --git a/tests/store_test.py b/tests/store_test.py
@@ -1,12 +1,15 @@
 from __future__ import annotations
 
+import logging
 import os.path
+import shlex
 import sqlite3
 import stat
 from unittest import mock
 
 import pytest
 
+import pre_commit.constants as C
 from pre_commit import git
 from pre_commit.store import _get_default_directory
 from pre_commit.store import _LOCAL_RESOURCES
@@ -91,6 +94,72 @@ def test_clone(store, tempdir_factory, caplog):
     assert store.select_all_repos() == [(path, rev, ret)]
 
 
+def test_warning_for_deprecated_stages_on_init(store, tempdir_factory, caplog):
+    manifest = '''\
+-   id: hook1
+    name: hook1
+    language: system
+    entry: echo hook1
+    stages: [commit, push]
+-   id: hook2
+    name: hook2
+    language: system
+    entry: echo hook2
+    stages: [push, merge-commit]
+'''
+
+    path = git_dir(tempdir_factory)
+    with open(os.path.join(path, C.MANIFEST_FILE), 'w') as f:
+        f.write(manifest)
+    cmd_output('git', 'add', '.', cwd=path)
+    git_commit(cwd=path)
+    rev = git.head_rev(path)
+
+    store.clone(path, rev)
+    assert caplog.record_tuples[1] == (
+        'pre_commit',
+        logging.WARNING,
+        f'repo `{path}` uses deprecated stage names '
+        f'(commit, push, merge-commit) which will be removed in a future '
+        f'version.  '
+        f'Hint: often `pre-commit autoupdate --repo {shlex.quote(path)}` '
+        f'will fix this.  '
+        f'if it does not -- consider reporting an issue to that repo.',
+    )
+
+    # should not re-warn
+    caplog.clear()
+    store.clone(path, rev)
+    assert caplog.record_tuples == []
+
+
+def test_no_warning_for_non_deprecated_stages_on_init(
+        store, tempdir_factory, caplog,
+):
+    manifest = '''\
+-   id: hook1
+    name: hook1
+    language: system
+    entry: echo hook1
+    stages: [pre-commit, pre-push]
+-   id: hook2
+    name: hook2
+    language: system
+    entry: echo hook2
+    stages: [pre-push, pre-merge-commit]
+'''
+
+    path = git_dir(tempdir_factory)
+    with open(os.path.join(path, C.MANIFEST_FILE), 'w') as f:
+        f.write(manifest)
+    cmd_output('git', 'add', '.', cwd=path)
+    git_commit(cwd=path)
+    rev = git.head_rev(path)
+
+    store.clone(path, rev)
+    assert logging.WARNING not in {tup[1] for tup in caplog.record_tuples}
+
+
 def test_clone_cleans_up_on_checkout_failure(store):
     with pytest.raises(Exception) as excinfo:
         # This raises an exception because you can't clone something that