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

Socket::bind_device missing on linux platforms #441

Closed
pabigot opened this issue May 13, 2023 · 9 comments
Closed

Socket::bind_device missing on linux platforms #441

pabigot opened this issue May 13, 2023 · 9 comments

Comments

@pabigot
Copy link
Contributor

pabigot commented May 13, 2023

Somewhere between v0.4.9 and v0.5 the method Socket::bind_device disappeared on linux platforms unless feature all is explicitly selected.

Without diving too deeply I suspect this is due to #249 removing an implicit requirement on all elsewhere.

The method is also no longer listed on the generic documentation, though bind_device_by_index remains.

I'm working around this by adding the all feature, but if this change wasn't intentional it'd be nice to get it fixed.

@Thomasdezeeuw
Copy link
Collaborator

I don't see a difference in the cfg attributes:
On master: https://github.com/rust-lang/socket2/blob/master/src/sys/unix.rs#L1793-L1803
On v0.4.x: https://github.com/rust-lang/socket2/blob/v0.4.x/src/sys/unix.rs#L1559-L1569

Are you sure another dependency didn't enable the all feature for you?

In any case the all feature is required because it's a Linux only feature (for both v0.4 and v0.5).

@pabigot
Copy link
Contributor Author

pabigot commented May 14, 2023

@Thomasdezeeuw Yes, it's more subtle than the function-specific attributes, which is why I think it was the removal by 5d61324 of an injected all that caused the change.

I agree all is appropriate in this case, but no, it wasn't required for this function to be available in 0.4.9.

Also is it intentional that methods only available on specific platforms are not documented in the generic crate docs? If so, how are people supposed to find the functions even exist?

@Thomasdezeeuw
Copy link
Collaborator

@Thomasdezeeuw Yes, it's more subtle than the function-specific attributes, which is why I think it was the removal by 5d61324 of an injected all that caused the change.

Are you refering to the correct commit? I don't see any changes to bind_device in it.

I agree all is appropriate in this case, but no, it wasn't required for this function to be available in 0.4.9.

Also is it intentional that methods only available on specific platforms are not documented in the generic crate docs? If so, how are people supposed to find the functions even exist?

No, that shouldn't be the case.

socket2/Cargo.toml

Lines 29 to 32 in 3047737

[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs"]
targets = ["aarch64-apple-ios", "aarch64-linux-android", "x86_64-apple-darwin", "x86_64-unknown-fuchsia", "x86_64-pc-windows-msvc", "x86_64-pc-solaris", "x86_64-unknown-freebsd", "x86_64-unknown-illumos", "x86_64-unknown-linux-gnu", "x86_64-unknown-linux-musl", "x86_64-unknown-netbsd", "x86_64-unknown-redox", "armv7-linux-androideabi", "i686-linux-android"]
should take care of that.

@pabigot
Copy link
Contributor Author

pabigot commented May 15, 2023

Are you referring to the correct commit? I don't see any changes to bind_device in it.

@Thomasdezeeuw Yes; I now think I was misreading the comment:

Since Unix sockets are now available on all tier1 platforms, this also removes all feature requirement from the SockAddr::unix` function.

as suggesting that the removal of all from that function might have propagated elsewhere.

I didn't see any other commits that seemed relevant, but before submitting the issue I did verify that an otherwise unchanged application that specified 0.4.9 in Cargo.toml compiled, and one that specified 0.5.0 did not because Socket::bind_device was no longer available. I'm actually using 0.5.3.

No, that shouldn't be the case.

socket2/Cargo.toml

Lines 29 to 32 in 3047737

[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs"]
targets = ["aarch64-apple-ios", "aarch64-linux-android", "x86_64-apple-darwin", "x86_64-unknown-fuchsia", "x86_64-pc-windows-msvc", "x86_64-pc-solaris", "x86_64-unknown-freebsd", "x86_64-unknown-illumos", "x86_64-unknown-linux-gnu", "x86_64-unknown-linux-musl", "x86_64-unknown-netbsd", "x86_64-unknown-redox", "armv7-linux-androideabi", "i686-linux-android"]

should take care of that.

It doesn't seem to, since the current API documentation claims to come from 0.5.3 (the commit you referenced), but it still lacks the bind_device function.

FYI, there is no v0.5.3 tag in the github repository.

@Thomasdezeeuw
Copy link
Collaborator

It doesn't seem to, since the current API documentation claims to come from 0.5.3 (the commit you referenced), but it still lacks the bind_device function.

Odd. It is available if you switch to the Linux documentation: https://docs.rs/socket2/latest/x86_64-unknown-linux-gnu/socket2/struct.Socket.html#method.bind_device.

I just noticed that other, e.g., Linux specific methods such as cpu_affinity are also missing, but (for some reason) the macOS/iOS methods such as device_index are documented. However if I copy the cfg attributes from device_index and modify them to be correct for bind_device they are the exact same. This might be some kind of bug elsewhere, either in the Rust feature or in our Cargo.toml.

FYI, there is no v0.5.3 tag in the github repository.

Thanks, I've fixed that.

@Thomasdezeeuw
Copy link
Collaborator

Ah, I've found the issue. We need to use #[cfg(any(/* actualy cfg/ */, doc))], so we need to fix all methods.

@Thomasdezeeuw
Copy link
Collaborator

So I looked into this a little more, but it seems quite difficult to actually document all items due to missing types/dependent crates, especially when we want to document Windows items on Unix or visa versa. For example if we want to document that Windows specific extension we need to include the Windows version of the sys module, but that requires the use of window_sys, which isn't available on Unix.

I'm not quite sure what the best way forward is here... For now we could at least recommend people look at the target specific docs so they aren't missing methods.

@Darksonn
Copy link
Collaborator

Yes, it's a pain. In Tokio, we have a doc-only module called tokio::doc that redefines missing types when building docs on platforms missing them. But it's a significant amount of work to add.

@Thomasdezeeuw
Copy link
Collaborator

Yes, it's a pain. In Tokio, we have a doc-only module called tokio::doc that redefines missing types when building docs on platforms missing them. But it's a significant amount of work to add.

Do you think it's worth it (for Socket2)?

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

No branches or pull requests

3 participants