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.copystat should copy Linux extended attributes #59443

Closed
larryhastings opened this issue Jul 1, 2012 · 13 comments
Closed

shutil.copystat should copy Linux extended attributes #59443

larryhastings opened this issue Jul 1, 2012 · 13 comments
Assignees
Labels
type-feature A feature request or enhancement

Comments

@larryhastings
Copy link
Contributor

BPO 15238
Nosy @birkenfeld, @larryhastings, @ned-deily, @skrah, @hynek, @serhiy-storchaka
Files
  • larry.copystat.xattrs.1.diff: patcho uno
  • 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/larryhastings'
    closed_at = <Date 2012-07-15.01:00:58.940>
    created_at = <Date 2012-07-01.15:13:28.731>
    labels = ['type-feature']
    title = 'shutil.copystat should copy Linux extended attributes'
    updated_at = <Date 2012-07-16.15:37:12.220>
    user = 'https://github.com/larryhastings'

    bugs.python.org fields:

    activity = <Date 2012-07-16.15:37:12.220>
    actor = 'hynek'
    assignee = 'larry'
    closed = True
    closed_date = <Date 2012-07-15.01:00:58.940>
    closer = 'larry'
    components = []
    creation = <Date 2012-07-01.15:13:28.731>
    creator = 'larry'
    dependencies = []
    files = ['26238']
    hgrepos = []
    issue_num = 15238
    keywords = ['patch']
    message_count = 13.0
    messages = ['164483', '164584', '165296', '165298', '165305', '165327', '165369', '165479', '165588', '165591', '165622', '165623', '165626']
    nosy_count = 7.0
    nosy_names = ['georg.brandl', 'larry', 'ned.deily', 'skrah', 'python-dev', 'hynek', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue15238'
    versions = ['Python 3.3']

    @larryhastings
    Copy link
    Contributor Author

    3.3 adds the *xattr() extended attribute functions on Linux. shutil implements a new internal function(_copyxattr) which copies these extended attributes. However, it's only used in shutil.copy2(). I assert that shutil.copystat() should also preserve this metadata. (Which, having subsumed this functionality, means that shutil.copy2() would no longer need to explicitly call it.)

    Also: it might be worth considering taking the same approach _copyxattr uses for the other conditional functionality (see "def lookup" inside copystat). Computing platform support at import time instead of runtime looks cleaner.

    Georg, how much of this (if any) do you consider a 3.3 bugfix?

    @larryhastings larryhastings self-assigned this Jul 1, 2012
    @larryhastings larryhastings added the type-feature A feature request or enhancement label Jul 1, 2012
    @larryhastings
    Copy link
    Contributor Author

    First patch.

    @hynek
    Copy link
    Member

    hynek commented Jul 12, 2012

    The patch looks good enough. Not sure if we should write Linux though. I sincerely hope we’ll support xattrs on other OSes soonish.

    Did you catch Georg somewhere whether this can still go in? Would be nice as xattrs are new to 3.3. Changing this in 3.4 would be rather unfortunate.

    @birkenfeld
    Copy link
    Member

    Yeah, good to go. But please re-insert a blank line in the test suite changes after the end of the method.

    @larryhastings
    Copy link
    Contributor Author

    Georg: done.

    Hynek: You must forgive me, I'm a recovering Windows programmer. I thought extended attributes were a Linux-only thing. Can you tell me what other platforms they are available on? And/or suggest some alternate language?

    @ned-deily
    Copy link
    Member

    See open bpo-12978.

    @hynek
    Copy link
    Member

    hynek commented Jul 13, 2012

    Hynek: You must forgive me, I'm a recovering Windows programmer. I thought extended attributes were a Linux-only thing. Can you tell me what other platforms they are available on? And/or suggest some alternate language?

    http://en.wikipedia.org/wiki/Xattr has a list. The question what’s most
    user friendly. The best way for the user would be your text. However
    we’ll have to dig through the docs once we support xattr on more platforms.

    However as I see it, we’re deep down this path already anyway
    (http://docs.python.org/dev/library/os.html?highlight=xattr#linux-extended-attributes),
    so I guess we can keep your docs.

    One nit: I’d swap your quoted "extended attributes" to unquoted but
    linked to the doc above.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 15, 2012

    New changeset 5f62c317c202 by Larry Hastings in branch 'default':

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Jul 16, 2012

    The new test fails on the Fedora bot:

    http://buildbot.python.org/all/builders/AMD64%20Fedora%20without%20threads%203.x/builds/2881/steps/test/logs/stdio

    ======================================================================
    FAIL: test_copyxattr (test.test_shutil.TestShutil)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/buildbot/buildarea/3.x.krah-fedora/build/Lib/test/test_shutil.py", line 420, in test_copyxattr
        self.assertEqual(os.listxattr(src), ['user.the_value'])
    AssertionError: Lists differ: ['security.selinux', 'user.the... != ['user.the_value']

    First differing element 0:
    security.selinux
    user.the_value

    First list contains 1 additional elements.
    First extra element 1:
    user.the_value

    • ['security.selinux', 'user.the_value']
      + ['user.the_value']

    @hynek
    Copy link
    Member

    hynek commented Jul 16, 2012

    You can’t just compare the xattr because some (like security.*) can be
    only copied as root. IIRC they come from SELinux.

    @larryhastings
    Copy link
    Contributor Author

    Hynek, it's clear you understand this far better than I do. Could I get you to fix the Fedora buildbot problem, etc, etc?

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 16, 2012

    New changeset 265e36e277f3 by Hynek Schlawack in branch 'default':
    bpo-15238: Fix xattr related shutil.copystat tests
    http://hg.python.org/cpython/rev/265e36e277f3

    @hynek
    Copy link
    Member

    hynek commented Jul 16, 2012

    I removed the check for good as we check for the existence of the xattr in the next line. However we check the destination now whether the xattrs have been copied. ;) Fedora is green again.

    @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
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants