From cc5645d899e34936e5c67ddbef90ee56d3065ae3 Mon Sep 17 00:00:00 2001 From: Emily Morehouse Date: Sun, 28 Jan 2018 12:06:02 -0700 Subject: [PATCH 01/14] bpo-32689: Updates shutil.move function to allow for Path objects to be used as source argument by casting to string before performing rstrip. Added documentation on why rstrip needs to be used in the helper _basename method. --- Lib/shutil.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Lib/shutil.py b/Lib/shutil.py index 3c02776a406551..a292b3b255eb01 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -513,6 +513,13 @@ def onerror(*args): def _basename(path): # A basename() variant which first strips the trailing slash, if present. # Thus we always get the last component of the path, even for directories. + # e.g. + # >>> os.path.basename('/bar/foo') + # 'foo' + # >>> os.path.basename('/bar/foo/') + # '' + # >>> _basename('/bar/foo/') + # 'foo' sep = os.path.sep + (os.path.altsep or '') return os.path.basename(path.rstrip(sep)) @@ -550,7 +557,12 @@ def move(src, dst, copy_function=copy2): os.rename(src, dst) return - real_dst = os.path.join(dst, _basename(src)) + # Using _basename instead of os.path.basename is important, as we must + # ignore any trailing slash to avoid the basename returning '' + # Forcing src to a string allows flexibility of objects being passed in, + # including Path objects + real_dst = os.path.join(dst, _basename(str(src))) + if os.path.exists(real_dst): raise Error("Destination path '%s' already exists" % real_dst) try: From 61f1019d963343308f112751f6b96d0ad233c3fd Mon Sep 17 00:00:00 2001 From: Emily Morehouse Date: Tue, 13 Feb 2018 18:02:33 -0700 Subject: [PATCH 02/14] Adding news entry --- .../next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst diff --git a/Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst b/Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst new file mode 100644 index 00000000000000..da95017dc251ba --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst @@ -0,0 +1,2 @@ +Updates shutil.move() function to allow for Path objects to be used as +source argument. Patch by Emily Morehouse. From 3f016ab1a04d78754b4b35cc686080aef252d2bc Mon Sep 17 00:00:00 2001 From: Maxwell McKinnon Date: Sun, 18 Aug 2019 05:43:56 -0700 Subject: [PATCH 03/14] Add test to show breakage for shutil.move where destination is a directory in the form of os.PathLike (pathlib.Path(dir)). --- Lib/shutil.py | 28 +++++++++++++++------------- Lib/test/test_shutil.py | 5 +++++ 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/Lib/shutil.py b/Lib/shutil.py index a292b3b255eb01..e9e38225a58182 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -511,15 +511,20 @@ def onerror(*args): rmtree.avoids_symlink_attacks = _use_fd_functions def _basename(path): - # A basename() variant which first strips the trailing slash, if present. - # Thus we always get the last component of the path, even for directories. - # e.g. - # >>> os.path.basename('/bar/foo') - # 'foo' - # >>> os.path.basename('/bar/foo/') - # '' - # >>> _basename('/bar/foo/') - # 'foo' + """A basename() variant which first strips the trailing slash, if present. + Thus we always get the last component of the path, even for directories. + + path: Union[PathLike, str] + + e.g. + >>> os.path.basename('/bar/foo') + 'foo' + >>> os.path.basename('/bar/foo/') + '' + >>> _basename('/bar/foo/') + 'foo' + """ + path = os.fspath(path) sep = os.path.sep + (os.path.altsep or '') return os.path.basename(path.rstrip(sep)) @@ -547,7 +552,6 @@ def move(src, dst, copy_function=copy2): A lot more could be done here... A look at a mv.c shows a lot of the issues this implementation glosses over. - """ real_dst = dst if os.path.isdir(dst): @@ -559,9 +563,7 @@ def move(src, dst, copy_function=copy2): # Using _basename instead of os.path.basename is important, as we must # ignore any trailing slash to avoid the basename returning '' - # Forcing src to a string allows flexibility of objects being passed in, - # including Path objects - real_dst = os.path.join(dst, _basename(str(src))) + real_dst = os.path.join(dst, _basename(src)) if os.path.exists(real_dst): raise Error("Destination path '%s' already exists" % real_dst) diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index f3cf43e50516c3..9580bbae0efa99 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1561,6 +1561,11 @@ def test_move_file_to_dir(self): # Move a file inside an existing dir on the same filesystem. self._check_move_file(self.src_file, self.dst_dir, self.dst_file) + def test_move_file_to_dir_pathlike(self): + # Move a file to another location on the same filesystem. + src = pathlib.Path(self.src_file) + self._check_move_file(src, self.dst_dir, self.dst_file) + @mock_rename def test_move_file_other_fs(self): # Move a file to an existing dir on another filesystem. From a66757ba4bbcd15c8219cd7af824097177b19619 Mon Sep 17 00:00:00 2001 From: Maxwell McKinnon Date: Sun, 18 Aug 2019 06:46:40 -0700 Subject: [PATCH 04/14] Taking a guess at what the whitespace issue was. --- Lib/shutil.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/shutil.py b/Lib/shutil.py index e9e38225a58182..b2935891a1a813 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -552,6 +552,7 @@ def move(src, dst, copy_function=copy2): A lot more could be done here... A look at a mv.c shows a lot of the issues this implementation glosses over. + """ real_dst = dst if os.path.isdir(dst): From b0716891b685da683aa084bc6b58b69431efd516 Mon Sep 17 00:00:00 2001 From: Maxwell McKinnon Date: Sun, 18 Aug 2019 07:04:32 -0700 Subject: [PATCH 05/14] The results of running reindent.check on the file. Hope this fixes the whitespace gating issue. --- Lib/shutil.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/shutil.py b/Lib/shutil.py index b2935891a1a813..db6896acac50c3 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -513,7 +513,7 @@ def onerror(*args): def _basename(path): """A basename() variant which first strips the trailing slash, if present. Thus we always get the last component of the path, even for directories. - + path: Union[PathLike, str] e.g. @@ -552,7 +552,7 @@ def move(src, dst, copy_function=copy2): A lot more could be done here... A look at a mv.c shows a lot of the issues this implementation glosses over. - + """ real_dst = dst if os.path.isdir(dst): From 0c935fc6d88a93bdc56c8bdcb85a61f4c4f92ad9 Mon Sep 17 00:00:00 2001 From: Maxwell A McKinnon Date: Wed, 28 Aug 2019 02:18:53 -0700 Subject: [PATCH 06/14] remove whitespace Will this fail the linter again? Co-Authored-By: Brandt Bucher --- Lib/shutil.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/shutil.py b/Lib/shutil.py index db6896acac50c3..088900d0f5af12 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -565,7 +565,6 @@ def move(src, dst, copy_function=copy2): # Using _basename instead of os.path.basename is important, as we must # ignore any trailing slash to avoid the basename returning '' real_dst = os.path.join(dst, _basename(src)) - if os.path.exists(real_dst): raise Error("Destination path '%s' already exists" % real_dst) try: From 538034961b9c367c744da4b183ba237dbde6bbb6 Mon Sep 17 00:00:00 2001 From: Maxwell McKinnon Date: Wed, 28 Aug 2019 02:31:11 -0700 Subject: [PATCH 07/14] Update doc .rst files --- Doc/library/shutil.rst | 3 +++ .../next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Doc/library/shutil.rst b/Doc/library/shutil.rst index 1527deb167f1e3..a2a25a28befffa 100644 --- a/Doc/library/shutil.rst +++ b/Doc/library/shutil.rst @@ -314,6 +314,9 @@ Directory and files operations .. versionchanged:: 3.5 Added the *copy_function* keyword argument. + .. versionchanged:: 3.9 + Accepts a :term:`path-like object` for both *src* and *dst*. + .. function:: disk_usage(path) Return disk usage statistics about the given path as a :term:`named tuple` diff --git a/Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst b/Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst index da95017dc251ba..3c601524b114cf 100644 --- a/Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst +++ b/Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst @@ -1,2 +1,2 @@ Updates shutil.move() function to allow for Path objects to be used as -source argument. Patch by Emily Morehouse. +source argument. Patch by Emily Morehouse and Maxwell "5.13b" McKinnon. From d1da9e3dc280b83081136da7e8f331d50cb4ad58 Mon Sep 17 00:00:00 2001 From: Maxwell A McKinnon Date: Thu, 5 Sep 2019 00:27:42 -0700 Subject: [PATCH 08/14] Update shutil.py Reverting the whitespace edit as the tests didn't pass --- Lib/shutil.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/shutil.py b/Lib/shutil.py index 817b7d3eedade6..a37a9b2ff6fd9f 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -783,6 +783,7 @@ def move(src, dst, copy_function=copy2): # Using _basename instead of os.path.basename is important, as we must # ignore any trailing slash to avoid the basename returning '' real_dst = os.path.join(dst, _basename(src)) + if os.path.exists(real_dst): raise Error("Destination path '%s' already exists" % real_dst) try: From 4777b4eb8c6623cd68f75e16f32f6c0dc78d90ab Mon Sep 17 00:00:00 2001 From: Maxwell A McKinnon Date: Thu, 5 Sep 2019 03:04:20 -0700 Subject: [PATCH 09/14] Update shutil.rst Fixed spacing failing doc building test --- Doc/library/shutil.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/library/shutil.rst b/Doc/library/shutil.rst index 5c3e3c4613b090..05251944aba58a 100644 --- a/Doc/library/shutil.rst +++ b/Doc/library/shutil.rst @@ -351,10 +351,10 @@ Directory and files operations Platform-specific fast-copy syscalls may be used internally in order to copy the file more efficiently. See :ref:`shutil-platform-dependent-efficient-copy-operations` section. - + .. versionchanged:: 3.9 Accepts a :term:`path-like object` for both *src* and *dst*. - + .. function:: disk_usage(path) From 54c752f1ebc5f4cc547c036c8f1e882d3a3513ef Mon Sep 17 00:00:00 2001 From: Maxwell McKinnon Date: Wed, 11 Sep 2019 08:29:34 -0700 Subject: [PATCH 10/14] ran python reindent.py -v ~/dev/cpython/Lib/shutil.py to fix whitespace break in CI --- Lib/shutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/shutil.py b/Lib/shutil.py index a37a9b2ff6fd9f..85e4a997e57e60 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -783,7 +783,7 @@ def move(src, dst, copy_function=copy2): # Using _basename instead of os.path.basename is important, as we must # ignore any trailing slash to avoid the basename returning '' real_dst = os.path.join(dst, _basename(src)) - + if os.path.exists(real_dst): raise Error("Destination path '%s' already exists" % real_dst) try: From 84832ebc40f4352eaf7f856ee7dd22a77e4defc4 Mon Sep 17 00:00:00 2001 From: Maxwell A McKinnon Date: Wed, 18 Sep 2019 15:30:51 -0700 Subject: [PATCH 11/14] Update Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst Co-Authored-By: Ammar Askar --- .../next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst b/Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst index 3c601524b114cf..d435351a3a75eb 100644 --- a/Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst +++ b/Misc/NEWS.d/next/Library/2018-02-13-17-58-30.bpo-32689.a-3SnH.rst @@ -1,2 +1,2 @@ -Updates shutil.move() function to allow for Path objects to be used as +Update :func:`shutil.move` function to allow for Path objects to be used as source argument. Patch by Emily Morehouse and Maxwell "5.13b" McKinnon. From 75f5d1177a90763155528fdb7038f84e27953985 Mon Sep 17 00:00:00 2001 From: Maxwell A McKinnon Date: Wed, 18 Sep 2019 15:32:23 -0700 Subject: [PATCH 12/14] Update shutil.rst Remove whitespace line after versionchanged:: 3.9 --- Doc/library/shutil.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/Doc/library/shutil.rst b/Doc/library/shutil.rst index 05251944aba58a..e2e0a13a9189d1 100644 --- a/Doc/library/shutil.rst +++ b/Doc/library/shutil.rst @@ -355,7 +355,6 @@ Directory and files operations .. versionchanged:: 3.9 Accepts a :term:`path-like object` for both *src* and *dst*. - .. function:: disk_usage(path) Return disk usage statistics about the given path as a :term:`named tuple` From 821d5103509088853855a6e6e54feb1e2270c0d5 Mon Sep 17 00:00:00 2001 From: Maxwell A McKinnon Date: Wed, 18 Sep 2019 15:38:01 -0700 Subject: [PATCH 13/14] Update test_shutil.py Added test for destination pathlike case as well (currently working but best to enforce the behavior!) --- Lib/test/test_shutil.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 782a5bf3f66af1..b0bfd4c9c44c71 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1825,10 +1825,15 @@ def test_move_file_to_dir(self): # Move a file inside an existing dir on the same filesystem. self._check_move_file(self.src_file, self.dst_dir, self.dst_file) - def test_move_file_to_dir_pathlike(self): - # Move a file to another location on the same filesystem. + def test_move_file_to_dir_pathlike_src(self): + # Move a pathlike file to another location on the same filesystem. src = pathlib.Path(self.src_file) self._check_move_file(src, self.dst_dir, self.dst_file) + + def test_move_file_to_dir_pathlike_dst(self): + # Move a file to another pathlike location on the same filesystem. + dst = pathlib.Path(self.dst_dir) + self._check_move_file(self.src_file, dst, self.dst_file) @mock_rename def test_move_file_other_fs(self): From 446d97153276f5979165d35e8e26c510cd6be138 Mon Sep 17 00:00:00 2001 From: Maxwell McKinnon Date: Thu, 19 Sep 2019 12:37:19 -0700 Subject: [PATCH 14/14] Fixed another whitespace blocker I introduced via online editing -- squash this later -- ran Tools/scripts/reindent.py -v ~/dev/cpython/Lib/test/test_shutil.py --- 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 b0bfd4c9c44c71..2735c3d7d6c2e2 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1829,7 +1829,7 @@ def test_move_file_to_dir_pathlike_src(self): # Move a pathlike file to another location on the same filesystem. src = pathlib.Path(self.src_file) self._check_move_file(src, self.dst_dir, self.dst_file) - + def test_move_file_to_dir_pathlike_dst(self): # Move a file to another pathlike location on the same filesystem. dst = pathlib.Path(self.dst_dir)