Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 48 additions & 19 deletions pre_commit_hooks/check_executables_have_shebangs.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,68 @@
import argparse
import shlex
import sys
from typing import List
from typing import Optional
from typing import Sequence
from typing import Set

from pre_commit_hooks.util import cmd_output

def check_has_shebang(path: str) -> int:
EXECUTABLE_VALUES = frozenset(('1', '3', '5', '7'))


def check_executables(paths: List[str]) -> int:
if sys.platform == 'win32': # pragma: win32 cover
return _check_git_filemode(paths)
else: # pragma: win32 no cover
retv = 0
for path in paths:
if not _check_has_shebang(path):
_message(path)
retv = 1

return retv


def _check_git_filemode(paths: Sequence[str]) -> int:
outs = cmd_output('git', 'ls-files', '--stage', '--', *paths)
seen: Set[str] = set()
for out in outs.splitlines():
metadata, path = out.split('\t')
tagmode = metadata.split(' ', 1)[0]

is_executable = any(b in EXECUTABLE_VALUES for b in tagmode[-3:])
has_shebang = _check_has_shebang(path)
if is_executable and not has_shebang:
_message(path)
seen.add(path)

return int(bool(seen))


def _check_has_shebang(path: str) -> int:
with open(path, 'rb') as f:
first_bytes = f.read(2)

if first_bytes != b'#!':
quoted = shlex.quote(path)
print(
f'{path}: marked executable but has no (or invalid) shebang!\n'
f" If it isn't supposed to be executable, try: "
f'`chmod -x {quoted}`\n'
f' If it is supposed to be executable, double-check its shebang.',
file=sys.stderr,
)
return 1
else:
return 0
return first_bytes == b'#!'


def _message(path: str) -> None:
print(
f'{path}: marked executable but has no (or invalid) shebang!\n'
f" If it isn't supposed to be executable, try: "
f'`chmod -x {shlex.quote(path)}`\n'
f' If it is supposed to be executable, double-check its shebang.',
file=sys.stderr,
)


def main(argv: Optional[Sequence[str]] = None) -> int:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument('filenames', nargs='*')
args = parser.parse_args(argv)

retv = 0

for filename in args.filenames:
retv |= check_has_shebang(filename)

return retv
return check_executables(args.filenames)


if __name__ == '__main__':
Expand Down
77 changes: 76 additions & 1 deletion tests/check_executables_have_shebangs_test.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
import os
import sys

import pytest

from pre_commit_hooks import check_executables_have_shebangs
from pre_commit_hooks.check_executables_have_shebangs import main
from pre_commit_hooks.util import cmd_output

skip_win32 = pytest.mark.skipif(
sys.platform == 'win32',
reason="non-git checks aren't relevant on windows",
)


@skip_win32 # pragma: win32 no cover
@pytest.mark.parametrize(
'content', (
b'#!/bin/bash\nhello world\n',
Expand All @@ -17,14 +28,14 @@ def test_has_shebang(content, tmpdir):
assert main((path.strpath,)) == 0


@skip_win32 # pragma: win32 no cover
@pytest.mark.parametrize(
'content', (
b'',
b' #!python\n',
b'\n#!python\n',
b'python\n',
'☃'.encode(),

),
)
def test_bad_shebang(content, tmpdir, capsys):
Expand All @@ -33,3 +44,67 @@ def test_bad_shebang(content, tmpdir, capsys):
assert main((path.strpath,)) == 1
_, stderr = capsys.readouterr()
assert stderr.startswith(f'{path}: marked executable but')


def test_check_git_filemode_passing(tmpdir):
with tmpdir.as_cwd():
cmd_output('git', 'init', '.')

f = tmpdir.join('f')
f.write('#!/usr/bin/env bash')
f_path = str(f)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other tests use .strpath, is one preferred over the other?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apparently strpath is not documented despite being a public attribute so I've mostly switched to str(...) now that I don't care about python2

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL

cmd_output('chmod', '+x', f_path)
cmd_output('git', 'add', f_path)
cmd_output('git', 'update-index', '--chmod=+x', f_path)

g = tmpdir.join('g').ensure()
g_path = str(g)
cmd_output('git', 'add', g_path)

# this is potentially a problem, but not something the script intends
# to check for -- we're only making sure that things that are
# executable have shebangs
h = tmpdir.join('h')
h.write('#!/usr/bin/env bash')
h_path = str(h)
cmd_output('git', 'add', h_path)

files = (f_path, g_path, h_path)
assert check_executables_have_shebangs._check_git_filemode(files) == 0


def test_check_git_filemode_failing(tmpdir):
with tmpdir.as_cwd():
cmd_output('git', 'init', '.')

f = tmpdir.join('f').ensure()
f_path = str(f)
cmd_output('chmod', '+x', f_path)
cmd_output('git', 'add', f_path)
cmd_output('git', 'update-index', '--chmod=+x', f_path)

files = (f_path,)
assert check_executables_have_shebangs._check_git_filemode(files) == 1


@pytest.mark.parametrize(
('content', 'mode', 'expected'),
(
pytest.param('#!python', '+x', 0, id='shebang with executable'),
pytest.param('#!python', '-x', 0, id='shebang without executable'),
pytest.param('', '+x', 1, id='no shebang with executable'),
pytest.param('', '-x', 0, id='no shebang without executable'),
),
)
def test_git_executable_shebang(temp_git_dir, content, mode, expected):
with temp_git_dir.as_cwd():
path = temp_git_dir.join('path')
path.write(content)
cmd_output('git', 'add', str(path))
cmd_output('chmod', mode, str(path))
cmd_output('git', 'update-index', f'--chmod={mode}', str(path))

# simulate how identify choses that something is executable
filenames = [path for path in [str(path)] if os.access(path, os.X_OK)]

assert main(filenames) == expected