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's location is ignored in the hooks in a monorepo #443

Closed
shamilovtim opened this issue Mar 9, 2023 · 3 comments
Closed

lefthook's location is ignored in the hooks in a monorepo #443

shamilovtim opened this issue Mar 9, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@shamilovtim
Copy link

shamilovtim commented Mar 9, 2023

🔧 Summary

The lefthook.yml supports the root: directive in order to run the hooks from a separate monorepo folder. However it does not pass this information to the hooks at all. So the hooks end up doing this:

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

  if lefthook -h >/dev/null 2>&1
  then
    eval lefthook $@
  elif test -f "$dir/node_modules/lefthook/bin/index.js"
  then
    eval "\"$dir/node_modules/lefthook/bin/index.js\" $@"
  elif test -f "$dir/node_modules/@evilmartians/lefthook/bin/lefthook_${osArch}_${cpuArch}/lefthook"
  then
    eval "\"$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
    eval "\"$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 npx @evilmartians/lefthook -h >/dev/null 2>&1
  then
    npx @evilmartians/lefthook $@
  else
    echo "Can't find lefthook in PATH"
  fi

git rev-parse --show-toplevel only gets the root of the whole repository. nowhere do the hooks account for lefthook being installed in a deeper nested directory. this is a problem because lefthook can be part of a monorepo which is not just a top level javascript project. for example:

./JS-project/
./go-project/
./ios-project/
./android-project/

No top level package.json is desirable here, and lefthook lives inside of ./js-project/ but tells the hooks to check ./node_modules/ rather than /JS-project/node_modules which is where it actually lives. as a fallback condition it tries to hack using npx lefthook which is even worse because it retrieves some random cached version somewhere globally on the machine, which is even worse and causes version mismatches.

Lefthook version

1.3.3

Possible Solution

  1. The hooks generated need to know where to look for lefthook in a monorepo. Add a directive to point to where lefthook is installed in the monorepo and propagate it to the hooks. For example lefthook_location: "my-js-dir/. Then pass that to the generated hooks as a param.

  2. For the npx fallback, this is an unrelated bug but it also needs to be fixed. Change:
    npx @evilmartians/lefthook $@
    to:
    npx @evilmartians/lefthook@latest $@

The former is incorrect and will always get a globally installed random version of lefthook that was cached on the user's machine at some random point in time. The latter will always get the latest released version with bugfixes.

@shamilovtim shamilovtim added the bug Something isn't working label Mar 9, 2023
@mrexox
Copy link
Member

mrexox commented Mar 9, 2023

The hooks generated need to know where to look for lefthook in a monorepo. Add a directive to point to where lefthook is installed in the monorepo and propagate it to the hooks. For example lefthook_location: "my-js-dir/. Then pass that to the generated hooks as a param.

Hey! I guess the problem is - it doesn't look obvious that root parameter only changes the cwd for the commands and scripts. If I properly understand - the problem is the following:

  • Your lefthook.yml lives in ./js-project/lefthook.yml
  • Your git root is in ./
  • You don't need to run hooks for other subprojects but you want a project root for lefthook to be in js-project

lefthook.yml is supposed to locate in the git root directory. Your case looks like a feature request.

Also things will become a bit more complicated if you have js1-project and js2-project with both having different versions of lefthook. How should the global git hooks work then? What if hooks interfere, how to resolve them? If we could find a solution I will consider adding this feature.

So, the main problem here is - for a monorepo with multiple projects we still have one set of git hooks, so we anyway need one reference for a lefthook executable.

BTW, do you have git modules? In this case things might be different but unfortunately I haven't tested this setup for a long time.

@shamilovtim
Copy link
Author

shamilovtim commented Mar 9, 2023

Thanks for responding! Yeah, your description is correct.

if you have js1-project and js2-project with both having different versions of lefthook. How should the global git hooks work then? What if hooks interfere, how to resolve them? If we could find a solution I will consider adding this feature.

I assume the easiest and most performant solution is to support a custom location directive in lefthook.yml for example lefthook_location:. This would avoid conflicts, since only one version of lefthook could be resolved through this directive.

Also aside from this stuff note point 2 above since if that worked correctly I probably never would've even noticed this. But it happened to install a bugged version of lefthook that got patched a while ago which was sitting in my npx cache.

BTW, do you have git modules

not using git modules

@mrexox
Copy link
Member

mrexox commented Dec 21, 2024

Hey! Now when you have lefthook NPM package installed in a subdirectory, and you have root option configured for that directory in your lefthook.yml (which must be in git repo root directory) – lefthook must be able to get the right executable from the <subdirectory>/node_modules. This feature added in this PR: #616. Please, report back if everything is working or not.

@mrexox mrexox closed this as completed Dec 21, 2024
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

No branches or pull requests

2 participants