Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-73991: Add pathlib.Path.move() #122073

Merged
merged 27 commits into from
Aug 25, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f6d09c0
GH-73991: Add `pathlib.Path.move()`
barneygale Jul 21, 2024
1dc0bfb
Simplify slightly
barneygale Jul 21, 2024
f488ff8
Test existing files/directories as destinations.
barneygale Jul 21, 2024
ff7746f
rename --> replace
barneygale Jul 21, 2024
29e51f5
Windows test fixes
barneygale Jul 21, 2024
36955ab
Revert "Windows test fixes"
barneygale Jul 21, 2024
d595047
Handle existing targets more consistently
barneygale Jul 21, 2024
5de963e
Use generic implementation on ERROR_ACCESS_DENIED.
barneygale Jul 21, 2024
e1c0d9e
Expand test coverage, lean into the Windows differences a bit.
barneygale Jul 22, 2024
a98aed4
Loosen tests for OSError types.
barneygale Jul 22, 2024
0af6396
Revert "Loosen tests for OSError types."
barneygale Jul 22, 2024
348cabd
Tweaks
barneygale Jul 22, 2024
aca70d1
Relax expectations once again.
barneygale Jul 22, 2024
aa27f48
skip tests affected by apparent os.replace() inconsistency
barneygale Jul 22, 2024
04b115a
More compatibility experiments.
barneygale Jul 22, 2024
be48d2a
Cunningly avoid specifying the problematic cases.
barneygale Jul 22, 2024
a5ee60a
Fix moving a file/directory to itself.
barneygale Jul 22, 2024
f9219ee
Undo unnecessary change
barneygale Jul 22, 2024
5ad2c1d
Ensure we exit from copytree() when copying directory over itself.
barneygale Jul 27, 2024
46b1fc2
Merge branch 'main' into path-move
barneygale Aug 7, 2024
d889dff
Merge branch 'main' into path-move
barneygale Aug 11, 2024
9d92108
Docs tweak
barneygale Aug 11, 2024
92cc785
Set `filename` and `filename2` in error messages.
barneygale Aug 11, 2024
b8372aa
Simplify patch
barneygale Aug 11, 2024
8199fd5
Undo changes from GH-122924.
barneygale Aug 13, 2024
175d687
Merge branch 'main' into path-move
barneygale Aug 23, 2024
8182a88
Merge branch 'main' into path-move
barneygale Aug 24, 2024
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
21 changes: 19 additions & 2 deletions Doc/library/pathlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1536,8 +1536,8 @@ Creating files and directories
available. In previous versions, :exc:`NotImplementedError` was raised.


Copying, renaming and deleting
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Copying, moving and deleting
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

.. method:: Path.copy(target, *, follow_symlinks=True, preserve_metadata=False)

Expand Down Expand Up @@ -1592,6 +1592,23 @@ Copying, renaming and deleting
.. versionadded:: 3.14


.. method:: Path.move(target)

Recursively move this file or directory tree to the given *target*, and
return a new :class:`!Path` instance pointing to *target*.

If the *target* doesn't exist it will be created. If both this path and the
*target* are existing files, then the target is overwritten. If both paths
point to the same file or directory, or the *target* is a non-empty
picnixz marked this conversation as resolved.
Show resolved Hide resolved
directory, then :exc:`OSError` is raised.

If both paths are on the same filesystem, the move is performed with
:func:`os.replace`. Otherwise, this path is copied (preserving metadata and
picnixz marked this conversation as resolved.
Show resolved Hide resolved
symlinks) and then deleted.

.. versionadded:: 3.14


.. method:: Path.rename(target)

Rename this file or directory to the given *target*, and return a new
Expand Down
5 changes: 4 additions & 1 deletion Doc/whatsnew/3.14.rst
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,17 @@ os
pathlib
-------

* Add methods to :class:`pathlib.Path` to recursively copy or remove files:
* Add methods to :class:`pathlib.Path` to recursively copy, move, or remove
files and directories:

* :meth:`~pathlib.Path.copy` copies the content of one file to another, like
:func:`shutil.copyfile`.
* :meth:`~pathlib.Path.copytree` copies one directory tree to another, like
:func:`shutil.copytree`.
* :meth:`~pathlib.Path.rmtree` recursively removes a directory tree, like
:func:`shutil.rmtree`.
* :meth:`~pathlib.Path.move` moves a file or directory tree, like
:func:`shutil.move`.

(Contributed by Barney Gale in :gh:`73991`.)

Expand Down
21 changes: 21 additions & 0 deletions Lib/pathlib/_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,11 @@ def copytree(self, target, *, follow_symlinks=True,
if on_error is None:
def on_error(err):
raise err
if target.is_relative_to(self):
try:
raise OSError(f"Cannot copy {self!r} inside itself: {target!r}")
barneygale marked this conversation as resolved.
Show resolved Hide resolved
except OSError as err:
on_error(err)
barneygale marked this conversation as resolved.
Show resolved Hide resolved
stack = [(self, target)]
while stack:
source_dir, target_dir = stack.pop()
Expand All @@ -869,6 +874,22 @@ def on_error(err):
except OSError as err:
on_error(err)

def move(self, target):
"""
Recursively move this file or directory tree to the given destination.
"""
if not isinstance(target, PathBase):
target = self.with_segments(target)
if self.is_dir(follow_symlinks=False):
copy_source = self.copytree
delete_source = self.rmtree
else:
copy_source = self.copy
delete_source = self.unlink
copy_source(target, follow_symlinks=False, preserve_metadata=True)
delete_source()
return target

def rename(self, target):
"""
Rename this path to the target path.
Expand Down
17 changes: 17 additions & 0 deletions Lib/pathlib/_local.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import errno
import io
import ntpath
import operator
Expand Down Expand Up @@ -849,6 +850,22 @@ def onexc(func, filename, err):
import shutil
shutil.rmtree(str(self), ignore_errors, onexc=onexc)

def move(self, target):
"""
Recursively move this file or directory tree to the given destination.
"""
if self._samefile_safe(target):
raise OSError(f"{self!r} and {target!r} are the same file")
barneygale marked this conversation as resolved.
Show resolved Hide resolved
try:
return self.replace(target)
except TypeError:
if not isinstance(target, PathBase):
raise
except OSError as err:
if err.errno != errno.EXDEV:
raise
return PathBase.move(self, target)

def rename(self, target):
"""
Rename this path to the target path.
Expand Down
52 changes: 52 additions & 0 deletions Lib/test/test_pathlib/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,19 @@
{os.open, os.stat, os.unlink, os.rmdir} <= os.supports_dir_fd and
os.listdir in os.supports_fd and os.stat in os.supports_follow_symlinks)

def patch_replace(old_test):
def new_replace(self, target):
raise OSError(errno.EXDEV, "Cross-device link", self, target)

def new_test(self):
old_replace = self.cls.replace
self.cls.replace = new_replace
try:
old_test(self)
finally:
self.cls.replace = old_replace
return new_test

#
# Tests for the pure classes.
#
Expand Down Expand Up @@ -751,6 +764,45 @@ def test_copytree_preserve_metadata_xattrs(self):
target_file = target.joinpath('dirD', 'fileD')
self.assertEqual(os.getxattr(target_file, b'user.foo'), b'42')

@patch_replace
def test_move_file_other_fs(self):
self.test_move_file()

@patch_replace
def test_move_file_to_file_other_fs(self):
self.test_move_file_to_file()

@patch_replace
def test_move_file_to_dir_other_fs(self):
self.test_move_file_to_dir()

@patch_replace
def test_move_dir_other_fs(self):
self.test_move_dir()

@patch_replace
def test_move_dir_to_dir_other_fs(self):
self.test_move_dir_to_dir()

@patch_replace
def test_move_dir_into_itself_other_fs(self):
self.test_move_dir_into_itself()

@patch_replace
@needs_symlinks
def test_move_file_symlink_other_fs(self):
self.test_move_file_symlink()

@patch_replace
@needs_symlinks
def test_move_dir_symlink_other_fs(self):
self.test_move_dir_symlink()

@patch_replace
@needs_symlinks
def test_move_dangling_symlink_other_fs(self):
self.test_move_dangling_symlink()

def test_resolve_nonexist_relative_issue38671(self):
p = self.cls('non', 'exist')

Expand Down
106 changes: 105 additions & 1 deletion Lib/test/test_pathlib/test_pathlib_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1503,7 +1503,9 @@ def iterdir(self):

def mkdir(self, mode=0o777, parents=False, exist_ok=False):
path = str(self.resolve())
if path in self._directories:
if path in self._files:
raise NotADirectoryError(errno.ENOTDIR, "Not a directory", path)
elif path in self._directories:
if exist_ok:
return
else:
Expand Down Expand Up @@ -1980,6 +1982,108 @@ def test_copytree_dangling_symlink(self):
self.assertTrue(target2.joinpath('link').is_symlink())
self.assertEqual(target2.joinpath('link').readlink(), self.cls('nonexistent'))

def test_move_file(self):
base = self.cls(self.base)
source = base / 'fileA'
source_text = source.read_text()
target = base / 'fileA_moved'
result = source.move(target)
picnixz marked this conversation as resolved.
Show resolved Hide resolved
self.assertEqual(result, target)
self.assertFalse(source.exists())
self.assertTrue(target.exists())
self.assertEqual(source_text, target.read_text())

def test_move_file_to_file(self):
base = self.cls(self.base)
source = base / 'fileA'
source_text = source.read_text()
target = base / 'dirB' / 'fileB'
result = source.move(target)
self.assertEqual(result, target)
self.assertFalse(source.exists())
self.assertTrue(target.exists())
self.assertEqual(source_text, target.read_text())

def test_move_file_to_dir(self):
base = self.cls(self.base)
source = base / 'fileA'
target = base / 'dirB'
picnixz marked this conversation as resolved.
Show resolved Hide resolved
self.assertRaises(OSError, source.move, target)
picnixz marked this conversation as resolved.
Show resolved Hide resolved

def test_move_file_to_itself(self):
base = self.cls(self.base)
source = base / 'fileA'
self.assertRaises(OSError, source.move, source)

def test_move_dir(self):
base = self.cls(self.base)
source = base / 'dirC'
target = base / 'dirC_moved'
result = source.move(target)
self.assertEqual(result, target)
self.assertFalse(source.exists())
self.assertTrue(target.is_dir())
self.assertTrue(target.joinpath('dirD').is_dir())
self.assertTrue(target.joinpath('dirD', 'fileD').is_file())
self.assertEqual(target.joinpath('dirD', 'fileD').read_text(),
"this is file D\n")
self.assertTrue(target.joinpath('fileC').is_file())
self.assertTrue(target.joinpath('fileC').read_text(),
"this is file C\n")

def test_move_dir_to_dir(self):
base = self.cls(self.base)
source = base / 'dirC'
target = base / 'dirB'
self.assertRaises(OSError, source.move, target)

def test_move_dir_to_itself(self):
base = self.cls(self.base)
source = base / 'dirC'
self.assertRaises(OSError, source.move, source)

def test_move_dir_into_itself(self):
picnixz marked this conversation as resolved.
Show resolved Hide resolved
base = self.cls(self.base)
source = base / 'dirC'
target = base / 'dirC' / 'bar'
self.assertRaises(OSError, source.move, target)

@needs_symlinks
def test_move_file_symlink(self):
base = self.cls(self.base)
source = base / 'linkA'
source_readlink = source.readlink()
target = base / 'linkA_moved'
result = source.move(target)
self.assertEqual(result, target)
self.assertFalse(source.exists())
self.assertTrue(target.is_symlink())
self.assertEqual(source_readlink, target.readlink())

@needs_symlinks
def test_move_dir_symlink(self):
base = self.cls(self.base)
source = base / 'linkB'
source_readlink = source.readlink()
target = base / 'linkB_moved'
result = source.move(target)
self.assertEqual(result, target)
self.assertFalse(source.exists())
self.assertTrue(target.is_symlink())
self.assertEqual(source_readlink, target.readlink())

@needs_symlinks
def test_move_dangling_symlink(self):
base = self.cls(self.base)
source = base / 'brokenLink'
source_readlink = source.readlink()
target = base / 'brokenLink_moved'
result = source.move(target)
self.assertEqual(result, target)
self.assertFalse(source.exists())
self.assertTrue(target.is_symlink())
self.assertEqual(source_readlink, target.readlink())

def test_iterdir(self):
P = self.cls
p = P(self.base)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add :meth:`pathlib.Path.move`, which moves a file or directory tree.
Loading