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

tarfile touches directories twice #54393

Closed
loewis mannequin opened this issue Oct 23, 2010 · 5 comments
Closed

tarfile touches directories twice #54393

loewis mannequin opened this issue Oct 23, 2010 · 5 comments
Assignees

Comments

@loewis
Copy link
Mannequin

loewis mannequin commented Oct 23, 2010

BPO 10184
Nosy @loewis, @gustaebel, @vstinner, @merwok
Files
  • tarfile.diff
  • tarfile-extract-directory-test.diff
  • tarfile2.diff
  • 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/gustaebel'
    closed_at = <Date 2010-11-01.21:39:48.827>
    created_at = <Date 2010-10-23.19:06:31.856>
    labels = []
    title = 'tarfile touches directories twice'
    updated_at = <Date 2010-11-01.21:39:48.827>
    user = 'https://github.com/loewis'

    bugs.python.org fields:

    activity = <Date 2010-11-01.21:39:48.827>
    actor = 'loewis'
    assignee = 'lars.gustaebel'
    closed = True
    closed_date = <Date 2010-11-01.21:39:48.827>
    closer = 'loewis'
    components = []
    creation = <Date 2010-10-23.19:06:31.856>
    creator = 'loewis'
    dependencies = []
    files = ['19345', '19352', '19354']
    hgrepos = []
    issue_num = 10184
    keywords = ['patch']
    message_count = 5.0
    messages = ['119467', '119510', '119524', '119545', '120182']
    nosy_count = 4.0
    nosy_names = ['loewis', 'lars.gustaebel', 'vstinner', 'eric.araujo']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue10184'
    versions = ['Python 3.1', 'Python 2.7', 'Python 3.2']

    @loewis
    Copy link
    Mannequin Author

    loewis mannequin commented Oct 23, 2010

    When extracting, the time stamps of directories are modified twice: once when creating the directories, and then once in reverse order when done.
    The first utimes call is redundant; it also causes issues with a buggy NFS server, see

    http://www.python.org/dev/buildbot/3.1/builders/AMD64%20debian%20parallel%203.1/builds/147/steps/test/logs/stdio

    (specifically, touching a file twice with the same second-resolution time stamp will increase the microsecond counter).

    The attached patch works around the issue; regardless of the NFS bug, I think that the redundant call should be eliminated.

    @loewis loewis mannequin assigned ghaering Oct 23, 2010
    @gustaebel
    Copy link
    Mannequin

    gustaebel mannequin commented Oct 24, 2010

    The patch goes a bit too far. Though it solves this particular problem with the way TarFile.extractall() works, it breaks TarFile.extract(). Running the testsuite does not expose this, simply because there's no testcase :-(

    Please see the attached testcase I just wrote which illustrates the problem.

    @loewis
    Copy link
    Mannequin Author

    loewis mannequin commented Oct 24, 2010

    I see. Here is an updated version that makes the touch operations optional.

    @loewis loewis mannequin assigned gustaebel and unassigned ghaering Oct 24, 2010
    @gustaebel
    Copy link
    Mannequin

    gustaebel mannequin commented Oct 25, 2010

    I'm not entirely happy with the name of the "touch" argument. Apart from it being nice and short, I think it's a little too unix-y and might be misleading because it is not only about setting the modification time as the name implies, but also owner and mode. My proposal would be "restore_attrs" or "set_attrs" which isn't half as nice as "touch", but sums up better what's actually done. I leave this up to you.

    I think the testcase wouldn't work on Windows the way it is now, would it?

    Apart from these minor issues the patch gets my blessing, go ahead ;-)

    @loewis
    Copy link
    Mannequin Author

    loewis mannequin commented Nov 1, 2010

    This is now committed as r86102. I opted to call the parameter set_attrs.

    @loewis loewis mannequin closed this as completed Nov 1, 2010
    @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
    None yet
    Projects
    None yet
    Development

    No branches or pull requests

    0 participants