Add ENOATTR for Linux #594
Conversation
libc-test/build.rs
Outdated
| @@ -171,6 +171,7 @@ fn main() { | |||
| if !uclibc { | |||
| // optionally included in uclibc | |||
| cfg.header("sys/xattr.h"); | |||
| cfg.header("attr/xattr.h"); | |||
There was a problem hiding this comment.
Not sure about this, but we test uclibc right?
PlasmaPower
force-pushed
the
linux-enoattr
branch
from
b17d7ea to
0f2ef41
Compare
|
The |
|
Also, this change is failing the linter as the constant definition comes at the end, but I think it needs to be there to use the platform specific |
|
Oh right now we don't actually test uclibc on CI yet, so it's ok to back out those changes if you'd like. Also no need to worry about definition order, it doesn't have to literally come after |
|
How should I deal with needing the attr headers for tests? They aren't present on some test systems. |
|
Whatever gets past CI is fine by me, I don't particularly have opinions how it gets past CI |
PlasmaPower
force-pushed
the
linux-enoattr
branch
2 times, most recently
from
1166da2 to
19e03f8
Compare
|
Do you mind if I just ignore the constant in the tests? The setxattr and getxattr man pages on linux specifically say |
|
Hm man pages typically tend to be right for one architecture and I've seen they don't always tend to be right across all of them. Do you know why this header can't be found? |
|
It comes from |
|
Because of that, maybe it shouldn't be in this Rust crate, but I think it's very important as other platforms define it and it's explicitly mentioned in the man page. |
|
From the libattr source: http://download.savannah.gnu.org/releases/attr/attr-2.4.47.src.tar.gz
#include <errno.h>
#ifndef ENOATTR
# define ENOATTR ENODATA /* No such attribute */
#endifIf defined, |
|
If this constant is only available on architecture, perhaps it could just be included there? The package should be installable via the various docker images we've got. |
PlasmaPower
force-pushed
the
linux-enoattr
branch
from
19e03f8 to
1713b76
Compare
|
Okay, I'll see if I can install them in docker. ENOATTR is available for all architectures. It's just that libattr isn't installed by default on all platforms. |
|
Hm if this isn't actually available in libc itself but requires an extra package to be installed, should it be included in the libc crate itself? |
|
I'm not sure. It is in libc for other platforms, so it's useful for cross-platform code. It's also mentioned in the |
|
Ok, well it sounds like the story here is fishy enough that it seems safe to just skip the constant, mind adding these comments to a comment as to why we're skipping it? |
PlasmaPower
force-pushed
the
linux-enoattr
branch
from
1713b76 to
98889cf
Compare
|
Done. I'm assuming we don't want rustdoc comments for the constant definition? We don't have them for any other constants. |
|
Nah that's ok, we typically don't have lots of rustdoc docs in libc (we rely on libc itself documentation for that) @bors: r+ |
|
📌 Commit 98889cf has been approved by |
|
⌛ Testing commit 98889cf with merge a3e79eb... |
Add ENOATTR for Linux It's defined differently for OSX and *BSD, so having a Linux definition is helpful for cross-platform code.
|
☀️ Test successful - status-appveyor, status-travis |
Add ENOATTR for Android PR based on #594 It's defined in Android sysroot so it should work without test workaround (CI should catch it otherwise?).
It's defined differently for OSX and *BSD, so having a Linux definition is helpful for cross-platform code.