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

options bugfix, pyxattr compatibility #38

Merged
merged 3 commits into from
Feb 28, 2016
Merged

options bugfix, pyxattr compatibility #38

merged 3 commits into from
Feb 28, 2016

Conversation

jmberg
Copy link
Member

@jmberg jmberg commented Apr 24, 2015

The first of these commits fixes a mix-up between XATTR_XATTR_* and XATTR_* options, which makes the options not work correctly (at least on Linux) because the constants are different; for example trying to XATTR_CREATE something will not actually fail if it exists because it really maps to nothing (XATTR_CREATE is 1, but so is XATTR_XATTR_NOFOLLOW, which obviously doesn't exist in the C API, so things get messed up.

The second adds a new compatibility module for pyxattr compatibility that can be imported as described in the commit log.

@etrepum
Copy link
Member

etrepum commented Apr 24, 2015

Looks like that makes the build fail on Mac (which is what Travis is testing here). I'll try and find some time to investigate this weekend if you don't have easy access to a Mac.

@jmberg
Copy link
Member Author

jmberg commented Apr 25, 2015

I don't, sorry. However, it looks like OSX is the only one that doesn't define XATTR_XATTR_*, so I guess it could/should just

#define XATTR_XATTR_NOFOLLOW XATTR_NOFOLLOW
...

and it seems likely that the

#ifndef XATTR_NOFOLLOW
#define XATTR_NOFOLLOW 0x0001
#endif

#ifndef XATTR_NOSECURITY
#define XATTR_NOSECURITY 0x0008
#endif

can be removed entirely.

The options passed to xattr are internally converted from XATTR_XATTR_*
to XATTR_* with the exception of NOFOLLOW which is in most cases handled
explicitly in the code.

In order to make this work properly, however, the correct constants need
to actually be used in python - that means the XATTR_XATTR_* ones must
be there rather than the OS's XATTR_* ones.
This adds compatibility with pyxattr version 0.5.3.

To use it in code written for pyxattr, import it like

  from xattr import pyxattr_compat as xattr

instead of just "import xattr".

It's about 100% slower but passes the pyxattr test suite.
The nofollow variable is set, but not used, while the
flag is removed from the 'options' and then tested.
Fix that by using the nofollow variable instead of
the useless test.
etrepum added a commit that referenced this pull request Feb 28, 2016
options bugfix, pyxattr compatibility
@etrepum etrepum merged commit cfddb9f into xattr:master Feb 28, 2016
@jmberg
Copy link
Member Author

jmberg commented Feb 28, 2016

Oh, thanks, I'd completely forgotten about this (after having decided that I didn't actually have xattrs worth backing up with bup, which I'd developed this for). I'll still try to get bup to use this though.

@etrepum
Copy link
Member

etrepum commented Feb 28, 2016

I had lost track of it as well, sorry about that!

clrpackages pushed a commit to clearlinux-pkgs/xattr that referenced this pull request Jul 12, 2016
Version 0.8.0 released 2016-02-28

* Use os.fsencode where available to better handle filesystem quirks related
  to surrogates
  xattr/xattr#46
* Options bugfix and compatibility module for pyxattr API
  xattr/xattr#38

Version 0.7.9 released 2016-02-12

* Added xattr/tests/*.py to MANIFEST.in
  xattr/xattr#43
clrpackages pushed a commit to clearlinux-pkgs/xattr that referenced this pull request Oct 24, 2016
Version 0.8.0 released 2016-02-28

* Use os.fsencode where available to better handle filesystem quirks related
  to surrogates
  xattr/xattr#46
* Options bugfix and compatibility module for pyxattr API
  xattr/xattr#38

Version 0.7.9 released 2016-02-12

* Added xattr/tests/*.py to MANIFEST.in
  xattr/xattr#43
clrpackages pushed a commit to clearlinux-pkgs/xattr that referenced this pull request Jan 11, 2017
Version 0.8.0 released 2016-02-28

* Use os.fsencode where available to better handle filesystem quirks related
  to surrogates
  xattr/xattr#46
* Options bugfix and compatibility module for pyxattr API
  xattr/xattr#38

Version 0.7.9 released 2016-02-12

* Added xattr/tests/*.py to MANIFEST.in
  xattr/xattr#43
clrpackages pushed a commit to clearlinux-pkgs/xattr that referenced this pull request Jan 17, 2017
Version 0.8.0 released 2016-02-28

* Use os.fsencode where available to better handle filesystem quirks related
  to surrogates
  xattr/xattr#46
* Options bugfix and compatibility module for pyxattr API
  xattr/xattr#38

Version 0.7.9 released 2016-02-12

* Added xattr/tests/*.py to MANIFEST.in
  xattr/xattr#43
clrpackages pushed a commit to clearlinux-pkgs/xattr that referenced this pull request Apr 18, 2017
Version 0.8.0 released 2016-02-28

* Use os.fsencode where available to better handle filesystem quirks related
  to surrogates
  xattr/xattr#46
* Options bugfix and compatibility module for pyxattr API
  xattr/xattr#38

Version 0.7.9 released 2016-02-12

* Added xattr/tests/*.py to MANIFEST.in
  xattr/xattr#43
clrpackages pushed a commit to clearlinux-pkgs/xattr that referenced this pull request May 5, 2017
Version 0.8.0 released 2016-02-28

* Use os.fsencode where available to better handle filesystem quirks related
  to surrogates
  xattr/xattr#46
* Options bugfix and compatibility module for pyxattr API
  xattr/xattr#38

Version 0.7.9 released 2016-02-12

* Added xattr/tests/*.py to MANIFEST.in
  xattr/xattr#43
clrpackages pushed a commit to clearlinux-pkgs/xattr that referenced this pull request May 25, 2017
Version 0.8.0 released 2016-02-28

* Use os.fsencode where available to better handle filesystem quirks related
  to surrogates
  xattr/xattr#46
* Options bugfix and compatibility module for pyxattr API
  xattr/xattr#38

Version 0.7.9 released 2016-02-12

* Added xattr/tests/*.py to MANIFEST.in
  xattr/xattr#43
clrpackages pushed a commit to clearlinux-pkgs/xattr that referenced this pull request Jul 15, 2017
Version 0.8.0 released 2016-02-28

* Use os.fsencode where available to better handle filesystem quirks related
  to surrogates
  xattr/xattr#46
* Options bugfix and compatibility module for pyxattr API
  xattr/xattr#38

Version 0.7.9 released 2016-02-12

* Added xattr/tests/*.py to MANIFEST.in
  xattr/xattr#43
jmberg added a commit to jmberg/bup that referenced this pull request Dec 10, 2019
Newer versions of python-xattr have pyxattr compatibility code
(since my pull request xattr/xattr#38).

Modify bup to allow using it; prefer pyxattr since it's faster,
but sometimes due to other dependencies python-xattr might be
present already (and since it conflicts with pyxattr, it's not
possible to install both.)

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
jmberg added a commit to jmberg/bup that referenced this pull request Dec 16, 2019
Newer versions of python-xattr have pyxattr compatibility code
(since my pull request xattr/xattr#38).

Modify bup to allow using it; prefer pyxattr since it's faster,
but sometimes due to other dependencies python-xattr might be
present already (and since it conflicts with pyxattr, it's not
possible to install both.)

Also add a .cirrus.yml task to test this on Debian.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
jmberg added a commit to jmberg/bup that referenced this pull request Dec 16, 2019
Newer versions of python-xattr have pyxattr compatibility code
(since my pull request xattr/xattr#38).

Modify bup to allow using it; prefer pyxattr since it's faster,
but sometimes due to other dependencies python-xattr might be
present already (and since it conflicts with pyxattr, it's not
possible to install both.)

Also add a .cirrus.yml task to test this on Debian.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
rlbdv pushed a commit to bup/bup that referenced this pull request Jan 9, 2020
Newer versions of python-xattr have pyxattr compatibility code
(since my pull request xattr/xattr#38).

Modify bup to allow using it; prefer pyxattr since it's faster,
but sometimes due to other dependencies python-xattr might be
present already (and since it conflicts with pyxattr, it's not
possible to install both.)

Also add a .cirrus.yml task to test this on Debian.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
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

Successfully merging this pull request may close these issues.

2 participants