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

shutil.move raises AttributeError if first argument is a pathlib.Path object and destination is a directory #76870

Closed
craigh mannequin opened this issue Jan 28, 2018 · 6 comments
Labels
3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@craigh
Copy link
Mannequin

craigh mannequin commented Jan 28, 2018

BPO 32689
Nosy @gvanrossum, @yan12125, @emilyemorehouse, @miss-islington, @maxwellmckinnon
PRs
  • bpo-32689: Updates shutil.move to allow for Path objects to be used as source arg #5393
  • bpo-32689: Updates shutil.move to allow for Path objects to be used as source arg #15326
  • 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 2019-10-01.02:46:32.949>
    created_at = <Date 2018-01-28.00:01:24.483>
    labels = ['type-bug', 'library', '3.9']
    title = 'shutil.move raises AttributeError if first argument is a pathlib.Path object and destination is a directory'
    updated_at = <Date 2019-10-01.02:46:32.941>
    user = 'https://bugs.python.org/craigh'

    bugs.python.org fields:

    activity = <Date 2019-10-01.02:46:32.941>
    actor = 'gvanrossum'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-10-01.02:46:32.949>
    closer = 'gvanrossum'
    components = ['Library (Lib)']
    creation = <Date 2018-01-28.00:01:24.483>
    creator = 'craigh'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32689
    keywords = ['patch']
    message_count = 6.0
    messages = ['310900', '310983', '310986', '311373', '353628', '353630']
    nosy_count = 6.0
    nosy_names = ['gvanrossum', 'craigh', 'yan12125', 'emilyemorehouse', 'miss-islington', 'bodom']
    pr_nums = ['5393', '15326']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue32689'
    versions = ['Python 3.9']

    @craigh
    Copy link
    Mannequin Author

    craigh mannequin commented Jan 28, 2018

    >>> import os, pathlib, shutil
    >>> os.mkdir('test1')
    >>> os.mkdir('test2')
    >>> path = pathlib.Path('test1')
    >>> shutil.move(path, 'test2')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python3.6/shutil.py", line 540, in move
        real_dst = os.path.join(dst, _basename(src))
      File "/usr/lib/python3.6/shutil.py", line 504, in _basename
        return os.path.basename(path.rstrip(sep))
    AttributeError: 'PosixPath' object has no attribute 'rstrip'

    @craigh craigh mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jan 28, 2018
    @emilyemorehouse
    Copy link
    Member

    Thanks for the bug report!

    shutil.move should certainly accept a path object, as shutil.copy does, though it should be noted that in your example, 'path' could become out of date as it does not refresh the path information. For example, with shutil.move fixed:

        >>> import os, pathlib, shutil
        >>> os.mkdir('test1')
        >>>
        >>> os.mkdir('test2')
        >>> path = pathlib.Path('test1')
        >>> path.absolute()
        PosixPath('/Users/e/Development/OSS/cpython/test1')
        >>> shutil.move(path, 'test2')
        'test2/test1'
        >>> path.absolute()
        PosixPath('/Users/e/Development/OSS/cpython/test2')

    test1 is now actually at '/Users/e/Development/OSS/cpython/test2/test1'

    For the fix:
    I did a bit of digging and the error comes from a helper method _basename that uses rstrip to remove a trailing separator, hence the error as rstrip doesn't exist for a path object (and I don't think it makes sense that it should, though that was one solution). Removing the trailing separator is, however, very important in determining the full destination path.

    After trying a few different approaches, I think the simplest way is to cast the src to a string before finding its appropriate basename. I also added some comments to make it more clear why _basename is used over os.path.basename to hopefully save someone else time in the future.

    A more robust option would be to explicitly handle Path objects or to handle exceptions for any dst that cannot be cast to a string. However, the current patch fixes the issue without introducing new problems.

    @craigh
    Copy link
    Mannequin Author

    craigh mannequin commented Jan 28, 2018

    In my test, the second call to path.absolute() is just returning the same result as the first call, which is what I would expect (as you say, the path object doesn't update automatically).

    However, your output shows it returning '/Users/e/Development/OSS/cpython/test2' instead of the (now broken) path from the first call. Maybe I'm missing something?

    @emilyemorehouse
    Copy link
    Member

    Ah, you're right. That was a typo when I was redacting my full path. The path object remains unchanged even though the directory has moved.

    Should have been:

        >>> import os, pathlib, shutil
        >>> os.mkdir('test1')
        >>> os.mkdir('test2')
        >>> path = pathlib.Path('test1')
        >>> path.absolute()
        PosixPath('/Users/e/Development/OSS/cpython/test1')
        >>> shutil.move(path, 'test2')
        'test2/test1'
        >>> path.absolute()
        PosixPath('/Users/e/Development/OSS/cpython/test1')

    test1 is now actually at '/Users/e/Development/OSS/cpython/test2/test1'

    @BoboTiG BoboTiG mannequin added 3.7 (EOL) end of life 3.8 only security fixes labels Jan 14, 2019
    @miss-islington
    Copy link
    Contributor

    New changeset cf57cab by Miss Islington (bot) (Maxwell A McKinnon) in branch 'master':
    bpo-32689: Updates shutil.move to allow for Path objects to be used as source arg (GH-15326)
    cf57cab

    @gvanrossum
    Copy link
    Member

    3.9 only!

    @gvanrossum gvanrossum added 3.9 only security fixes and removed 3.7 (EOL) end of life 3.8 only security fixes labels Oct 1, 2019
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    q0w added a commit to q0w/pip that referenced this issue Jun 15, 2022
    q0w added a commit to q0w/pip that referenced this issue Jun 15, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants