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.

Sign up for GitHub

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

Add use_srcentry parameter to shutil.copytree() #79829

Closed
xxxxxxx mannequin opened this issue Jan 3, 2019 · 6 comments
Closed

Add use_srcentry parameter to shutil.copytree() #79829

xxxxxxx mannequin opened this issue Jan 3, 2019 · 6 comments
Labels
3.8 only security fixes docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@xxxxxxx
Copy link
Mannequin

xxxxxxx mannequin commented Jan 3, 2019

BPO 35648
Nosy @giampaolo
PRs
  • bpo-35648 Add use_srcentry parameter to shutil.copytree() #11421
  • bpo-35648 Add use_srcentry parameter to shutil.copytree() #11421
  • bpo-35648 Add use_srcentry parameter to shutil.copytree() #11421
  • bpo-35648 Add use_srcentry parameter to shutil.copytree() #11421
  • bpo-35652: shutil.copytree(copy_function=...): pass DirEntry instead of path str  #11997
  • bpo-35652 Add use_srcentry parameter to shutil.copytree() II #11425
    		•

    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-02-24.17:46:05.877>
created_at = <Date 2019-01-03.09:06:12.505>
labels = ['3.8', 'type-feature', 'library', 'invalid', 'docs']
title = 'Add use_srcentry parameter to shutil.copytree()'
updated_at = <Date 2019-02-24.17:46:05.877>
user = 'https://bugs.python.org/xxxxxxx'

    bugs.python.org fields:

    activity = <Date 2019-02-24.17:46:05.877>
actor = 'xxxxxxx'
assignee = 'none'
closed = True
closed_date = <Date 2019-02-24.17:46:05.877>
closer = 'xxxxxxx'
components = ['Documentation', 'Library (Lib)']
creation = <Date 2019-01-03.09:06:12.505>
creator = 'xxxxxxx'
dependencies = []
files = []
hgrepos = []
issue_num = 35648
keywords = ['patch', 'patch', 'patch', 'patch']
message_count = 6.0
messages = ['332923', '332942', '336392', '336394', '336452', '336479']
nosy_count = 2.0
nosy_names = ['giampaolo.rodola', 'xxxxxxx']
pr_nums = ['11421', '11421', '11421', '11421', '11997', '11425']
priority = 'normal'
resolution = 'not a bug'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue35648'
versions = ['Python 3.8']
    @xxxxxxx
    Copy link
    Mannequin Author

    xxxxxxx mannequin commented Jan 3, 2019

    Currently it is decided if to use the srcentry in the copy_function by checking if the copy_function is copy() or copy2(). This will fail if the copy_function is a modified copy() or copy2() function. To control if the copy_function gets a srcentry or srcname parameter, added the use_srcentry parameter.

    @xxxxxxx xxxxxxx mannequin assigned docspython Jan 3, 2019
    @xxxxxxx xxxxxxx mannequin added 3.8 only security fixes docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jan 3, 2019
    @xxxxxxx
    Copy link
    Mannequin Author

    xxxxxxx mannequin commented Jan 3, 2019

    A new PR is started.
    See https://bugs.python.org/issue35652 and #11425

    @xxxxxxx xxxxxxx mannequin closed this as completed Jan 3, 2019
    @giampaolo giampaolo reopened this Feb 23, 2019
    @giampaolo
    Copy link
    Contributor

    It turns out that's a bug I introduced in PR-7874. I'm providing:
    #11997
    ...which fixes it without introducing a new parameter. I also verified that monkey patching shutil.copy2 and shutil.copy doesn't cause any issue.

    @xxxxxxx
    Copy link
    Mannequin Author

    xxxxxxx mannequin commented Feb 23, 2019

    You are right about the bug. The funny thing is, I never saw it.

    The problem I see is another.
    The use_srcentry variable in the _copytree function is set by determining if the copy_function is the function copy2 or copy. If so the srcentry is passed to the copy_function. If the user passes a custom copy_function to copytree, this function will ever become only the srcname and not the srcentry. To summarize it: The integrated copy_functions get the srcentry and custom copy_functions get the srcname.
    But what is if the custom copy_function is a edited copy2 function or needs the srcentry? In order to fix this was my idea to have a parameter to control if the custom copy_function get the srcentry or a srcname.
    Please write me if there is anything misconception in my thoughts or other objections?

    @giampaolo
    Copy link
    Contributor

    When user uses copy_function argument, the callback function should always receive path strings, both for consistency and for not breaking backward compatibility. This line:

        use_srcentry = copy_function is copy2 or copy_function is copy

    ...makes sure of that (only passes DirEntry instances around if copy_function is not specified by user).

    @xxxxxxx
    Copy link
    Mannequin Author

    xxxxxxx mannequin commented Feb 24, 2019

    Okay, than this behaviour is wanted.
    I only thought that the user should have more control about it, but you are right, only the path string is better to handle.
    I will close the PR.

    @xxxxxxx xxxxxxx mannequin closed this as completed Feb 24, 2019
    @xxxxxxx xxxxxxx mannequin added the invalid label Feb 24, 2019
    @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
    3.8 only security fixes docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @giampaolo