Add Windows support (socket types, OpenSSL linking, IPv6 fallback)#40
Add Windows support (socket types, OpenSSL linking, IPv6 fallback)#40AliRezaBeigy wants to merge 10 commits intoMygod:mainfrom
Conversation
fd96c36 to
095bce3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd96c36d12
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive Windows support to the Slipstream QUIC proxy by implementing platform-specific socket type handling, fixing OpenSSL linking, and adding IPv6 to IPv4 fallback for UDP socket binding.
Changes:
- Added Windows-compatible socket type abstractions using
winapicrate with conditional compilation - Updated OpenSSL and pthread library linking to use Windows-appropriate libraries (
libssl/libcryptoandws2_32) - Implemented IPv6→IPv4 fallback for UDP socket binding to handle Windows systems without IPv6 support
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/slipstream-ffi/Cargo.toml | Adds winapi dependency for Windows socket types |
| crates/slipstream-ffi/build.rs | Conditionally links OpenSSL libraries (libssl/libcrypto on Windows) and replaces pthread with ws2_32 on Windows |
| crates/slipstream-ffi/src/runtime.rs | Implements Windows-compatible SockaddrStorage type alias and socket conversion functions using winapi structures |
| crates/slipstream-ffi/src/picoquic.rs | Updates imports to use Windows socket types conditionally |
| crates/slipstream-ffi/src/lib.rs | Exports SockaddrStorage type for use by other crates |
| crates/slipstream-server/src/udp_fallback.rs | Updates type signatures and test helper to use platform-agnostic SockaddrStorage |
| crates/slipstream-server/src/server.rs | Updates socket address storage variables to use SockaddrStorage type |
| crates/slipstream-client/src/runtime/setup.rs | Adds IPv6 to IPv4 fallback for UDP socket binding |
| crates/slipstream-client/src/runtime/path.rs | Updates socket address storage to use SockaddrStorage type |
| crates/slipstream-client/src/runtime.rs | Updates socket address storage variables to use SockaddrStorage type |
| crates/slipstream-client/src/dns/response.rs | Updates type signature to use SockaddrStorage |
| crates/slipstream-client/src/dns/resolver.rs | Updates struct fields and function signatures to use SockaddrStorage |
| crates/slipstream-client/src/dns/poll.rs | Updates socket address storage variables to use SockaddrStorage type |
| crates/slipstream-client/src/dns/path.rs | Updates socket address storage variable to use SockaddrStorage type |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Sorry for the late reply, I need more time to manage the test and copilot. Before that I have to manage picoquic. I will create a pr for that too. |
|
Hi I have limited bandwidth so it will be a while until I get to this. Thanks for the patience. :) |
bc95b4c to
eb895f4
Compare
|
Branch has been rebased and all tests have passed successfully. However, this branch requires Mygod/slipstream-picoquic#1 to achieve full Windows compatibility. Please take a look that PR first if you find time. |
94daf09 to
99dd3ff
Compare
|
@Mygod, I think the problem is that I switch to bash in the code, and in Windows, when I run a bash command, it's looking for WSL if the bash doesn't exist. I suggest two solutions: 1. Install bash; 2. Revert that forced bash shell (I didn't recommend because I faced some errors in PowerShell while building). |
|
|
||
| let mut command = Command::new(script); | ||
| let mut command = if cfg!(windows) { | ||
| let mut cmd = Command::new("bash"); |
There was a problem hiding this comment.
Here I switch to bash.
|
Ok please fix CI for me to review. ;) |
1c88839 to
8f90c7e
Compare
8f90c7e to
d1bdf5d
Compare
|
@Mygod Everything in here is good now. |
|
Vendored is too slow. No chance using precompiled? |
|
Installing openssl in windows is taking same time as vendored build time and with prebuild openssl user require to install the openssl when they want to use it. |
|
I'm asking for CI. |
|
Let's try it. Give me a moment. |
|
I thought I needed to install openssl with choco that takes too much time, but I was wrong. |
Mygod
left a comment
There was a problem hiding this comment.
As mentioned at Mygod/slipstream-picoquic#1 (comment), every picoquic change must be as minimal as possible since we need to account for future updates to the library. If a change can be justified in a context outside of slipstream, then the PR should be opened upstream to private-octopus's repo.
d8e0d6f to
c3dc75e
Compare
Adds Windows support with socket type compatibility, OpenSSL linking fixes, and IPv6 fallback for UDP socket binding.
Changes:
winapidependency for Windows socket typeslibssl/libcryptoinstead ofssl/crypto)pthreadwithws2_32on Windowssockaddr_storageusingwinapisocket_addr_to_storageandsockaddr_storage_to_socket_addrSockaddrStoragetypeI test and build the changes in https://github.com/AliRezaBeigy/slipstream-rust-deploy