diff --git a/pre_commit_hooks/check_shebang_scripts_are_executable.py b/pre_commit_hooks/check_shebang_scripts_are_executable.py index 937425b0..07c43fad 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 collections.abc import Sequence @@ -9,13 +10,34 @@ 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 CalledProcessError +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) + # Use the index when available: its executable bits are authoritative for + # staged files, especially on platforms that cannot track file modes. + try: + return _check_git_filemode(paths) + except CalledProcessError: + # Do not hide an unrelated Git failure in a working tree. + if cmd_output( + 'git', 'rev-parse', '--is-inside-work-tree', retcode=None, + ).strip() == 'true': + raise + if sys.platform == 'win32': + # Windows cannot reliably determine Unix executable bits. + raise + return _check_fs_filemode(paths) + + +def _check_fs_filemode(paths: Sequence[str]) -> int: + found = False + for path in paths: + if not os.access(path, os.X_OK) and has_shebang(path): + _message(path) + found = True + return int(found) 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..a3bc24d9 100644 --- a/tests/check_shebang_scripts_are_executable_test.py +++ b/tests/check_shebang_scripts_are_executable_test.py @@ -1,9 +1,12 @@ from __future__ import annotations import os +import sys +from types import SimpleNamespace import pytest +import pre_commit_hooks.check_shebang_scripts_are_executable as shebang_hook from pre_commit_hooks.check_shebang_scripts_are_executable import \ _check_git_filemode from pre_commit_hooks.check_shebang_scripts_are_executable import main @@ -87,3 +90,51 @@ def test_git_executable_shebang(temp_git_dir, content, mode, expected): filenames = [path for path in [str(path)] if os.access(path, os.X_OK)] assert main(filenames) == expected + + +@pytest.mark.parametrize( + ('content', 'executable', 'expected'), + ( + ('#!/bin/sh\n', False, 1), + ('#!/bin/sh\n', True, 0), + ('echo hello\n', False, 0), + ), +) +def test_without_git_repository(tmp_path, monkeypatch, content, executable, expected): + script = tmp_path / 'example.sh' + script.write_text(content) + monkeypatch.chdir(tmp_path) + monkeypatch.setattr( + shebang_hook, 'os', + SimpleNamespace(access=lambda path, mode: executable, X_OK=os.X_OK), + ) + monkeypatch.setattr( + shebang_hook, 'sys', SimpleNamespace(platform='linux', stderr=sys.stderr), + ) + + assert main([str(script)]) == expected + + +def test_without_git_repository_on_windows_raises(tmp_path, monkeypatch): + script = tmp_path / 'example.sh' + script.write_text('#!/bin/sh\n') + monkeypatch.chdir(tmp_path) + monkeypatch.setattr( + shebang_hook, 'sys', SimpleNamespace(platform='win32', stderr=sys.stderr), + ) + + with pytest.raises(shebang_hook.CalledProcessError): + main([str(script)]) + + +def test_git_failure_inside_repository_is_not_hidden(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + cmd_output('git', 'init', '.') + + def broken_ls_files(paths): + raise shebang_hook.CalledProcessError('simulated git error') + + monkeypatch.setattr(shebang_hook, 'git_ls_files', broken_ls_files) + + with pytest.raises(shebang_hook.CalledProcessError, match='simulated'): + main([])