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

add listxattr/getxattr/setxattr/removexattr on Linux and Android #1847

Closed

Conversation

SteveLauC
Copy link
Member

@SteveLauC SteveLauC commented Oct 14, 2022

What this PR does:

This PR adds listxattr, getxattr, setxattr, removexattr and their variants on Linux and Android

pub fn listxattr<P: ?Sized + NixPath>(path: &P) -> Result<Vec<OsString>>
pub fn llistxattr<P: ?Sized + NixPath>(path: &P) -> Result<Vec<OsString>>
pub fn flistxattr(fd: RawFd) -> Result<Vec<OsString>>

pub fn getxattr<P, S>(path: &P, name: S) -> Result<Vec<u8>>
pub fn lgetxattr<P, S>(path: &P, name: S) -> Result<Vec<u8>>
pub fn fgetxattr<S>(fd: RawFd, name: S) -> Result<Vec<u8>>

pub fn setxattr<P, S, B>(
    path: &P,
    name: S,
    value: B,
    flags: SetxattrFlag,
) -> Result<()>
pub fn lsetxattr<P, S, B>(
    path: &P,
    name: S,
    value: B,
    flags: SetxattrFlag,
) -> Result<()>
pub fn fsetxattr<S, B>(
    fd: RawFd,
    name: S,
    value: B,
    flags: SetxattrFlag,
) -> Result<()>

pub fn removexattr<P, S>(path: &P, name: S) -> Result<()>
pub fn lremovexattr<P, S>(path: &P, name: S) -> Result<()>
pub fn fremovexattr<S>(fd: RawFd, name: S) -> Result<()>

man pages

What about other platforms

NetBSD should also be added in this PR, but their current libc bindings are wrong.

Update: I just found that NetBSD has another collection of xattr APIs, which is compatible with FreeBSD, and the libc bindings are added in libc#1114. Then I prefer to put these two BSD OSes together.

Module

These APIs are placed in sys/xattr.rs

Tests

Tests are placed in test/sys/test-xattr.rs. Symbolic link versions of APIs are not covered.

If they should be covered, I will add them.

pub fn llistxattr<P: ?Sized + NixPath>(path: &P) -> Result<Vec<OsString>>
pub fn lgetxattr<P, S>(path: &P, name: S) -> Result<Vec<u8>>
pub fn lsetxattr<P, S>(
    path: &P,
    name: S,
    value: B,
    flags: SetxattrFlag,
) -> Result<()>
pub fn lremovexattr<P, S>(path: &P, name: S) -> Result<()>

In the tests, temporary directory are created in the current working directory instead of under /tmp, this is needed because we need to test user EA, tmpfs does not support this, see this answer.

@asomers
Copy link
Member

asomers commented Nov 29, 2022

I haven't read the whole PR yet. But does this have any advantages over the existing xattr crate?

@SteveLauC
Copy link
Member Author

SteveLauC commented Nov 29, 2022

I haven't read the whole PR yet. But does this have any advantages over the existing xattr crate?

Yes, it has some minor advantages, see Why another crate for EA? Any difference from xattr.

As you can see, I have made a dedicated crate for this, and have almost all the platforms supported. If you want it to be merged into nix, I am happy to contribute. If you don't, feel free to close this PR.

@asomers
Copy link
Member

asomers commented Nov 29, 2022

Yes, it has some minor advantages, see Why another crate for EA? Any difference from xattr.

As you can see, I have made a dedicated crate for this, and have almost all the platforms supported. If you want it to be merged into nix, I am happy to contribute. If you don't, feel free to close this PR.

And this PR merges the entirety of your extattr crate into Nix? Are you intending to shut down the other one?

@SteveLauC
Copy link
Member Author

And this PR merges the entirety of your extattr crate into Nix?

I have no strong view on this, it depends on if nix wants it or not. It does not matter whether these bindings exist in nix or extattr as long as Rust users have such an option. Though I do tend to get this merged into nix as nix has some great facilities for handling UNIX syscalls.

Are you intending to shut down the other one?

If nix wants it, I won't shut it down immediately, adding some deprecation messages to let users (if I have) migrate to nix would be the option.

@asomers
Copy link
Member

asomers commented Nov 29, 2022

Ok, but I don't have time to review it now. Postponing to the next release.

@asomers asomers added this to the 0.27.0 milestone Nov 29, 2022
@asomers
Copy link
Member

asomers commented Aug 11, 2023

I just got back around to reviewing this, because I'm going to make a new release soon. I looked at your extattr crate, and I like it. In fact, I think I have another project that could benefit from it. However, Nix, as you may have noticed, has been getting rather bloated. And since you've already got this stuff wrapped up in a neatly self-contained crate, I propose that we keep it there rather than merge into Nix. Do you agree with that?

@SteveLauC
Copy link
Member Author

I just got back around to reviewing this, because I'm going to make a new release soon.

Happy to hear that there will be a new release!

I propose that we keep it there rather than merge into Nix. Do you agree with that?

Yeah, Nix is indeed some kind of bloated, so i am ok with this proposal, I will close this PR.

@SteveLauC SteveLauC closed this Aug 11, 2023
@SteveLauC SteveLauC deleted the xattr-syscalls-on-Android-and-Linux branch December 11, 2023 22:41
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.

None yet

2 participants