Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for manual dependency management for local repos, system or script entry-points #13

Open
lorenzwalthert opened this issue Nov 1, 2020 · 21 comments
Labels
enhancement New feature or request pro

Comments

@lorenzwalthert
Copy link

lorenzwalthert commented Nov 1, 2020

I know there is no explicit dependency management for script or system hooks or local repos. This is not a problem when used locally, because people could install the dependencies once. However, there is no way to install these dependencies for https://pre-commit.ci to my knowledge. This means that any pre-commit.ci reun will fail if it has at least one local, system or script hook if I am not mistaken. An example is this run.

I saw there is #11 which would provide maximum flexibility. Maybe there is also a way to limit the scope and run custom code only before the hook check?

If not, maybe it would be helpful to indicate in .pre-commit-config.yaml if a hook should run on pre-commit.ci. Or set an env variable that can be checked in a system, script or local hook.

Looking forward for this to reach production readiness 🥳

cc: @katrinleinweber. Maybe we should anyways aim to support R as a language, as discussed in pre-commit/pre-commit#926. There is support for conda now, maybe we can follow the PR and use renv: pre-commit/pre-commit#1232. If you are interested, let's discuss in lorenzwalthert/precommit#215. In any case, this would not solve the problem for local hooks and system or script hook in general.

@asottile
Copy link
Member

asottile commented Nov 1, 2020

arbitrary installation won't be supported for the free tier as it exposes too many risks and system / script are the escape hatch

@lorenzwalthert
Copy link
Author

Ok. So are third level languages the only ones not supported by pre-commit.ci and the service will install required global system requirements of second level languages like conda?

@asottile
Copy link
Member

asottile commented Nov 2, 2020

for alpha currently the only languages I've added are python, node, and ruby -- but I'll be accepting PRs to add the other supported languages in the runner-image (which is public for this reason!)

but yes, by the time it's out of alpha it should have support for second class languages (maybe without docker / docker_image because I'm not sure how I can do that efficiently / safely)

@Cielquan
Copy link

Couldn't one e.g. for python stuff write a python script which gets called as a script hook at the top of .pre-commit-config.yaml which then determines if it runs inside pre-commit.ci or not and if so uses pip to install dependencies for other local hooks?

I did not look into the runner-image source yet.

@asottile
Copy link
Member

the free tier intentionally does not allow network access at runtime so that wouldn't work

@Cielquan
Copy link

Ah ok thanks for the info. But if I interpret your answer correctly it would be a possibility for the paid plan than?

@asottile
Copy link
Member

yes or something like it

@eggplants
Copy link

I know this problem is avoidable by using https://github.com/pre-commit/action instead. But I would like to use even if I had to pay for it...

@asottile
Copy link
Member

asottile commented May 1, 2022

@eggplants is there a specific tool you'd like made available? I can probably design a system for providing them

@eggplants
Copy link

Thank you! Here is:
https://github.com/pecigonzalo/pre-commit-shfmt

@asottile
Copy link
Member

asottile commented May 1, 2022

good news, you can use shfmt without needing special system tools installed \o/ (since it is written in go)

repos:
-   repo: local
    hooks:
    -   id: shfmt
        name: shfmt
        entry: shfmt -w -i 4
        language: golang
        additional_dependencies: [mvdan.cc/sh/v3/cmd/[email protected]]
        types: [shell]

@eggplants
Copy link

This workaround looks good, thank you so much.

@eggplants
Copy link

@asottile Could you create PR for pecigonzalo/pre-commit-shfmt?

@asottile
Copy link
Member

asottile commented May 1, 2022

@eggplants feel free to take that yaml block above and dedent it and add it to .pre-commit-hooks.yaml in that repo! (though I would perhaps remove -i 4 from args if it's going to be extended?)

@asottile
Copy link
Member

I've just released a brand new "lite" version of pre-commit.ci which may be of interest for non-managed hooks (such as those requiring special unsupported setup to make language: system or language: script work). you can read more about it at https://pre-commit.ci/lite -- the tl;dr is adds autofixing to GitHub Actions

@gilesw
Copy link

gilesw commented Feb 2, 2023

I love that pre-commit-ci is so fast and my company would pay for private repos. I tried out github super-linter and it's really slow because it has to pull a huge docker image. It's a shame that the pre-commit-ci team can't add all the common dependencies themselves to their image. I'm assuming it's fast because the image doesn't have to be pulled down every time it runs.

It's not clear what this mechanism does under the hood:-

additional_dependencies: [mvdan.cc/sh/v3/cmd/[email protected]]

If it works for any go project then maybe install actionlint.

If I can't then I'm going to go back to running pre-commit in gha and installing dependencies with adsf and using caching to speed that up.

@asottile
Copy link
Member

asottile commented Feb 2, 2023

It's a shame that the pre-commit-ci team can't add all the common dependencies themselves to their image

that misses the entire point of pre-commit -- the whole idea is that the tool installs and manages exactly the tools you specify in the configuration. if your contributors need to set up a bunch of manual stuff outside the tool they're unlikely to do it. the configuration above for shfmt uses the framework to manage a language: golant dependency

@gilesw
Copy link

gilesw commented Feb 2, 2023

I can agree that it's strange that for such a mature system that pre-commit hooks often don't install the dependencies they need.

@asottile
Copy link
Member

asottile commented Feb 2, 2023

fwiw this seems to work in-framework (though, also highlights a bug in actionlint!):

$ pre-commit  run actionlint --all-files
actionlint...............................................................Failed
- hook id: actionlint
- exit code: 1

.github/workflows/main.yml:6:10: string should not be empty [syntax-check]
  |
6 |     tags:
  |          ^
repos:
-   repo: local
    hooks:
    -   id: actionlint
        name: actionlint
        language: golang
        entry: actionlint
        additional_dependencies: [github.com/rhysd/actionlint/cmd/[email protected]]
        files: ^\.github/workflows/.*\.(yml|yaml)$

@gilesw
Copy link

gilesw commented Feb 2, 2023

Well I'm happy that pre-commit-ci now runs it and does it fast. So no blockers unless I find another strange hook that needs something unusual. I might do a pr on the actionlint repo so at least they can include an example of running it that way but it sounds like you're going to get this question asked again.

@asottile
Copy link
Member

asottile commented Feb 2, 2023

it's kinda odd that they don't have a language: golang integration directly -- seems they have a docker and system one but not the one that would ~generally be best for users

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pro
Development

No branches or pull requests

5 participants