From c41d4241b9535977133fdf899fc31d312bc2f5f4 Mon Sep 17 00:00:00 2001 From: Cheryl Sabella Date: Mon, 11 Feb 2019 09:28:35 -0500 Subject: [PATCH 1/4] bpo-18283: Add support for bytes to shutil.which --- Doc/library/shutil.rst | 3 ++ Lib/shutil.py | 28 +++++++++++++------ Lib/test/test_shutil.py | 21 +++++++++++--- .../2019-02-11-09-24-08.bpo-18283.BT3Jhc.rst | 1 + 4 files changed, 40 insertions(+), 13 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-02-11-09-24-08.bpo-18283.BT3Jhc.rst diff --git a/Doc/library/shutil.rst b/Doc/library/shutil.rst index 427a120159633d4..79d6bd4a06c8065 100644 --- a/Doc/library/shutil.rst +++ b/Doc/library/shutil.rst @@ -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 diff --git a/Lib/shutil.py b/Lib/shutil.py index 8d0de72b44a3a42..f8200f93e71f873 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -1279,6 +1279,13 @@ 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): + 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 @@ -1289,13 +1296,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 @@ -1308,15 +1308,25 @@ def _access_check(fn, mode): path = os.environ.get("PATH", os.defpath) if not path: return None - path = path.split(os.pathsep) + if isinstance(cmd, 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) + if isinstance(cmd, bytes): + path.insert(0, os.fsencode(os.curdir)) + else: + path.insert(0, os.curdir) # PATHEXT is necessary to check on Windows. pathext = os.environ.get("PATHEXT", "").split(os.pathsep) + if isinstance(cmd, bytes): + pathext = list(map(os.fsencode, 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 diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 6f22e5378ff22cb..fcc30c51919574b 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -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. @@ -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) @@ -1581,11 +1584,11 @@ 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) @@ -1593,7 +1596,7 @@ 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) @@ -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 = b".EXE" + + class TestMove(unittest.TestCase): def setUp(self): diff --git a/Misc/NEWS.d/next/Library/2019-02-11-09-24-08.bpo-18283.BT3Jhc.rst b/Misc/NEWS.d/next/Library/2019-02-11-09-24-08.bpo-18283.BT3Jhc.rst new file mode 100644 index 000000000000000..85704a37d3074bd --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-02-11-09-24-08.bpo-18283.BT3Jhc.rst @@ -0,0 +1 @@ +Add support for bytes to :func:`shutil.which`. From ecc185cb638684df6de4efc53de3fcc5d64edfdd Mon Sep 17 00:00:00 2001 From: Cheryl Sabella Date: Mon, 11 Feb 2019 12:00:29 -0500 Subject: [PATCH 2/4] Fix comparison between bytes and string for os.curdir --- Lib/shutil.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/shutil.py b/Lib/shutil.py index f8200f93e71f873..3f4840fd24e1768 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -1317,11 +1317,11 @@ def which(cmd, mode=os.F_OK | os.X_OK, path=None): if sys.platform == "win32": # The current directory takes precedence on Windows. - if not os.curdir in path: - if isinstance(cmd, bytes): - path.insert(0, os.fsencode(os.curdir)) - else: - path.insert(0, os.curdir) + curdir = os.curdir + if isinstance(cmd, 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) From 8bf7afe740b95e2345ab831b6cfb5669ef876b8f Mon Sep 17 00:00:00 2001 From: Cheryl Sabella Date: Tue, 12 Feb 2019 09:13:36 -0500 Subject: [PATCH 3/4] Cleanup per code review --- Lib/shutil.py | 12 ++++++++---- Lib/test/test_shutil.py | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Lib/shutil.py b/Lib/shutil.py index 3f4840fd24e1768..065e08bc5c343e8 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -1279,6 +1279,7 @@ 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. @@ -1286,6 +1287,7 @@ def _access_check(fn, mode): 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 @@ -1304,11 +1306,13 @@ def which(cmd, mode=os.F_OK | os.X_OK, path=None): 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 - if isinstance(cmd, bytes): + if use_bytes: path = os.fsencode(path) path = path.split(os.fsencode(os.pathsep)) else: @@ -1318,15 +1322,15 @@ def which(cmd, mode=os.F_OK | os.X_OK, path=None): if sys.platform == "win32": # The current directory takes precedence on Windows. curdir = os.curdir - if isinstance(cmd, bytes): + 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 isinstance(cmd, bytes): - pathext = list(map(os.fsencode, pathext)) + 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 diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index fcc30c51919574b..b8991abf9016b79 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1614,7 +1614,7 @@ def setUp(self): self.file = os.fsencode(self.file) self.temp_file.name = os.fsencode(self.temp_file.name) self.curdir = os.fsencode(self.curdir) - self.ext = b".EXE" + self.ext = os.fsencode(".EXE") class TestMove(unittest.TestCase): From 829581b638491d4a61e25cfdda05d8ba3e26b513 Mon Sep 17 00:00:00 2001 From: Cheryl Sabella Date: Tue, 12 Feb 2019 14:43:52 -0500 Subject: [PATCH 4/4] Fix encoding call for test --- Lib/test/test_shutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index b8991abf9016b79..e3a0e702eee3ca4 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1614,7 +1614,7 @@ def setUp(self): 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(".EXE") + self.ext = os.fsencode(self.ext) class TestMove(unittest.TestCase):