-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Update Ubuntu to 25.10 #4698
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?
Update Ubuntu to 25.10 #4698
Conversation
I assume the failures may be coming from:
|
Account for [1]. [1]: bminor/glibc@3263675
The MS_NOUSER fix is easy. Baud rates I'm less sure about; our definitions match those from Linux https://github.com/torvalds/linux/blob/992d4e481e958c6898fe750afd429d1b585fff93/include/uapi/asm-generic/termbits-common.h but glibc doesn't match? https://github.com/bminor/glibc/blob/3fd794264e3f062bfbf0c8727cef82f16d51450b/bits/termios-baud.h. Guessing that the commit above made it so we're starting to see the glibc definitions. |
https://sourceware.org/bugzilla/show_bug.cgi?id=33340, totally missed the discussion here at #4692. |
pub const MS_NOUSER: c_ulong = 0xffffffff80000000; | ||
// FIXME: should this be an int? The suffix is `U` not `UL`. | ||
pub const MS_NOUSER: c_ulong = 1 << 31; |
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.
I wrote the glibc patch. Sorry, I didn't expect breakage here.
Here is my rational from the commit message [1]:
Using gcc -Wshift-overflow=2 -Wsystem-headers to compile a file
including <sys/mount.h> will cause a warning since 1 << 31 is undefined
behavior on platforms where int is 32-bits.
Technically unsigned integers aren't allowed in C enum types, it is a GCC extension that is widely supported. We decided it was safe to use since the extension is already required for EPOLLONESHOT
and EPOLLET
[2].
Regarding the FIXME
, I am not super familiar with Rust, but the mount
function uses unsigned long
for the flags
argument. Until C23 enums could not have a type other than int
, otherwise UL
probably would have made more sense. Hopefully that helps you a bit in your decision.
[1] https://inbox.sourceware.org/libc-alpha/[email protected]/
[2] https://inbox.sourceware.org/libc-alpha/[email protected]/
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.
Your patch makes complete sense, I assume the intent was always to be 0x80000000
rather than the sign-extended 0xffffffff80000000
. I just left that fixme as something to double check after the experimentation here; surprised you found this PR out of the blue, but thank you for saving me the work :)
I wrote the glibc patch. Sorry, I didn't expect breakage here.
Not at all a problem, there's nothing user-visible—just a test failure checking the equality of constants. Expected whenever the libraries we test against get updated.
Related to termio failures: llvm/llvm-project#137403 |
Cherry picked from #4695