Auto merge of #9153 - calavera:strip_options, r=ehuss

Allow `true` and `false` as options for `strip` option

This follows the convention of `lto` and `debug` that allow `true` for
the highest level, and `false` for disabled.

Signed-off-by: David Calavera <[email protected]>

Author: bors

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 => {}
     }
@@ -590,9 +588,12 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) {
     if let Some(incremental) = toml.incremental {
         profile.incremental = incremental;
     }
-    if let Some(strip) = toml.strip {
-        profile.strip = strip;
-    }
+    profile.strip = match toml.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)),
+    };
 }
 
 /// The root profile (dev/release).
@@ -809,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)]
@@ -1249,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")
+}

src/cargo/util/toml/mod.rs

@@ -16,7 +16,6 @@ use url::Url;
 use crate::core::dependency::DepKind;
 use crate::core::manifest::{ManifestMetadata, TargetSourcePath, Warnings};
 use crate::core::nightly_features_allowed;
-use crate::core::profiles::Strip;
 use crate::core::resolver::ResolveBehavior;
 use crate::core::{Dependency, Manifest, PackageId, Summary, Target};
 use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest, Workspace};
@@ -442,7 +441,7 @@ pub struct TomlProfile {
     pub build_override: Option<Box<TomlProfile>>,
     pub dir_name: Option<InternedString>,
     pub inherits: Option<InternedString>,
-    pub strip: Option<Strip>,
+    pub strip: Option<StringOrBool>,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)]
@@ -686,8 +685,8 @@ impl TomlProfile {
             self.dir_name = Some(*v);
         }
 
-        if let Some(v) = profile.strip {
-            self.strip = Some(v);
+        if let Some(v) = &profile.strip {
+            self.strip = Some(v.clone());
         }
     }
 }

src/doc/src/reference/unstable.md

@@ -765,8 +765,11 @@ cargo-features = ["strip"]
 strip = "debuginfo"
 ```
 
-Other possible values of `strip` are `none` and `symbols`. The default is
-`none`.
+Other possible string values of `strip` are `none`, `symbols`, and `off`. The default is `none`.
+
+You can also configure this option with the two absolute boolean values
+`true` and `false`. The former enables `strip` at its higher level, `symbols`,
+whilst the later disables `strip` completely.
 
 ### rustdoc-map
 * Tracking Issue: [#8296](https://github.com/rust-lang/cargo/issues/8296)

tests/testsuite/config.rs

@@ -1,6 +1,5 @@
 //! Tests for config settings.
 
-use cargo::core::profiles::Strip;
 use cargo::core::{enable_nightly_features, Shell};
 use cargo::util::config::{self, Config, SslVersionConfig, StringList};
 use cargo::util::interning::InternedString;
@@ -1446,7 +1445,7 @@ fn string_list_advanced_env() {
 }
 
 #[cargo_test]
-fn parse_enum() {
+fn parse_strip_with_string() {
     write_config(
         "\
 [profile.release]
@@ -1458,28 +1457,5 @@ strip = 'debuginfo'
 
     let p: toml::TomlProfile = config.get("profile.release").unwrap();
     let strip = p.strip.unwrap();
-    assert_eq!(strip, Strip::DebugInfo);
-}
-
-#[cargo_test]
-fn parse_enum_fail() {
-    write_config(
-        "\
-[profile.release]
-strip = 'invalid'
-",
-    );
-
-    let config = new_config();
-
-    assert_error(
-        config
-            .get::<toml::TomlProfile>("profile.release")
-            .unwrap_err(),
-        "\
-error in [..]/.cargo/config: could not load config key `profile.release.strip`
-
-Caused by:
-  unknown variant `invalid`, expected one of `debuginfo`, `none`, `symbols`",
-    );
+    assert_eq!(strip, toml::StringOrBool::String("debuginfo".to_string()));
 }

tests/testsuite/profiles.rs

@@ -509,6 +509,11 @@ fn strip_works() {
 
 #[cargo_test]
 fn strip_requires_cargo_feature() {
+    if !is_nightly() {
+        // -Zstrip is unstable
+        return;
+    }
+
     let p = project()
         .file(
             "Cargo.toml",
@@ -541,7 +546,12 @@ Caused by:
 }
 
 #[cargo_test]
-fn strip_rejects_invalid_option() {
+fn strip_passes_unknown_option_to_rustc() {
+    if !is_nightly() {
+        // -Zstrip is unstable
+        return;
+    }
+
     let p = project()
         .file(
             "Cargo.toml",
@@ -553,7 +563,7 @@ fn strip_rejects_invalid_option() {
                 version = "0.1.0"
 
                 [profile.release]
-                strip = 'wrong'
+                strip = 'unknown'
             "#,
         )
         .file("src/main.rs", "fn main() {}")
@@ -562,13 +572,77 @@ fn strip_rejects_invalid_option() {
     p.cargo("build --release -v")
         .masquerade_as_nightly_cargo()
         .with_status(101)
-        .with_stderr(
+        .with_stderr_contains(
             "\
-[ERROR] failed to parse manifest at `[CWD]/Cargo.toml`
+[COMPILING] foo [..]
+[RUNNING] `rustc [..] -Z strip=unknown [..]`
+error: incorrect value `unknown` for debugging option `strip` - either `none`, `debuginfo`, or `symbols` was expected
+",
+        )
+        .run();
+}
 
-Caused by:
-  unknown variant `wrong`, expected one of `debuginfo`, `none`, `symbols` for key [..]
+#[cargo_test]
+fn strip_accepts_true_to_strip_symbols() {
+    if !is_nightly() {
+        // -Zstrip is unstable
+        return;
+    }
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                cargo-features = ["strip"]
+
+                [package]
+                name = "foo"
+                version = "0.1.0"
+
+                [profile.release]
+                strip = true
+            "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    p.cargo("build --release -v")
+        .masquerade_as_nightly_cargo()
+        .with_stderr(
+            "\
+[COMPILING] foo [..]
+[RUNNING] `rustc [..] -Z strip=symbols [..]`
+[FINISHED] [..]
 ",
         )
         .run();
 }
+
+#[cargo_test]
+fn strip_accepts_false_to_disable_strip() {
+    if !is_nightly() {
+        // -Zstrip is unstable
+        return;
+    }
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                cargo-features = ["strip"]
+
+                [package]
+                name = "foo"
+                version = "0.1.0"
+
+                [profile.release]
+                strip = false
+            "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    p.cargo("build --release -v")
+        .masquerade_as_nightly_cargo()
+        .with_stderr_does_not_contain("-Z strip")
+        .run();
+}