Add Solaris operating system support (using flag mio_unsupported_forc… by psumbera · Pull Request #1715 · tokio-rs/mio

psumbera · 2023-09-01T09:48:24Z

…e_poll_poll)

Thomasdezeeuw · 2023-09-01T10:18:15Z

Since solaris doesn't support any selector other than poll you don't need the mio_unsupported_force_poll_poll cfg.

But are problems listed in #1152 and #1328 (and #1528, #1542) resolved?

notgull · 2023-09-01T16:56:45Z

Solaris and Illumos support a mechanism called event ports, which can be used to implement event listening in a more efficient way than poll().

Thomasdezeeuw · 2023-09-02T15:35:52Z

Solaris and Illumos support a mechanism called event ports, which can be used to implement event listening in a more efficient way than poll().

Illumos already uses epoll, so this will be Solaris specific.

psumbera · 2023-09-04T08:12:29Z

Since solaris doesn't support any selector other than poll you don't need the mio_unsupported_force_poll_poll cfg.

How is that possible? So far I had to use RUSTFLAGS="--cfg mio_unsupported_force_poll_poll".

But are problems listed in #1152 and #1328 (and #1528, #1542) resolved?

#1152 could be considered as resolved once this is merged in. Originally I was thinking about implementing it via Solaris event ports but it's not possible right now since we couln't check it using continous integration.

#1328 my commit uses SOCK_CLOEXEC and SOCK_NONBLOCK as it's for Illumos. Solaris 10 will never support Rust. It's very old and now with mininal support. It was released in 2004.

Not sure about others. I can modifiy README file and remove Solaris line. But mio_unsupported_force_poll_poll is mentined there as not really supported thing.

Thomasdezeeuw · 2023-09-04T08:34:39Z

Since solaris doesn't support any selector other than poll you don't need the mio_unsupported_force_poll_poll cfg.

How is that possible? So far I had to use RUSTFLAGS="--cfg mio_unsupported_force_poll_poll".

By adding the correct cfg attributes for Solaris.

But are problems listed in #1152 and #1328 (and #1528, #1542) resolved?

#1152 could be considered as resolved once this is merged in. Originally I was thinking about implementing it via Solaris event ports but it's not possible right now since we couln't check it using continous integration.

👍

#1328 my commit uses SOCK_CLOEXEC and SOCK_NONBLOCK as it's for Illumos. Solaris 10 will never support Rust. It's very old and now with mininal support. It was released in 2004.

I'm not familiar with Solaris, but do newer version have support for SOCK_CLOEXEC and SOCK_NONBLOCK?

Not sure about others. I can modifiy README file and remove Solaris line.

Let's leave it for now, I want to make sure it actually works this time.

But mio_unsupported_force_poll_poll is mentined there as not really supported thing.

They key word in mio_unsupported_force_poll_poll is force, it overwrites the preferred polling method on the platform to use a poll(2) based implementation instead. This is mainly used for testing and thus the flag itself unsupported. However the poll(2) implementation is fully supported on platforms that don't have anything "better", for example ESP-IDF. We can add Solaris to this list, even though it has event ports.

psumbera · 2023-09-04T12:00:34Z

Since solaris doesn't support any selector other than poll you don't need the mio_unsupported_force_poll_poll cfg.

How is that possible? So far I had to use RUSTFLAGS="--cfg mio_unsupported_force_poll_poll".

By adding the correct cfg attributes for Solaris.

Sorry! I think I'm lost. What and where should I put anything? Thanks!

But are problems listed in #1152 and #1328 (and #1528, #1542) resolved?

#1328 my commit uses SOCK_CLOEXEC and SOCK_NONBLOCK as it's for Illumos. Solaris 10 will never support Rust. It's very old and now with mininal support. It was released in 2004.

I'm not familiar with Solaris, but do newer version have support for SOCK_CLOEXEC and SOCK_NONBLOCK?

Yes, all versions of Solaris 11.4 (first release in 2018) do support them.

But mio_unsupported_force_poll_poll is mentined there as not really supported thing.

Thomasdezeeuw · 2023-09-04T12:20:05Z

By adding the correct cfg attributes for Solaris.

Sorry! I think I'm lost. What and where should I put anything? Thanks!

Try compiling without RUSTFLAGS="--cfg mio_unsupported_force_poll_poll" and rustc will tell you ;)

I'm not familiar with Solaris, but do newer version have support for SOCK_CLOEXEC and SOCK_NONBLOCK?

Yes, all versions of Solaris 11.4 (first release in 2018) do support them.

👍

psumbera · 2023-09-04T12:32:52Z

By adding the correct cfg attributes for Solaris.

Sorry! I think I'm lost. What and where should I put anything? Thanks!

Try compiling without RUSTFLAGS="--cfg mio_unsupported_force_poll_poll" and rustc will tell you ;)

No it doesn't help:

$ RUSTFLAGS="--cfg mio_unsupported_force_poll_poll" cargo build
   Compiling libc v0.2.147
   Compiling log v0.4.20
   Compiling mio v0.8.8 (/builds/psumbera/mio-solaris)
    Finished dev [unoptimized + debuginfo] target(s) in 6.07s

Thomasdezeeuw · 2023-09-04T12:36:37Z

No it doesn't help:

$ RUSTFLAGS="--cfg mio_unsupported_force_poll_poll" cargo build
   Compiling libc v0.2.147
   Compiling log v0.4.20
   Compiling mio v0.8.8 (/builds/psumbera/mio-solaris)
    Finished dev [unoptimized + debuginfo] target(s) in 6.07s

Try enabling all features --all-features. And as I said don't use RUSTFLAGS="--cfg mio_unsupported_force_poll_poll". So just run cargo check --all-features on Solaris and it should fail.

psumbera · 2023-09-04T13:05:28Z

Try enabling all features --all-features. And as I said don't use RUSTFLAGS="--cfg mio_unsupported_force_poll_poll". So just run cargo check --all-features on Solaris and it should fail. But still have no issue how to define

Sorry about with[out] RUSTFLAGS. It fails like this now. But still have no clue how to define mio_unsupported_force_poll_poll cfg attribute (ideally in Config.toml or src/macros.rs).

cargo check --all-features
    Checking mio v0.8.8 (/builds/psumbera/mio-solaris)
error[E0432]: unresolved imports `self::selector::event`, `self::selector::Event`, `self::selector::Events`, `self::selector::Selector`
  --> src/sys/unix/mod.rs:18:37
   |
18 |     pub(crate) use self::selector::{event, Event, Events, Selector};
   |                                     ^^^^^  ^^^^^  ^^^^^^  ^^^^^^^^ no `Selector` in `sys::unix::selector`
   |                                     |      |      |
   |                                     |      |      no `Events` in `sys::unix::selector`
   |                                     |      no `Event` in `sys::unix::selector`
   |                                     no `event` in `sys::unix::selector`
   |
   = help: consider importing this module instead:
           crate::event
   = help: consider importing this struct instead:
           crate::event::Event
   = help: consider importing this struct instead:
           crate::Events

warning: unused macro definition: `debug_detail`
  --> src/sys/mod.rs:18:18
   |
18 |     macro_rules! debug_detail {
   |                  ^^^^^^^^^^^^
   |
   = note: `#[warn(unused_macros)]` on by default

For more information about this error, try `rustc --explain E0432`.
warning: `mio` (lib) generated 1 warning
error: could not compile `mio` (lib) due to previous error; 1 warning emitted

Thomasdezeeuw · 2023-09-04T13:19:11Z

Well... you need to resolve those issues to ensure that Solaris works...

psumbera · 2023-09-04T14:13:38Z

Ok, with my new change Solaris builds without need to define mio_unsupported_force_poll_poll.

Thomasdezeeuw · 2023-09-05T14:47:43Z

@psumbera can you add some CI setup? cargo test would be best, but cargo check is sufficient for now. Also see https://github.com/tokio-rs/mio/blob/master/Makefile#L2.

psumbera · 2023-09-05T16:10:58Z

@psumbera can you add some CI setup? cargo test would be best, but cargo check is sufficient for now.

Confused again. Where? .github/workflows/ci.yml? Any example for that?
cargo test passes for me. But there is no Solaris instance which is accesible from github for CI.

Also see https://github.com/tokio-rs/mio/blob/master/Makefile#L2.

Solaris isn't supported by rustup right now. I think there is no need to extend the list.

Thomasdezeeuw · 2023-09-10T16:00:24Z

@psumbera can you add some CI setup? cargo test would be best, but cargo check is sufficient for now.

Confused again. Where? .github/workflows/ci.yml? Any example for that? cargo test passes for me. But there is no Solaris instance which is accesible from github for CI.

... I mean come on... At some point you have put the time in. It's really not that complicated once you find the GitHub Actions file.

Also see https://github.com/tokio-rs/mio/blob/master/Makefile#L2.

Solaris isn't supported by rustup right now. I think there is no need to extend the list.

Do you have any other means to add Solaris support to the CI?

Thomasdezeeuw · 2023-09-06T09:26:42Z

@@ -1,5 +1,5 @@
 #[cfg(all(
-    not(mio_unsupported_force_poll_poll),
+    not(any(target_os = "solaris", mio_unsupported_force_poll_poll)),


This is not needed as solaris is not any of the targets listed below.

Thomasdezeeuw · 2023-09-06T09:26:50Z


 #[cfg(all(
-    not(mio_unsupported_force_poll_poll),
+    not(any(target_os = "solaris", mio_unsupported_force_poll_poll)),


Again not needed.

Thomasdezeeuw · 2023-09-06T09:27:09Z


 #[cfg(all(
-    not(mio_unsupported_force_poll_poll),
+    not(any(target_os = "solaris", mio_unsupported_force_poll_poll)),


Not needed.

Thomasdezeeuw · 2023-09-06T09:27:14Z


 #[cfg(all(
-    not(mio_unsupported_force_poll_poll),
+    not(any(target_os = "solaris", mio_unsupported_force_poll_poll)),


Not needed.

Thomasdezeeuw · 2023-09-10T16:01:50Z

@@ -1,5 +1,5 @@
 #[cfg(all(
-    not(mio_unsupported_force_poll_poll),
+    not(any(target_os = "solaris", mio_unsupported_force_poll_poll)),


Not needed.

I got following error without this change:

error: field `waker` is never read --> src/sys/unix/waker.rs:38:9 | 37 | pub struct Waker { | ----- field in this struct 38 | waker: WakerInternal, | ^^^^^ | = note: `Waker` has a derived impl for the trait `Debug`, but this is intentionally ignored during dead code analysis note: the lint level is defined here --> src/lib.rs:6:5 | 6 | dead_code | ^^^^^^^^^ error: associated items `new` and `wake` are never used --> src/sys/unix/waker.rs:42:16 | 41 | impl Waker { | ---------- associated items in this implementation 42 | pub fn new(selector: &Selector, token: Token) -> io::Result<Waker> { | ^^^ ... 48 | pub fn wake(&self) -> io::Result<()> { | ^^^^ ```

I think it uses pub use self::poll::Waker; and thus cannot use pub use self::fdbased::Waker.

But on Solaris this attribute should never return true, even without the change made. I don't have time to look into it at the moment, but this change is not needed and only made the cfg attribute more complex.

@Thomasdezeeuw How can possibly following attribute be false on Solaris?

#[cfg(all( not(mio_unsupported_force_poll_poll), not(all( not(mio_unsupported_force_waker_pipe), any( target_os = "freebsd", target_os = "ios", target_os = "macos", target_os = "tvos", target_os = "watchos", ) )) ))]

Thomasdezeeuw · 2023-09-10T16:01:55Z

 mod fdbased {
    #[cfg(all(
-        not(mio_unsupported_force_waker_pipe),
+        not(any(target_os = "solaris", mio_unsupported_force_waker_pipe)),


Not needed.

Thomasdezeeuw · 2023-09-10T16:02:04Z


 #[cfg(all(
-    not(mio_unsupported_force_poll_poll),
+    not(any(target_os = "solaris", mio_unsupported_force_poll_poll)),


Not needed.

Thomasdezeeuw · 2023-09-10T16:02:28Z

        }

-        #[cfg(mio_unsupported_force_poll_poll)]
+        #[cfg(any(target_os = "solaris", mio_unsupported_force_poll_poll))]


I don't think this is needed, but not 100% sure.

Thomasdezeeuw · 2023-09-10T16:02:38Z


 #[cfg(all(
-    mio_unsupported_force_poll_poll,
+    any(target_os = "solaris", mio_unsupported_force_poll_poll),


Not needed.

Thomasdezeeuw · 2023-09-10T16:03:01Z


 #[cfg(all(
-    mio_unsupported_force_poll_poll,
+    any(target_os = "solaris", mio_unsupported_force_poll_poll),


Not need, you already added it below.

I got following errir without this change:

error[E0432]: unresolved import `crate::sys::unix::waker::WakerInternal` --> src/sys/unix/selector/poll.rs:7:5 | 7 | use crate::sys::unix::waker::WakerInternal; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `WakerInternal` in `sys::unix::waker` error: unused import: `AsRawFd` --> src/sys/unix/selector/poll.rs:11:25 | 11 | use std::os::unix::io::{AsRawFd, RawFd}; | ^^^^^^^ | note: the lint level is defined here --> src/lib.rs:5:5 | 5 | unused_imports, | ^^^^^^^^^^^^^^

psumbera · 2023-09-11T08:58:20Z

@psumbera can you add some CI setup? cargo test would be best, but cargo check is sufficient for now.

Confused again. Where? .github/workflows/ci.yml? Any example for that? cargo test passes for me. But there is no Solaris instance which is accesible from github for CI.

... I mean come on... At some point you have put the time in. It's really not that complicated once you find the GitHub Actions file.

Still confused. If you mean '.cirrus.yml'. Cirrus CI doesn't support Solaris OS.

Also see https://github.com/tokio-rs/mio/blob/master/Makefile#L2.

Solaris isn't supported by rustup right now. I think there is no need to extend the list.

Do you have any other means to add Solaris support to the CI?

No.

psumbera · 2023-09-19T08:18:27Z

@Thomasdezeeuw do you have any other commets or suggestions? Thank you!

Thomasdezeeuw

@Thomasdezeeuw do you have any other commets or suggestions? Thank you!

Ideally we can setup some kind of CI, but that doesn't seem likely.

Also some of the comments I made still need to be resolved, I see you answered them, but I think the warnings should we resolved in another way.

Thomasdezeeuw · 2023-09-19T09:47:01Z

@@ -1,5 +1,5 @@
 #[cfg(all(
-    not(mio_unsupported_force_poll_poll),
+    not(any(target_os = "solaris", mio_unsupported_force_poll_poll)),


But on Solaris this attribute should never return true, even without the change made. I don't have time to look into it at the moment, but this change is not needed and only made the cfg attribute more complex.

Thomasdezeeuw · 2023-10-18T14:32:19Z

I need to clean up the cfg a bit so that Solaris (and vita, see #1721) can use the epoll(2) implementation without any RUSTFLAGS.

Thomasdezeeuw · 2023-10-20T15:23:11Z

@psumbera can you rebase this? I think after we merged #1721 we got a reasonable way forward. I'll like Solaris to be supported in a similar way, that is without any flags.

Thomasdezeeuw · 2023-11-06T15:56:57Z

@psumbera did you mean to close this?

psumbera · 2023-11-06T16:00:14Z

@psumbera did you mean to close this?

No. I'm now applying manually my changes on top of latest repo. I will push change to my repo later...

psumbera · 2023-11-06T16:55:18Z

Ok. I wasn't able to reuse this pull request. Instead I have to create #1724.

psumbera force-pushed the solaris-poll branch from 48eceef to 1e82cd8 Compare September 4, 2023 07:46

psumbera force-pushed the solaris-poll branch from 1e82cd8 to bc54006 Compare September 4, 2023 14:11

Thomasdezeeuw reviewed Sep 10, 2023

View reviewed changes

psumbera force-pushed the solaris-poll branch 2 times, most recently from 7c90608 to accc36b Compare September 11, 2023 08:39

Thomasdezeeuw reviewed Sep 19, 2023

View reviewed changes

Thomasdezeeuw mentioned this pull request Oct 18, 2023

Added support for vita target #1721

Merged

Thomasdezeeuw mentioned this pull request Oct 19, 2023

Possibly fragile coordination between mio and tokio for Windows event-triggered simulation tokio-rs/tokio#5866

Closed

psumbera closed this Nov 6, 2023

psumbera force-pushed the solaris-poll branch from accc36b to e80c3b2 Compare November 6, 2023 14:54

Conversation

psumbera commented Sep 1, 2023

Uh oh!

Thomasdezeeuw commented Sep 1, 2023

Uh oh!

notgull commented Sep 1, 2023

Uh oh!

Thomasdezeeuw commented Sep 2, 2023

Uh oh!

psumbera commented Sep 4, 2023

Uh oh!

Thomasdezeeuw commented Sep 4, 2023

Uh oh!

psumbera commented Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Thomasdezeeuw commented Sep 4, 2023

Uh oh!

psumbera commented Sep 4, 2023

Uh oh!

Thomasdezeeuw commented Sep 4, 2023

Uh oh!

psumbera commented Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Thomasdezeeuw commented Sep 4, 2023

Uh oh!

psumbera commented Sep 4, 2023

Uh oh!

Thomasdezeeuw commented Sep 5, 2023

Uh oh!

psumbera commented Sep 5, 2023

Uh oh!

Thomasdezeeuw commented Sep 10, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

psumbera commented Sep 11, 2023

Uh oh!

psumbera commented Sep 19, 2023

Uh oh!

Thomasdezeeuw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Thomasdezeeuw commented Oct 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Thomasdezeeuw commented Oct 20, 2023

psumbera commented Sep 4, 2023 •

edited

Loading

psumbera commented Sep 4, 2023 •

edited

Loading

Thomasdezeeuw commented Oct 18, 2023 •

edited

Loading