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

Jump to bottom

shutil.move fails on symlink source #54202

Closed
jniehof mannequin opened this issue Sep 29, 2010 · 15 comments
Closed

shutil.move fails on symlink source #54202

jniehof mannequin opened this issue Sep 29, 2010 · 15 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@jniehof
Copy link
Mannequin

jniehof mannequin commented Sep 29, 2010

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
    Copy link
    Mannequin Author

    jniehof mannequin commented Sep 29, 2010

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

    @jniehof jniehof mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Sep 29, 2010
    @bitdancer
    Copy link
    Member

    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
    Copy link
    Member

    hynek commented Aug 21, 2011

    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
    Copy link
    Member

    merwok commented Aug 21, 2011

    Can you update the documentation?

    @jniehof
    Copy link
    Mannequin Author

    jniehof mannequin commented Aug 22, 2011

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

    @merwok
    Copy link
    Member

    merwok commented Aug 22, 2011

    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
    Copy link
    Mannequin Author

    jniehof mannequin commented Aug 23, 2011

    É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
    Copy link
    Member

    pitrou commented Aug 23, 2011

    Looks good to me.

    @merwok
    Copy link
    Member

    merwok commented Jan 2, 2012

    Antoine, would you mind taking this one?

    @pitrou
    Copy link
    Member

    pitrou commented Jan 2, 2012

    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
    Copy link
    Member

    hynek commented Jan 5, 2012

    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
    Copy link
    Member

    pitrou commented Jan 5, 2012

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

    @hynek
    Copy link
    Member

    hynek commented Jan 6, 2012

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 6, 2012

    New changeset 1ea8b7233fd7 by Antoine Pitrou in branch 'default':
    Issue bpo-9993: When the source and destination are on different filesystems,
    http://hg.python.org/cpython/rev/1ea8b7233fd7

    @pitrou
    Copy link
    Member

    pitrou commented Jan 6, 2012

    Patch now committed to 3.3. Thank you Jonathan and Hynek!

    @pitrou pitrou closed this as completed Jan 6, 2012
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @hynek @bitdancer @merwok @pitrou