Add getregs()/setregs()/getregset()/setregset() for Linux/musl/aarch64#2502
Conversation
orhun
force-pushed
the
getregs/aarch64-unknown-linux-musl
branch
from
37f409a to
eb6acf0
Compare
Here is a test, maybe you can take a look and try enabling it for musl as well:) |
orhun
changed the title
getregs()/getregset()/setregset() for Linux/musl/aarch64
|
I expanded the scope of this PR a bit and enable the tests. Thought I still wonder why Linux/musl/aarch64 wasn't enabled in the initial implementation in #2044 |
|
The tests seem to be passing 🤔 |
I think it is because the author's use case does not target
Looks like |
SteveLauC
changed the title
getregs()/getregset()/setregset() for Linux/musl/aarch64getregs()/setregs()/getregset()/setregset() for Linux/musl/aarch64
SteveLauC
left a comment
SteveLauC
left a comment
There was a problem hiding this comment.
LGTM, thanks for the patch!
|
Awesome, thanks for the merge! When should we expect a new release? 😇 |
Probably not soon, the I/O safety port is still not done, even though we finish it quickly, we still need to test it. Can you guys use a git dependency, though this would stop you from releasing to crates.io, I am not sure if this will block lurk from being included in Alpine Linux |
|
Thanks for letting me know!
Yeah, weekly alpha/rc releases would be really useful for these cases. Both for accessing the versions quickly and testing it :)
I'm not sure as well, just asked them 👀 |
| all(target_env = "musl", target_arch = "aarch64") | ||
| ) | ||
| ))] | ||
| pub fn getregset<S: RegisterSet>(pid: Pid) -> Result<S::Regs> { |
There was a problem hiding this comment.
We forgot to enable the RegisterSet trait and enum RegisterSetValue for linux/aarch64/musl in this PR, I found this while reviewing #2507, the CI does not catch it as well because we don't have CI for this triple:<
There was a problem hiding this comment.
I added support for aarch64-unknown-linux-musl for getregs() function and it builds fine now, but I don't have a way to test it.
I am kinda surprised that Rust does not catch this at compile time. 🤔
@orhun, could you please give this a fix and if possible, give this a try with lurk?
There was a problem hiding this comment.
I try to fix this issue and add the CI for aarch64/musl in #2510, but it looks like the constants needed by enum RegisterSetValue have not been defined in the libc crate, so we need to add them there first.
There was a problem hiding this comment.
Ah damn, good catch, these arch stuff goes deep!
Do you want me to test it out with lurk? Is there any other thing that I can help here?
There was a problem hiding this comment.
these arch stuff goes deep!
Exactly 😪
Do you want me to test it out with lurk?
Yeah, that would be great! Once #2510 is merged, we can start the test:)
Is there any other thing that I can help here?
Currently, no, I may ping you when I do need you 😁
2 participants
What does this PR do
I needed this specific API for packaging my project for Alpine Linux distribution:
I added support for
aarch64-unknown-linux-muslforgetregs()function and it builds fine now, but I don't have a way to test it.Also, I'm not sure if this is the correct way to add support for this architecture - but should be fine!
Let me know what you think and I can try adding support for other functions (such as
setregs()) as well.BTW this is my first PR to the
nixcrate so please let me know if I'm doing something wrong 🐻Checklist:
CONTRIBUTING.md