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 support on windows #61

Closed
wants to merge 42 commits into from
Closed

Conversation

NobodyXu
Copy link
Member

Fixed #46

@NobodyXu NobodyXu added the enhancement New feature or request label Mar 11, 2022
@NobodyXu NobodyXu added this to the windows-support milestone Mar 11, 2022
@NobodyXu NobodyXu requested a review from jonhoo March 11, 2022 08:19
@NobodyXu NobodyXu self-assigned this Mar 11, 2022
@jonhoo
Copy link
Collaborator

jonhoo commented Mar 11, 2022

This change is Reviewable

@NobodyXu
Copy link
Member Author

The Windows CI does not support running docker container, so I didn't run the integration tests on Windows CI.

 - tempdir
 - dirs
 - libc
 - once_cell
 - io-lifetimes
 - tokio-pipe

are now only enabled on unix.

Signed-off-by: Jiahao XU <[email protected]>
Signed-off-by: Jiahao XU <[email protected]>
by marking them `cfg(unix)` or `cfg(not(windows))`

Signed-off-by: Jiahao XU <[email protected]>
Accept `tokio::proess::Command` instead of `std::process::Command`

Signed-off-by: Jiahao XU <[email protected]>
Since Windows does not support container.

Signed-off-by: Jiahao XU <[email protected]>
I copied it from `stdio/unix.rs` and I forgot to change `RawFd` to
`RawHandle`.

Signed-off-by: Jiahao XU <[email protected]>
Remove unused parameter `&self`.

Signed-off-by: Jiahao XU <[email protected]>
@jonhoo
Copy link
Collaborator

jonhoo commented Mar 26, 2022

While I appreciate the work you've done on this, I don't think this is something I want to merge. We don't have a reliable way to test it in CI, it introduces a lot of conditional compilation, it has a completely different execution model (which will be unexpected for users), and it will become unsupported again if we eventually move to just supporting the native mux.

@NobodyXu
Copy link
Member Author

While I appreciate the work you've done on this, I don't think this is something I want to merge. We don't have a reliable way to test it in CI, it introduces a lot of conditional compilation, it has a completely different execution model (which will be unexpected for users), and it will become unsupported again if we eventually move to just supporting the native mux.

I have to agree, as this makes this crate significantly more complicated and I haven't figure out how to run sshd on windows in the CI yet.

IMHO it will be much easier if unix socket in windows supports passing file handler/fd.

@NobodyXu NobodyXu closed this Mar 27, 2022
@NobodyXu NobodyXu deleted the support-windows branch March 27, 2022 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Unable to establish connection on Windows 11
2 participants