shutil.move fails on symlink source

[jniehof]

BPO	9993
Nosy	@pitrou, @vstinner, @tarekziade, @merwok, @bitdancer, @hynek
Files	shutil_move_symlinks.patch: Patch to shutil.move: move symlinks rather than copy contents shutil_move_doc.patch: Documentation update for new shutil.move behaviour shutil_move_symlinks.patch: Patch to shutil.move: move symlinks rather than copy contents – fixed tests for mock based aproach shutil_move_symlinks.patch: Fixes shutil.move, adds docs with proper tag.

^{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 = <Date 2012-01-06.19:18:31.687>
created_at = <Date 2010-09-29.23:03:11.449>
labels = ['type-bug', 'library']
title = 'shutil.move fails on symlink source'
updated_at = <Date 2012-01-06.19:18:31.686>
user = 'https://bugs.python.org/jniehof'

bugs.python.org fields:

activity = <Date 2012-01-06.19:18:31.686>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2012-01-06.19:18:31.687>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2010-09-29.23:03:11.449>
creator = 'jniehof'
dependencies = []
files = ['19063', '23002', '24146', '24149']
hgrepos = []
issue_num = 9993
keywords = ['patch']
message_count = 15.0
messages = ['117673', '117751', '142604', '142610', '142734', '142765', '142824', '142829', '150467', '150472', '150696', '150698', '150717', '150763', '150764']
nosy_count = 8.0
nosy_names = ['pitrou', 'vstinner', 'tarek', 'eric.araujo', 'r.david.murray', 'jniehof', 'python-dev', 'hynek']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue9993'
versions = ['Python 3.3']

[jniehof]

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

[bitdancer]

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 bpo-9999) and pass after the patch. The patch itself looks good to me and does accord better with how 'mv' works.

[hynek]

I work on the superficially related bpo-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.

[merwok]

Can you update the documentation?

[jniehof]

Éric, here's a quick docs-only patch against current default...does this do the job?

[merwok]

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?

[jniehof]

É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."

[pitrou]

Looks good to me.

[merwok]

Antoine, would you mind taking this one?

[pitrou]

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.

[hynek]

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

[pitrou]

Thanks! I think we also need a doc update for the change in behaviour (with a "versionchanged" tag).

[hynek]

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.