Pathlib.rename isn't robust

[oz123]

BPO	46317
Nosy	@oz123, @barneygale
PRs	GH-73991: Add pathlib.Path.move that can handle rename across FS #30650

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2022-01-09.14:52:56.361>
labels = ['library', '3.9', '3.10', '3.11']
title = "Pathlib.rename isn't robust"
updated_at = <Date 2022-01-17.19:38:48.397>
user = 'https://github.com/oz123'

bugs.python.org fields:

activity = <Date 2022-01-17.19:38:48.397>
actor = 'Oz.Tiram'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2022-01-09.14:52:56.361>
creator = 'Oz.Tiram'
dependencies = []
files = []
hgrepos = []
issue_num = 46317
keywords = ['patch']
message_count = 6.0
messages = ['410155', '410590', '410593', '410595', '410648', '410649']
nosy_count = 2.0
nosy_names = ['Oz.Tiram', 'barneygale']
pr_nums = ['30650']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue46317'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

[oz123]

Pathlib.rename will fail across file system with:

OSError: [Errno 18] Invalid cross-device link

e.g:
-> path_dict["current_path"].rename(path_dict["destination"])
(Pdb) n
OSError: [Errno 18] Invalid cross-device link: '/tmp/pipenv-k1m0oynt-yaml/PyYAML-6.0/lib/yaml' -> '/home/oz123/Software/pipenv/pipenv/patched/yaml3'

This is because it uses os.rename under the hood:
https://github.com/python/cpython/blob/3.10/Lib/pathlib.py#L306

One can either replace it with shutil.move which works, or one could
add another method to Pathlib.move(...) with similar signature and return value, which calls shutil.move under the hood.

Before submitting a patch for that, I would like to get feedback for that.

[barneygale]

Sounds good. Would you expose the copy_function argument in pathlib, or do something else (like metadata=True)?

[oz123]

@Barney, I am not sure that I understand your question.

I think adding another method Pathlib.Path and Pathlib._Accessor is my preferred way. The would be something like:

    class _NormalAccessor(_Accessor):
       ...
       self.move = shutil.move
    

    class Path:
       ....

       def move(self, src, dest):
          self._accessor.move(self, target)
          return self.__class__(target)

Now, this is hardly a patch. I need to submit a PR with proper docs, tests and NEWS entry... I will be glad to work on it. However, I guess I need someone to "sponsor" it and merge it.

[barneygale]

shutil.move() accepts a copy_function argument:

    shutil.move(src, dst, copy_function=copy2)

It's possible to set copy_function=copy to skip copying file metadata.

[oz123]

Thanks for the answer, it makes sense now. Yes, I would adopt this.
Allowing users to use copy2 (or any other functio ...) using a keyword.

[barneygale]

Fair enough. Users who wanted to avoid copying file metadata would then do something like this, I suppose?

    import pathlib
    import shutil

    path = pathlib.Path('foo')
    path.move('bar', copy_function=shutil.copy)

I guess the downside here is that users would still need to import shutil to do this. But I see the utility of allowing any copy_function to be supplied!

[barneygale]

Hey @oz123, I've just spotted that this issue is a duplicate of #73991. There's some more discussion there. I'll resolve this issue as a duplicate, and re-title your PR. Hope that's alright, thanks!