From b719e1809c2c81283e930086faebd7d6050cd5d7 Mon Sep 17 00:00:00 2001 From: David Briscoe Date: Wed, 12 Jan 2022 23:39:10 -0800 Subject: [PATCH] Use bash to open extensionless hooks on windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #971. Partly resolve #703. If the hook doesn't have a file extension, then Windows won't know how to run it and you'll get "[WinError 193] %1 is not a valid Win32 application". It's very likely that it's a shell script of some kind, so use bash.exe (commonly installed via Windows Subsystem for Linux). We don't want to run all hooks with bash because they could be .bat files. Update tests to get several hook ones working. More work necessary to get commit-msg hook working. The hook writes to the wrong file because it's not using forward slashes in the path: C:\Users\idbrii\AppData\Local\Temp\bare_test_commit_msg_hook_successy5fo00du\CUsersidbriiAppDataLocalTempbare_test_commit_msg_hook_successy5fo00duCOMMIT_EDITMSG --- git/index/fun.py | 15 ++++++++++++++- test/test_index.py | 9 ++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/git/index/fun.py b/git/index/fun.py index 16ec744e2..59fa1be19 100644 --- a/git/index/fun.py +++ b/git/index/fun.py @@ -3,6 +3,7 @@ # NOTE: Autodoc hates it if this is a docstring from io import BytesIO +from pathlib import Path import os from stat import ( S_IFDIR, @@ -21,6 +22,7 @@ force_text, force_bytes, is_posix, + is_win, safe_decode, ) from git.exc import ( @@ -76,6 +78,10 @@ def hook_path(name: str, git_dir: PathLike) -> str: return osp.join(git_dir, 'hooks', name) +def _has_file_extension(path): + return osp.splitext(path)[1] + + def run_commit_hook(name: str, index: 'IndexFile', *args: str) -> None: """Run the commit hook of the given name. Silently ignores hooks that do not exist. :param name: name of hook, like 'pre-commit' @@ -89,8 +95,15 @@ def run_commit_hook(name: str, index: 'IndexFile', *args: str) -> None: env = os.environ.copy() env['GIT_INDEX_FILE'] = safe_decode(str(index.path)) env['GIT_EDITOR'] = ':' + cmd = [hp] try: - cmd = subprocess.Popen([hp] + list(args), + if is_win and not _has_file_extension(hp): + # Windows only uses extensions to determine how to open files + # (doesn't understand shebangs). Try using bash to run the hook. + relative_hp = Path(hp).relative_to(index.repo.working_dir).as_posix() + cmd = ["bash.exe", relative_hp] + + cmd = subprocess.Popen(cmd + list(args), env=env, stdout=subprocess.PIPE, stderr=subprocess.PIPE, diff --git a/test/test_index.py b/test/test_index.py index 02cb4e813..233a4c643 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -13,6 +13,7 @@ ) import tempfile from unittest import skipIf +import shutil from git import ( IndexFile, @@ -52,8 +53,9 @@ HOOKS_SHEBANG = "#!/usr/bin/env sh\n" +is_win_without_bash = is_win and not shutil.which('bash.exe') + -@skipIf(HIDE_WINDOWS_KNOWN_ERRORS, "TODO: fix hooks execution on Windows: #703") def _make_hook(git_dir, name, content, make_exec=True): """A helper to create a hook""" hp = hook_path(name, git_dir) @@ -881,7 +883,7 @@ def test_pre_commit_hook_fail(self, rw_repo): try: index.commit("This should fail") except HookExecutionError as err: - if is_win: + if is_win_without_bash: self.assertIsInstance(err.status, OSError) self.assertEqual(err.command, [hp]) self.assertEqual(err.stdout, '') @@ -896,6 +898,7 @@ def test_pre_commit_hook_fail(self, rw_repo): else: raise AssertionError("Should have caught a HookExecutionError") + @skipIf(HIDE_WINDOWS_KNOWN_ERRORS, "TODO: fix hooks execution on Windows: #703") @with_rw_repo('HEAD', bare=True) def test_commit_msg_hook_success(self, rw_repo): commit_message = "commit default head by Frèderic Çaufl€" @@ -920,7 +923,7 @@ def test_commit_msg_hook_fail(self, rw_repo): try: index.commit("This should fail") except HookExecutionError as err: - if is_win: + if is_win_without_bash: self.assertIsInstance(err.status, OSError) self.assertEqual(err.command, [hp]) self.assertEqual(err.stdout, '')