CI skip not working correctly?

[TylerYep]

Even though I specified the skip flag, pre-commit-ci still tries to install the environment for mypy? It works fine if I put the command in local, but not if I specify the repo url.

ci:
  skip: [mypy]
repos:
  - repo: https://github.com/pre-commit/mirrors-mypy
    rev: v0.812
    hooks:
      - id: mypy
        additional_dependencies: [torch]

(the config is truncated to only include mypy)

build attempt 1...
    => timeout after 60s
    [INFO] Installing environment for https://github.com/pre-commit/mirrors-mypy.
    [INFO] Once installed this environment will be reused.
    [INFO] This may take a few minutes...
    Interrupted (^C): KeyboardInterrupt: 
    Check the log at /pc/pre-commit.log
build attempt 2...
    => timeout after 60s
    [INFO] Installing environment for https://github.com/pre-commit/mirrors-mypy.
    [INFO] Once installed this environment will be reused.
    [INFO] This may take a few minutes...
    Interrupted (^C): KeyboardInterrupt: 
    Check the log at /pc/pre-commit.log
build attempt 3...
    => timeout after 60s
    [INFO] Installing environment for https://github.com/pre-commit/mirrors-mypy.
    [INFO] Once installed this environment will be reused.
    [INFO] This may take a few minutes...
    Interrupted (^C): KeyboardInterrupt: 
    Check the log at /pc/pre-commit.log

[asottile]

yeah, installation must still occur and succeed to eventually run pre-commit run

[asottile]

it's possible that pre-commit itself could be improved for this case:

$ git diff
diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py
index 05c3268..0fef50d 100644
--- a/pre_commit/commands/run.py
+++ b/pre_commit/commands/run.py
@@ -271,11 +271,11 @@ def _get_diff() -> bytes:
 def _run_hooks(
         config: Dict[str, Any],
         hooks: Sequence[Hook],
+        skips: Set[str],
         args: argparse.Namespace,
         environ: MutableMapping[str, str],
 ) -> int:
     """Actually run the hooks."""
-    skips = _get_skips(environ)
     cols = _compute_cols(hooks)
     classifier = Classifier.from_config(
         _all_filenames(args), config['files'], config['exclude'],
@@ -403,9 +403,11 @@ def run(
             )
             return 1

-        install_hook_envs(hooks, store)
+        skips = _get_skips(environ)
+        to_install = [hook for hook in hooks if hook.id not in skips]
+        install_hook_envs(to_install, store)

-        return _run_hooks(config, hooks, args, environ)
+        return _run_hooks(config, hooks, skips, args, environ)

     # https://github.com/python/mypy/issues/7726
     raise AssertionError('unreachable')

this would skip installation if a hookid is inside SKIP (though it still needs to "clone" to ensure the metadata is correct)

[asottile]

I'll see what I think of this tomorrow: https://github.com/pre-commit/pre-commit/pull/1875

[lachmanfrantisek]

Is this already deployed on pre-commit.ci?

Looks like I am still hitting #25 for ansible-lint that I've marked as skipped (https://github.com/packit/deployment/pull/211/files).

Thanks for working on this!

[asottile]

@lachmanfrantisek it is not, that is why this issue is still open

[lachmanfrantisek]

Thanks for your response. I should have made that clear in my question, but do you plan to deploy that? I don't know, how (often) you update the pre-commit itself in the CI part. Just want to know if it makes sense to wait a bit or do something with that on our side. Nevertheless, thanks!

[asottile]

bumping the pre-commit version is trivial, teaching the distributed cache mechanism about this requires a code change

[asottile]

this is implemented now -- you should be able to use ci: skip: [...] to avoid problematic builds such as ansible or torch

thanks again for the issue !

[lachmanfrantisek]

Thanks a lot!