search

iustin / pyxattr

A python module for accessing filesystem Extended Attributes
https://pyxattr.k1024.org/
GNU Lesser General Public License v2.1
30 stars 15 forks source link

xattr.c: There is no more attr/xattr.h with >=attr-2.4.48 #15

Closed Polynomial-C closed 6 years ago

Polynomial-C commented 6 years ago

See also: http://git.savannah.nongnu.org/cgit/attr.git/commit/?id=7921157890d07858d092f4003ca4c6bae9fd2c38

iustin commented 6 years ago

Thanks for the patch, but - I don't see any version checks, so how will this work when compiled against older versions, especially ones that - like Debian Hurd - don't define ENOATTR?

Polynomial-C commented 6 years ago

To be honest I only looked at attr/xattr.h from attr-2.4.47 which simply redefines ENOATTR:

#ifndef ENOATTR
# define ENOATTR ENODATA        /* No such attribute */
#endif
Polynomial-C commented 6 years ago

Wondering what the status is here. Do you want me to change something? To be honest, you are the first one asking for a version check. I've never seen this in any other project using xattr. And I tried to make this backward compatible. That's why I replaced ENOATTR with ENODATA. That should also be available in GNU Hurd.

iustin commented 6 years ago

Hey, sorry, I just dropped the ball on this one. I'll review the patch on the weekend.

My comment about version was just about backwards compatibility, if that is achieved simply via ifdefs, all good.

eli-schwartz commented 6 years ago

Status?

szpak commented 6 years ago

The pyxattr build in Fedora rawhide (a development version) started to fail recently after libattr update due to missing ENOATTR. I planned to patch it locally in RPM just for Fedora, but it would be good to have some common idea how to fix it in general.

Related Bugzilla ticket and build logs.

eli-schwartz commented 6 years ago

IMO the fix is to merge this PR and get rid of code deprecated for 3 years, with inlining the single define which the file provided.

It's what we did for Arch Linux: https://git.archlinux.org/svntogit/community.git/commit/trunk?h=packages/python-pyxattr&id=97d5258ce2c604c3c1c9366d536b5b65b2405ebf

iustin commented 6 years ago

Finally had some time to test. The main point I was missing is for how long this was available in glibc - since 2.3, which was released ages ago. As such, just switching is fine for any modern distribution.

I'll accept this PR, but it needs further small update to the setup.py file to remove the requirement of the attr library, since it will be no longer needed, and the README.

For the record, I found this discussion interesting/somewhat relevant: https://patchwork.ozlabs.org/patch/363203/.

iustin commented 6 years ago

Released in version 0.6.1.