Use shutil.which for Path.which

kiwi/path.py

@@ -19,6 +19,7 @@
 import logging
 import collections
 import pathlib
+import shutil
 from typing import Dict, List, MutableMapping, Optional
 
 # project
@@ -199,17 +200,17 @@ def rebase_to_root(root: str, elements: List[str]) -> List[str]:
 
     @staticmethod
     def which(
-        filename: str, alternative_lookup_paths: Optional[List[str]] = None,
+        filename: str,
         custom_env: Optional[MutableMapping[str, str]] = None,
-        access_mode: Optional[int] = None,
+        access_mode: int = os.F_OK | os.X_OK,
         root_dir: Optional[str] = None
     ) -> Optional[str]:
         """
         Lookup file name in PATH
 
         :param string filename: file base name
         :param list alternative_lookup_paths: list of additional lookup paths
-        :param list custom_env: a custom os.environ
+        :param list custom_env: a custom os.environ used to obtain ``$PATH``
         :param int access_mode: one of the os access modes or a combination of
          them (os.R_OK, os.W_OK and os.X_OK). If the provided access mode
          does not match the file is considered not existing
@@ -219,31 +220,10 @@ def which(
 
         :rtype: str
         """
-        lookup_paths = []
-        multipart_message = [
-            '"%s": ' % filename, 'exists: unknown', 'mode match: not checked'
-        ]
-        system_path = os.environ.get('PATH')
-        if custom_env:
-            system_path = custom_env.get('PATH')
-        if system_path:
-            lookup_paths = system_path.split(os.pathsep)
-        if alternative_lookup_paths:
-            lookup_paths += alternative_lookup_paths
+        system_path = (custom_env.get("PATH") if custom_env else os.environ.get("PATH")) or os.defpath
+
+        lookup_paths = system_path.split(os.pathsep)
         if root_dir:
             lookup_paths = Path.rebase_to_root(root_dir, lookup_paths)
-        multipart_message[0] += 'in paths "%s"' % ':'.join(lookup_paths)
-        for path in lookup_paths:
-            location = os.path.join(path, filename)
-            file_exists = os.path.exists(location)
-            multipart_message[1] = 'exists: "%s"' % file_exists
-            if access_mode and file_exists:
-                mode_match = os.access(location, access_mode)
-                multipart_message[2] = 'mode match: "%s"' % mode_match
-                if mode_match:
-                    return location
-            elif file_exists:
-                return location
-
-        log.debug(' '.join(multipart_message))
-        return None
+        log.debug(f"Looking for {filename} in {os.pathsep.join(lookup_paths)}")
+        return shutil.which(filename, access_mode, path=os.pathsep.join(lookup_paths))

test/unit/command_test.py

@@ -38,7 +38,7 @@ def test_run_failure(self, mock_popen, mock_which):
 
     def test_run_invalid_environment(self):
         with raises(KiwiCommandNotFound):
-            Command.run(['command', 'args'], {'HOME': '/root'})
+            Command.run(['command', 'args'], custom_env={'PATH': '/root'})
 
     @patch('kiwi.path.Path.which')
     @patch('subprocess.Popen')

test/unit/path_test.py

@@ -7,6 +7,8 @@
 
 import os
 
+import pytest
+
 from kiwi.path import Path
 from kiwi.exceptions import KiwiFileAccessError
 
@@ -85,54 +87,36 @@ def test_which(self, mock_exists, mock_env, mock_access):
         assert Path.which('some-file') == '/usr/local/bin/some-file'
         mock_exists.return_value = False
         assert Path.which('some-file') is None
-        mock_env.return_value = None
-        mock_exists.return_value = True
-        assert Path.which('some-file', alternative_lookup_paths=['alternative']) == \
-            'alternative/some-file'
         mock_access.return_value = False
         mock_env.return_value = '/usr/local/bin:/usr/bin:/bin'
         assert Path.which('some-file', access_mode=os.X_OK) is None
-        mock_access.return_value = True
-        assert Path.which('some-file', access_mode=os.X_OK) == \
-            '/usr/local/bin/some-file'
-        assert Path.which('some-file', custom_env={'PATH': 'custom_path'}) == \
-            'custom_path/some-file'
+
+    def test_which_with_real_data(self, pytestconfig: pytest.Config):
         assert Path.which(
-            'some-file',
-            custom_env={'PATH': 'custom_path'},
-            root_dir='/root_dir'
-        ) == '/root_dir/custom_path/some-file'
+            'tox.ini',
+            custom_env={'PATH': str(pytestconfig.rootpath)},
+            access_mode=os.F_OK
+        ) == str(pytestconfig.rootpath / "tox.ini")
+
+        assert Path.which(
+            '__init__.py',
+            custom_env={'PATH': "/kiwi/"},
+            access_mode=os.F_OK,
+            root_dir=str(pytestconfig.rootpath)
+        ) == str(pytestconfig.rootpath / "kiwi" / "__init__.py")
 
     @patch('os.access')
     @patch('os.environ.get')
     @patch('os.path.exists')
     def test_which_not_found_log(
         self, mock_exists, mock_env, mock_access
     ):
-        mock_env.return_value = '/usr/local/bin:/usr/bin:/bin'
+        PATH = '/usr/local/bin:/usr/bin:/bin'
+        mock_env.return_value = PATH
         mock_exists.return_value = False
         with self._caplog.at_level(logging.DEBUG):
             assert Path.which('file') is None
-            assert (
-                '"file": in paths "{0}" exists: "False" mode match: '
-                'not checked'
-            ).format(mock_env.return_value) in self._caplog.text
-
-    @patch('os.access')
-    @patch('os.environ.get')
-    @patch('os.path.exists')
-    def test_which_not_found_for_mode_log(
-        self, mock_exists, mock_env, mock_access
-    ):
-        mock_env.return_value = '/usr/local/bin:/usr/bin:/bin'
-        mock_exists.return_value = True
-        mock_access.return_value = False
-        with self._caplog.at_level(logging.DEBUG):
-            assert Path.which('file', access_mode=os.X_OK) is None
-            assert (
-                '"file": in paths "{0}" exists: "True" mode match: '
-                '"False"'
-            ).format(mock_env.return_value) in self._caplog.text
+            assert ('Looking for file in ' + PATH) in self._caplog.text
 
     def test_access_invalid_mode(self):
         with raises(ValueError) as issue: