check-executables-have-shebangs not correct on windows

[dstandish]

does not seem to work right on windows

i am dealing with a primarily linux / python project but on a windows machine.

i have set git config core.filemode false

i created a new file and stage and i verify that filemode is 644:

>git ls-files -s newfile.py
100644 3edd36f71bf2081c70a0eaf39dec6980d0a9f791 0       newfile.py

but hook still fails

hookid: check-executables-have-shebangs

newfile.py: marked executable but has no (or invalid) shebang!
  If it isn't supposed to be executable, try: chmod -x newfile.py
  If it is supposed to be executable, double-check its shebang.

why is this file causing error?

[asottile]

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?

[dstandish]

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?

[asottile]

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?)

[dstandish]

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?

[dstandish]

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....

[asottile]

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

[asottile]

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 🤷‍♂

[dstandish]

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

[asottile]

one way would be to use SKIP=...

[dstandish]

nice -- thanks

[asottile]

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

[dstandish]

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?

[asottile]

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

[jfboismenu]

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

[asottile]

nope! feel free to take it 🎉