diff --git a/pre_commit_hooks/check_executables_have_shebangs.py b/pre_commit_hooks/check_executables_have_shebangs.py index c34c7b70..1c50ea02 100644 --- a/pre_commit_hooks/check_executables_have_shebangs.py +++ b/pre_commit_hooks/check_executables_have_shebangs.py @@ -2,26 +2,60 @@ 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: @@ -29,12 +63,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: 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__': diff --git a/tests/check_executables_have_shebangs_test.py b/tests/check_executables_have_shebangs_test.py index 15f0c790..3b776128 100644 --- a/tests/check_executables_have_shebangs_test.py +++ b/tests/check_executables_have_shebangs_test.py @@ -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', @@ -17,6 +28,7 @@ def test_has_shebang(content, tmpdir): assert main((path.strpath,)) == 0 +@skip_win32 # pragma: win32 no cover @pytest.mark.parametrize( 'content', ( b'', @@ -24,7 +36,6 @@ def test_has_shebang(content, tmpdir): b'\n#!python\n', b'python\n', '☃'.encode(), - ), ) def test_bad_shebang(content, tmpdir, capsys): @@ -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) + 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