search

tarmolov / git-hooks-js

A tool to manage and run project git hooks
167 stars 30 forks source link

.gitignored files are still executed #30

Closed codebling closed 7 years ago

codebling commented 8 years ago

We check the hook names against the output of git check-ignore, but git check-ignore does some nominal path normalisation, so any non-trivial path will not match the output.

Furthermore, we only ever filter against the first file in the list of ignored files provided by the output, so at most one hook will ever be filtered.

I ran into this issue while fixing the tests for Windows. The test only verifies that gitignored files which also have the wrong permissions are not executed. Since hooks with the wrong permissions will never be executed anyways, that eclipsed ability to spot the issue with not executing gitignored files.

Will create a PR to resolve the issue

rjmunro commented 7 years ago

I'm confused as to exactly what this is and how it's desirable. Why would you want a file in the .githooks folder, but neither commit it nor or run it as a hook?

What if I want to have a git hook specific to my machine or something? Without this feature I could put it in .gitignore and not commit it, but still use the .githooks folder structure to manage it alongside other hooks that are for all users. With this feature, it inexplicably won't be run.

If there is a real need to put things in the .githooks folders that aren't hooks, I think they should have their own ignore mechanism, separate to .gitignore.

codebling commented 7 years ago

@rjmunro I think you've misunderstood. This issue report concerns a feature which is already implemented, specifically with the fact that it the implementation contains a bug. If the feature is not desirable, it should be removed. If the feature is in the code, it should work.

I don't presume to know why this feature exists or what purpose it may serve, but I have attempted to contribute to this project by at very least making the implementation perform consistently.

rjmunro commented 7 years ago

@codebling Sorry, I probably should have started a new bug to debate merits of the feature, rather than to try to use this one.

codebling commented 7 years ago

@rjmunro haha no worries. I was pretty taken aback when I found this feature and almost opened a 'why?!!?!?' issue myself.

tarmolov commented 7 years ago

https://github.com/tarmolov/git-hooks-js/issues/12