Simplify Strip option

- Accept a string option that's passed to rustc to decide how to handle it. - Remove early validation. Signed-off-by: David Calavera <david.calavera@gmail.com>
rust-lang · Feb 9, 2021 · eb34617 · eb34617
1 parent 933cbc4
commit eb34617
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 57 deletions.
diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs
@@ -552,9 +552,7 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) {
     }
     match toml.lto {
         Some(StringOrBool::Bool(b)) => profile.lto = Lto::Bool(b),
-        Some(StringOrBool::String(ref n)) if matches!(n.as_str(), "off" | "n" | "no") => {
-            profile.lto = Lto::Off
-        }
+        Some(StringOrBool::String(ref n)) if is_off(n.as_str()) => profile.lto = Lto::Off,
         Some(StringOrBool::String(ref n)) => profile.lto = Lto::Named(InternedString::new(n)),
         None => {}
     }
@@ -591,19 +589,10 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) {
         profile.incremental = incremental;
     }
     profile.strip = match toml.strip {
-        Some(StringOrBool::Bool(enabled)) if enabled => Strip::Symbols,
-        Some(StringOrBool::Bool(enabled)) if !enabled => Strip::None,
-        Some(StringOrBool::String(ref n)) if matches!(n.as_str(), "off" | "n" | "no" | "none") => {
-            Strip::None
-        }
-        Some(StringOrBool::String(ref n)) if matches!(n.as_str(), "debuginfo") => Strip::DebugInfo,
-        Some(StringOrBool::String(ref n)) if matches!(n.as_str(), "symbols") => Strip::Symbols,
-        None => Strip::None,
-        Some(ref strip) => panic!(
-            "unknown variant `{}`, expected one of `debuginfo`, `none`, `symbols` for key `strip`
-            ",
-            strip
-        ),
+        Some(StringOrBool::Bool(true)) => Strip::Named(InternedString::new("symbols")),
+        None | Some(StringOrBool::Bool(false)) => Strip::None,
+        Some(StringOrBool::String(ref n)) if is_off(n.as_str()) => Strip::None,
+        Some(StringOrBool::String(ref n)) => Strip::Named(InternedString::new(n)),
     };
 }
 
@@ -821,24 +810,22 @@ impl fmt::Display for PanicStrategy {
 )]
 #[serde(rename_all = "lowercase")]
 pub enum Strip {
-    /// Only strip debugging symbols
-    DebugInfo,
     /// Don't remove any symbols
     None,
-    /// Strip all non-exported symbols from the final binary
-    Symbols,
+    /// Named Strip settings
+    Named(InternedString),
 }
 
 impl fmt::Display for Strip {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match *self {
-            Strip::DebugInfo => "debuginfo",
             Strip::None => "none",
-            Strip::Symbols => "symbols",
+            Strip::Named(s) => s.as_str(),
         }
         .fmt(f)
     }
 }
+
 /// Flags used in creating `Unit`s to indicate the purpose for the target, and
 /// to ensure the target's dependencies have the correct settings.
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
@@ -1261,3 +1248,8 @@ fn validate_packages_unmatched(
     }
     Ok(())
 }
+
+/// Returns `true` if a string is a toggle that turns an option off.
+fn is_off(s: &str) -> bool {
+    matches!(s, "off" | "n" | "no" | "none")
+}
diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs
@@ -560,18 +560,6 @@ impl TomlProfile {
 
         if self.strip.is_some() {
             features.require(Feature::strip())?;
-            match self.strip {
-                Some(StringOrBool::Bool(_)) => {}
-                Some(StringOrBool::String(ref n)) => match n.as_str() {
-                    "off" | "n" | "none" | "no" | "debuginfo" | "symbols" => {}
-                    _ => bail!(
-                        "`strip` setting of `{}` is not a valid setting,\
-                         must be `symbols`, `debuginfo`, `none`, `true`, or `false`",
-                        n
-                    ),
-                },
-                None => {}
-            }
         }
         Ok(())
     }
@@ -780,15 +768,6 @@ impl<'de> de::Deserialize<'de> for StringOrBool {
     }
 }
 
-impl fmt::Display for StringOrBool {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::String(s) => write!(f, "{}", &s),
-            Self::Bool(b) => write!(f, "{}", b),
-        }
-    }
-}
-
 #[derive(PartialEq, Clone, Debug, Serialize)]
 #[serde(untagged)]
 pub enum VecStringOrBool {

diff --git a/tests/testsuite/profiles.rs b/tests/testsuite/profiles.rs
@@ -546,7 +546,7 @@ Caused by:
 }
 
 #[cargo_test]
-fn strip_rejects_invalid_option() {
+fn strip_passes_unknown_option_to_rustc() {
     if !is_nightly() {
         // -Zstrip is unstable
         return;
@@ -563,7 +563,7 @@ fn strip_rejects_invalid_option() {
                 version = "0.1.0"
 
                 [profile.release]
-                strip = 'wrong'
+                strip = 'unknown'
             "#,
         )
         .file("src/main.rs", "fn main() {}")
@@ -574,10 +574,9 @@ fn strip_rejects_invalid_option() {
         .with_status(101)
         .with_stderr(
             "\
-[ERROR] failed to parse manifest at `[CWD]/Cargo.toml`
-
-Caused by:
-  unknown variant `wrong`, expected one of `debuginfo`, `none`, `symbols` for key `strip`
+[COMPILING] foo [..]
+[RUNNING] `rustc [..] -Z strip=unknown [..]`
+[FINISHED] [..]
 ",
         )
         .run();
@@ -644,13 +643,6 @@ fn strip_accepts_false_to_disable_strip() {
 
     p.cargo("build --release -v")
         .masquerade_as_nightly_cargo()
-        .with_stderr(
-            "\
-[COMPILING] foo [..]
-[RUNNING] `rustc [..] -Z strip=none
- [..]`
-[FINISHED] [..]
-",
-        )
+        .with_stderr_does_not_contain("-Z strip")
         .run();
 }