desktop: Improve context on pre-commit hook failures and provide escape hatch

This came up again in #11802. If a contributor wants to take a crack at adding a “Commit anyway” button to the error dialog, I think that’d be a great next step.

A ➕ to make this happen [9]

I’ve confirmed with @j-f1 that git config stash.usebuiltin false fixes the issue, so this can be used as a workaround.

It’s still unclear why does lint-staged executes the system git binary in some systems. This could probably be caused by some specific bash configuration…

Anyways, I see 3 different things going on here:

1. There’s an issue on git, which causes git to spawn internally a binary of a different version causing failures when using GIT_EXEC_PATH.
2. The git version bundled in Desktop is a bit old, upgrading it to the latest one would fix that issue.
3. Desktop doesn’t have any escape hatch when such pre-commit issues occur, so users are helpless and need to fallback to the terminal.

Maybe we can refocus this issue (or create a new issue) to try to address point 3. so:

• Errors happening on pre-commit hooks are clear and as easy to understand as possible.
• Desktop offers a way to “Commit anyways” when a pre-commit error happens.

Why can’t there be a button to “commit anyway” on the error window that uses --no-verify? That is a no brainer since this window only shows for people with pre-commit.

I’ve been trying to move to github desktop from another gui tool, but I guess I might have to find yet another option

Thanks for reporting @j-f1 !

After some offline discussion with @j-f1 , he realized that the issue comes from the lint-staged npm package.

These are the events that I think are happening based on the conversation with @j-f1 (I haven’t been able to reproduce the problem in my machine though, so take this with a grain of salt):

1. husky creates a pre-commit git hook, which gets executed by the version of git that gets bundled by Desktop.
2. This pre-commit hook executes an sh script that ends up executing lint-staged on the excalibur/excalibur project (source).
3. The pre-commit hook gets executed with the GIT_EXEC_PATH env variable set to the path of the git binary bundled in Desktop and adds its path to the beginning of the PATH env var.
4. lint-staged runs git stash save --include-untracked --keep-index 'lint-staged automatic backup' (source).
5. The git binary used for the previous command gets resolved to the system version of git (I have no idea why this happens??? lint-staged uses execa to execute git).
6. If the system version of git is above v2.25.0, the git stash command will internally call git update-index --ignore-skip-worktree-entries -z --add --remove --stdin (source).
7. Now, the git command will get executed using the git binary bundled in Desktop (thanks to the GIT_EXEC_PATH env var).
8. The git binary bundled in Desktop is v2.23.1, which doesn’t support the --ignore-skip-worktree-entries param so the command fails.

Based on the sourcecode of git stash, looks like this behaviour can be avoided by using setting the stash.useBuiltin config param to false (this way, the old legacy stash logic will get executed):

$ git config stash.usebuiltin false

(this is just a potential workaround and may cause other issues).

The main question here is regarding point 5., and why does lint-staged ignores the path and resolves to the system git binary in some systems. I think that if we can figure out that issue and resolve it, this issue will go away.