fix: use BTreeSet for std-features; add unit tests

* fix formatting of string in front of std_features
* rename `std_features` to `std-features` in config.toml

Author: shrirambalaji

config.example.toml

@@ -760,7 +760,7 @@
 #validate-mir-opts = 3
 
 # Configure `std` features used during bootstrap
-# std_features = ["panic_unwind"]
+# std-features = ["panic_unwind", "backtrace"]
 
 # =============================================================================
 # Options for specific targets

src/bootstrap/src/core/build_steps/compile.rs

@@ -553,7 +553,9 @@ pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, stage: u32, car
             .arg("--features")
             .arg(features);
     } else {
-        features += &builder.std_features(target);
+        let builder_std_features = format!(" {}", &builder.std_features(target));
+
+        features += &builder_std_features;
         features.push_str(compiler_builtins_c_feature);
 
         cargo

src/bootstrap/src/core/config/config.rs

@@ -4,7 +4,7 @@
 //! how the build runs.
 
 use std::cell::{Cell, RefCell};
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeSet, HashMap, HashSet};
 use std::fmt::{self, Display};
 use std::io::IsTerminal;
 use std::path::{absolute, Path, PathBuf};
@@ -287,7 +287,7 @@ pub struct Config {
     pub rust_profile_generate: Option<String>,
     pub rust_lto: RustcLto,
     pub rust_validate_mir_opts: Option<u32>,
-    pub rust_std_features: Option<Vec<String>>,
+    pub rust_std_features: Option<BTreeSet<String>>,
     pub llvm_profile_use: Option<String>,
     pub llvm_profile_generate: bool,
     pub llvm_libunwind_default: Option<LlvmLibunwind>,
@@ -1142,7 +1142,7 @@ define_config! {
         download_rustc: Option<StringOrBool> = "download-rustc",
         lto: Option<String> = "lto",
         validate_mir_opts: Option<u32> = "validate-mir-opts",
-        std_features: Option<Vec<String>> = "std_features",
+        std_features: Option<Vec<String>> = "std-features",
     }
 }
 
@@ -2097,7 +2097,9 @@ impl Config {
         }
 
         // std_features chosen during bootstrap
-        config.rust_std_features = std_features;
+        if let Some(std_features) = std_features {
+            config.rust_std_features = Some(BTreeSet::from_iter(std_features.into_iter()));
+        }
 
         let default = debug == Some(true);
         config.rust_debug_assertions = debug_assertions.unwrap_or(default);

src/bootstrap/src/core/config/tests.rs

@@ -1,3 +1,4 @@
+use std::collections::BTreeSet;
 use std::env;
 use std::fs::{remove_file, File};
 use std::io::Write;
@@ -326,3 +327,24 @@ fn verbose_tests_default_value() {
     let config = Config::parse(Flags::parse(&["build".into(), "compiler".into(), "-v".into()]));
     assert_eq!(config.verbose_tests, true);
 }
+
+#[test]
+fn parse_rust_std_features() {
+    let config = parse("rust.std-features = [\"panic-unwind\", \"backtrace\"]");
+    let expected_features: BTreeSet<String> =
+        ["panic-unwind", "backtrace"].into_iter().map(|s| s.to_string()).collect();
+    assert_eq!(config.rust_std_features, Some(expected_features));
+}
+
+#[test]
+fn parse_rust_std_features_empty() {
+    let config = parse("rust.std-features = []");
+    let expected_features: BTreeSet<String> = BTreeSet::new();
+    assert_eq!(config.rust_std_features, Some(expected_features));
+}
+
+#[test]
+#[should_panic]
+fn parse_rust_std_features_invalid() {
+    parse("rust.std-features = \"backtrace\"");
+}

src/bootstrap/src/lib.rs

@@ -17,7 +17,7 @@
 //! also check out the `src/bootstrap/README.md` file for more information.
 
 use std::cell::{Cell, RefCell};
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeSet, HashMap, HashSet};
 use std::fmt::Display;
 use std::fs::{self, File};
 use std::path::{Path, PathBuf};
@@ -661,38 +661,35 @@ impl Build {
         &self.config.rust_info
     }
 
-    /// Gets the space-separated set of activated features for the standard
-    /// library. This can be configured with the `std_features` key in config.toml.
+    /// Gets the space-separated set of activated features for the standard ibrary. This can be configured with the `std-features` key in config.toml.
     fn std_features(&self, target: TargetSelection) -> String {
-        let mut features = if let Some(features) = &self.config.rust_std_features {
-            features.iter().map(|s| s.as_ref()).collect::<Vec<&str>>()
+        let mut features: BTreeSet<&str> = if let Some(features) = &self.config.rust_std_features {
+            features.iter().map(|s| s.as_str()).collect()
         } else {
-            vec![]
+            BTreeSet::new()
         };
 
-        if !features.contains(&"panic-unwind") {
-            features.push("panic-unwind");
-        }
+        features.insert("panic-unwind");
 
         match self.config.llvm_libunwind(target) {
-            LlvmLibunwind::InTree => features.push("llvm-libunwind"),
-            LlvmLibunwind::System => features.push("system-llvm-libunwind"),
-            LlvmLibunwind::No => {}
-        }
+            LlvmLibunwind::InTree => features.insert("llvm-libunwind"),
+            LlvmLibunwind::System => features.insert("system-llvm-libunwind"),
+            LlvmLibunwind::No => false,
+        };
+
         if self.config.backtrace {
-            features.push("backtrace");
+            features.insert("backtrace");
         }
         if self.config.profiler_enabled(target) {
-            features.push("profiler");
+            features.insert("profiler");
         }
         // Generate memcpy, etc.  FIXME: Remove this once compiler-builtins
         // automatically detects this target.
         if target.contains("zkvm") {
-            features.push("compiler-builtins-mem");
+            features.insert("compiler-builtins-mem");
         }
 
-        // remove duplicates
-        features.into_iter().collect::<HashSet<_>>().into_iter().collect::<Vec<_>>().join(" ")
+        features.into_iter().collect::<Vec<_>>().join(" ")
     }
 
     /// Gets the space-separated set of activated features for the compiler.