Merge pull request #2677 from taoufik07/fix_cmd_norm_env

Fix command normalization when a custom env is passed

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:]

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:

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 = {
-            env={
+            'HOME': homedir,
-                'HOME': homedir,
+            'PATH': _path_without_us(),
-                'PATH': _path_without_us(),
+            # Git needs this to make a commit
-                # Git needs this to make a commit
+            'GIT_AUTHOR_NAME': os.environ['GIT_AUTHOR_NAME'],
-                'GIT_AUTHOR_NAME': os.environ['GIT_AUTHOR_NAME'],
+            'GIT_COMMITTER_NAME': os.environ['GIT_COMMITTER_NAME'],
-                'GIT_COMMITTER_NAME': os.environ['GIT_COMMITTER_NAME'],
+            'GIT_AUTHOR_EMAIL': os.environ['GIT_AUTHOR_EMAIL'],
-                'GIT_AUTHOR_EMAIL': os.environ['GIT_AUTHOR_EMAIL'],
+            'GIT_COMMITTER_EMAIL': os.environ['GIT_COMMITTER_EMAIL'],
-                'GIT_COMMITTER_EMAIL': os.environ['GIT_COMMITTER_EMAIL'],
+        }
-            },
+        if os.name == 'nt' and 'PATHEXT' in os.environ:  # pragma: no cover
-            check=False,
+            env['PATHEXT'] = os.environ['PATHEXT']
-        )
+
+        ret, out = git_commit(env=env, check=False)
         assert ret == 1
         assert out == (
             '`pre-commit` not found.  '

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

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
+    assert parse_shebang.find_executable('run', env=env_path) is None
-    ret = parse_shebang.find_executable('run.myext', _environ=env_path)
+    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