[issue46317] Pathlib.rename isn't robust

[Oz Tiram]

Change by Oz Tiram :


--
keywords: +patch
pull_requests: +28851
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/30650

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46317] Pathlib.rename isn't robust

[Barney Gale]

Barney Gale  added the comment:

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!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46317] Pathlib.rename isn't robust

[Oz Tiram]

Oz Tiram  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46317] Pathlib.rename isn't robust

[Barney Gale]

Barney Gale  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46317] Pathlib.rename isn't robust

[Oz Tiram]

Oz Tiram  added the comment:

@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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46317] Pathlib.rename isn't robust

[Barney Gale]

Barney Gale  added the comment:

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

--
nosy: +barneygale

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46317] Pathlib.rename isn't robust

[Oz Tiram]

New submission from Oz Tiram :

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.

--
components: Library (Lib)
messages: 410155
nosy: Oz.Tiram
priority: normal
severity: normal
status: open
title: Pathlib.rename isn't robust
versions: Python 3.10, Python 3.11, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com