Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new feature refetch remotes config sync hooks fail #743

Closed
SirNoSir opened this issue Jun 4, 2024 · 15 comments · Fixed by #745
Closed

new feature refetch remotes config sync hooks fail #743

SirNoSir opened this issue Jun 4, 2024 · 15 comments · Fixed by #745
Labels
bug Something isn't working

Comments

@SirNoSir
Copy link

SirNoSir commented Jun 4, 2024

🔧 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.

@SirNoSir SirNoSir added the bug Something isn't working label Jun 4, 2024
@mrexox
Copy link
Member

mrexox commented Jun 4, 2024

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
Copy link
Author

SirNoSir commented Jun 4, 2024

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
Copy link
Member

mrexox commented Jun 4, 2024

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
Copy link
Author

SirNoSir commented Jun 4, 2024

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
Copy link
Member

mrexox commented Jun 4, 2024

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
Copy link
Author

SirNoSir commented Jun 5, 2024

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
Copy link
Member

mrexox commented Jun 6, 2024

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
Copy link
Member

mrexox commented Jun 6, 2024

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?

@mrexox mrexox closed this as completed Jun 6, 2024
@mrexox mrexox reopened this Jun 6, 2024
@SirNoSir
Copy link
Author

SirNoSir commented Jun 7, 2024

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

@SirNoSir SirNoSir closed this as completed Jun 7, 2024
@SirNoSir SirNoSir reopened this Jun 7, 2024
@mrexox
Copy link
Member

mrexox commented Jun 7, 2024

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
Copy link
Author

SirNoSir commented Jun 10, 2024

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
Copy link
Member

mrexox commented Jun 10, 2024

Yes, I think so, I will try to implement this approach 👍

@mrexox
Copy link
Member

mrexox commented Jun 10, 2024

@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
Copy link
Author

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! 😄

@mrexox
Copy link
Member

mrexox commented Jun 10, 2024

Nice! I will prepare the release later this week 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants