From bf1a1fa5fd6de3633b033847964b51a60ffbd0d5 Mon Sep 17 00:00:00 2001 From: taoufik07 Date: Thu, 5 Jan 2023 13:31:28 +0100 Subject: [PATCH] Fix command normalization when a custom env is passed --- pre_commit/parse_shebang.py | 18 +++++++++------ pre_commit/util.py | 2 +- tests/commands/install_uninstall_test.py | 29 ++++++++++++------------ tests/languages/rust_test.py | 7 ++++-- tests/parse_shebang_test.py | 6 ++--- 5 files changed, 35 insertions(+), 27 deletions(-) diff --git a/pre_commit/parse_shebang.py b/pre_commit/parse_shebang.py index 3ac933c0..3ee04e8d 100644 --- a/pre_commit/parse_shebang.py +++ b/pre_commit/parse_shebang.py @@ -20,13 +20,13 @@ def parse_filename(filename: str) -> tuple[str, ...]: def find_executable( - exe: str, _environ: Mapping[str, str] | None = None, + exe: str, *, env: Mapping[str, str] | None = None, ) -> str | None: exe = os.path.normpath(exe) if os.sep in exe: return exe - environ = _environ if _environ is not None else os.environ + environ = env if env is not None else os.environ if 'PATHEXT' in environ: exts = environ['PATHEXT'].split(os.pathsep) @@ -43,12 +43,12 @@ def find_executable( return None -def normexe(orig: str) -> str: +def normexe(orig: str, *, env: Mapping[str, str] | None = None) -> str: def _error(msg: str) -> NoReturn: raise ExecutableNotFoundError(f'Executable `{orig}` {msg}') if os.sep not in orig and (not os.altsep or os.altsep not in orig): - exe = find_executable(orig) + exe = find_executable(orig, env=env) if exe is None: _error('not found') return exe @@ -62,7 +62,11 @@ def normexe(orig: str) -> str: return orig -def normalize_cmd(cmd: tuple[str, ...]) -> tuple[str, ...]: +def normalize_cmd( + cmd: tuple[str, ...], + *, + env: Mapping[str, str] | None = None, +) -> tuple[str, ...]: """Fixes for the following issues on windows - https://bugs.python.org/issue8557 - windows does not parse shebangs @@ -70,12 +74,12 @@ def normalize_cmd(cmd: tuple[str, ...]) -> tuple[str, ...]: This function also makes deep-path shebangs work just fine """ # Use PATH to determine the executable - exe = normexe(cmd[0]) + exe = normexe(cmd[0], env=env) # Figure out the shebang from the resulting command cmd = parse_filename(exe) + (exe,) + cmd[1:] # This could have given us back another bare executable - exe = normexe(cmd[0]) + exe = normexe(cmd[0], env=env) return (exe,) + cmd[1:] diff --git a/pre_commit/util.py b/pre_commit/util.py index d51fd32d..8ea48446 100644 --- a/pre_commit/util.py +++ b/pre_commit/util.py @@ -99,7 +99,7 @@ def cmd_output_b( _setdefault_kwargs(kwargs) try: - cmd = parse_shebang.normalize_cmd(cmd) + cmd = parse_shebang.normalize_cmd(cmd, env=kwargs.get('env')) except parse_shebang.ExecutableNotFoundError as e: returncode, stdout_b, stderr_b = e.to_output() else: diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index e3943773..a1ecda86 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -248,7 +248,7 @@ def test_install_idempotent(tempdir_factory, store): def _path_without_us(): # Choose a path which *probably* doesn't include us env = dict(os.environ) - exe = find_executable('pre-commit', _environ=env) + exe = find_executable('pre-commit', env=env) while exe: parts = env['PATH'].split(os.pathsep) after = [ @@ -258,7 +258,7 @@ def _path_without_us(): if parts == after: raise AssertionError(exe, parts) env['PATH'] = os.pathsep.join(after) - exe = find_executable('pre-commit', _environ=env) + exe = find_executable('pre-commit', env=env) return env['PATH'] @@ -276,18 +276,19 @@ def test_environment_not_sourced(tempdir_factory, store): # Use a specific homedir to ignore --user installs homedir = tempdir_factory.get() - ret, out = git_commit( - env={ - 'HOME': homedir, - 'PATH': _path_without_us(), - # Git needs this to make a commit - 'GIT_AUTHOR_NAME': os.environ['GIT_AUTHOR_NAME'], - 'GIT_COMMITTER_NAME': os.environ['GIT_COMMITTER_NAME'], - 'GIT_AUTHOR_EMAIL': os.environ['GIT_AUTHOR_EMAIL'], - 'GIT_COMMITTER_EMAIL': os.environ['GIT_COMMITTER_EMAIL'], - }, - check=False, - ) + env = { + 'HOME': homedir, + 'PATH': _path_without_us(), + # Git needs this to make a commit + 'GIT_AUTHOR_NAME': os.environ['GIT_AUTHOR_NAME'], + 'GIT_COMMITTER_NAME': os.environ['GIT_COMMITTER_NAME'], + 'GIT_AUTHOR_EMAIL': os.environ['GIT_AUTHOR_EMAIL'], + 'GIT_COMMITTER_EMAIL': os.environ['GIT_COMMITTER_EMAIL'], + } + if os.name == 'nt' and 'PATHEXT' in os.environ: # pragma: no cover + env['PATHEXT'] = os.environ['PATHEXT'] + + ret, out = git_commit(env=env, check=False) assert ret == 1 assert out == ( '`pre-commit` not found. ' diff --git a/tests/languages/rust_test.py b/tests/languages/rust_test.py index f011e719..b8167a9e 100644 --- a/tests/languages/rust_test.py +++ b/tests/languages/rust_test.py @@ -1,5 +1,6 @@ from __future__ import annotations +from typing import Mapping from unittest import mock import pytest @@ -48,7 +49,9 @@ def test_installs_with_bootstrapped_rustup(tmpdir, language_version): original_find_executable = parse_shebang.find_executable - def mocked_find_executable(exe: str) -> str | None: + def mocked_find_executable( + exe: str, *, env: Mapping[str, str] | None = None, + ) -> str | None: """ Return `None` the first time `find_executable` is called to ensure that the bootstrapping code is executed, then just let the function @@ -59,7 +62,7 @@ def test_installs_with_bootstrapped_rustup(tmpdir, language_version): find_executable_exes.append(exe) if len(find_executable_exes) == 1: return None - return original_find_executable(exe) + return original_find_executable(exe, env=env) with mock.patch.object(parse_shebang, 'find_executable') as find_exe_mck: find_exe_mck.side_effect = mocked_find_executable diff --git a/tests/parse_shebang_test.py b/tests/parse_shebang_test.py index d7acbf57..2fcb29ee 100644 --- a/tests/parse_shebang_test.py +++ b/tests/parse_shebang_test.py @@ -75,10 +75,10 @@ def test_find_executable_path_ext(in_tmpdir): env_path = {'PATH': os.path.dirname(exe_path)} env_path_ext = dict(env_path, PATHEXT=os.pathsep.join(('.exe', '.myext'))) assert parse_shebang.find_executable('run') is None - assert parse_shebang.find_executable('run', _environ=env_path) is None - ret = parse_shebang.find_executable('run.myext', _environ=env_path) + assert parse_shebang.find_executable('run', env=env_path) is None + ret = parse_shebang.find_executable('run.myext', env=env_path) assert ret == exe_path - ret = parse_shebang.find_executable('run', _environ=env_path_ext) + ret = parse_shebang.find_executable('run', env=env_path_ext) assert ret == exe_path