classification
Title: shutil.copystat should copy Linux extended attributes
Type: enhancement Stage: resolved
Components: Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: larry Nosy List: georg.brandl, hynek, larry, ned.deily, python-dev, serhiy.storchaka, skrah
Priority: normal Keywords: patch

Created on 2012-07-01 15:13 by larry, last changed 2012-07-16 15:37 by hynek. This issue is now closed.

Files
File name Uploaded Description Edit
larry.copystat.xattrs.1.diff larry, 2012-07-03 07:16 patcho uno review
Messages (13)
msg164483 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-07-01 15:13
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?
msg164584 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-07-03 07:16
First patch.
msg165296 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2012-07-12 11:21
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.
msg165298 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-07-12 11:36
Yeah, good to go.  But please re-insert a blank line in the test suite changes after the end of the method.
msg165305 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-07-12 15:03
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?
msg165327 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2012-07-12 18:39
See open Issue12978.
msg165369 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2012-07-13 07:34
> 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.
msg165479 - (view) Author: Roundup Robot (python-dev) Date: 2012-07-15 00:55
New changeset 5f62c317c202 by Larry Hastings in branch 'default':
- Issue #15238: shutil.copystat now copies Linux "extended attributes".
http://hg.python.org/cpython/rev/5f62c317c202
msg165588 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-07-16 11:26
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']
msg165591 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2012-07-16 12:00
You can’t just compare the xattr because some (like security.*) can be
only copied as root. IIRC they come from SELinux.
msg165622 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-07-16 14:51
Hynek, it's clear you understand this far better than I do.  Could I get you to fix the Fedora buildbot problem, etc, etc?
msg165623 - (view) Author: Roundup Robot (python-dev) Date: 2012-07-16 15:15
New changeset 265e36e277f3 by Hynek Schlawack in branch 'default':
#15238: Fix xattr related shutil.copystat tests
http://hg.python.org/cpython/rev/265e36e277f3
msg165626 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2012-07-16 15:37
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.
History
Date User Action Args
2012-07-16 15:37:12hyneksetmessages: + msg165626
2012-07-16 15:15:25python-devsetmessages: + msg165623
2012-07-16 14:51:54larrysetmessages: + msg165622
2012-07-16 12:00:31hyneksetmessages: + msg165591
2012-07-16 11:26:36skrahsetnosy: + skrah
messages: + msg165588
2012-07-15 01:00:58larrysetstatus: open -> closed
resolution: fixed
stage: needs patch -> resolved
2012-07-15 00:55:27python-devsetnosy: + python-dev
messages: + msg165479
2012-07-13 07:34:23hyneksetmessages: + msg165369
2012-07-12 18:39:30ned.deilysetnosy: + ned.deily
messages: + msg165327
2012-07-12 15:03:01larrysetmessages: + msg165305
2012-07-12 11:36:15georg.brandlsetmessages: + msg165298
2012-07-12 11:21:34hyneksetmessages: + msg165296
2012-07-03 07:16:30larrysetfiles: + larry.copystat.xattrs.1.diff
keywords: + patch
messages: + msg164584
2012-07-01 15:14:54pitrousetnosy: + hynek
2012-07-01 15:13:28larrycreate