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

Lefthook causes git commit to hang indefinitely when internet connection is unavailable #610

Closed
evnp opened this issue Jan 14, 2024 · 9 comments · Fixed by #616
Closed
Labels
bug Something isn't working

Comments

@evnp
Copy link

evnp commented Jan 14, 2024

🔧 Summary

Lefthook causes git commit to hang indefinitely when internet connection is unavailable. Adding LEFTHOOK=0 to the start of the git commit command makes the command succeed instantly.

Want to add – really love the tool, its design/simplicity are amazing compared to alternatives I've dealt with. I'd love to understand what is causing this particular issue and if some small configuration change can fix it OR if this is due to some environment-specific issue on my side. I work from low-connectivity settings often and being blocked from committing changes normally (with linting/formatting) at these times is very inconvenient.

Lefthook version

$ lefthook version -f
1.5.5 9b072e6622857ceb8d40a173ba39ae97afb35957

Steps to reproduce

  1. Turn off device wifi / internet connectivity.
  2. Stage some code changes.
  3. Run git commit -m "test"
  4. Observe that the command hangs indefinitely.
  5. Run LEFTHOOK=0 git commit -m "test"
  6. Observe that the command finishes with no delay.

Expected results

The git commit command succeeds, with lefthook invoked as normal, and with identical output as when internet connectivity was available. I wouldn't expect git commit to behave differently when internet connectivity is disabled vs enabled.

Actual results

The git commit command hangs indefinitely, with no output (from lefthook or anything else).

Possible Solution

Could some remote version check be happening that I'm unaware of? I scanned the documentation and searched through all open/closed repo issues, couldn't find anything relevant to internet connectivity.

Logs / Screenshots

radiopaper frontend ● cat ../lefthook.yml
# Lefthook Documentation:
# https://github.com/evilmartians/lefthook/blob/master/docs/configuration.md
pre-commit:
  parallel: true
  commands:
    lint-format:
      root: "frontend/"
      glob: "*.{js,ts,jsx,tsx,vue,html,svg,json,yml,yaml}"
      run: npm run lint-format {staged_files}
    typecheck:
      root: "frontend/"
      glob: "*.{js,ts,jsx,tsx,vue}"
      run: npm run typecheck
radiopaper frontend ● LEFTHOOK_VERBOSE=true git commit --message "test"
// waited 60 seconds ...^C
radiopaper frontend ● LEFTHOOK=0 LEFTHOOK_VERBOSE=true git commit --message "test"
[refactor-validate 778d54e8] test
 1 file changed, 1 insertion(+)
radiopaper frontend ●
Screen Shot 2024-01-14 at 12 55 38 PM

Device information:

radiopaper frontend ● system_profiler SPSoftwareDataType SPHardwareDataType
Software:

    System Software Overview:

      System Version: macOS 12.3 (21E230)
      Kernel Version: Darwin 21.4.0
      Boot Volume: Macintosh HD
      Boot Mode: Normal
      Secure Virtual Memory: Enabled
      System Integrity Protection: Enabled

Hardware:

    Hardware Overview:

      Model Name: MacBook Pro
      Model Identifier: MacBookPro17,1
      Chip: Apple M1
      Total Number of Cores: 8 (4 performance and 4 efficiency)
      Memory: 16 GB
      System Firmware Version: 7459.101.2
      OS Loader Version: 7459.101.2
      Activation Lock Status: Enabled
@evnp evnp added the bug Something isn't working label Jan 14, 2024
@mrexox
Copy link
Member

mrexox commented Jan 15, 2024

Hey! Thank you for this issue. I'd love to understand the reason of this problem too. Could you please show the output when you run git commit with LEFTHOOK_VERBOSE=true env variable set?

LEFTHOOK_VERBOSE=true git commit

Or even without git

lefthook run pre-commit -v

@evnp
Copy link
Author

evnp commented Jan 15, 2024

Hey @mrexox, sure thing. Thanks for taking a look.

LEFTHOOK_VERBOSE=true git commit

This unfortunately seems to hang before producing any output:

Screen Shot 2024-01-15 at 9 13 27 AM

It's as if lefthook isn't even being invoked, which makes me wonder if it's some git config issue on my side. What's odd is that adding LEFTHOOK=0 makes git commit work as normal.

lefthook run pre-commit -v

This does run lefthook as normal when wifi is off, with normal expected output. So it's some kind of interaction with git for sure.

Screen Shot 2024-01-15 at 9 15 14 AM

@mrexox
Copy link
Member

mrexox commented Jan 16, 2024

Thank you. That's confusing. Could you show the content of the .git/hooks/pre-commit? Probably it is outdated and doesn't work for some reason. lefthook install must update it. Could you try the same thing after running lefthook install?

@evnp
Copy link
Author

evnp commented Jan 17, 2024

For sure, sorry about the delay. Here's that:

$ 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 @evilmartians/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" "$@"

Here's my global .gitconfig as well, in case that might have any relevance.

$ cat ~/.gitconfig
# This is Git's per-user configuration file.

[user]
  name = Evan Purcer
  email = ep@e2.gg

[init]
  defaultBranch = main

[core]
  pager = delta

[interactive]
  diffFilter = delta --color-only

[add.interactive]
  useBuiltin = false # required for git 2.37.0

[delta]
  navigate = true    # use n and N to move between diff sections
  light = false      # set to true if you're in a terminal w/ a light background color (e.g. the default macOS terminal)
  side-by-side = true
  line-numbers = false

[merge]
  conflictstyle = diff3

[diff]
  colorMoved = default

@evnp
Copy link
Author

evnp commented Jan 17, 2024

Your pointing out .git/hooks/pre-commit led me to some kind of answer. Within that file it looks like the npx command here is what hangs silently when wifi is off:

    elif command -v npx >/dev/null 2>&1
    then
      npx @evilmartians/lefthook "$@"

If I replace that line with this:

    elif command -v npx >/dev/null 2>&1
    then
      # npx @evilmartians/lefthook "$@"
      ./frontend/node_modules/.bin/lefthook "$@"

then everything works as expected – awkward path required though, since I've installed lefthook locally for this project via npm, not globally.

What's the recommended setup for this sort of case? A bit more context, I installed lefthook with

npm install --save-dev 

and after adding "install-githooks": "lefthook install" to package.json scripts, ran this to install hooks:

npm run install-githooks

@evnp
Copy link
Author

evnp commented Jan 17, 2024

I guess two clear solutions are jumping out at me:
A. install lefthook globally, then it looks like .git/hooks/pre-commit will use that binary instead
B. modify the .git/hooks/pre-commit file manually to correct the node_modules path for our project:

---    elif test -f "$dir/node_modules/lefthook/bin/index.js"
+++    elif test -f "$dir/frontend/node_modules/lefthook/bin/index.js"
       then
---       "$dir/node_modules/lefthook/bin/index.js" "$@"
+++       "$dir/frontend/node_modules/lefthook/bin/index.js" "$@"

Is there any way to accomplish B automatically when lefthook install runs? (basically change the location where .git/hooks/pre-commit looks for the project-local lefthook script)

I'd like to avoid installing globally if possible – it'd be great if npm install && npm run install-githooks was all that users of our repo needed to run to get set up, with nothing modifying the surrounding environment.

@mrexox
Copy link
Member

mrexox commented Jan 17, 2024

Oh, I see, you have node_modules not in the repo root. This is a known issue but there's no general solution for it yet. Yes, the fallback to npx is bad and the only recommendation is to have lefthook installed globally.

Anyway thank you for creating this issue I will try to figure this out and provide some fix. The main complication is that root is an option of the config while git hook is being installed globally, and if you have, say, frontend-1 and frontend-2 folders with different apps and only one has lefthook dependency, it's impossible to predict which path should be used in a git hook.

The fix will probably require some extra config option. I haven't thought out this yet. If you have an idea, please share it with me.

@evnp
Copy link
Author

evnp commented Jan 17, 2024

Hey thanks for the explanation, makes total sense. I'm open to anything here and I know it's a significant ask to add config options to something with (wonderfully) minimal surface area.

With admittedly not a ton of broader context outside the npm use case - maybe adding an npm-root option at the top level of lefthook.yml would make sense? This would contain a path similar to root options under rules, which would be used to prefix node_modules/ in the pre-commit file if it's set. Something like this:

# lefthook.yml
npm-root: "frontend/"
pre-commit:
  parallel: true
  commands:
    lint-format:
      root: "frontend/"
      ...

Alternately (or maybe both?) an --npm-root option could be added to lefthook install. In my case I'd then change my package.json script to

// package.json
"scripts": {
  "install-githooks": "lefthook install --npm-root frontend/"
  ...

which would have the same effect the config above.

For now, I can use this package.json script to fully resolve the issue for my use case, so thank you for the help getting me there!

// package.json
"scripts": {
  "install-githooks": "lefthook install && perl -pi -e 's/node_modules/frontend\\/node_modules/g' ../.git/hooks/pre-commit"
  ...
  // note - using perl because sed doesn't behave well on osx vs linux

(by the way, I'd be happy to contribute a PR with any of the above – but understand if it's better this be handled by maintainers)

@evnp
Copy link
Author

evnp commented Mar 16, 2024

@mrexox Sorry about my huge delay in taking a look at this. I can confirm that on v1.5.5 with wifi off I see the hanging behavior, and after upgrading to v1.6.7 lefthook behaves exactly as desired. This is without any other config changes which is an unexpected bonus. Appreciate your work here!

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