-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Separate L4Re code, add aarch64 and enable tests #4383
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
base: main
Are you sure you want to change the base?
Conversation
7de90e1
to
f452341
Compare
edit: resolved (see below) @tgross35 The nightly freebsd fails are not due to this change. But I don't understand why the style check of src/l4re/mod.rs fails. I executed
is locally successful for me (with rustfmt 1.8.0-nightly (2fa8b11f09 2025-04-06) from current rust nightly in rustup). Any idea why? |
ddd5281
to
e83d80f
Compare
This was because I did not run the style check with the extra_traits feature. I fixed that. However it now shows that
fails but also that succeeds for me locally and I sorted the file via Update: sort apparently sorts differently depending on LC_ALL, I sorted again with LC_ALL=C |
592e367
to
d54c3e6
Compare
The L4Re code was previously attached to the Linux code which was not correct in many ways. This commit separates the L4Re code and enables the libc-tests and includes the fixes for the failing tests. Aarch64 is added as a second supported architecture (more to come).
Thanks for this PR, I'll try to take a look soon. We recently decided to do some reorganization that would flip the module order around, so e.g. |
Good idea to turn the order around! The thing is that much of the L4Re support was incorrect while still attached to the linux code path and the change also removes unnecessary l4re exceptions from the unix, linux_like and linux mods. So I think, the refactoring (turning the order around) would be easier if we applied this change first. I also can help with the refactoring once you want to start with it. Additionally, since we would like to bring aarch64 support for L4Re into the main rust repo (to be able to build for this arch) and the support in this crate is a precondition, my preference would be to have it merged before the refactoring. Would that be ok for you? |
@tgross35 any thoughts about this? do you think we can get the aarch64 support into the next libc release? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry this has taken so long; there is a massive diff here and it has taken me a while to get through it.
It seems like a lot of this code comes verbatim from linux/mod.rs
; there is still config for musl, arm, 32-bit x86, and other things that aren't relevant for our l4re targets. This would require significant cleanup and we would still have nearly identical code in two different places, so unfortunately I don't think the approach here is the best way forward.
The refactoring I mentioned above is still a little while off so we shouldn't wait for that. Instead, I think we could work with something like this:
- Move this PR's
src/l4re/mod.rs
tolinux_like/l4re/{mod.rs, ...}
- Move the code you need from
linux_like/linux/mod.rs
tolinux_like/shared.rs
- Import everything from
shared
for both l4re and linux #[cfg(...)]
out anything relevant inshared
(or the unix parent modules) that is on linux but not l4re
The last step is going to be the trickiest since we don't do this anywhere yet, but will need a pattern like this in the near future anyway. The style check might yell at you but you can disregard it for now (will need to be updated before this merges).
Similarly,, libc-test/build.rs
should use test_linux
and gate relevant bits behind if l4re
, rather than duplicating most of that function.
Description
The L4Re code was previously attached to the Linux code which was not correct in many ways. This commit separates the L4Re code and enables the libc-tests and includes the fixes for the failing tests. Aarch64 is added as a second supported architecture (more to come).
Sources
L4Re-adapted version of uclibc: https://github.com/kernkonzept/l4re-core/tree/master/uclibc/lib.
Checklist
libc-test/semver
have been updated*LAST
or*MAX
areincluded (see #3131)
cd libc-test && cargo test --target mytarget
);especially relevant for platforms that may not be checked in CI