-
Notifications
You must be signed in to change notification settings - Fork 1.2k
musl: 64-bit time support #4463
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
|
Hmm, seems like the style checker isn't happy with mixing |
|
Since #4433 seems close to merging, I can rebase on top of it once merged. |
8b32bb3 to
e91f182
Compare
|
Rebased on top of the GNU changes :) |
|
Thanks! Sorry it's taken me a while, I'll try to look at this very soon |
tgross35
left a comment
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.
Thanks for working on this! Left a few comments here.
For the style check, are the changes it is suggesting possible? If not, that will probably have to be updated unfortunately (hopefully we will be able to reorganize things at some point so it's less annoying...)
|
Thanks for the review! I'll get around to implementing the changes hopefully in the next few days. I'll try figure out how to pass the style lints - but it might need a new file. After this merges and when 1.0 is closer to release feel free to ping me on GitHub and I can help fully complete the transition for these changes in the 1.0 branch on the musl side :). |
9b5ca40 to
f669296
Compare
Fyi we're skipping hexagon now, should be picked up if you rebase |
|
@xbjfk gentle ping here, would you be able to take a look at this? If you're able to rebase I can probably apply at least a few of the patches from this series, to reduce the remaining surface area. |
This comment has been minimized.
This comment has been minimized.
c20d0e7 to
b2c6f8e
Compare
|
Thanks for the nudge! Have rebased and applied most comments. Still need to confirm why |
src/new/mod.rs
Outdated
| } else if #[cfg(target_os = "emscripten")] { | ||
| mod emscripten; | ||
| pub(crate) use emscripten::*; | ||
| pub use musl::sched::sched_util; |
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.
For the test failure: drop this bit...
src/unix/linux_like/mod.rs
Outdated
| #[cfg(not(any( target_env = "musl", target_os = "emscripten", target_env = "ohos")))] | ||
| pub struct sched_param { | ||
| pub sched_priority: c_int, | ||
| #[cfg(any(target_env = "musl", target_os = "emscripten", target_env = "ohos"))] | ||
| pub sched_ss_low_priority: c_int, | ||
| #[cfg(any(target_env = "musl", target_os = "emscripten", target_env = "ohos"))] | ||
| pub sched_ss_repl_period: crate::timespec, | ||
| #[cfg(any(target_env = "musl", target_os = "emscripten", target_env = "ohos"))] | ||
| pub sched_ss_init_budget: crate::timespec, | ||
| #[cfg(any(target_env = "musl", target_os = "emscripten", target_env = "ohos"))] | ||
| pub sched_ss_max_repl: c_int, | ||
| } |
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.
... and restore these fields, but with only #[cfg(target_os = "emscripten")] (update the cfg(not) of course.
Or create a src/new/emscripten/sched.rs file and copy the deprecated version from musl there.
Oh, sorry, that's most of what I just left comments about 😆 I think this is very close to ready though |
The structures are the same - and this follows the upstream definition On musl at least, blkcnt64_t == blkcnt_t == i64 and ino_t == ino64_t == u64
This feature is enabled with independently from musl_v1_2_3 to support time64. Defining this feature makes this roughly equivalent to upstream commit bminor/musl@f12bd8e.
This is equivalent to upstream commit bminor/musl@9b2921b.
This corresponds to upstream commit bminor/musl@1febd21 (most symbols) and bminor/musl@22daaea (only dlsym)
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
A bunch of properties were removed upstream and set to reserved. This matches upstream commit bminor/musl@827aa8f and bminor/musl@2d69fcf
d15a765 to
81dcc04
Compare
Change time_t type to i64 Change struct stat, msqid_ds and shmid_ds to reflect This commit follows upstream bminor/musl@3814333 and bminor/musl@d6dcfe4 It also implements a fix from bminor/musl@0fbd7d6
This is primarily based on a small part of bminor/musl@3814333. This also integrates bminor/musl@3c02bac, which update MSG_STAT, SEM_STAT, SEM_STAT_ANY. These are based on the value of IPC_STAT, however we can just use `cfg` as it is effectively the same.
This fixes test failures on musl.
Namely, this allows `target_endian` as well as adds a constant array where certain configs are explicitly allowed (e.g. musl32_time64).
Unfortunately does not seem to handle cfg_if well
This reverts commit 34e7583.
This reverts commit 72fe537.
|
Okay - I should have resolved everything. I added a commit to change I also reverted the commit which disabled time related tests. I'll fix any test failures hopefully tomorrow. |
Description
This change includes time64 support for applicable architectures (x86, arm, mips and powerpc). This is based on the previous PRs to this repo as well as the musl changelog from 1.1.24 -> 1.2. It can be enabled with the environment variable
RUST_LIBC_UNSTABLE_MUSL_TIME64only when musl_v1_2_3 is enabled and the architecture is supported.A lot of structures, especially ones with mixed endian became excessively complicated, so I used
cfg_ifto separate them. It looks like you can only nests! {}incfg_if! {}, but not vice versa.As a note, I'm considering removing
musl_not_time64, and just keeping the old logic of allowing deprecated for function definitions involvingtime_tas it introduces necessary complexity.When libc 1.0 is released, I believe the best path would be to remove the
musl_v1_2_3feature, making it unconditionally enabled, keepingmusl_time64which will expand to(musl && time64_arch).Tested through QEMU for all architectures.
Sources
Sources are located on each commit, in the form of upstream commits
Checklist
libc-test/semverhave been updated*LASTor*MAXareincluded (see rust-lang/libc#3131)
cd libc-test && cargo test --target mytarget);especially relevant for platforms that may not be checked in CI
@rustbot label +stable-nominated