From 28e7831e4a4ddb747af17dccf7d0e487a484ba23 Mon Sep 17 00:00:00 2001 From: barneygale Date: Thu, 5 Jan 2023 23:28:23 +0000 Subject: [PATCH 01/10] gh-100809: Fix handling of drive-relative paths in pathlib.Path.absolute() If it's available, use `nt._getfullpathname()` to retrieve an absolute path. This allows paths such as 'X:' to be made absolute even when `os.getcwd()` returns a path on another drive. It follows the behaviour of `os.path.abspath()`, except that no path normalisation is performed. --- Lib/pathlib.py | 13 ++++++++++++- Lib/test/test_pathlib.py | 18 ++++++++++++++---- ...3-01-06-21-14-41.gh-issue-100809.I697UT.rst | 3 +++ 3 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-01-06-21-14-41.gh-issue-100809.I697UT.rst diff --git a/Lib/pathlib.py b/Lib/pathlib.py index a0678f61b63211..67d3c3707bdf6e 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -19,6 +19,17 @@ from operator import attrgetter from stat import S_ISDIR, S_ISLNK, S_ISREG, S_ISSOCK, S_ISBLK, S_ISCHR, S_ISFIFO from urllib.parse import quote_from_bytes as urlquote_from_bytes +try: + from nt import _getfullpathname +except ImportError: + def _absolute_parts(path): + return [os.getcwd()] + path._parts +else: + def _absolute_parts(path): + try: + return [_getfullpathname(path)] + except (OSError, ValueError): + return [os.getcwd()] + path._parts __all__ = [ @@ -827,7 +838,7 @@ def absolute(self): """ if self.is_absolute(): return self - return self._from_parts([os.getcwd()] + self._parts) + return self._from_parts(_absolute_parts(self)) def resolve(self, strict=False): """ diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index 7d4d782cf5f075..06a6c9b43182a6 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -1554,8 +1554,7 @@ def test_cwd(self): def test_absolute_common(self): P = self.cls - with mock.patch("os.getcwd") as getcwd: - getcwd.return_value = BASE + with os_helper.change_cwd(BASE): # Simple relative paths. self.assertEqual(str(P().absolute()), BASE) @@ -2993,15 +2992,26 @@ def test_absolute(self): self.assertEqual(str(P(share + 'a\\b').absolute()), share + 'a\\b') # UNC relative paths. - with mock.patch("os.getcwd") as getcwd: - getcwd.return_value = share + def getfullpathname(path): + return os.path.join(share, path) + with mock.patch("nt._getfullpathname", getfullpathname): self.assertEqual(str(P().absolute()), share) self.assertEqual(str(P('.').absolute()), share) self.assertEqual(str(P('a').absolute()), os.path.join(share, 'a')) self.assertEqual(str(P('a', 'b', 'c').absolute()), os.path.join(share, 'a', 'b', 'c')) + drive = os.path.splitdrive(BASE)[0] + with os_helper.change_cwd(BASE): + # Relative path with drive + self.assertEqual(str(P(drive).absolute()), BASE) + self.assertEqual(str(P(drive + 'foo').absolute()), os.path.join(BASE, 'foo')) + + # Relative path with root + self.assertEqual(str(P('\\').absolute()), drive + '\\') + self.assertEqual(str(P('\\foo').absolute()), drive + '\\foo') + def test_glob(self): P = self.cls diff --git a/Misc/NEWS.d/next/Library/2023-01-06-21-14-41.gh-issue-100809.I697UT.rst b/Misc/NEWS.d/next/Library/2023-01-06-21-14-41.gh-issue-100809.I697UT.rst new file mode 100644 index 00000000000000..d9586d7866fb7b --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-01-06-21-14-41.gh-issue-100809.I697UT.rst @@ -0,0 +1,3 @@ +Fix handling of drive-relative paths (like 'C:' and 'C:foo') in +:meth:`pathlib.Path.absolute`. This method now calls +``nt._getfullpathname()`` on Windows. From 4b5d3331692347bf33610be545a8c52f1c3fe369 Mon Sep 17 00:00:00 2001 From: barneygale Date: Fri, 6 Jan 2023 23:12:36 +0000 Subject: [PATCH 02/10] Call _getfullpathname() on the drive (if any), not the full path --- Lib/pathlib.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Lib/pathlib.py b/Lib/pathlib.py index 67d3c3707bdf6e..93b4db3426ffce 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -22,14 +22,7 @@ try: from nt import _getfullpathname except ImportError: - def _absolute_parts(path): - return [os.getcwd()] + path._parts -else: - def _absolute_parts(path): - try: - return [_getfullpathname(path)] - except (OSError, ValueError): - return [os.getcwd()] + path._parts + _getfullpathname = None __all__ = [ @@ -838,7 +831,14 @@ def absolute(self): """ if self.is_absolute(): return self - return self._from_parts(_absolute_parts(self)) + elif self._drv and _getfullpathname: + try: + cwd = _getfullpathname(self._drv) + except (ValueError, OSError): + cwd = os.getcwd() + else: + cwd = os.getcwd() + return self._from_parts([cwd] + self._parts) def resolve(self, strict=False): """ From d2649a67591e3bcafd6823631b23671e06a01b1e Mon Sep 17 00:00:00 2001 From: barneygale Date: Fri, 6 Jan 2023 23:24:17 +0000 Subject: [PATCH 03/10] Undo unnecessary test changes --- Lib/test/test_pathlib.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index 06a6c9b43182a6..4ee65b024ba190 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -1554,7 +1554,8 @@ def test_cwd(self): def test_absolute_common(self): P = self.cls - with os_helper.change_cwd(BASE): + with mock.patch("os.getcwd") as getcwd: + getcwd.return_value = BASE # Simple relative paths. self.assertEqual(str(P().absolute()), BASE) @@ -2992,10 +2993,9 @@ def test_absolute(self): self.assertEqual(str(P(share + 'a\\b').absolute()), share + 'a\\b') # UNC relative paths. - def getfullpathname(path): - return os.path.join(share, path) + with mock.patch("os.getcwd") as getcwd: + getcwd.return_value = share - with mock.patch("nt._getfullpathname", getfullpathname): self.assertEqual(str(P().absolute()), share) self.assertEqual(str(P('.').absolute()), share) self.assertEqual(str(P('a').absolute()), os.path.join(share, 'a')) From 2271a4e4a4f71488f5fc9324bc861b3cd0be5b94 Mon Sep 17 00:00:00 2001 From: barneygale Date: Sat, 7 Jan 2023 13:00:52 +0000 Subject: [PATCH 04/10] Stop handling ValueError and OSError from _getfullpathname() --- Lib/pathlib.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Lib/pathlib.py b/Lib/pathlib.py index 93b4db3426ffce..106123270307d1 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -832,10 +832,8 @@ def absolute(self): if self.is_absolute(): return self elif self._drv and _getfullpathname: - try: - cwd = _getfullpathname(self._drv) - except (ValueError, OSError): - cwd = os.getcwd() + # There is a CWD on each drive-letter drive. + cwd = _getfullpathname(self._drv) else: cwd = os.getcwd() return self._from_parts([cwd] + self._parts) From ad9db67b41ee4fb146954e053b2e07d9df053bc9 Mon Sep 17 00:00:00 2001 From: barneygale Date: Sat, 7 Jan 2023 21:08:29 +0000 Subject: [PATCH 05/10] Add test for drive-specific current directory on non-current drive. Co-authored-by: Eryk Sun --- Lib/test/support/os_helper.py | 35 +++++++++++++++++++++++++++++++++++ Lib/test/test_pathlib.py | 17 +++++++++++++---- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/Lib/test/support/os_helper.py b/Lib/test/support/os_helper.py index 2d4356a1191b1e..4eb079e0867482 100644 --- a/Lib/test/support/os_helper.py +++ b/Lib/test/support/os_helper.py @@ -4,6 +4,7 @@ import os import re import stat +import string import sys import time import unittest @@ -716,3 +717,37 @@ def __exit__(self, *ignore_exc): else: self._environ[k] = v os.environ = self._environ + + +try: + import ctypes + kernel32 = ctypes.WinDLL('kernel32', use_last_error=True) +except (ImportError, AttributeError): + def subst_drive(path): + raise NotImplementedError('ctypes or kernel32 is not available') +else: + ERROR_FILE_NOT_FOUND = 2 + DDD_REMOVE_DEFINITION = 2 + DDD_EXACT_MATCH_ON_REMOVE = 4 + DDD_NO_BROADCAST_SYSTEM = 8 + + @contextlib.contextmanager + def subst_drive(path): + """Temporarily yield a substitute drive for a given path.""" + for c in reversed(string.ascii_uppercase): + drive = f'{c}:' + if (not kernel32.QueryDosDeviceW(drive, None, 0) and + ctypes.get_last_error() == ERROR_FILE_NOT_FOUND): + break + else: + raise FileExistsError('no available logical drive') + if not kernel32.DefineDosDeviceW( + DDD_NO_BROADCAST_SYSTEM, drive, path): + raise ctypes.WinError(ctypes.get_last_error()) + try: + yield drive + finally: + if not kernel32.DefineDosDeviceW( + DDD_REMOVE_DEFINITION | DDD_EXACT_MATCH_ON_REMOVE, + drive, path): + raise ctypes.WinError(ctypes.get_last_error()) diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index 4ee65b024ba190..b67a92e676e8d0 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -3004,14 +3004,23 @@ def test_absolute(self): drive = os.path.splitdrive(BASE)[0] with os_helper.change_cwd(BASE): - # Relative path with drive - self.assertEqual(str(P(drive).absolute()), BASE) - self.assertEqual(str(P(drive + 'foo').absolute()), os.path.join(BASE, 'foo')) - # Relative path with root self.assertEqual(str(P('\\').absolute()), drive + '\\') self.assertEqual(str(P('\\foo').absolute()), drive + '\\foo') + # Relative path on current drive + self.assertEqual(str(P(drive).absolute()), BASE) + self.assertEqual(str(P(drive + 'foo').absolute()), os.path.join(BASE, 'foo')) + + with os_helper.subst_drive(BASE) as other_drive: + other_cwd = f'{other_drive}\\dirA' + with os_helper.change_cwd(other_cwd): + pass + + # Relative path on another drive + self.assertEqual(str(P(other_drive).absolute()), other_cwd) + self.assertEqual(str(P(other_drive + 'foo').absolute()), other_cwd + '\\foo') + def test_glob(self): P = self.cls From 4bb81ee9d7a1c992302229d4d36465aab5acf96a Mon Sep 17 00:00:00 2001 From: Barney Gale Date: Sun, 8 Jan 2023 14:10:04 +0000 Subject: [PATCH 06/10] Apply suggestions from code review Co-authored-by: Eryk Sun --- Lib/test/support/os_helper.py | 4 ++-- Lib/test/test_pathlib.py | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Lib/test/support/os_helper.py b/Lib/test/support/os_helper.py index 4eb079e0867482..f0a53e39329a50 100644 --- a/Lib/test/support/os_helper.py +++ b/Lib/test/support/os_helper.py @@ -724,7 +724,7 @@ def __exit__(self, *ignore_exc): kernel32 = ctypes.WinDLL('kernel32', use_last_error=True) except (ImportError, AttributeError): def subst_drive(path): - raise NotImplementedError('ctypes or kernel32 is not available') + raise unittest.SkipTest('ctypes or kernel32 is not available') else: ERROR_FILE_NOT_FOUND = 2 DDD_REMOVE_DEFINITION = 2 @@ -740,7 +740,7 @@ def subst_drive(path): ctypes.get_last_error() == ERROR_FILE_NOT_FOUND): break else: - raise FileExistsError('no available logical drive') + raise unittest.SkipTest('no available logical drive') if not kernel32.DefineDosDeviceW( DDD_NO_BROADCAST_SYSTEM, drive, path): raise ctypes.WinError(ctypes.get_last_error()) diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index b67a92e676e8d0..dcfa00c3ede390 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -3014,8 +3014,10 @@ def test_absolute(self): with os_helper.subst_drive(BASE) as other_drive: other_cwd = f'{other_drive}\\dirA' - with os_helper.change_cwd(other_cwd): - pass + # set the working directory on the substitute drive + saved_dir = os.getcwd() + os.chdir(other_cwd) + os.chdir(saved_dir) # Relative path on another drive self.assertEqual(str(P(other_drive).absolute()), other_cwd) From 5dd9af2a830f6a526d2f14b66e9ceb4113a39517 Mon Sep 17 00:00:00 2001 From: barneygale Date: Sun, 8 Jan 2023 14:11:21 +0000 Subject: [PATCH 07/10] Move DOS device constants into `try:` block --- Lib/test/support/os_helper.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/test/support/os_helper.py b/Lib/test/support/os_helper.py index f0a53e39329a50..821a4b1ffd5077 100644 --- a/Lib/test/support/os_helper.py +++ b/Lib/test/support/os_helper.py @@ -722,15 +722,15 @@ def __exit__(self, *ignore_exc): try: import ctypes kernel32 = ctypes.WinDLL('kernel32', use_last_error=True) -except (ImportError, AttributeError): - def subst_drive(path): - raise unittest.SkipTest('ctypes or kernel32 is not available') -else: + ERROR_FILE_NOT_FOUND = 2 DDD_REMOVE_DEFINITION = 2 DDD_EXACT_MATCH_ON_REMOVE = 4 DDD_NO_BROADCAST_SYSTEM = 8 - +except (ImportError, AttributeError): + def subst_drive(path): + raise unittest.SkipTest('ctypes or kernel32 is not available') +else: @contextlib.contextmanager def subst_drive(path): """Temporarily yield a substitute drive for a given path.""" From b19e6a072007f9e2011e30770a02e840a741a0f4 Mon Sep 17 00:00:00 2001 From: barneygale Date: Sun, 8 Jan 2023 14:32:22 +0000 Subject: [PATCH 08/10] Tidy up test code slightly --- Lib/test/test_pathlib.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index dcfa00c3ede390..80e2befb51588b 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -3013,17 +3013,16 @@ def test_absolute(self): self.assertEqual(str(P(drive + 'foo').absolute()), os.path.join(BASE, 'foo')) with os_helper.subst_drive(BASE) as other_drive: + # Set the working directory on the substitute drive + saved_cwd = os.getcwd() other_cwd = f'{other_drive}\\dirA' - # set the working directory on the substitute drive - saved_dir = os.getcwd() os.chdir(other_cwd) - os.chdir(saved_dir) + os.chdir(saved_cwd) # Relative path on another drive self.assertEqual(str(P(other_drive).absolute()), other_cwd) self.assertEqual(str(P(other_drive + 'foo').absolute()), other_cwd + '\\foo') - def test_glob(self): P = self.cls p = P(BASE) From 5e1d4aa982c98c5d3e2a5226bdc3d4ee0ac42483 Mon Sep 17 00:00:00 2001 From: barneygale Date: Thu, 16 Feb 2023 18:55:22 +0000 Subject: [PATCH 09/10] Simplify implementation --- Lib/pathlib.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Lib/pathlib.py b/Lib/pathlib.py index 106123270307d1..f70fb4afdebf09 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -19,10 +19,6 @@ from operator import attrgetter from stat import S_ISDIR, S_ISLNK, S_ISREG, S_ISSOCK, S_ISBLK, S_ISCHR, S_ISFIFO from urllib.parse import quote_from_bytes as urlquote_from_bytes -try: - from nt import _getfullpathname -except ImportError: - _getfullpathname = None __all__ = [ @@ -831,9 +827,9 @@ def absolute(self): """ if self.is_absolute(): return self - elif self._drv and _getfullpathname: + elif self._drv: # There is a CWD on each drive-letter drive. - cwd = _getfullpathname(self._drv) + cwd = self._flavour.abspath(self._drv) else: cwd = os.getcwd() return self._from_parts([cwd] + self._parts) From 5f1cf01a0f82edb324c28383da0ccad76f25c851 Mon Sep 17 00:00:00 2001 From: Barney Gale Date: Fri, 17 Feb 2023 12:52:23 +0000 Subject: [PATCH 10/10] Update Misc/NEWS.d/next/Library/2023-01-06-21-14-41.gh-issue-100809.I697UT.rst Co-authored-by: Steve Dower --- .../Library/2023-01-06-21-14-41.gh-issue-100809.I697UT.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2023-01-06-21-14-41.gh-issue-100809.I697UT.rst b/Misc/NEWS.d/next/Library/2023-01-06-21-14-41.gh-issue-100809.I697UT.rst index d9586d7866fb7b..54082de88ccf4a 100644 --- a/Misc/NEWS.d/next/Library/2023-01-06-21-14-41.gh-issue-100809.I697UT.rst +++ b/Misc/NEWS.d/next/Library/2023-01-06-21-14-41.gh-issue-100809.I697UT.rst @@ -1,3 +1,3 @@ Fix handling of drive-relative paths (like 'C:' and 'C:foo') in -:meth:`pathlib.Path.absolute`. This method now calls -``nt._getfullpathname()`` on Windows. +:meth:`pathlib.Path.absolute`. This method now uses the OS API +to retrieve the correct current working directory for the drive.