-
Notifications
You must be signed in to change notification settings - Fork 357
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 DAITA V2 support for wireguard-go #7158
Conversation
ee89ebc
to
132ab1a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 11 of 12 files at r1, 3 of 8 files at r2, all commit messages.
Reviewable status: 13 of 18 files reviewed, all discussions resolved
082ad9e
to
7390f59
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 8 files at r2, 14 of 14 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
talpid-tunnel-config-client/src/lib.rs
line 249 at r3 (raw file):
proto::DaitaPlatform::IosWgGo } }
⛏️ Consider re-writing this function using a combination of const
and if-else
in conjunction with cfg!
instead. It will play nicer with people's editors, it is shorter and we can customize the compilation error message 😊
const fn get_platform() -> proto::DaitaPlatform {
use proto::DaitaPlatform;
const PLATFORM: DaitaPlatform = if cfg!(windows) {
DaitaPlatform::WindowsNative
} else if cfg!(target_os = "linux") {
DaitaPlatform::LinuxWgGo
} else if cfg!(target_os = "macos") {
DaitaPlatform::MacosWgGo
} else if cfg!(target_os = "android") {
DaitaPlatform::AndroidWgGo
} else if cfg!(target_os = "ios") {
DaitaPlatform::IosWgGo
} else {
panic!("This platform does not support DAITA V2!")
};
PLATFORM
}
Which results in the following compiler error if we try to compile the crate on a platform where get_platform
is not implemented:
error[E0080]: evaluation of constant value failed
--> talpid-tunnel-config-client/src/lib.rs:234:9
|
234 | panic!("This platform does not support DAITA V2!")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'This platform does not support DAITA V2!', talpid-tunnel-config-client/src/lib.rs:234:9
|
= note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
note: erroneous constant encountered
--> talpid-tunnel-config-client/src/lib.rs:236:5
|
236 | PLATFORM
| ^^^^^^^^
For more information about this error, try `rustc --explain E0080`.
error: could not compile `talpid-tunnel-config-client` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
Code quote:
#[cfg(all(unix, not(target_os = "ios")))]
fn get_platform() -> proto::DaitaPlatform {
#[cfg(windows)]
{
proto::DaitaPlatform::WindowsNative
}
#[cfg(target_os = "linux")]
{
proto::DaitaPlatform::LinuxWgGo
}
#[cfg(target_os = "macos")]
{
proto::DaitaPlatform::MacosWgGo
}
#[cfg(target_os = "android")]
{
proto::DaitaPlatform::AndroidWgGo
}
#[cfg(target_os = "ios")]
{
proto::DaitaPlatform::IosWgGo
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 17 of 20 files reviewed, all discussions resolved (waiting on @MarkusPettersson98)
talpid-tunnel-config-client/src/lib.rs
line 249 at r3 (raw file):
Previously, MarkusPettersson98 (Markus Pettersson) wrote…
⛏️ Consider re-writing this function using a combination of
const
andif-else
in conjunction withcfg!
instead. It will play nicer with people's editors, it is shorter and we can customize the compilation error message 😊const fn get_platform() -> proto::DaitaPlatform { use proto::DaitaPlatform; const PLATFORM: DaitaPlatform = if cfg!(windows) { DaitaPlatform::WindowsNative } else if cfg!(target_os = "linux") { DaitaPlatform::LinuxWgGo } else if cfg!(target_os = "macos") { DaitaPlatform::MacosWgGo } else if cfg!(target_os = "android") { DaitaPlatform::AndroidWgGo } else if cfg!(target_os = "ios") { DaitaPlatform::IosWgGo } else { panic!("This platform does not support DAITA V2!") }; PLATFORM }Which results in the following compiler error if we try to compile the crate on a platform where
get_platform
is not implemented:error[E0080]: evaluation of constant value failed --> talpid-tunnel-config-client/src/lib.rs:234:9 | 234 | panic!("This platform does not support DAITA V2!") | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'This platform does not support DAITA V2!', talpid-tunnel-config-client/src/lib.rs:234:9 | = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered --> talpid-tunnel-config-client/src/lib.rs:236:5 | 236 | PLATFORM | ^^^^^^^^ For more information about this error, try `rustc --explain E0080`. error: could not compile `talpid-tunnel-config-client` (lib) due to 1 previous error warning: build failed, waiting for other jobs to finish...
Nice suggestion!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
Co-authored-by: Markus Pettersson <[email protected]>
Co-authored-by: Markus Pettersson <[email protected]>
Previous size resulted in occasional dropped events
This PR enables DAITA V2 for all platforms that use wireguard go (except on iOS).
Related PR: mullvad/wireguard-go#18.
Fix DES-1386.
This change is