Want illumos ci#5237
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| pub const PTHREAD_MUTEX_RECURSIVE: c_int = 4; | ||
|
|
||
| #[cfg(target_os = "illumos")] | ||
| pub const PTHREAD_MUTEX_DEFAULT: c_int = 0x8; |
There was a problem hiding this comment.
fwiw I am discussing this with a few people before we land this change.
This comes from: https://www.illumos.org/issues/16200, however this value is not in the current sysroot being used to cross compile rustc. There is a plan to bump the illumos sysroot, but even this version will not contain the header value change. This means any software built with this libc change wouldn't function correctly on a sufficiently old illumos kernel.
We could potentially keep the previous behavior and exclude the header check in the tests but we will have to remind ourselves to later go back and fix this in libc whenever we do have a sufficiently new sysroot.
There was a problem hiding this comment.
for "sufficiently old" meaning "before ~jan 2024 when 16200 was fixed", though - I think that points away from just changing libc! it's also "weird" to me that when building a normal release Rust, the sysroot there would have a pthread.h with PTHREAD_MUTEX_DEFAULT that disagrees with this file.
in some sense, the header check here is checking the wrong pthread.h for anyone that uses the sync types via libstd, since whatever happens to be around for CI today is only relevant for anyone building libc today. but it's the right pthread.h if your code goes and does pthread things, whereas libstd's pthread stuff could be lightly confusing in that case (though PTHREAD_MUTEX_NORMAL didn't change so maybe that's all fine).
I've at least talked myself into thinking that leaving this as-is with a note for cutting a future sysroot to clean this up is probably the expedient thing to do?
There was a problem hiding this comment.
We don't appear to directly use PTHREAD_MUTEX_DEFAULT in rust-lang/rust so I'm not worried about this change.
See rust-lang/rust#150756 for tracking the update.
| pub const PTHREAD_MUTEX_RECURSIVE: c_int = 4; | ||
|
|
||
| #[cfg(target_os = "illumos")] | ||
| pub const PTHREAD_MUTEX_DEFAULT: c_int = 0x8; |
There was a problem hiding this comment.
We don't appear to directly use PTHREAD_MUTEX_DEFAULT in rust-lang/rust so I'm not worried about this change.
See rust-lang/rust#150756 for tracking the update.
5 participants
Fixes: #1405
This adds illumos ci via the
vmactions/omnios-vmgithub action and relies on the latest omnios LTS release (r151054 )which is supported until 2028-05-01.Additionally this PR fixes a few test issues that I discovered while working on this.