diff --git a/pre_commit_hooks/check_shebang_scripts_are_executable.py b/pre_commit_hooks/check_shebang_scripts_are_executable.py index 621696ce..0436df0c 100644 --- a/pre_commit_hooks/check_shebang_scripts_are_executable.py +++ b/pre_commit_hooks/check_shebang_scripts_are_executable.py @@ -2,6 +2,7 @@ from __future__ import annotations import argparse +import os import shlex import sys from typing import Sequence @@ -9,13 +10,30 @@ from pre_commit_hooks.check_executables_have_shebangs import EXECUTABLE_VALUES from pre_commit_hooks.check_executables_have_shebangs import git_ls_files from pre_commit_hooks.check_executables_have_shebangs import has_shebang +from pre_commit_hooks.util import cmd_output def check_shebangs(paths: list[str]) -> int: - # Cannot optimize on non-executability here if we intend this check to - # work on win32 -- and that's where problems caused by non-executability - # (elsewhere) are most likely to arise from. - return _check_git_filemode(paths) + fs_tracks_executable_bit = cmd_output( + 'git', 'config', 'core.fileMode', retcode=None, + ).strip() + return ( + _check_git_filemode(paths) + if fs_tracks_executable_bit == 'false' + else _check_fs_filemode(paths) + ) + + +def _check_fs_filemode( + paths: list[str], +) -> int: # pragma: win32 no cover + retv = 0 + for path in paths: + if not os.access(path, os.X_OK) and has_shebang(path): + _message(path) + retv = 1 + + return retv def _check_git_filemode(paths: Sequence[str]) -> int: diff --git a/tests/check_shebang_scripts_are_executable_test.py b/tests/check_shebang_scripts_are_executable_test.py index e4bd07ca..9c204fbb 100644 --- a/tests/check_shebang_scripts_are_executable_test.py +++ b/tests/check_shebang_scripts_are_executable_test.py @@ -1,6 +1,6 @@ from __future__ import annotations -import os +import sys import pytest @@ -9,6 +9,62 @@ from pre_commit_hooks.check_shebang_scripts_are_executable import main from pre_commit_hooks.util import cmd_output +skip_win32 = pytest.mark.xfail( + 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', + b'#!/usr/bin/env python3.10', + b'#!python', + '#!☃'.encode(), + ), +) +def test_executable_shebang(content, tmpdir): + path = tmpdir.join('path') + path.write(content, 'wb') + cmd_output('chmod', '+x', path) + assert main((str(path),)) == 0 + + +@skip_win32 # pragma: win32 no cover +@pytest.mark.parametrize( + 'content', ( + b'#!/bin/bash\nhello world\n', + b'#!/usr/bin/env python3.10', + b'#!python', + '#!☃'.encode(), + ), +) +def test_not_executable_shebang(content, tmpdir, capsys): + path = tmpdir.join('path') + path.write(content, 'wb') + assert main((str(path),)) == 1 + _, stderr = capsys.readouterr() + assert stderr.startswith( + f'{path}: has a shebang but is not marked executable!', + ) + + +@skip_win32 # pragma: win32 no cover +@pytest.mark.parametrize( + 'content', ( + b'', + b' #!python\n', + b'\n#!python\n', + b'python\n', + '☃'.encode(), + ), +) +def test_not_executable_no_shebang(content, tmpdir, capsys): + path = tmpdir.join('path') + path.write(content, 'wb') + assert main((str(path),)) == 0 + def test_check_git_filemode_passing(tmpdir): with tmpdir.as_cwd(): @@ -83,7 +139,5 @@ def test_git_executable_shebang(temp_git_dir, content, mode, expected): cmd_output('chmod', mode, str(path)) cmd_output('git', 'update-index', f'--chmod={mode}', str(path)) - # simulate how identify chooses that something is executable - filenames = [path for path in [str(path)] if os.access(path, os.X_OK)] - - assert main(filenames) == expected + files = (str(path),) + assert main(files) == expected