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

"extends" path using an environment variable not working as expected #135

Closed
qjcg opened this issue Jul 9, 2020 · 3 comments
Closed

"extends" path using an environment variable not working as expected #135

qjcg opened this issue Jul 9, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@qjcg
Copy link
Contributor

qjcg commented Jul 9, 2020

I'm trying to get lefthook to use hooks from a custom external directory defined via an environment variable
(e.g. extends: $HOOKDIR/hooks.yml).

So far I haven't been able to get it working, but maybe I'm missing something obvious.

Steps to reproduce (tested using bash):

# 1. A basic hook for testing purposes.
$ cat hooks.yml 
pre-commit:
  commands:
    greet:
      run: echo 'Extended hooks!'

# 2. Export a HOOKDIR environment variable and copy our hook file there.
$ export HOOKDIR=/tmp
$ cp hooks.yml $HOOKDIR

# 3. Tell lefthook about our external hooks file and test running the hook. It fails :( <--THIS IS THE ISSUE
$ echo 'extends: $HOOKDIR/hooks.yml' > lefthook.yml
$ lefthook run pre-commit
runtime error: slice bounds out of range [:-1]

# 4. Repeat steps 2 & 3 using $HOME instead of $HOOKDIR. It works! :)
$ cp hooks.yml $HOME
$ echo 'extends: $HOME/hooks.yml' > lefthook.yml
$ lefthook run pre-commit
Lefthook v0.7.2
RUNNING HOOKS GROUP: pre-commit

  EXECUTE > greet
Extended hooks!

SUMMARY: (done in 0.02 seconds)
✔️  greet

For context:

Remote config as in #118 would be a better solution for my use case (sharing a standard set of common hooks across repos for a team of developers), but unless / until that PR gets merged, I'll be looking at extends: plus an env-defined local path as the next best thing.

@Arkweid Arkweid added the bug Something isn't working label Jul 10, 2020
@qjcg
Copy link
Contributor Author

qjcg commented Jul 31, 2020

Looking into this, the root cause of this issue is viper's absPathify function (as called by the AddConfigPath method). It currently fails when provided with a path starting with an environment variable and containing a single pre-expansion path separator (excluding $HOME, which is handled differently than other env vars).

So:

  • $HOOKDIR/hooks.yml will fail (one pre-expansion path separator), but
  • $HOOKDIR/shared/hooks.yml (2 pre-expansion path separators) will succeed

The good news is, there's spf13/viper#495 which fixes the issue and adds a solid unit test (outside of this PR, viper does not currently test absPathify). The bad news is, the PR's been pending, with no comments, for over 2 years...

So, until there's an upstream fix in viper, a workaround for lefthook users could be to always use at least 2 pre-expansion path separators when using non-$HOME environment variables with extends, as in:

extends: $HOOKDIR/shared/hooks.yml.

@qjcg
Copy link
Contributor Author

qjcg commented Jul 31, 2020

Finally I got a fix merged for this upstream: spf13/viper#940

So the issue described above in lefthook should be resolved by updating the viper dependency to v1.7.1.

@qjcg
Copy link
Contributor Author

qjcg commented Aug 21, 2020

Closed by #148.

@qjcg qjcg closed this as completed Aug 21, 2020
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