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

tarfile does not support pathlib #72417

Closed
ethanfurman opened this issue Sep 21, 2016 · 9 comments
Closed

tarfile does not support pathlib #72417

ethanfurman opened this issue Sep 21, 2016 · 9 comments
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ethanfurman
Copy link
Member

BPO 28230
Nosy @brettcannon, @gustaebel, @ned-deily, @ethanfurman, @berkerpeksag, @serhiy-storchaka
PRs
  • bpo-28230: Document the pathlib support in tarfile and add tests. #512
  • [3.6] bpo-28230: Document the pathlib support in tarfile and add tests. #559
    		•
    Files
  • open-tarfile.stoneleaf.patch
  • issue28230_v2.diff
  • test_tarfile_pathlike.patch
    		•

    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 2017-03-15.12:52:09.446>
created_at = <Date 2016-09-21.07:31:34.259>
labels = ['3.7', 'type-bug', 'library']
title = 'tarfile does not support pathlib'
updated_at = <Date 2017-03-24.22:41:49.995>
user = 'https://github.com/ethanfurman'

    bugs.python.org fields:

    activity = <Date 2017-03-24.22:41:49.995>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2017-03-15.12:52:09.446>
closer = 'berker.peksag'
components = ['Library (Lib)']
creation = <Date 2016-09-21.07:31:34.259>
creator = 'ethan.furman'
dependencies = []
files = ['44772', '45042', '45051']
hgrepos = []
issue_num = 28230
keywords = ['patch']
message_count = 9.0
messages = ['278414', '278416', '278428', '278444', '288604', '289096', '289671', '290265', '290267']
nosy_count = 6.0
nosy_names = ['brett.cannon', 'lars.gustaebel', 'ned.deily', 'ethan.furman', 'berker.peksag', 'serhiy.storchaka']
pr_nums = ['512', '559']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue28230'
versions = ['Python 3.6', 'Python 3.7']
    @ethanfurman ethanfurman added the type-bug An unexpected behavior, bug, or error label Sep 21, 2016
    @ned-deily ned-deily added the 3.7 (EOL) end of life label Sep 23, 2016
    @berkerpeksag berkerpeksag added the stdlib Python modules in the Lib dir label Oct 5, 2016
    @berkerpeksag
    Copy link
    Member

    Here's an updated patch with different tests and documentation changes. I simplified Lib/tarfile.py a bit (see my review comments)

    A slightly off-topic question: I had to update both docstrings and documentation. Should we use this as an opportunity to simplify the docstrings?

    @serhiy-storchaka
    Copy link
    Member

    There are two kinds of paths in tarinfo:

    1. External paths that correspond to filesystem paths. The path of the tar file itself, patches of added files and target directory for extraction. Supporting path protocol looks reasonable for them.

    2. Internal paths, they are just string keys inside an archive. They come from TarFile.getnames() and always are strings. I'm not sure that pathlib have relation to this. This issue needs more thoughts. I would not haste with this.

    @ethanfurman
    Copy link
    Member Author

    As Serhiy was alluding to, if the incoming path is for the actual tar file and is only passed along to Python itself then we probably don't need to worry about os.fspath(). For names that will be interally stored, or are for accessing internal files, then the proper sequence is check for and call os.fspath if necessary, and then double-check that the name to be used is a str. The double-check is needed because os.fspath may return a bytes object, which tar does not allow.

    @serhiy-storchaka
    Copy link
    Member

    It seems to me that tarfile already supports path protocol for all external patches and no changes for tarfile are needed.

    Proposed patch adds tests for (already implemented) support of pathlike protocol in tarfile.

    @berkerpeksag
    Copy link
    Member

    Serhiy's patch looks good to me, but it would be nice to document the support for PathLike objects in the tarfile documentation.

    @serhiy-storchaka
    Copy link
    Member

    Updated documentation.

    Note that path-like objects are supported only for external names. Internal names are not OS paths.

    @berkerpeksag
    Copy link
    Member

    PR 512 has been merged and backported to 3.6 branch. I think this can be closed now. Thanks, Ethan and Serhiy!

    @serhiy-storchaka
    Copy link
    Member

    New changeset 666165f by Serhiy Storchaka in branch '3.6':
    [3.6] bpo-28230: Document the pathlib support in tarfile and add tests. (#559)
    666165f

    @serhiy-storchaka
    Copy link
    Member

    New changeset c45cd16 by Serhiy Storchaka in branch 'master':
    bpo-28230: Document the pathlib support in tarfile and add tests. (#512)
    c45cd16

    @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.7 (EOL) end of life 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
    @berkerpeksag @serhiy-storchaka @ned-deily @ethanfurman