From 314fa5366d88305a4b23b10a97c56d9247e1f806 Mon Sep 17 00:00:00 2001 From: Kurt von Laven Date: Sun, 10 Apr 2022 12:52:36 -0700 Subject: [PATCH] Support checking executable bit without Git. The check-shebang-scripts-are-executable hook already avoided false negatives in a Git repository by looking up the Git file mode rather than relying on the file mode in the file system. Git already automatically probes the file system for executable bit support. Use the file mode in the file system when we are not in a Git clone or it is trusted by Git according to its core.fileMode config variable. --- .../check_shebang_scripts_are_executable.py | 26 +++++++-- ...eck_shebang_scripts_are_executable_test.py | 58 +++++++++++++++++++ 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/pre_commit_hooks/check_shebang_scripts_are_executable.py b/pre_commit_hooks/check_shebang_scripts_are_executable.py index 621696c..0436df0 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 typing import Sequence 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 7c2696d..9c204fb 100644 --- a/tests/check_shebang_scripts_are_executable_test.py +++ b/tests/check_shebang_scripts_are_executable_test.py @@ -1,5 +1,7 @@ from __future__ import annotations +import sys + import pytest from pre_commit_hooks.check_shebang_scripts_are_executable import \ @@ -7,6 +9,62 @@ from pre_commit_hooks.check_shebang_scripts_are_executable import \ 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():