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

Be more specific about winapi features #4737

Closed
Darksonn opened this issue Jun 3, 2022 · 7 comments
Closed

Be more specific about winapi features #4737

Darksonn opened this issue Jun 3, 2022 · 7 comments
Labels
A-tokio Area: The main tokio crate E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue. M-net Module: tokio/net

Comments

@Darksonn
Copy link
Contributor

Darksonn commented Jun 3, 2022

Our dependency on winapi is listed in the following way:

[target.'cfg(windows)'.dependencies.winapi]
version = "0.3.8"
default-features = false
features = ["std", "winsock2", "mswsock", "handleapi", "ws2ipdef", "ws2tcpip"]
optional = true

Additionally, the various features of Tokio that require something from winapi can enable additional winapi features, e.g.:

net = [
  "libc",
  "mio/os-poll",
  "mio/os-ext",
  "mio/net",
  "socket2",
  "winapi/namedpipeapi",
]

However, the features that are listed in the [target.'cfg(windows)'.dependencies.winapi] section are also specific to various other features of Tokio, and hence enabled more often than necessary. We would like to be more specific about which features we need, and only enable each feature when we need it by moving them to be listed on other dependencies like "winapi/namedpipeapi".

@Darksonn Darksonn added E-help-wanted Call for participation: Help is requested to fix this issue. E-easy Call for participation: Experience needed to fix: Easy / not much A-tokio Area: The main tokio crate M-net Module: tokio/net labels Jun 3, 2022
@notgull
Copy link
Contributor

notgull commented Jun 5, 2022

I disabled all of the winapi features that are on by default and, at least on my build, it doesn't seem to raise any issues. Might be on my end, let me run the CI just to be sure.

@Darksonn
Copy link
Contributor Author

Darksonn commented Jun 6, 2022

The features were added like this to fix #4662, so if you remove them, that issue should come back. (Of course, only on windows builds.)

@gftea
Copy link
Contributor

gftea commented Jun 11, 2022

try to work on this, I am new to rust, and can you help below question @Darksonn

in net/windows/named_pipe.rs

in below codes, as I understand crate:io refer to io module in src folder, but I cannot find os module in it, so which module crate::os refer to?

use crate::io::{AsyncRead, AsyncWrite, Interest, PollEvented, ReadBuf, Ready};
use crate::os::windows::io::{AsRawHandle, FromRawHandle, RawHandle};

@Darksonn
Copy link
Contributor Author

The crate::os module is a re-export of std::os.

It exists because Tokio replaces it with something else when building documentation to show windows stuff in docs built on Linux.

@gftea
Copy link
Contributor

gftea commented Jun 11, 2022

@Darksonn , tests pass with above change in PR, I am unclear why we do not have error regarding unresolved import even though in named_pipe.rs, there are below import

    pub(super) use crate::os::windows::ffi::OsStrExt;
    pub(super) use crate::winapi::shared::minwindef::{DWORD, FALSE};
    pub(super) use crate::winapi::um::fileapi;
    pub(super) use crate::winapi::um::handleapi;
    pub(super) use crate::winapi::um::namedpipeapi;
    pub(super) use crate::winapi::um::winbase;
    pub(super) use crate::winapi::um::winnt;

@gftea
Copy link
Contributor

gftea commented Jun 29, 2022

This issue can be closed because the PR merged? @Darksonn

@Darksonn
Copy link
Contributor Author

Ah, thanks. I didn't notice that the PR wasn't set up to auto-close this issue when merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue. M-net Module: tokio/net
Projects
None yet
Development

No branches or pull requests

3 participants