aws / s2n-quic

An implementation of the IETF QUIC protocol
https://crates.io/crates/s2n-quic
Apache License 2.0
1.13k stars 120 forks source link

A means to turn off all platform-specific features in `s2n-quic-platform` #1850

Closed dongcarl closed 1 year ago

dongcarl commented 1 year ago

The conversation arose out of: https://github.com/aws/s2n-quic/pull/1849

Basically, non-s2n_quic_platform_socket_mmsg and non-s2n_quic_platform_socket_msg (as in, falling back to s2n_quic_platform::message::simple) codepaths were not being compiled, leading to the issue that #1849 fixed.

@camshaft indicated that it would be good to have some way of disabling all platform-specific features to prevent issues like this.

I had patched build.rs to notice an env var, although a cfg var can be used too and is probably more appropriate.

camshaft commented 1 year ago

Thinking about it a bit, I think the best approach is to use a environment variable rather than a cfg, since that would require passing that cfg to all of the dependencies and force a recompilation if changed. For the env var, I think something like S2N_QUIC_PLATFORM_FEATURES_OVERRIDE=msg,pktinfo,gso would be the most flexible for testing weird combinations in CI. If you want to disable all of the features, then you can simply pass an empty env value.

dongcarl commented 1 year ago

Here's a patch, I won't open the PR since I have no idea how the CI works:

diff --git a/quic/s2n-quic-platform/build.rs b/quic/s2n-quic-platform/build.rs
index 0777e6cf..ca2012ce 100644
--- a/quic/s2n-quic-platform/build.rs
+++ b/quic/s2n-quic-platform/build.rs
@@ -1,9 +1,22 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

-use std::{fs::read_dir, io::Error, path::Path, process::Command};
+use std::{error, fs::read_dir, io, path::Path, process::Command};
+
+fn main() -> Result<(), Box<dyn error::Error>> {
+    const FEATURES_OVERRIDE_ENV_VAR: &str = "S2N_QUIC_PLATFORM_FEATURES_OVERRIDE";
+    println!("cargo:rerun-if-env-changed={}", FEATURES_OVERRIDE_ENV_VAR);
+    match std::env::var(FEATURES_OVERRIDE_ENV_VAR) {
+        Ok(val) => {
+            for feature in val.split(",") {
+                supports(feature);
+            }
+            return Ok(());
+        }
+        Err(std::env::VarError::NotPresent) => {},
+        Err(e) => return Err(e.into()),
+    }

-fn main() -> Result<(), Error> {
     let env = Env::new();

     for feature in read_dir("features")? {
@@ -77,7 +90,7 @@ impl Env {
     }

     // Tries to compile the program and returns if it was successful
-    fn check(&self, path: &Path) -> Result<bool, Error> {
+    fn check(&self, path: &Path) -> Result<bool, io::Error> {
         let mut command = Command::new(&self.rustc);

         command