Skip to content

Commit

Permalink
Auto merge of #8246 - GabrielMajeri:add-strip-option, r=alexcrichton
Browse files Browse the repository at this point in the history
Add option to strip binaries

This PR adds a Cargo option for stripping symbols from generated binaries.

This is based on the `-Z strip` flag for `rustc`, which has been [recently implemented](rust-lang/rust#71757).

Notes for reviewers: I'm not entirely sure how we test this, I've created a crate locally and it seems to be working.

Supersedes my previous work in #8191.
Fixes #3483.
  • Loading branch information
bors committed May 19, 2020
2 parents 500b2bd + 2cd41e1 commit d18e4b3
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 6 deletions.
7 changes: 6 additions & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -732,6 +732,7 @@ fn build_base_args(
rpath,
ref panic,
incremental,
strip,
..
} = unit.profile;
let test = unit.mode.is_any_test();
Expand Down Expand Up @@ -910,6 +911,10 @@ 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
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,9 @@ features! {

// Opt-in new-resolver behavior.
[unstable] resolver: bool,

// Allow to specify whether binaries should be stripped.
[unstable] strip: bool,
}
}

Expand Down
32 changes: 32 additions & 0 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,9 @@ 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;
}
}

/// The root profile (dev/release).
Expand Down Expand Up @@ -595,6 +598,7 @@ pub struct Profile {
pub rpath: bool,
pub incremental: bool,
pub panic: PanicStrategy,
pub strip: Strip,
}

impl Default for Profile {
Expand All @@ -611,6 +615,7 @@ impl Default for Profile {
rpath: false,
incremental: false,
panic: PanicStrategy::Unwind,
strip: Strip::None,
}
}
}
Expand All @@ -635,6 +640,7 @@ compact_debug! {
rpath
incremental
panic
strip
)]
}
}
Expand Down Expand Up @@ -721,6 +727,7 @@ impl Profile {
bool,
bool,
PanicStrategy,
Strip,
) {
(
self.opt_level,
Expand All @@ -732,6 +739,7 @@ impl Profile {
self.rpath,
self.incremental,
self.panic,
self.strip,
)
}
}
Expand Down Expand Up @@ -776,6 +784,30 @@ 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::Deserialize,
)]
#[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 => "debuginfo",
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)]
Expand Down
10 changes: 10 additions & 0 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -407,6 +408,7 @@ pub struct TomlProfile {
pub build_override: Option<Box<TomlProfile>>,
pub dir_name: Option<InternedString>,
pub inherits: Option<InternedString>,
pub strip: Option<Strip>,
}

#[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)]
Expand Down Expand Up @@ -522,6 +524,10 @@ impl TomlProfile {
);
}
}

if self.strip.is_some() {
features.require(Feature::strip())?;
}
Ok(())
}

Expand Down Expand Up @@ -641,6 +647,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);
}
}
}

Expand Down
110 changes: 109 additions & 1 deletion tests/testsuite/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use std::env;

use cargo_test_support::project;
use cargo_test_support::{is_nightly, project};

#[cargo_test]
fn profile_overrides() {
Expand Down Expand Up @@ -468,3 +468,111 @@ fn thin_lto_works() {
)
.run();
}

#[cargo_test]
fn strip_works() {
if !is_nightly() {
return;
}

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();
}

#[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();
}
12 changes: 8 additions & 4 deletions tests/testsuite/unit_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ fn simple() {
"overflow_checks": true,
"rpath": false,
"incremental": false,
"panic": "unwind"
"panic": "unwind",
"strip": "none"
},
"platform": null,
"mode": "build",
Expand Down Expand Up @@ -115,7 +116,8 @@ fn simple() {
"overflow_checks": true,
"rpath": false,
"incremental": false,
"panic": "unwind"
"panic": "unwind",
"strip": "none"
},
"platform": null,
"mode": "build",
Expand Down Expand Up @@ -155,7 +157,8 @@ fn simple() {
"overflow_checks": true,
"rpath": false,
"incremental": false,
"panic": "unwind"
"panic": "unwind",
"strip": "none"
},
"platform": null,
"mode": "build",
Expand Down Expand Up @@ -188,7 +191,8 @@ fn simple() {
"overflow_checks": true,
"rpath": false,
"incremental": false,
"panic": "unwind"
"panic": "unwind",
"strip": "none"
},
"platform": null,
"mode": "build",
Expand Down

0 comments on commit d18e4b3

Please sign in to comment.