From e7475cab62da1d1951cefddbcc3982bd314861fb Mon Sep 17 00:00:00 2001 From: Gabriel Majeri Date: Fri, 1 May 2020 16:37:05 +0300 Subject: [PATCH 1/9] Add option to strip binaries --- src/cargo/core/compiler/mod.rs | 8 +++++++- src/cargo/core/profiles.rs | 35 ++++++++++++++++++++++++++++++++++ src/cargo/util/toml/mod.rs | 15 +++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index f662495a1a1..c43cf6bfd2d 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -46,7 +46,7 @@ use self::output_depinfo::output_depinfo; use self::unit_graph::UnitDep; pub use crate::core::compiler::unit::{Unit, UnitInterner}; use crate::core::manifest::TargetSourcePath; -use crate::core::profiles::{PanicStrategy, Profile}; +use crate::core::profiles::{PanicStrategy, Profile, Strip}; use crate::core::{Edition, Feature, InternedString, PackageId, Target}; use crate::util::errors::{self, CargoResult, CargoResultExt, ProcessError, VerboseError}; use crate::util::machine_message::Message; @@ -732,6 +732,7 @@ fn build_base_args( rpath, ref panic, incremental, + strip, .. } = unit.profile; let test = unit.mode.is_any_test(); @@ -910,6 +911,11 @@ fn build_base_args( opt(cmd, "-C", "incremental=", Some(dir)); } + if strip != Strip::None { + cmd.arg("-Z").arg(format!("strip={}", strip)); + } + + if unit.is_std { // -Zforce-unstable-if-unmarked prevents the accidental use of // unstable crates within the sysroot (such as "extern crate libc" or diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index d4769389abd..2f2445fdc28 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -565,6 +565,14 @@ 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 = match strip.as_str() { + "debuginfo" => Strip::DebugInfo, + "none" => Strip::None, + "symbols" => Strip::Symbols, + _ => panic!("Unexpected strip option `{}`", strip), + }; + } } /// The root profile (dev/release). @@ -595,6 +603,7 @@ pub struct Profile { pub rpath: bool, pub incremental: bool, pub panic: PanicStrategy, + pub strip: Strip, } impl Default for Profile { @@ -611,6 +620,7 @@ impl Default for Profile { rpath: false, incremental: false, panic: PanicStrategy::Unwind, + strip: Strip::None, } } } @@ -635,6 +645,7 @@ compact_debug! { rpath incremental panic + strip )] } } @@ -721,6 +732,7 @@ impl Profile { bool, bool, PanicStrategy, + Strip, ) { ( self.opt_level, @@ -732,6 +744,7 @@ impl Profile { self.rpath, self.incremental, self.panic, + self.strip, ) } } @@ -776,6 +789,28 @@ impl fmt::Display for PanicStrategy { } } +/// The setting for choosing which symbols to strip +#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, PartialOrd, Ord, serde::Serialize)] +#[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, +} + +impl fmt::Display for Strip { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match *self { + Strip::DebugInfo => "unwind", + Strip::None => "abort", + Strip::Symbols => "symbols", + } + .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)] diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 246bd37f145..897812e9476 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -407,6 +407,7 @@ pub struct TomlProfile { pub build_override: Option>, pub dir_name: Option, pub inherits: Option, + pub strip: Option, } #[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)] @@ -522,6 +523,16 @@ impl TomlProfile { ); } } + + if let Some(strip) = &self.strip { + if strip != "debuginfo" && strip != "none" && strip != "symbols" { + bail!( + "`strip` setting of `{}` is not a valid setting,\ + must be `debuginfo`, `none` or `symbols`", + strip + ); + } + } Ok(()) } @@ -641,6 +652,10 @@ impl TomlProfile { if let Some(v) = &profile.dir_name { self.dir_name = Some(*v); } + + if let Some(v) = profile.strip { + self.strip = Some(v); + } } } From c6529dd66bdc53d0958eb2fab369303cb9cf92cc Mon Sep 17 00:00:00 2001 From: Gabriel Majeri Date: Sat, 16 May 2020 20:47:55 +0300 Subject: [PATCH 2/9] Fix formatting --- src/cargo/core/compiler/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index c43cf6bfd2d..1ba65031e97 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -915,7 +915,6 @@ fn build_base_args( cmd.arg("-Z").arg(format!("strip={}", strip)); } - if unit.is_std { // -Zforce-unstable-if-unmarked prevents the accidental use of // unstable crates within the sysroot (such as "extern crate libc" or From d36e793753319b602e95b35c5b5b5e37c73e1646 Mon Sep 17 00:00:00 2001 From: Gabriel Majeri Date: Sat, 16 May 2020 22:28:06 +0300 Subject: [PATCH 3/9] Fix tests --- tests/testsuite/unit_graph.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/testsuite/unit_graph.rs b/tests/testsuite/unit_graph.rs index 878e7c3b4f5..2313a3ce6d5 100644 --- a/tests/testsuite/unit_graph.rs +++ b/tests/testsuite/unit_graph.rs @@ -75,7 +75,8 @@ fn simple() { "overflow_checks": true, "rpath": false, "incremental": false, - "panic": "unwind" + "panic": "unwind", + "strip": "none" }, "platform": null, "mode": "build", @@ -115,7 +116,8 @@ fn simple() { "overflow_checks": true, "rpath": false, "incremental": false, - "panic": "unwind" + "panic": "unwind", + "strip": "none" }, "platform": null, "mode": "build", @@ -155,7 +157,8 @@ fn simple() { "overflow_checks": true, "rpath": false, "incremental": false, - "panic": "unwind" + "panic": "unwind", + "strip": "none" }, "platform": null, "mode": "build", @@ -188,7 +191,8 @@ fn simple() { "overflow_checks": true, "rpath": false, "incremental": false, - "panic": "unwind" + "panic": "unwind", + "strip": "none" }, "platform": null, "mode": "build", From 348043a25c015c1e3831135f8b50c67ac735dd63 Mon Sep 17 00:00:00 2001 From: Gabriel Majeri Date: Mon, 18 May 2020 21:56:59 +0300 Subject: [PATCH 4/9] Fix typo --- src/cargo/core/profiles.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 2f2445fdc28..964a2549491 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -804,7 +804,7 @@ pub enum Strip { impl fmt::Display for Strip { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self { - Strip::DebugInfo => "unwind", + Strip::DebugInfo => "debuginfo", Strip::None => "abort", Strip::Symbols => "symbols", } From 9551ed34c11179b681461d04b6e09fa9de96b9f1 Mon Sep 17 00:00:00 2001 From: Gabriel Majeri Date: Mon, 18 May 2020 21:57:09 +0300 Subject: [PATCH 5/9] Add nightly feature --- src/cargo/core/features.rs | 5 +++++ src/cargo/util/toml/mod.rs | 1 + 2 files changed, 6 insertions(+) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 1c82a033d49..a876c49cc1e 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -214,6 +214,9 @@ features! { // Opt-in new-resolver behavior. [unstable] resolver: bool, + + // Allow to specify whether binaries should be stripped. + [unstable] strip: bool, } } @@ -353,6 +356,7 @@ pub struct CliUnstable { pub crate_versions: bool, pub separate_nightlies: bool, pub multitarget: bool, + pub strip: bool, } impl CliUnstable { @@ -432,6 +436,7 @@ impl CliUnstable { "crate-versions" => self.crate_versions = parse_empty(k, v)?, "separate-nightlies" => self.separate_nightlies = parse_empty(k, v)?, "multitarget" => self.multitarget = parse_empty(k, v)?, + "strip" => self.strip = parse_empty(k, v)?, _ => bail!("unknown `-Z` flag specified: {}", k), } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 897812e9476..fa24d196925 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -525,6 +525,7 @@ impl TomlProfile { } if let Some(strip) = &self.strip { + features.require(Feature::strip())?; if strip != "debuginfo" && strip != "none" && strip != "symbols" { bail!( "`strip` setting of `{}` is not a valid setting,\ From a10eb862814b1c1a1c0c7b251f655b24a7f96bbe Mon Sep 17 00:00:00 2001 From: Gabriel Majeri Date: Mon, 18 May 2020 21:57:12 +0300 Subject: [PATCH 6/9] Add test --- tests/testsuite/profiles.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/testsuite/profiles.rs b/tests/testsuite/profiles.rs index ad8bbe7db1a..0db2a3de6e6 100644 --- a/tests/testsuite/profiles.rs +++ b/tests/testsuite/profiles.rs @@ -468,3 +468,34 @@ fn thin_lto_works() { ) .run(); } + +#[cargo_test] +fn strip_works() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["strip"] + + [package] + name = "foo" + version = "0.1.0" + + [profile.release] + strip = 'symbols' + "#, + ) + .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(); +} From e768b17a873e329a29f5a74ee04076d239172213 Mon Sep 17 00:00:00 2001 From: Gabriel Majeri Date: Tue, 19 May 2020 21:02:23 +0300 Subject: [PATCH 7/9] Remove unstable CLI option --- src/cargo/core/features.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index a876c49cc1e..6a05b0f091f 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -356,7 +356,6 @@ pub struct CliUnstable { pub crate_versions: bool, pub separate_nightlies: bool, pub multitarget: bool, - pub strip: bool, } impl CliUnstable { @@ -436,7 +435,6 @@ impl CliUnstable { "crate-versions" => self.crate_versions = parse_empty(k, v)?, "separate-nightlies" => self.separate_nightlies = parse_empty(k, v)?, "multitarget" => self.multitarget = parse_empty(k, v)?, - "strip" => self.strip = parse_empty(k, v)?, _ => bail!("unknown `-Z` flag specified: {}", k), } From 5353f81ef2492f6738a8d70a3cfbc8d8e57c211d Mon Sep 17 00:00:00 2001 From: Gabriel Majeri Date: Tue, 19 May 2020 21:03:52 +0300 Subject: [PATCH 8/9] Use serde for deserializing strip option --- src/cargo/core/profiles.rs | 11 ++++------- src/cargo/util/toml/mod.rs | 12 +++--------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 964a2549491..fc5fe31bccb 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -566,12 +566,7 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) { profile.incremental = incremental; } if let Some(strip) = toml.strip { - profile.strip = match strip.as_str() { - "debuginfo" => Strip::DebugInfo, - "none" => Strip::None, - "symbols" => Strip::Symbols, - _ => panic!("Unexpected strip option `{}`", strip), - }; + profile.strip = strip; } } @@ -790,7 +785,9 @@ impl fmt::Display for PanicStrategy { } /// The setting for choosing which symbols to strip -#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, PartialOrd, Ord, serde::Serialize)] +#[derive( + Clone, Copy, PartialEq, Eq, Debug, Hash, PartialOrd, Ord, serde::Serialize, serde::Deserialize, +)] #[serde(rename_all = "lowercase")] pub enum Strip { /// Only strip debugging symbols diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index fa24d196925..7cb2240e71f 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -15,6 +15,7 @@ use url::Url; use crate::core::dependency::DepKind; use crate::core::manifest::{ManifestMetadata, TargetSourcePath, Warnings}; +use crate::core::profiles::Strip; use crate::core::resolver::ResolveBehavior; use crate::core::{Dependency, InternedString, Manifest, PackageId, Summary, Target}; use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest, Workspace}; @@ -407,7 +408,7 @@ pub struct TomlProfile { pub build_override: Option>, pub dir_name: Option, pub inherits: Option, - pub strip: Option, + pub strip: Option, } #[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)] @@ -524,15 +525,8 @@ impl TomlProfile { } } - if let Some(strip) = &self.strip { + if self.strip.is_some() { features.require(Feature::strip())?; - if strip != "debuginfo" && strip != "none" && strip != "symbols" { - bail!( - "`strip` setting of `{}` is not a valid setting,\ - must be `debuginfo`, `none` or `symbols`", - strip - ); - } } Ok(()) } From 2cd41e1d7b8413f9a4e5e1029ba747825845df26 Mon Sep 17 00:00:00 2001 From: Gabriel Majeri Date: Tue, 19 May 2020 21:03:59 +0300 Subject: [PATCH 9/9] Add more tests --- tests/testsuite/profiles.rs | 79 ++++++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/tests/testsuite/profiles.rs b/tests/testsuite/profiles.rs index 0db2a3de6e6..1a51d20b0eb 100644 --- a/tests/testsuite/profiles.rs +++ b/tests/testsuite/profiles.rs @@ -2,7 +2,7 @@ use std::env; -use cargo_test_support::project; +use cargo_test_support::{is_nightly, project}; #[cargo_test] fn profile_overrides() { @@ -471,6 +471,10 @@ fn thin_lto_works() { #[cargo_test] fn strip_works() { + if !is_nightly() { + return; + } + let p = project() .file( "Cargo.toml", @@ -499,3 +503,76 @@ fn strip_works() { ) .run(); } + +#[cargo_test] +fn strip_requires_cargo_feature() { + if !is_nightly() { + return; + } + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [profile.release] + strip = 'symbols' + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("build --release -v") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr( + "\ +[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + feature `strip` is required + +consider adding `cargo-features = [\"strip\"]` to the manifest +", + ) + .run(); +} +#[cargo_test] +fn strip_rejects_invalid_option() { + if !is_nightly() { + return; + } + + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["strip"] + + [package] + name = "foo" + version = "0.1.0" + + [profile.release] + strip = 'wrong' + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("build --release -v") + .masquerade_as_nightly_cargo() + .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 [..] +", + ) + .run(); +}