l4re: change bit widths of file offset types - #5173
Conversation
a64dd66 to
66c2f42
Compare
66c2f42 to
244c00b
Compare
This comment has been minimized.
This comment has been minimized.
244c00b to
f0bf0bf
Compare
This comment has been minimized.
This comment has been minimized.
caac209 to
e16db76
Compare
|
I'm pretty fine deprecating these since the platform has pretty low use, but we should figure out the other uclibc PR first. @SnoozeThis #5165 -> remove label S-blocked, add label S-waiting-on-review |
|
(https://snoozeth.is/u1Hjq3aITKU) I will wait until #5165 is merged and then add label S-waiting-on-review and remove label S-blocked. @rustbot claim. |
e16db76 to
0c5f454
Compare
This comment has been minimized.
This comment has been minimized.
e1d5fdc to
fb8fb9e
Compare
This comment has been minimized.
This comment has been minimized.
fb8fb9e to
a2d1a13
Compare
This comment has been minimized.
This comment has been minimized.
a2d1a13 to
e09560e
Compare
|
@rustbot blocked |
@farao I'm not sure I understand what you mean here. I don't think there's a When I pointed out that it was odd, I meant that we should be getting LFS |
This comment has been minimized.
This comment has been minimized.
3abdeba to
95633b6
Compare
This comment has been minimized.
This comment has been minimized.
|
Sorry, I was apparently confused. I had a closer look now and I believe the For flock64, there's an additional problem since flock64 is currently missing completely from the l4re part of the libc crate (which I believe is just a mistake, it should be added). Btw, it might very well be that we are going to add 32 bit targets in the next year for l4re. Just to keep it in mind that optimizations based on targets missing for 32 bits might need to be reversed in the future. |
95633b6 to
d1595a2
Compare
This comment has been minimized.
This comment has been minimized.
|
Thanks for clarifying. I just changed the patch to only conditionally I don't think we can rename the How do you want to handle the |
d1595a2 to
8ae5d8b
Compare
| // FIXME(1.0,deprecate): lfs binding to be removed | ||
| #[cfg_attr( | ||
| all(target_os = "l4re", target_pointer_width = "64"), | ||
| deprecated( | ||
| since = "0.2.190", | ||
| note = "Use `fstatfs` instead. LFS is being phased out, see \ | ||
| rust-lang/libc#4805." | ||
| ) | ||
| )] | ||
| pub fn fstatfs64(fd: c_int, buf: *mut statfs64) -> c_int; |
There was a problem hiding this comment.
Fwiw I think the comment covers these, we may as well deprecate everything at once
There was a problem hiding this comment.
Done. Except for those where there was no FIXME comment.
I think adding flock64 would be the correct thing to do, would you agree? |
8ae5d8b to
19d2499
Compare
This comment has been minimized.
This comment has been minimized.
Done. @rustbot ready |
c8f396c to
74915e8
Compare
|
I ran libc-test again, there remains a problem with __sword_type: The name in the uclibc-ng is actually __SWORD_TYPE but I think that's not ok to use as a name as it goes against the rust naming conventions? Either way, please remove the declaration of fsword_t from src/unix/linux_like/l4re/uclibc/*/mod.rs and in order to get rid of the warnings above adapt this line: Line 4235 in 1e84ea3 |
The entirety of libc goes against Rust naming conventions :) so feel free to match source |
74915e8 to
9c46506
Compare
This comment has been minimized.
This comment has been minimized.
|
Done. I've ketp the same casing as is prevalent across type aliases in |
|
Now please remove the fsword_t declaration in the two arch specific files (otherwise that will also prevent the tests to be run) |
Add deprecation warnings to LFS bindings under L4Re uClibc. This patch adds notices to types and routines. The only supported target is 64-bits wide. Its suffixed and unsuffixed types should always be equivalent.
|
I don’t think I can outright remove the symbol. I’ve deprecated it and have set up a skip for it in libc-test’s build script (I missed setting up the skip yesterday.) That should make the tests pass again. |
9c46506 to
13c23b6
Compare
|
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. |
Description
This PR deprecates a number of symbols from the
unix/linux_like/l4remodule. This has also come with further deprecations and modifications in other top-level modules.At present, the definitions in upstream l4re-core are always 64-bit wide for the 64-bit targets we seem to support in this module. Though rustc only has support for x86_64 under this target OS.
Sources
Sources on the uClibc changes are documented in #5165. A regex search of the affected symbols on l4re-core's
libcdirectory reveals no modifications in their patches would necessitate changes in this patch.Checklist
libc-test/semverhave been updated*LASTor*MAXare included (see #3131)cd libc-test && cargo test --target mytarget); especially relevant for platforms that may not be checked in CI@rustbot label +stable-nominated