Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Doc/library/shutil.rst
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,9 @@ Directory and files operations

.. versionadded:: 3.3

.. versionchanged:: 3.8
The :class:`bytes` type is now accepted. If *cmd* type is
:class:`bytes`, the result type is also :class:`bytes`.

.. exception:: Error

Expand Down
34 changes: 24 additions & 10 deletions Lib/shutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -1279,6 +1279,15 @@ def get_terminal_size(fallback=(80, 24)):

return os.terminal_size((columns, lines))


# Check that a given file can be accessed with the correct mode.
# Additionally check that `file` is not a directory, as on Windows
# directories pass the os.access check.
def _access_check(fn, mode):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious any reason over moving this outside which?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an author of the original patch, I can explain: it's inefficient to define a temporary function for each which() call. Moreover, there is no technical reason to use a nested function.

return (os.path.exists(fn) and os.access(fn, mode)
and not os.path.isdir(fn))


def which(cmd, mode=os.F_OK | os.X_OK, path=None):
"""Given a command, mode, and a PATH string, return the path which
conforms to the given mode on the PATH, or None if there is no such
Expand All @@ -1289,13 +1298,6 @@ def which(cmd, mode=os.F_OK | os.X_OK, path=None):
path.

"""
# Check that a given file can be accessed with the correct mode.
# Additionally check that `file` is not a directory, as on Windows
# directories pass the os.access check.
def _access_check(fn, mode):
return (os.path.exists(fn) and os.access(fn, mode)
and not os.path.isdir(fn))

# If we're given a path with a directory part, look it up directly rather
# than referring to PATH directories. This includes checking relative to the
# current directory, e.g. ./script
Expand All @@ -1304,19 +1306,31 @@ def _access_check(fn, mode):
return cmd
return None

use_bytes = isinstance(cmd, bytes)

if path is None:
path = os.environ.get("PATH", os.defpath)
if not path:
return None
path = path.split(os.pathsep)
if use_bytes:
path = os.fsencode(path)
path = path.split(os.fsencode(os.pathsep))
else:
path = os.fsdecode(path)
path = path.split(os.pathsep)

if sys.platform == "win32":
# The current directory takes precedence on Windows.
if not os.curdir in path:
path.insert(0, os.curdir)
curdir = os.curdir
if use_bytes:
curdir = os.fsencode(curdir)
if curdir not in path:
path.insert(0, curdir)

# PATHEXT is necessary to check on Windows.
pathext = os.environ.get("PATHEXT", "").split(os.pathsep)
if use_bytes:
pathext = [os.fsencode(ext) for ext in pathext]
# See if the given file matches any of the expected path extensions.
# This will allow us to short circuit when given "python.exe".
# If it does match, only test that one, otherwise we have to try
Expand Down
21 changes: 17 additions & 4 deletions Lib/test/test_shutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -1517,6 +1517,9 @@ def setUp(self):
os.chmod(self.temp_file.name, stat.S_IXUSR)
self.addCleanup(self.temp_file.close)
self.dir, self.file = os.path.split(self.temp_file.name)
self.env_path = self.dir
self.curdir = os.curdir
self.ext = ".EXE"

def test_basic(self):
# Given an EXE in a directory, it should be returned.
Expand Down Expand Up @@ -1549,7 +1552,7 @@ def test_cwd(self):
rv = shutil.which(self.file, path=base_dir)
if sys.platform == "win32":
# Windows: current directory implicitly on PATH
self.assertEqual(rv, os.path.join(os.curdir, self.file))
self.assertEqual(rv, os.path.join(self.curdir, self.file))
else:
# Other platforms: shouldn't match in the current directory.
self.assertIsNone(rv)
Expand Down Expand Up @@ -1581,19 +1584,19 @@ def test_pathext_checking(self):
# Ask for the file without the ".exe" extension, then ensure that
# it gets found properly with the extension.
rv = shutil.which(self.file[:-4], path=self.dir)
self.assertEqual(rv, self.temp_file.name[:-4] + ".EXE")
self.assertEqual(rv, self.temp_file.name[:-4] + self.ext)

def test_environ_path(self):
with support.EnvironmentVarGuard() as env:
env['PATH'] = self.dir
env['PATH'] = self.env_path
rv = shutil.which(self.file)
self.assertEqual(rv, self.temp_file.name)

def test_empty_path(self):
base_dir = os.path.dirname(self.dir)
with support.change_cwd(path=self.dir), \
support.EnvironmentVarGuard() as env:
env['PATH'] = self.dir
env['PATH'] = self.env_path
rv = shutil.which(self.file, path='')
self.assertIsNone(rv)

Expand All @@ -1604,6 +1607,16 @@ def test_empty_path_no_PATH(self):
self.assertIsNone(rv)


class TestWhichBytes(TestWhich):
def setUp(self):
TestWhich.setUp(self)
self.dir = os.fsencode(self.dir)
self.file = os.fsencode(self.file)
self.temp_file.name = os.fsencode(self.temp_file.name)
self.curdir = os.fsencode(self.curdir)
self.ext = os.fsencode(self.ext)


class TestMove(unittest.TestCase):

def setUp(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for bytes to :func:`shutil.which`.