homepage

shutil doesn't provide any function which copies extended attributes on files. Note that "cp -a" does copy xattrs, but "cp -p" doesn't (and copy2() claims to work like "cp -p").

> copy2() claims to work like "cp -p"
It probably does, for a behavior of "cp -p" that predates extended attributes <wink>.
Do you think the best way is to always copy xattrs, add a new parameter, add a new copy function?

I'd tend to always copy xattrs – it seems that's what the user would expect to happen. A new parameter to _forbid_ it might make sense. However, I feel that there are already enough parameters in place. :-/

I'm also in favor of adding extended attributes to copy2:
"""
Similar to shutil.copy(), but metadata is copied as well 
"""
extended attributes are metadata. And there are already too many copy functions...

If nobody objects, I'd cook up a patch.

Sounds good.

This ticket has a small catch:
There are several namespaces. According to http://en.wikipedia.org/wiki/Xattr#Linux :
- user: can be set by anyone
- trusted: root only
- security: root only
- system: even root can’t do that, at least not in my vm
I’m writing a shutil.copyxattr() first which could simple get another argument for the namespaces that should be copied.
However what to do inside copy2()?
I’m tending to either:
1. copy only user.*
2. ignore errors in any namespace != user
Personally, I find the second approach rather non-deterministic.
So I’d suggest:
- copyxattr has an extra argument called namespaces with default being ['user'], so that in theory someone who wants to do something more sophisticated can do it.
- copy2 copies only user.* though because that’s what you usually want.
Please let me know what you think about it.

> I’m writing a shutil.copyxattr() first which could simple get another
> argument for the namespaces that should be copied.
Sounds good to me :-)
> However what to do inside copy2()?
> 
> I’m tending to either:
> 
> 1. copy only user.*
> 2. ignore errors in any namespace != user
> 
> Personally, I find the second approach rather non-deterministic.
But it's also more practical, e.g. when running as root you would
probably be surprised if only a subset of xattrs get copied, wouldn't
you? "Practicality beats purity."
For reference, here is part of the documentation for GNU cp's "-a"
option:
`-a'
`--archive'
 Preserve as much as possible of the structure and attributes of the
 original files in the copy (but do not attempt to preserve internal
 directory structure; i.e., `ls -U' may list the entries in a copied
 directory in a different order). Try to preserve SELinux security
 context and extended attributes (xattr), but ignore any failure to
 do that and print no corresponding diagnostic. Equivalent to `-dR
 --preserve=all' with the reduced diagnostics.
Meaning that "cp -a" tries to copy all xattrs and silences errors when
it's not possible to do so.
"cp --preserve=all" seems to have a similar error-silencing behaviour:
 `all'
 Preserve all file attributes. Equivalent to specifying all
 of the above, but with the difference that failure to
 preserve SELinux security context or extended attributes does
 not change `cp''s exit status. In contrast to `-a', all but
 `Operation not supported' warnings are output.

Ok, so I’ve added a function `copyxattr()` and `copy2()` tries to copy all possible namespaces.
Tests pass on Linux and Mac OS X.

Hynek, did you get a notification of my review on Rietveld?

I didn't. :/ I'll look into it tomorrow.

I have answered to the (two weeks old :-/) review. There are three open questions in there we'll have to figure out before I fix the patch:
- should copyxattr() remove xattrs in dst that aren't present in src? Make it an option like `remove_missing_xattr`?
- use "None" for `namespaces` in copyxattrs() to indicate we want to copy all of them?
- add a ignore_errors option?
ISTM, that "all namespaces" don't make much sense without ignore_errors as there seem to be some internal xattr etc. 
Suggestion: copyxattrs() has ignore_errors as default and returns a list of xattr it couldn't copy as (xattr, exception) tuples? Or an "on error" handler like in rmtree? I'd prefer the first one as ISTM that failures happen more often than not.

I really like the idea of adding extended attributes copy to shutil.cop2().
However, I'm not convinced that exposing the raw copyxattr() is
necessary (I can't really come up with a use case for this).
So I'd suggest make copyxattr() private, and wait until someone asks
for the functionality.

> I really like the idea of adding extended attributes copy to shutil.cop2().
> However, I'm not convinced that exposing the raw copyxattr() is
> necessary (I can't really come up with a use case for this).
> 
> So I'd suggest make copyxattr() private, and wait until someone asks
> for the functionality.
I agree. If someone needs it, (s)he will be able to advice on useful
semantics re replacement. For copy2() it’s a non-issue.

So here's an updated patch. I've stripped everything away we don't need for copy2 integration. It catches EPERM, ENOTSUP and ENODATA (for race conditions).
Happy reviewing. :)

The copyxattr() function should be private (_copyxattr()).
For some reason the tests are failing here:
======================================================================
ERROR: test_copy2_xattr (test.test_shutil.TestShutil)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "/home/antoine/cpython/default/Lib/test/test_shutil.py", line 410, in test_copy2_xattr
 os.setxattr(src, 'user.foo', b'42')
OSError: [Errno 95] Operation not supported
======================================================================
ERROR: test_copyxattr (test.test_shutil.TestShutil)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "/home/antoine/cpython/default/Lib/test/test_shutil.py", line 296, in test_copyxattr
 os.setxattr(src, 'user.foo', b'42')
OSError: [Errno 95] Operation not supported

> The copyxattr() function should be private (_copyxattr()).
Ok. I presumed that not adding it to __all__ is "private enough".
> For some reason the tests are failing here:
> 
> ======================================================================
> ERROR: test_copy2_xattr (test.test_shutil.TestShutil)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/home/antoine/cpython/default/Lib/test/test_shutil.py", line 410, in test_copy2_xattr
> os.setxattr(src, 'user.foo', b'42')
> OSError: [Errno 95] Operation not supported
> 
> ======================================================================
> ERROR: test_copyxattr (test.test_shutil.TestShutil)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/home/antoine/cpython/default/Lib/test/test_shutil.py", line 296, in test_copyxattr
> os.setxattr(src, 'user.foo', b'42')
> OSError: [Errno 95] Operation not supported
Looks like your file system Python uses for tmp files doesn't support
xattr. That's bad because you can't verify. How should I cope with that?
try/catch on the first setxattr() and skip the test if it fails? Is the
an official way to do that?

> Looks like your file system Python uses for tmp files doesn't support
> xattr. That's bad because you can't verify. How should I cope with that?
> try/catch on the first setxattr() and skip the test if it fails? Is the
> an official way to do that?
Well, apparently the extended attributes tests in test_os get skipped
with the following message: "no non-broken extended attribute support".
Perhaps you need the same condition somewhere.

Ok, I've extracted the xattr checker from test_os.py and transformed it into a caching decorator like skip_unless_symlinks.
_copyxattr rename is also done.

The patch looks ok, but it would be nice if someone with non-broken xattr support could test it. Although I suppose you did run the test suite, Hynek?

I did. The only error I got was
ERROR: test_idna (test.test_socket.GeneralModuleTests)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "/home/vagrant/p2/Lib/test/test_socket.py", line 1182, in test_idna
 socket.gethostbyname('испытание.python.org')
socket.gaierror: [Errno -3] Temporary failure in name resolution
I presume it's unrelated. :)

New changeset 85824b819bcb by Antoine Pitrou in branch 'default':
Issue #14082: shutil.copy2() now copies extended attributes, if possible.
http://hg.python.org/cpython/rev/85824b819bcb 

Pushed now. Hopefully the buildbots won't moan.

Looks like we have our first buildbot failure:
======================================================================
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 346, in test_copyxattr
 self.assertEqual(['user.bar'], os.listxattr(dst))
AssertionError: Lists differ: ['user.bar'] != ['security.selinux', 'user.bar...
First differing element 0:
user.bar
security.selinux
Second list contains 1 additional elements.
First extra element 1:
user.bar
- ['user.bar']
+ ['security.selinux', 'user.bar']

Hynek's patch (communicated over IRC, committed in 8d85f9920878) seems to have fixed the failure.