check-executables-have-shebangs not correct on windows #435

windows unfortunately considers all files as os.X_OK (since windows doesn't really have a concept of non-executable files)

this check is really only designed for posix, we could maybe change it to just do nothing on windows? what do you think about that?

the thing is, even on windows, you can still tell executable status as recorded by git.

e.g. you might see this:

>git ls-files -s build.sh
100755 3edd36f71bf2081c70a0eaf39dec6980d0a9f791 0      build.sh

the thing i'm pointing out here is that in this repo build.sh is executable and it therefore shows up as 755. while the python script in my initial comment is not executable and is 644.

maybe this hook can rely on git filemode rather than system status? after all, the git filemode is what really matters as far as the repo is concerned right?

hmmm yes and no -- most of the scripts work standalone -- but I could see handling it just as the git mode (perhaps just on windows?)

so perhaps it could look at what it set in core.filemode

if false that means "can't trust os filemode"

so if false, then look at git filemode; otherwise, os is fine?

i guess this is actually an issue with pre-commit repo and not pre-commit-hooks. because what it boils down to is the file type classification executable....

which comes from identify which doesn't have anything to do with git 🤔

which at that point is quite the stack of complexity -- the practical choice is probably either "don't use this hook on windows" or make the hook just always pass on windows 🤷‍♂

do you know how to make it skip in windows???

one way would be to use SKIP=...

nice -- thanks

I still think it might make sense to call out to git here to improve the output (but maybe just in pre-commit/pre-commit-hooks?) -- we do similar things for check-added-large-files where the filtering isn't quite sufficient

yeah i mean doing that i think would make this hook work correctly on windows

i looked a bit at gitpython. seemed a bit daunting to work with, and seemed like might just be easier to do popen or something. you reckon you'd use popen?

I would not use gitpython, subprocess is fine (take a look at the current calls to git in the project for inspiration please 🎉)

Has anyone taken a stab at this one? We're running into this issue here. I might be tempted to fix it.

nope! feel free to take it 🎉

It would be cool if each hook would have a "skip-on-platform" or "only-apply-on-platform" attribute. Is there something like that?

there's SKIP which you can set in your environment

making something auto-skip in a gating tool is not a good idea :)

this is also already fixed and your question is off topic here

I bypassed the issue by running this command in my Windows Powershell.

git update-index --chmod=+x .\filename.py

I bypassed the issue by running this command in my Windows Powershell.

git update-index --chmod=+x .\filename.py

Thanks a lot. In my case, I'm on linux and the hook was complaining about my .eslintrc.js file, which didn't have executable permissions. I've just ran git update-index --chmod=-x .eslintrc.js and the problem was solved