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

Make pre-commit mirror? #3405

Closed
MarcoGorelli opened this issue Dec 6, 2022 · 23 comments · Fixed by #3828
Closed

Make pre-commit mirror? #3405

MarcoGorelli opened this issue Dec 6, 2022 · 23 comments · Fixed by #3828
Labels
C: integrations Editor plugins and other integrations C: performance Black is too slow. Or too fast. T: enhancement New feature or request

Comments

@MarcoGorelli
Copy link
Contributor

Thanks to mypyc, black runs twice as fast when installed by PyPI vs in a pre-commit hook pandas-dev/pandas#49947

Would you be open to making a mirror repo, like this one that exists for ruff, which will install black wheels and thus be fast?

@ichard26 ichard26 added C: performance Black is too slow. Or too fast. C: integrations Editor plugins and other integrations T: enhancement New feature or request labels Dec 7, 2022
@ichard26
Copy link
Collaborator

ichard26 commented Dec 7, 2022

I'm generally in favour, but I have no idea how it'd work. I suppose we could create a mirror similar to mirrors-mypy and then advertise it heavily in the docs and other areas. We can't really deprecate the hook from here though (commit SHAs would never work in w/ a mirror).

@MarcoGorelli
Copy link
Contributor Author

do people pin hooks to commit SHAs? I think tags are way more common, and if this would make black twice as fast for 99% of users, then it might be worth it?

@ichard26
Copy link
Collaborator

ichard26 commented Dec 7, 2022

Wait, are you suggesting to replace the hook here to pull Black from PyPI? I thought we were discussing creating a mirror. I was just noting it's not ideal having two official hooks "forever" since some people do pin down to the commit.

@MarcoGorelli
Copy link
Contributor Author

Yes, my suggestion is to create a mirror (which pulls black from PyPI) - I don't think it's possible to get the one here to pull from PyPI (at least, with setuptools-scm)

Alternatively, there might be some workaround to allow the hook to be pulled from PyPI within the hook here?

@ichard26
Copy link
Collaborator

ichard26 commented Dec 14, 2022

(note: I got bored and felt like writing hacky code. This idea is stupid and would easily break.)

A hacky idea would be to edit .pre-commit-hooks.yaml only on the tagged release commits to invoke a shim (see below) that installs Black from PyPI at runtime before passing off control to Black. All other commits would have the current normal version of .pre-commit-hooks.yaml. This is totally unsupported by pre-commit and AFAIK the environment is not meant to change after initialization, but this is one option.

# pre_commit_shim.py

checker_code = """
import sys, black
sys.stdout.buffer.write(str(black.COMPILED).encode("utf-8"))
"""

try:
    import subprocess
    import sys

    proc = subprocess.run([sys.executable, "-c", checker_code], check=True, stdout=subprocess.PIPE, encoding="utf-8")
    if proc.stdout.strip() == "False":
        subprocess.run([sys.executable, "-m" , "pip", "install", "black==22.12.0"], check=True)    
except:
    pass

import black
black.main(sys.argv[1:])

A mirror is certainly better and less prone to breakage though :)

@hauntsaninja
Copy link
Collaborator

I was tired of slow pre-commit, so I made a mirror here: https://github.com/hauntsaninja/black-pre-commit-mirror

Thanks for all the mypyc work!

@MarcoGorelli
Copy link
Contributor Author

legeeeeeend 🙌 🥳 worth pointing to that one in the readme?

@cdce8p
Copy link
Contributor

cdce8p commented Jul 4, 2023

Is it possible to make the mirror a bit more "official"? For example by moving the repo to the psf org?
For bigger projects it's difficult to convince maintainers to use a "random" mirror repo for black.

See: home-assistant/core#95605 (comment)

@cooperlees
Copy link
Collaborator

Just so I understand, is the advantage of the pre-commit mirror so it's a small faster mirror to check out rather than our larger checkout this repo would cause? Is it slow even with a shallow checkout?

If so, @ambv how do we go about asking for a psf/black-pre-commit-mirror or something of the likes?

@cdce8p
Copy link
Contributor

cdce8p commented Jul 4, 2023

Just so I understand, is the advantage of the pre-commit mirror so it's a small faster mirror to check out rather than our larger checkout this repo would cause? Is it slow even with a shallow checkout?

No. The difference is how pre-commit installs black. With the "normal" pre-commit hook https://github.com/psf/black it's basically installed from the Python source code in the repo. With the mirror, black is used as dependency and thus the compiled wheels are fetched from PyPI which are a lot faster.

@ambv
Copy link
Collaborator

ambv commented Jul 4, 2023

Ha, that makes it attractive given that Black on PyPI is mypyc-compiled whereas the one installed from source most certainly is not.

@ambv
Copy link
Collaborator

ambv commented Jul 4, 2023

I can start ambv/black-pre-commit and move it over to psf/ but it won't happen today as it's Fireworks Day in the US.

@cdce8p
Copy link
Contributor

cdce8p commented Jul 4, 2023

Ha, that makes it attractive given that Black on PyPI is mypyc-compiled whereas the one installed from source most certainly is not.

Exactly, just as example for Home Assistant the normal run is ~5min whereas the mirror only takes ~3min.
The mirror from @hauntsaninja works fine, the question is just if it can be made a bit more "official" / moved to the psf org.

@cooperlees
Copy link
Collaborator

Right. I get it now. I did miss that. I like the ambv -> psf plan. Thanks Łukasz.

If no one beats me, I'll update docs once this is all setup.

@ambv
Copy link
Collaborator

ambv commented Jul 4, 2023

Actually, since @hauntsaninja's mirror already works, let's just move that one to psf/. I'm fine with that, too. So whenever you're ready Shantanu, make the move on GitHub. I will let PSF know to expect it.

@hauntsaninja
Copy link
Collaborator

Hm, when I tried the transfer I got:

You don’t have the permission to create public repositories on psf

Might need to coordinate a little more on this one. I've added you all as collaborators to the mirror repo (didn't see a way to directly add you as admin).

@cdce8p
Copy link
Contributor

cdce8p commented Jul 9, 2023

Hm, when I tried the transfer I got:

You don’t have the permission to create public repositories on psf

Might need to coordinate a little more on this one. I've added you all as collaborators to the mirror repo (didn't see a way to directly add you as admin).

@hauntsaninja I believe you might first need to transfer ownership to @ambv from the repo settings page. Collaborator access isn't enough. After that @ambv should be able to transfer it over to the psf org.

@cdce8p
Copy link
Contributor

cdce8p commented Jul 28, 2023

Any update here? Would love to use it and get the speedup. However, difficult to do unless it's moved to the psf org as mentioned before.

Is it possible to make the mirror a bit more "official"? For example by moving the repo to the psf org? For bigger projects it's difficult to convince maintainers to use a "random" mirror repo for black.

See: home-assistant/core#95605 (comment)

Please let me know if there is anything I can to do help.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Aug 1, 2023

I couldn't move it directly to psf/. I'd transferred to ambv/, but I think he might need to accept

@ambv
Copy link
Collaborator

ambv commented Aug 1, 2023

Yeah, sorry, I missed the notification and the transfer expired. Can we try again?

@hauntsaninja
Copy link
Collaborator

Aborted and retried!

@ambv
Copy link
Collaborator

ambv commented Aug 2, 2023

This is now moved to psf/black-pre-commit-mirror. All we need is to update docs.

@hauntsaninja
Copy link
Collaborator

Documented in #3828. Thanks Łukasz!

GenevieveBuckley pushed a commit to napari/napari that referenced this issue Aug 15, 2023
# Description
Use black compiled with mypyc instead of pure python version to get it
faster

# References
psf/black#3405

## Type of change
- [x] Maintenance (changes required to run napari, tests, & CI smoothly)
scop added a commit to scop/hashpipe that referenced this issue Aug 15, 2023
scop added a commit to scop/hashpipe that referenced this issue Aug 15, 2023
scop added a commit to scop/pytekukko that referenced this issue Aug 15, 2023
scop added a commit to scop/pytekukko that referenced this issue Aug 15, 2023
scop added a commit to scop/home-assistant-jatekukko that referenced this issue Aug 15, 2023
scop added a commit to scop/bash-completion that referenced this issue Aug 15, 2023
scop added a commit to scop/home-assistant-jatekukko that referenced this issue Aug 15, 2023
scop added a commit to guludo/venv-run that referenced this issue Aug 15, 2023
scop added a commit to scop/bash-completion that referenced this issue Aug 16, 2023
scop added a commit to guludo/venv-run that referenced this issue Sep 8, 2023
scop added a commit to guludo/venv-run that referenced this issue Sep 8, 2023
mcdonnnj added a commit to cisagov/skeleton-generic that referenced this issue Sep 11, 2023
This mirror was created to leverage performance optimizations from
mypyc wheels that are available if black is installed from PyPI. These
wheels are not available if black is installed from source as it would
be using the old URL. Please see psf/black#3828 and psf/black#3405 for
more information.
mcdonnnj added a commit to cisagov/skeleton-generic that referenced this issue Sep 13, 2023
This mirror was created to leverage performance optimizations from
mypyc wheels that are available if black is installed from PyPI. These
wheels are not available if black is installed from source as it would
be using the old URL. Please see psf/black#3828 and psf/black#3405 for
more information.
dav3r added a commit to cisagov/cyhy-feeds that referenced this issue Nov 9, 2023
This mirror was created to leverage performance optimizations from
mypyc wheels that are available if black is installed from PyPI. These
wheels are not available if black is installed from source as it would
be using the old URL. Please see psf/black#3828 and psf/black#3405 for
more information.

Co-authored-by: Nick <50747025+mcdonnnj@users.noreply.github.com>
jsf9k pushed a commit to cisagov/ansible-role-clamav that referenced this issue Dec 19, 2023
This mirror was created to leverage performance optimizations from
mypyc wheels that are available if black is installed from PyPI. These
wheels are not available if black is installed from source as it would
be using the old URL. Please see psf/black#3828 and psf/black#3405 for
more information.
pokey added a commit to talonhub/community that referenced this issue Feb 1, 2024
nriley pushed a commit to talonhub/community that referenced this issue Feb 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: integrations Editor plugins and other integrations C: performance Black is too slow. Or too fast. T: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants