evilmartians / lefthook

Fast and powerful Git hooks manager for any type of projects.
MIT License
4.66k stars 211 forks source link

new feature refetch remotes config sync hooks fail #743

Closed SirNoSir closed 2 months ago

SirNoSir commented 2 months ago

:wrench: Summary

I test the new feature refetch #736 but a problem occured while synchronizing git hooks.

Lefthook version

I test version 1.6.15 in windows environment.

Steps to reproduce

set refetch: true and send a commit.

Expected results

Actual results

sync hooks fail, and git hooks files was removed after executing the commit.

Possible Solution

I suspect that the reason for the problem may be that the git hooks script(like prepare-commit-msg etc.) is trying to update itself during execution, the operating system reject this update command. Also, before using refetch feature I add lefthook install command in the rc file, the result is the same and error was file prepare-commit-msg access denied. Maybe just download the remote file without updating git hooks is a way to fix this problem?

Logs / Screenshots

+ '[' '' = 0 ']'
+ '[' -f ./lefthookrc ']'
+ . ./lefthookrc
+ call_lefthook run prepare-commit-msg .git/COMMIT_EDITMSG
+ test -n ''
+ lefthook.exe -h
+ lefthook.exe run prepare-commit-msg .git/COMMIT_EDITMSG
│ [lefthook] cmd: [git rev-parse --show-toplevel]
│ [lefthook] err: <nil>
│ [lefthook] out: D:/source/

│ [lefthook] cmd: [git rev-parse --git-path hooks]
│ [lefthook] err: <nil>
│ [lefthook] out: .git/hooks

│ [lefthook] cmd: [git rev-parse --git-path info]
│ [lefthook] err: <nil>
│ [lefthook] out: .git/info

│ [lefthook] cmd: [git rev-parse --git-dir]
│ [lefthook] err: <nil>
│ [lefthook] out: .git

│ [lefthook] cmd: [git hash-object -t tree /dev/null]
│ [lefthook] err: <nil>
│ [lefthook] out: 4b825dc642cb6eb9a060e54bf8d69288fbee4904

│ Merging remote config: http://***/tools/lefthook-config: .git\info\lefthook-remotes\lefthook-config\remote_main.yml
╭───────────────────────────────────────────────╮
│ 🥊 lefthook v1.6.15  hook: prepare-commit-msg │
╰───────────────────────────────────────────────╯
│ Updating remote config repository: .git\info\lefthook-remotes\lefthook-config
│ [lefthook] cmd: [git -C .git\info\lefthook-remotes\lefthook-config pull --quiet]
│ [lefthook] dir: D:/source/
│ [lefthook] err: <nil>
│ [lefthook] out:
│ Searching config in:D:/source/
│ Merging remote config: http://***/tools/lefthook-config: .git\info\lefthook-remotes\lefthook-config\remote_main.yml
sync hooks: ❌
⚠️  There was a problem with synchronizing git hooks.
Run 'lefthook install' manually.
mrexox commented 2 months ago

Hey! I found a place where the error happens however lefthook does not log the error, so it's hard to say what was the actual problem. Does it fail the same way if you don't use refetch: true and manually run lefthook install && lefthook run prepare-commit-msg?

SirNoSir commented 2 months ago

whether I set refetch true or false, It works well when i run lefthook install && lefthook run prepare-commit-msg. Any ideas?

lefthook install && lefthook run prepare-commit-msg
sync hooks: ✔️ (prepare-commit-msg)
╭───────────────────────────────────────────────╮
│ 🥊 lefthook v1.6.15  hook: prepare-commit-msg │
╰───────────────────────────────────────────────╯
┃ remote-script.sh ❯

                                      d in %PATH%
  ────────────────────────────────────
summary: (done in 0.06 seconds)
🥊  remote-script.sh
mrexox commented 2 months ago

So, it only fails if you run lefthook implicitly with git commit ... command, right?

Could you please check these steps:

  1. remove refetch: true from the remotes
  2. remove lefthook install from the rc file
  3. git add -A && git commit

Since you've touched the lefthook.yml file lefthook should try to overwrite the hooks and display sync hooks: ✔️ (prepare-commit-msg). I just wonder if it fails to overwrite the hooks or the problem is related to fetching/merging the configs after fetching the remotes.

SirNoSir commented 2 months ago

These are the details of my operations, I hope it can help.

  1. When I followed the steps above, the hooks sync failed. However, it still ran my script anyway.
  2. After that, I checked ./.git/hooks/ in my project, and the prepare-commit-msg file was removed.
  3. The next time I ran git commit, lefthook was not triggered since the hooks file was not present.
git add -A && git commit
warning: in the working copy of 'lefthook.yml', LF will be replaced by CRLF the next time Git touches it
╭───────────────────────────────────────────────╮
│ 🥊 lefthook v1.6.15  hook: prepare-commit-msg │
╰───────────────────────────────────────────────╯
sync hooks: ❌
⚠️  There was a problem with synchronizing git hooks.
Run 'lefthook install' manually.
┃  remote-script.sh ❯
git add -A && git commit
Aborting commit due to empty commit message.
mrexox commented 2 months ago

Ok, I will try to prepare a fix that at least displays the error.

I assume that this may be related to configuration merging and prepare-commig-msg file may be dropped while being installed.

Could you share your lefthook.yml hooks and the remote config (just the basic structure without the real commands/scripts)?

SirNoSir commented 2 months ago

Of course, My config is very simple like:

# lefthook.yml
remotes:
  - git_url: http://git_address/tools/lefthook-config
    configs:
    - remote_main.yml
# remote_main.yml
source_dir: ./
prepare-commit-msg:
  scripts:
    "remote-script.sh":
      runner: bash
mrexox commented 2 months ago

Thank you. I could not reproduce the error but I will release a new version that prints the error so we can have more context on what is the reason.

mrexox commented 2 months ago

If you have a chance to install lefthook with go install github.com/evilmartians/lefthook@latest, could you try the lefthook version that is currently in master?

SirNoSir commented 2 months ago

Hey, The log I used master version with refetch option is as follows:

+ call_lefthook run prepare-commit-msg .git/COMMIT_EDITMSG
+ test -n ''
+ lefthook.exe -h
+ lefthook.exe run prepare-commit-msg .git/COMMIT_EDITMSG
│ [lefthook] cmd: [git rev-parse --show-toplevel]
│ [lefthook] err: <nil>
│ [lefthook] out: D:/source/IA-Git/main

│ [lefthook] cmd: [git rev-parse --git-path hooks]
│ [lefthook] err: <nil>
│ [lefthook] out: .git/hooks

│ [lefthook] cmd: [git rev-parse --git-path info]
│ [lefthook] err: <nil>
│ [lefthook] out: .git/info

│ [lefthook] cmd: [git rev-parse --git-dir]
│ [lefthook] err: <nil>
│ [lefthook] out: .git

│ [lefthook] cmd: [git hash-object -t tree /dev/null]
│ [lefthook] err: <nil>
│ [lefthook] out: 4b825dc642cb6eb9a060e54bf8d69288fbee4904

│ Merging remote config: http://git_address/tools/lefthook-config: .git\info\lefthook-remotes\lefthook-config\remote_main.yml
╭───────────────────────────────────────────────╮
│ 🥊 lefthook v1.6.15  hook: prepare-commit-msg │
╰───────────────────────────────────────────────╯
│ Updating remote config repository: .git\info\lefthook-remotes\lefthook-config
│ [lefthook] cmd: [git -C .git\info\lefthook-remotes\lefthook-config pull --quiet]
│ [lefthook] dir: D:/source/MyProject/main
│ [lefthook] err: <nil>
│ [lefthook] out:
│ Searching config in:D:/source/MyProject/main
│ Merging remote config: http://git_address/tools/lefthook-config: .git\info\lefthook-remotes\lefthook-config\remote_main.yml
sync hooks: ❌
⚠️  There was a problem with synchronizing git hooks. Run 'lefthook install' manually.
   Error: could not add the hook: open D:\source\MyProject\main\.git\hooks\prepare-commit-msg: Access is denied.
┃  remote-script.sh ❯

My config was like this:

# lefthook.yml
remotes:
  - git_url: http://git_address/tools/lefthook-config
    refetch: true
    configs:
    - remote_main.yml
mrexox commented 2 months ago

Ok, I see that the problem is related to opening a file which is probably being executed at the moment of calling the script. I will need to think out a solution for this issue 🤔

SirNoSir commented 2 months ago

Hey, thank you for your reply. Although I am not familiar with Go and Bash, I would like to share my approach. I don't think updating prepare-commit-msg scripts during execution is necessary in this "refetch" case. Would it be feasible to update only remote scripts in the ./.git/info directory instead?

mrexox commented 2 months ago

Yes, I think so, I will try to implement this approach :+1:

mrexox commented 2 months ago

@SirNoSir , could you please check the unpublished version (using the same go install ... command as before). I've skipped installing the hooks if nothing is touched in the main config. So, this should fix the "Access is denied" error.

SirNoSir commented 2 months ago

Great! I just tested the latest version. It ran perfectly and correctly, executed the script I had just modified on Git. Thank you for your help! :smile:

mrexox commented 2 months ago

Nice! I will prepare the release later this week :+1: