classification
Title: shutil.move fails on symlink source
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eric.araujo, haypo, hynek, jniehof, pitrou, python-dev, r.david.murray, tarek
Priority: normal Keywords: patch

Created on 2010-09-29 23:03 by jniehof, last changed 2012-01-06 19:18 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
shutil_move_symlinks.patch jniehof, 2010-09-29 23:03 Patch to shutil.move: move symlinks rather than copy contents review
shutil_move_doc.patch jniehof, 2011-08-22 17:24 Documentation update for new shutil.move behaviour review
shutil_move_symlinks.patch hynek, 2012-01-05 21:53 Patch to shutil.move: move symlinks rather than copy contents – fixed tests for mock based aproach review
shutil_move_symlinks.patch hynek, 2012-01-06 08:25 Fixes shutil.move, adds docs with proper tag. review
Messages (15)
msg117673 - (view) Author: Jonathan Niehof (jniehof) Date: 2010-09-29 23:03
shutil.move does not behave as I expect when moving a symlink across filesystems. (This is when src itself is a symlink, not when it is a directory tree including symlinks.)

-If src is a symlink to file, rather than moving the symlink, it copies the contents of the file
-If src is a symlink to a directory, rather than moving the symlink, it copies the contents of the directory to a new directory
-If src is a dangling symlink, it errors out

Attached patch against the py3k branch adds tests for these cases and adds the expected behaviour, which is to recreate the symlink.

(I have encountered this on 2.6 - current SVN; it is probably in 2.5 as well but I have not confirmed.)
msg117751 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-09-30 16:27
I can confirm that the tests fail before the patch (but if an only if /tmp is a different file system from the one where the build is, see issue 9999) and pass after the patch.  The patch itself looks good to me and does accord better with how 'mv' works.
msg142604 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2011-08-21 09:51
I work on the superficially related #12715 (symlinks for shutil). As we try to mimic the POSIX tools and `mv` indeed doesn't follow links, I agree with the approach of this patch.
msg142610 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-08-21 11:01
Can you update the documentation?
msg142734 - (view) Author: Jonathan Niehof (jniehof) Date: 2011-08-22 17:24
Éric, here's a quick docs-only patch against current default...does this do the job?
msg142765 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-08-22 22:25
Thanks for the doc patch, which looks good.  I have one question: if a relative symlink is copied, should the resulting symlink be relative too?
msg142824 - (view) Author: Jonathan Niehof (jniehof) Date: 2011-08-23 13:41
Éric: I think copying a relative symlink should also be relative, and that's the behaviour of this patch. That was the use case that tripped me up with the original behaviour of shutil.move: a relative symlink which was dangling in its original location, but not once moved. (This was, believe it or not, intentional design....)

This is also the behaviour when src and dst are on the same filesystem. Basically my intention was to remove the distinction between "same filesystem" and "different filesystem."
msg142829 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-08-23 14:07
Looks good to me.
msg150467 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-01-02 17:21
Antoine, would you mind taking this one?
msg150472 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-02 18:22
Ow, I'm sorry for overlooking this. I thought you would take it.
Unfortunately the patch is now broken because of 5b61334bb776.
(technically it applies, but the tests don't run anymore because the "other filesystem" now uses a mocking approach)

By the way, I think this shouldn't be applied to bugfix branches, since it's a behaviour change.
msg150696 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2012-01-05 21:53
I took the liberty to fix the tests.

Basically I've adapted them to the new mock based cross file system approach (that doesn't depend on luck anymore :)).

I also had to add one more `os.path.realpath` because on some OS (like OS X) the tmp directory path already consists of symlinks.

I didn't touch the actual code.

Tested on OS X and Ubuntu Linux (Oneiric).
msg150698 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-05 22:04
Thanks! I think we also need a doc update for the change in behaviour (with a "versionchanged" tag).
msg150717 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2012-01-06 08:25
Oh sorry, I didn't look into the doc patch.

I unified both into one patch and added the versionchanged tag and also updated the docstring of shutil.move.
msg150763 - (view) Author: Roundup Robot (python-dev) Date: 2012-01-06 19:17
New changeset 1ea8b7233fd7 by Antoine Pitrou in branch 'default':
Issue #9993: When the source and destination are on different filesystems,
http://hg.python.org/cpython/rev/1ea8b7233fd7
msg150764 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-01-06 19:18
Patch now committed to 3.3. Thank you Jonathan and Hynek!
History
Date User Action Args
2012-01-06 19:18:31pitrousetstatus: open -> closed
resolution: fixed
messages: + msg150764

stage: patch review -> resolved
2012-01-06 19:17:45python-devsetnosy: + python-dev
messages: + msg150763
2012-01-06 08:25:03hyneksetfiles: + shutil_move_symlinks.patch

messages: + msg150717
2012-01-05 22:04:36pitrousetmessages: + msg150698
2012-01-05 21:53:37hyneksetfiles: + shutil_move_symlinks.patch

messages: + msg150696
2012-01-02 18:22:26pitrousetmessages: + msg150472
versions: - Python 2.7, Python 3.2
2012-01-02 17:21:57eric.araujosetmessages: + msg150467
2011-08-23 14:07:38pitrousetmessages: + msg142829
2011-08-23 13:43:14hayposetnosy: + pitrou, haypo
2011-08-23 13:41:38jniehofsetmessages: + msg142824
2011-08-22 22:25:38eric.araujosetmessages: + msg142765
2011-08-22 17:24:51jniehofsetfiles: + shutil_move_doc.patch

messages: + msg142734
2011-08-21 11:01:15eric.araujosetnosy: + eric.araujo

messages: + msg142610
versions: + Python 3.3, - Python 3.1
2011-08-21 09:51:08hyneksetnosy: + hynek
messages: + msg142604
2010-09-30 16:27:46r.david.murraysetnosy: + r.david.murray
messages: + msg117751
2010-09-30 03:25:21ned.deilysetnosy: + tarek
stage: patch review

versions: - Python 2.6
2010-09-29 23:03:11jniehofcreate