black icon indicating copy to clipboard operation
black copied to clipboard

Make pre-commit mirror?

Open MarcoGorelli opened this issue 3 years ago • 5 comments

Thanks to mypyc, black runs twice as fast when installed by PyPI vs in a pre-commit hook https://github.com/pandas-dev/pandas/pull/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?

MarcoGorelli avatar Dec 06 '22 15:12 MarcoGorelli

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).

ichard26 avatar Dec 07 '22 20:12 ichard26

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?

MarcoGorelli avatar Dec 07 '22 20:12 MarcoGorelli

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.

ichard26 avatar Dec 07 '22 20:12 ichard26

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?

MarcoGorelli avatar Dec 07 '22 20:12 MarcoGorelli

(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 :)

ichard26 avatar Dec 14 '22 04:12 ichard26

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!

hauntsaninja avatar Jun 21 '23 05:06 hauntsaninja

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

MarcoGorelli avatar Jun 21 '23 09:06 MarcoGorelli

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: https://github.com/home-assistant/core/pull/95605#issuecomment-1620017691

cdce8p avatar Jul 04 '23 11:07 cdce8p

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?

cooperlees avatar Jul 04 '23 11:07 cooperlees

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.

cdce8p avatar Jul 04 '23 11:07 cdce8p

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

ambv avatar Jul 04 '23 11:07 ambv

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.

ambv avatar Jul 04 '23 11:07 ambv

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.

cdce8p avatar Jul 04 '23 11:07 cdce8p

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.

cooperlees avatar Jul 04 '23 11:07 cooperlees

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.

ambv avatar Jul 04 '23 14:07 ambv

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 avatar Jul 04 '23 18:07 hauntsaninja

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 avatar Jul 09 '23 15:07 cdce8p

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.

cdce8p avatar Jul 28 '23 10:07 cdce8p

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

hauntsaninja avatar Aug 01 '23 19:08 hauntsaninja

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

ambv avatar Aug 01 '23 19:08 ambv

Aborted and retried!

hauntsaninja avatar Aug 01 '23 19:08 hauntsaninja

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

ambv avatar Aug 02 '23 15:08 ambv

Documented in https://github.com/psf/black/pull/3828. Thanks Łukasz!

hauntsaninja avatar Aug 04 '23 01:08 hauntsaninja