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.copytree - 3.8 changed argument types of the ignore callback #83571

Closed
mbarkhau mannequin opened this issue Jan 19, 2020 · 14 comments
Closed

shutil.copytree - 3.8 changed argument types of the ignore callback #83571

mbarkhau mannequin opened this issue Jan 19, 2020 · 14 comments
Assignees
Labels
3.8 only security fixes 3.9 only security fixes docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@mbarkhau
Copy link
Mannequin

mbarkhau mannequin commented Jan 19, 2020

BPO 39390
Nosy @vstinner, @giampaolo, @serhiy-storchaka, @mbarkhau
PRs
  • bpo-39390: Update shutil.copytree doc - Add versionchanged 3.8 notice for ignore callable #18069
  • bpo-39390: fix argument types for ignore callback of shutil.copytree #18122
  • [3.8] bpo-39390 shutil: fix argument types for ignore callback (GH-18122). #18168
  • 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 = 'https://github.com/giampaolo'
    closed_at = <Date 2020-01-27.23:48:24.425>
    created_at = <Date 2020-01-19.20:40:46.651>
    labels = ['3.8', 'type-bug', 'library', '3.9', 'docs']
    title = 'shutil.copytree - 3.8 changed argument types of the ignore callback'
    updated_at = <Date 2020-01-27.23:48:24.425>
    user = 'https://github.com/mbarkhau'

    bugs.python.org fields:

    activity = <Date 2020-01-27.23:48:24.425>
    actor = 'giampaolo.rodola'
    assignee = 'giampaolo.rodola'
    closed = True
    closed_date = <Date 2020-01-27.23:48:24.425>
    closer = 'giampaolo.rodola'
    components = ['Documentation', 'Library (Lib)']
    creation = <Date 2020-01-19.20:40:46.651>
    creator = 'mbarkhau'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39390
    keywords = ['patch', '3.8regression']
    message_count = 14.0
    messages = ['360271', '360300', '360303', '360427', '360431', '360435', '360437', '360621', '360622', '360624', '360625', '360626', '360627', '360825']
    nosy_count = 5.0
    nosy_names = ['vstinner', 'giampaolo.rodola', 'docs@python', 'serhiy.storchaka', 'mbarkhau']
    pr_nums = ['18069', '18122', '18168']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue39390'
    versions = ['Python 3.8', 'Python 3.9']

    @mbarkhau
    Copy link
    Mannequin Author

    mbarkhau mannequin commented Jan 19, 2020

    In Python 3.8, the types of the parameters to the ignore callable appear to have changed.

    Previously the src parameter was a string and the names parameter was a list of strings. Now the src parameter appears to be either a pathlib.Path or an os.DirEntry, while the names parameter is a set of strings.

    I would suggest adding the following to the documentation https://github.com/python/cpython/blob/master/Doc/library/shutil.rst

    .. versionchanged:: 3.8
    The types of arguments to *ignore* have changed. The first argument
    (the directory being visited) is a func:`os.DirEntry` or a
    func:`pathlib.Path`; Previously it was a string. The second argument is
    a set of strings; previously it was a list of strings.

    @mbarkhau mbarkhau mannequin assigned docspython Jan 19, 2020
    @mbarkhau mbarkhau mannequin added 3.8 only security fixes 3.9 only security fixes docs Documentation in the Doc dir stdlib Python modules in the Lib dir labels Jan 19, 2020
    @mbarkhau mbarkhau mannequin assigned docspython Jan 19, 2020
    @mbarkhau mbarkhau mannequin added type-bug An unexpected behavior, bug, or error 3.8 only security fixes 3.9 only security fixes docs Documentation in the Doc dir stdlib Python modules in the Lib dir labels Jan 19, 2020
    @mbarkhau mbarkhau mannequin changed the title shutil.copytree - ignore callback behaviour change shutil.copytree - 3.8 changed argument types of the ignore callback Jan 19, 2020
    @mbarkhau mbarkhau mannequin changed the title shutil.copytree - ignore callback behaviour change shutil.copytree - 3.8 changed argument types of the ignore callback Jan 19, 2020
    @serhiy-storchaka
    Copy link
    Member

    This looks like a backward incompatible change in 3.8. Should not copytree convert arguments of the ignore callback to str and list correspondingly?

    @mbarkhau
    Copy link
    Mannequin Author

    mbarkhau mannequin commented Jan 20, 2020

    This looks like a backward incompatible change in 3.8.

    Indeed.

    Should not copytree convert arguments of the ignore callback to str and list correspondingly?

    Well, since any existing code probably expects that behavior (or at least probably works if that is the case), I would be for such a change. I'm not sure what your policy is though. If there is recently written code that assumes the new types, are you OK with that breaking if it is changed back?

    I guess since it's an undocumented breaking change, it shouldn't be too much of an issue.

    @mbarkhau
    Copy link
    Mannequin Author

    mbarkhau mannequin commented Jan 21, 2020

    Is there anything I can do to help move this forward?

    @giampaolo
    Copy link
    Contributor

    Should not copytree convert arguments of the ignore callback to str and list correspondingly?

    It should. I think it makes sense to just do this for Python 3.8.2 instead of updating the doc.

    @mbarkhau
    Copy link
    Mannequin Author

    mbarkhau mannequin commented Jan 21, 2020

    Unless somebody else wants to, I could have a go at an PR to update shutil.py

    @giampaolo
    Copy link
    Contributor

    Yes, thanks. Whoever got bit by this is either getting an exception or not the intended behavior (due to failed string comparison). I doubt anybody is relying on the new type checking since it's not documented. If they are, they are probably just doing:

        def callback(name, names):
            if not isinstance(name, str):  # bugfix 3.8
                name = name.name
            ...

    @giampaolo
    Copy link
    Contributor

    New changeset 8870433 by Giampaolo Rodola (mbarkhau) in branch 'master':
    bpo-39390 shutil: fix argument types for ignore callback (GH-18122)
    8870433

    @giampaolo
    Copy link
    Contributor

    For completeness, a similar problem is present also on python < 3.8 if passing a pathlib.Path type as *src*: the callback function will receive a pathlib.Path type once, and then string types.

    @mbarkhau
    Copy link
    Mannequin Author

    mbarkhau mannequin commented Jan 24, 2020

    For completeness, a similar problem is present also on python < 3.8

    Fair point. I'll have a look.

    @serhiy-storchaka
    Copy link
    Member

    For completeness, a similar problem is present also on python < 3.8 if passing a pathlib.Path type as *src*

    I do not think this is a problem. If you pass a string, you will get a string, so existing code will continue to work as before. The only problem if you pass a string, but get an unexpected type.

    @giampaolo
    Copy link
    Contributor

    I don't think we need to change anything on < 3.8, but 3.8 and 3.9 will always convert *src* to str via os.fspath(), which IMO is more consistent (e.g. os.path.* functions and others do the same).

    @mbarkhau
    Copy link
    Mannequin Author

    mbarkhau mannequin commented Jan 24, 2020

    If you pass a string, you will get a string, so existing code will continue to work as before.

    Somebody might have code that is running against a flat directory and have written their ignore function expecting to get a pathlib.Path, because that's the only case they encountered. This change would break their code and so would an upgrade to 3.9 with the patch that was just merged.

    @giampaolo
    Copy link
    Contributor

    New changeset cf9d005 by Giampaolo Rodola (mbarkhau) in branch '3.8':
    [3.8] bpo-39390 shutil: fix argument types for ignore callback (GH-18122)
    cf9d005

    @giampaolo giampaolo self-assigned this Jan 27, 2020
    @giampaolo giampaolo assigned giampaolo and unassigned docspython Jan 27, 2020
    @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
    Labels
    3.8 only security fixes 3.9 only security fixes docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants