gitbutlerapp / gitbutler

The GitButler version control client, backed by Git, powered by Tauri/Rust/Svelte
https://gitbutler.com
Other
11.52k stars 467 forks source link

Git hooks are not executed (on the correct .git/index) #2615

Open nathanael-ruf opened 4 months ago

nathanael-ruf commented 4 months ago

For me git pre-commit hooks aren't executed. Judging from https://github.com/gitbutlerapp/gitbutler/issues/2551 it seems like they should be, just without being able to view the output?

nolith commented 4 months ago

Same for me, I was testing a new command I added in lefthook and I noticed that gitbutler happily committed something that was supposed to make the pre-commit check fail

krlvi commented 4 months ago

Thank you for reporting this. I am trying to reproduce this - are you able to share your pre-commit hook (or part of it?). Im testing with something like this, which does executed...

#!/bin/sh
echo "asdf"
exit 1
nolith commented 4 months ago

I'm using https://github.com/evilmartians/lefthook and it installs a script like this one.

 cat .git/hooks/pre-commit
#!/bin/sh

if [ "$LEFTHOOK" = "0" ]; then
  exit 0
fi

call_lefthook()
{
  dir="$(git rev-parse --show-toplevel)"
  osArch=$(uname | tr '[:upper:]' '[:lower:]')
  cpuArch=$(uname -m | sed 's/aarch64/arm64/')

  if lefthook -h >/dev/null 2>&1
  then
    lefthook "$@"
  elif test -f "$dir/node_modules/lefthook/bin/index.js"
  then
    "$dir/node_modules/lefthook/bin/index.js" "$@"
  elif test -f "$dir/node_modules/@evilmartians/lefthook/bin/lefthook_${osArch}_${cpuArch}/lefthook"
  then
    "$dir/node_modules/@evilmartians/lefthook/bin/lefthook_${osArch}_${cpuArch}/lefthook" "$@"
  elif test -f "$dir/node_modules/@evilmartians/lefthook-installer/bin/lefthook_${osArch}_${cpuArch}/lefthook"
  then
    "$dir/node_modules/@evilmartians/lefthook-installer/bin/lefthook_${osArch}_${cpuArch}/lefthook" "$@"
  elif bundle exec lefthook -h >/dev/null 2>&1
  then
    bundle exec lefthook "$@"
  elif yarn lefthook -h >/dev/null 2>&1
  then
    yarn lefthook "$@"
  elif pnpm lefthook -h >/dev/null 2>&1
  then
    pnpm lefthook "$@"
  elif command -v npx >/dev/null 2>&1
  then
    npx lefthook "$@"
  elif swift package plugin lefthook >/dev/null 2>&1
  then
    swift package --disable-sandbox plugin lefthook "$@"
  else
    echo "Can't find lefthook in PATH"
  fi
}

call_lefthook run "pre-commit" "$@"
nolith commented 4 months ago

@krlvi this morning I was on another mac and decided to test if this issue was depending on my machine.

I started with your pre-commit hook and it worked as expected, then I added a lefthook.yml with a pre-commit command that calls false and tested it with git, it worked as well, but in gitbutler it does not work, the commit is generated

nolith commented 4 months ago

I think I found where is the issue, lefthook seems to interact with the staged files and I'm quite sure gitbutler don't make use of it.

With a bunch of eprintln! and some tweak to the lefthook generated pre-commit file I managed to confirm that gitbutler invokes lefthook but then lefthook skips all the validation because it thinks that the staging area is empty.

╭──────────────────────────────────────╮
│ 🥊 lefthook v1.5.6  hook: pre-commit │
╰──────────────────────────────────────╯
│  fail (skip) no matching staged files

  ────────────────────────────────────
summary: (skip) empty

In the above output, fail is the name of the command that should run false and always fail.

I'll give a try with other git-hooks managers, but lefthook is incompatible with gitbutler.

yeliex commented 4 months ago

@nolith husky and lint-staged for node ecosystem not work as well

schacon commented 4 months ago

Ah, yeah. I see what the problem is. We don't use the index for staging the next commit. We can't, because we basically do that in memory. We keep the index matching what the union of the commit trees on our applied branches are, so status works more or less how you expect, but we don't actually use it to commit. So if pre-commit relies on that (which makes sense) rather than the working directory, then it's not going to match what is about to be committed in the virtual branch.

I'm actually not sure of a good way around this. Perhaps we can replace the index right before running this command and then revert it after.

jhnns commented 3 months ago

Damn, I'm also using husky and lint-staged 😁. Is there any plan to fix this or is GitButler's approach just incompatible with husky/lint-staged?

borama commented 2 months ago

It seems that the same issue is with Overcommit as well. No pre-commit hooks are run for me.