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

Allow recursion into {pre,post}-up hook directories #9

Closed
wants to merge 3 commits into from

Conversation

pablox-cl
Copy link

When I found the hooks, I found that rcm only checks for an executable file (pre-up or post-up). Following some common sense I imagined that executable files inside those directories (something like the git hooks) should work too.

It's quite precarious, and variables could be a bit better, but works fine.

If you like it, I can add it this feature to the documentation and make the pull after that.

@mike-burns
Copy link
Contributor

Interesting. Is there a use case for a directory over a file?

@pablox-cl
Copy link
Author

Well, actually it's more because it's something I expected to work that way... you know, there's a lot of software that use that kind of functionality and that's (git hooks, wicd, netcfg, ecc) what I did in the first place (yep I check the docs later and it's clear about it's a file and not a directory) only to found it didn't work.

Also it's more modular, you can have "shareable" hooks and finally you don't have to put different unrelated stuff in the same file.

@pbrisbin
Copy link
Contributor

pbrisbin commented Dec 9, 2013

I can see the benefit here as you can have different hooks in different languages. That's the only real technical advantage I see, though I also like it from an organizational standpoint.

@pablox-cl, Have you thought about:

find "$dotfiles_dir/hooks/$when-$direction" -type f -executable -exec {} \;

You lose a little debug output, but it's mighty terse, and I think it works for both the file and directory cases. I'd have to confirm its portability with @mike-burns though.

Alternatively, rcm ignores the hooks directory entirely, so you could just add separate scripts to hooks/* then write a $when-$direction which finds and runs them.

@mike-burns
Copy link
Contributor

This would need a documentation update. Otherwise this looks good, either as-is or with @pbrisbin 's overly-terse but rather clever find solution.

-executable isn't in POSIX, but -perm +111 is.

@pablox-cl
Copy link
Author

No, it's fine. I did think in using find or file globbing (but then I thought it could be an issue with POSIX). I decided to use that because it gives more output. But actually it's overly complex when it shouldn't be. I'm gonna test if it actually works for both cases, and if it is, I'll update the PR and I will add the documentation too.

@mike-burns
Copy link
Contributor

Merged as 83c4875 , thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants