Add tier 2 illumos CI #5237
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
papertigers
force-pushed
the
illumos-ci
branch
2 times, most recently
from
36af7ea to
46a02de
Compare
| 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.
There was a problem hiding this comment.
+1 for my part, but i dunno if @tgross35 would want one of the Known illumos People to also chime in
|
I'm fine to take this, if we hear differently from them it's easy enough to revert. @papertigers could you squash with the PR title/description as the commit message please? Unfortunately we don't have squash+merge on this repo. |
This comment has been minimized.
This comment has been minimized.
Fixes: rust-lang#1405 As discussed in rust-lang#1405 we want CI support for illumos. It looks like solaris has already added CI support in a previous commit so we can safely mark the issue as closed. This adds illumos ci via the vmactions/omnios-vm github action and relies on the latest omnios LTS release (r151054) which is supported until 2028-05-01. Additionally this PR fixes an epoll test issue that was looking for timerfd that I discovered while working on this.
|
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. |
|
@tgross35 I squashed the commits and amended the commit message. Should be good to go, let me know if we need to take care of anything else. |
5 participants
Fixes: #1405
As discussed in #1405 we want CI support for illumos. It looks like solaris has already added CI support in a previous
commit so we can safely mark the issue as closed.
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 an epoll test issue that was looking for timerfd that I discovered while working on this.