Skip to content

Conversation

@Stebalien
Copy link

This will make it possible to implement systemd socket activation in rqbit (ikatson/rqbit#505).

#[derive(thiserror::Error, Debug)]
pub enum Error {
#[error("error taking ownership of a socket from a file descriptor: {0}")]
SocketFromFd(std::io::Error),
Copy link
Author

Choose a reason for hiding this comment

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

Note: this is a breaking change.

Copy link
Owner

@ikatson ikatson left a comment

Choose a reason for hiding this comment

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

Please address the comment otherwise lgtm

SocketAddr::V4(addr) => SocketAddrKind::V4(addr),
SocketAddr::V6(addr) => SocketAddrKind::V6 {
addr,
is_dualstack: addr.ip().is_unspecified()
Copy link
Owner

Choose a reason for hiding this comment

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

Might need to check the flag V6_V6ONLY (don't remember the exact name)

Copy link
Author

Choose a reason for hiding this comment

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

next line

Copy link
Author

Choose a reason for hiding this comment

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

Gah. Except that line is incorrect. Fixing...

Copy link
Author

Choose a reason for hiding this comment

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

Ok, fixed. I'm not sure how I managed to do that (the tests were correct, at least...).

@ikatson
Copy link
Owner

ikatson commented Nov 22, 2025

Hey this doesn't compile on Mac, please address, can't merge otherwise

This will make it possible to implement systemd socket activation in
rqbit [1]. This is restricted to Linux because MacOS is missing some
getsockopt features needed by `Socket::protocol` and
`Socket::is_listener`.

[1] ikatson/rqbit#505
@Stebalien
Copy link
Author

I've restricted this change to Linux only. It appears that MacOS is missing a few features and it's not like we need it on MacOS anyways.

I've also enabled CI in my fork now. It passes on Windows and Linux, but I'm still seeing (unrelated) failures in the multicast tests:

multicast::tests::bind_multiple_same_port
multicast::tests::test_v4_received
multicast::tests::test_v6_received

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