pre-commit hooks do not run

[WaylonWalker]

I have been usuing gitui quite regularly for a few weeks and I am in love with it. The only downside is that pre-commit hooks are not running and sometimes I don't see errors until they hit CI.

Describe the bug
pre-commit hooks do not run.

To Reproduce
Create a commit in a repo with pre-commit hooks installed.

Try with a sample pre-commit.

pip instal pre-commit
pre-commit sample-config > .pre-commit-config.yaml
pre-commit install

Expected behavior
pre-commit hooks run after the message is entered.

Screenshots
If applicable, add screenshots to help explain your problem.

Context (please complete the following information):

• wsl/Debian 9
• docker/stretch
• GitUI Version: 0.10.1

[extrawurst]

I am having trouble reproducing this. also the unittests for pre-commit hooks do run for me. can you let me know what is in this sample-config? is it supposed to block the commit or how do you know it was not run?

[WaylonWalker]

I created a sample repo, not sure if it helps, but you can clone it and play with it to get a better feel for how pre-commit hooks work. I created a sample dialog below to possibly help paint a better picture of the workflow when pre-commit is installed.

As far as whats in the sample config. trailing-whitespace is an autoformatter that automatically trims whitespace from the end of every line in the files being commited. end-of-file-fixer does the same for newlines at the end of files. check-yaml checks yaml syntax, but does not fix it. check-added-large-files check for files that are generally discouraged to store in git. There are many more pre-configured hooks at https://pre-commit.com/hooks.html, and if you have a tool that is not included you can also run a shell command.

The pre-commit library is a handy library to make creating a sharing pre-commit hooks much easier, You might be able to figure out how to test gits pre-commit hooks without using the library itself.

---

After cloning the repo I pip install pre-commit then pre-commit install. Here is what happens upon first commit after installing pre-commit. I made the config, added it, then commited. Everything passes and

pre-commit-sample on  main [+] via 🅒  pre-commit
❯ git commit -m "add pre-commit-config.yml"
[INFO] Initializing environment for https://github.com/pre-commit/pre-commit-hooks.
[INFO] Installing environment for https://github.com/pre-commit/pre-commit-hooks.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
Trim Trailing Whitespace.................................................Passed
Fix End of Files.........................................................Passed
Check Yaml...............................................................Passed
Check for added large files..............................................Passed
[main 8c0a360] add pre-commit-config.yml
 1 file changed, 10 insertions(+)
 create mode 100644 .pre-commit-config.yaml

Since everything passed this commit is now in the git log.

❯ git log
commit 8c0a3606949d675bc166f73bede9f3b7319a9987 (HEAD -> main)
Author: Waylon Walker <quadmx08@gmail.com>
Date:   Sat Oct 17 09:07:54 2020 -0500

    add pre-commit-config.yml

commit c78a6c27fb735c14432d237d152391b5202d7c37 (origin/main, origin/HEAD)
Author: Waylon Walker <quadmx08@gmail.com>
Date:   Sat Oct 17 09:02:26 2020 -0500

    Initial commit

Next I will add something to the repo that violates our trailing whitespace rule. If I add some trailing whitespace to the first line of the readme,

pre-commit-sample on  main [⇡] via 🅒  pre-commit
❯ sed -i "s/sample/sample repo      /g" README.md

We can see the space added to the readme in the diff.

pre-commit-sample on  main [!⇡] via 🅒  pre-commit
❯ git diff
diff --git a/README.md b/README.md
index 69c7683..7a0dcfc 100644
--- a/README.md
+++ b/README.md
@@ -1 +1 @@
-# pre-commit-sample
\ No newline at end of file
+# pre-commit-sample repo██████
\ No newline at end of file

Lets try to commit this.

pre-commit-sample on  main [!⇡] via 🅒  pre-commit
❯ git add .

pre-commit-sample on  main [+⇡] via 🅒  pre-commit
❯ git commit -m "sample -> sample repo"
Trim Trailing Whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing README.md

Fix End of Files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing README.md

Check Yaml...........................................(no files to check)Skipped
Check for added large files..............................................Passed

Since the pre-commit formatters made some changes to our code, the pre-commit step failed and the code that violated our trailing whitespace rule was not allowed to ever be in our git history.

pre-commit-sample on  main [!+⇡] via 🅒  pre-commit
❯ git status
On branch main
Your branch is ahead of 'origin/main' by 1 commit.
  (use "git push" to publish your local commits)

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   README.md

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   README.md

This step was an auto-formatter so it actually fixed the issue for us. If we simply add the changed file again the commit will pass.

pre-commit-sample on  main [!+⇡] via 🅒  pre-commit
❯ git add .

pre-commit-sample on  main [+⇡] via 🅒  pre-commit
❯ git commit -m "sample -> sample repo"
Trim Trailing Whitespace.................................................Passed
Fix End of Files.........................................................Passed
Check Yaml...........................................(no files to check)Skipped
Check for added large files..............................................Passed
[main 1ea9518] sample -> sample repo
 1 file changed, 1 insertion(+), 1 deletion(-)

pre-commit-sample on  main [⇡] via 🅒  pre-commit
❯ git log
commit 1ea95181090d6c123eb1b6c36a82add9a9148b3a (HEAD -> main)
Author: Waylon Walker <quadmx08@gmail.com>
Date:   Sat Oct 17 09:31:58 2020 -0500

    sample -> sample repo

commit 8c0a3606949d675bc166f73bede9f3b7319a9987
Author: Waylon Walker <quadmx08@gmail.com>
Date:   Sat Oct 17 09:07:54 2020 -0500

    add pre-commit-config.yml

commit c78a6c27fb735c14432d237d152391b5202d7c37 (origin/main, origin/HEAD)
Author: Waylon Walker <quadmx08@gmail.com>
Date:   Sat Oct 17 09:02:26 2020 -0500

    Initial commit

[extrawurst]

did you validate that regular shell based pre-commits are also not working on your setup? it looks like it is a pip pre-commit specific issue or maybe even windows specific 🤔

[pm100]

as far as can see there is no code to run pre-commit checks in gitui. Only commit-msg and post-commit

[extrawurst]

Ok I was confusing pre with post commit hooks, indeed pre commit hooks are missing right now - sorry for the confusion

[pm100]

I can take this one if you like

[pm100]

heads up: this is going to take a little longer than I expected (which I am fine with). The way gitui runs hooks does not work for the python scripts generated by pre-commit, tested on both windows and linux.

I looked to steal how vscode does it - they simply run git to do the heavy lifting - so thats a dead end :(

So now I have to see exactly what git is doing

[extrawurst]

hm that's weird. since we simply run them in a bash it should automatically run python scripts accordingly, right?

[extrawurst]

and also this means, adding pre-commit hook support is not the issue but supporting python based hooks is the issue? can we then split that off in two different issues?

[pm100]

thats not how she-bang things work. You hooks work becuase they are bash scripts. she-bang things are wired up differently
c

here is python script

pm100@paul-surf3:~/work/pctest$ cat .git/hooks/pre-commit
#!/usr/bin/env python3.8
# File generated by pre-commit: https://pre-commit.com
# ID: 138fd403232d2ddd5efb44317e38bf03
import os
import sys

here i run it from shell prompt directly

pm100@paul-surf3:~/work/pctest$ .git/hooks/pre-commit
Trim Trailing Whitespace.................................................Passed
Fix End of Files.........................................................Passed
Check Yaml...............................................................Passed
Check for added large files..............................................Passed
pm100@paul-surf3:~/work/pctest$

works nicely

here is what gitui does

pm100@paul-surf3:~/work/pctest$ bash .git/hooks/pre-commit
.git/hooks/pre-commit: line 4: import: command not found
.git/hooks/pre-commit: line 5: import: command not found
.git/hooks/pre-commit: line 11: syntax error near unexpected token `('
.git/hooks/pre-commit: line 11: `if sys.version_info < (3, 3):'
pm100@paul-surf3:~/work/pctest$

still gets treats as shell script. I have found what git cli does on windows, I am looking into linux (I guess osx will be the same)

[extrawurst]

how is a tool like nushell doing this then?

[extrawurst]

@pm100 so I just did a little test and for me running python scripts using shebang works perfectly fine with the method we are using: https://github.com/extrawurst/shebangrust

[pm100]

i see and it works for me too, but the python from pre-commit fails as I showed. I will find out why. I changed gitui run code to what I think it should be and it works with the pre-commit scripts, but I want to understand the difference

And I think windows will be a different story too.

NOTE- in your test you do 'sh', in gitui you do 'bash' - but its not the cause of the difference

[pm100]

OK the subtle difference is that #! is interpreted by the kernel exec loader , its not done by the shell. In gitui you do 'bash 'file'' this says to bash 'run these commands', the #! is just a comment

in your test you say bash -c 'file' which says run this command, its not a builtin so its passed to the exec loader, that reads the #! and runs /bin/env python

[pm100]

so here is precommit working on linux

The output is not very nice tho, compared to the mini report generated by git cli

I will add a test. I also need to get windows going.

[extrawurst]

I am not very concerned about the output formatting tbh
Thanks for looking into this

[WaylonWalker]

You guys are awesome, this is looking great.

Looks like you have it covered. As far as I know the python package just manages the install and setup of these. I believe that there are some written in ruby and some in nodejs.

[pm100]

I think you should be concerned about the output, the 'standard' precommit tooling goes to a lot of trouble to have nice output

its a definite step back to loose that layout. I will clean it up - I have a plan for it. THe color is togher but at least maintain the layout.

[pm100]

As regards windows. I can make it work but its waaay more complex.

The way you have it at the moment only works by luck, most windows dev systems do not have bash installed, so even the simple bash of a shell script wont work. And it certainly wont work with arbitrary scripts (python for example)

I have looked at how git does it (boy that was a tough exercise since it requires msys2 / mingw - so I learned a lot). git sets up the msys2 runtime and it uses tooling from that. I can make that work in gitui. It will require the installation of git for windows - that does not seem too bad. Basically git parses the #! script to work out what binary to load to actually run it. pre-commit toolset uses "#!/bin/env python exe" so git runs 'env' and passes it the script. The thing I couldnt understand was whee it was getting 'env' from, finally worked it out.

This solution would not have gitui running git, merely taking advantage of all the pseudo unix plumbing that git installs

[pm100]

I have code to make the msg popup dynamically size itself and to stop stripping the \n characters so that line breaks are preserved. I suspect that stripping was an unintended side effect of the conversion to spans

This improves all error message output IMHO. I can make this a separate PR / issue if you like

Note that I have not preserved the message coloring, thats much harder

[extrawurst]

This improves all error message output IMHO. I can make this a separate PR / issue if you like

cool stuff, but without seeing code it is hard to argue about - maybe open a PR first, then we can better reason about it. but I have the feeling this makes sense to be in a separate issue/PR

Note that I have not preserved the message coloring, thats much harder

don't worry about that.

[extrawurst]

@dr-BEat was the last hero to work on windows hooks (#236) maybe he can chime in aswell

[extrawurst]

@pm100 this was fixed in #386 - right?

[pm100]

@extrawurst yes