Skip to content
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

Add Solaris operating system support #1724

Merged
merged 2 commits into from
Nov 16, 2023

Conversation

psumbera
Copy link
Contributor

@psumbera psumbera commented Nov 6, 2023

No description provided.

@psumbera
Copy link
Contributor Author

psumbera commented Nov 6, 2023

@Thomasdezeeuw can you please look here?

Copy link
Collaborator

@Thomasdezeeuw Thomasdezeeuw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you confirm that it now works without RUSTFLAGS?

Also could you add Solaris to

target: ["armv7-sony-vita-newlibeabihf"]
? Then we can make sure it builds.

@psumbera
Copy link
Contributor Author

psumbera commented Nov 6, 2023

I can confirm that cargo test --features="log,os-poll,os-ext,net" passes on Solaris 11.4.

@Thomasdezeeuw
Copy link
Collaborator

@psumbera can you fix the CI?

@psumbera
Copy link
Contributor Author

psumbera commented Nov 8, 2023

@psumbera can you fix the CI?

Not really sure... I can reproduce it on Linux. But now I don't know what to do...

Why it's failing for armv7-sony-vita-newlibeabihf even without my changes?

@Thomasdezeeuw
Copy link
Collaborator

Maybe try (I don't have my regular dev setup atm, so can't test)

diff --git a/src/sys/unix/mod.rs b/src/sys/unix/mod.rs
index 7804236..28e0bea 100644
--- a/src/sys/unix/mod.rs
+++ b/src/sys/unix/mod.rs
@@ -17,7 +17,9 @@ cfg_os_poll! {
     mod selector;
     pub(crate) use self::selector::{event, Event, Events, Selector};
 
+    #[cfg(feature = "os-ext")]
     mod sourcefd;
+    #[cfg(feature = "os-ext")]
     pub use self::sourcefd::SourceFd;
 
     mod waker;

@psumbera
Copy link
Contributor Author

psumbera commented Nov 8, 2023

Now I'm getting (with --features os-poll) following. Not sure what to do with it.

error: glob import doesn't reexport anything because no candidate is public enough
  --> src/sys/mod.rs:57:13
   |
57 |     pub use self::unix::*;
   |             ^^^^^^^^^^^^^
   |
note: the lint level is defined here

@psumbera
Copy link
Contributor Author

psumbera commented Nov 9, 2023

Now I'm getting (with --features os-poll) following. Not sure what to do with it.

error: glob import doesn't reexport anything because no candidate is public enough

I think this might be known Rust issue: rust-lang/rust#115966 . Adding #[warn(unused_imports)] seems to resolve the issue for now.

Though not sure what is status of CI now?

@psumbera
Copy link
Contributor Author

psumbera commented Nov 9, 2023

Though not sure what is status of CI now?

I see now new results. Thank you! The only failure is MSRV (ubuntu-latest). Have no idea what is wrong there...

@psumbera
Copy link
Contributor Author

@Thomasdezeeuw can this be merged now please?

src/sys/mod.rs Outdated
@@ -54,6 +54,7 @@ cfg_os_poll! {
#[cfg(unix)]
cfg_os_poll! {
mod unix;
#[warn(unused_imports)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't make sense. Did you mean to disable the lint? That would be the following, but really we shouldn't need it at all.

Suggested change
#[warn(unused_imports)]
#[allow(unused_imports)]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, #[allow(unused_imports)] does make sense. Fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Thomasdezeeuw can you please merge it now?

…solaris --no-default-features --features net (and os-poll)
@Thomasdezeeuw Thomasdezeeuw merged commit f6a20da into tokio-rs:master Nov 16, 2023
24 checks passed
@Thomasdezeeuw
Copy link
Collaborator

Thanks @psumbera

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants