black as pre-commit hook always fails my commits

Question

I'm trying to use pre-commit to manage Black as a Git pre-commit hook, but I must be doing it wrong.

In my pre-commit config file I have:

-   repo: https://github.com/psf/black
    rev: 19.3b0
    hooks:
    -   id: black

What I'm expecting to happen is for Black to just modify the staged file, and for the commit to succeed. Because the whole point of Black is that it auto-enforces Python code style rules, no questions asked.

What actually happens when I stage a (non-black-compliant) file and try to commit: Black goes ahead and modifies the file to make it compliant, as intended... But the problem is that it returns a "fail". So the commit fails. And then I have to unstage the file, then restage it before committing again... and only then does the commit succeed.

This is a huge annoyance and can't possibly be the intended workflow?

What am I doing wrong?

Solution 1

(author of pre-commit here)

the framework intentionally does not provide a way to auto-commit modifications. Here's a few issues which have asked for such:

• pre-commit/pre-commit#806
• pre-commit/pre-commit#747

A comment from one of those issues:

pre-commit itself will never touch the staging area. These are good ways to silently break commits. In my mind this is one of the worst things that [other frameworks do and suggest] -- hooks are very frequently not perfect and magically changing what's being committed should not be taken lightly.

That said, if you would like to foot gun, your hook can call git add -u and pre-commit won't know any better :) a sketch of that (untested, discouraged)

  - id: yapf
    entry: bash -c 'yapf "$@"; git add -u' --

(note: using bash will potentially reduce portability)

Another comment notes

Fortunately, git add -u && !! is pretty easy to run if you're ok firing from the hip :)

Solution 2

One of my developers came with a good tip, in case you have a fail by commit due to black (for example due to the single/double quotes) that's resolved with the pre-commit-hook (like with double-quote-string-fixer). you get in kind of 'nobodies land git situation'. There is a changed file in the staged files, but cannot be committted by the pre-commit-hook, git status won't see any changes, but commit fails (a realy black hole in my opinion). You only get a fail on commit, but nothing can be done (excecpt a reset head on this file). Once you are in thsi situation and run: use commit -m 'Resolving pre-commit-hook changes' --no-verify ..... tada!: it's resolved.

Solution 3

Looking at black's README, you probably want to use the --check option, which simply exits successfully or unsuccessfully depending on whether the file meets standards. That will cause the commit to fail without modifying the file.

Solution 4

I'm the same boat as you. From what I've been researching, commits can't be amended by pre-commit hooks.

As far as I can figure out, the best next thing is what bk2204 outlined. We ask black to prevent any commits that include python files that haven't been formatted properly by black. It still makes sure that any commits are formatted, but it is indeed annoying that it won't just automatically format the files for us.

It makes sense. Any changes in a commit have to be staged. If we could do that from the git hook, then our problem would be solved. You got halfway there by modifying the file directly from the git hook. The next half would be staging all the changes of the modified files. But evidently... "You cannot amend a commit in a pre commit hook" which means no staging. https://stackoverflow.com/a/14641656/6032076

I would have commented on bk2204's answer, but I don't have 50 rep yet.

Scratch all that, this answer (https://stackoverflow.com/a/16832764/6032076) claims that commits can be changed in a pre commit hook. In that case files are being added, so I bet in our case files can be restaged/amended.

View more solutions

Comments

[Anakhand]

I also think that for a tool like black, which is intended specifically for modifying files without the need of human supervision, should commit the files even if it has changed them. I've opened an issue on the black repository: github.com/psf/black/issues/1857

[Jean-François Corbett]

I see what you mean, but that's the opposite of what I'm trying to do. I'd like black to just modify the file, and for the commit to succeed. Now clarified in the question.

[Jean-François Corbett]

Sounds reasonable enough. I'm left struggling to understand what the black pre-commit hook is supposed to do / how it's supposed to be used. Because the whole point of black is that it modifies files, no questions asked. I guess that's a questions for the authors of that hook.

[michel.iamit]

where do you add this entry? both black and pre-commit-hook say: yapf is not present in repository <either black or pre-commit-hooks>. Typo? Perhaps it is introduced in a newer version? Often pre-commit autoupdate fixes this.

[anthony sottile]

@michel.iamit yapf is just an example for yapf (copied from the ticket), you want to make sure you're configuring the hook you want to configure (id: black for instance)

[Andreas]

git commit -m 'Resolving pre-commit-hook changes' --no-verify did the trick, great suggestion!

[mChristos]

I don't understand how this answer is accepted - I'm left confused just like @Jean-FrançoisCorbett. Whats the use of a pre-commit hook that formats code if it doesn't modify the file in staging?