Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add preferred default release profile settings #55

Merged
merged 11 commits into from
Jun 16, 2020
38 changes: 38 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ walkdir = "2.3.1"

[dev-dependencies]
assert_matches = "1.3.0"
pretty_assertions = "0.6.1"
wabt = "0.9.2"

[features]
Expand Down
9 changes: 6 additions & 3 deletions src/cmd/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use std::{

use crate::{
util,
workspace::{ManifestPath, Workspace},
workspace::{ManifestPath, Profile, Workspace},
UnstableFlags, Verbosity,
};
use anyhow::{Context, Result};
Expand Down Expand Up @@ -100,7 +100,10 @@ pub fn collect_crate_metadata(manifest_path: &ManifestPath) -> Result<CrateMetad
/// The original Cargo.toml will be amended to remove the `rlib` crate type in order to minimize
/// the final Wasm binary size.
///
/// To disable this and use the `Cargo.toml` as is then pass the `-Z original_manifest` flag.
/// Preferred default `[profile.release]` settings will be added if they are missing, existing
/// user-defined settings will be preserved.
///
/// To disable this and use the original `Cargo.toml` as is then pass the `-Z original_manifest` flag.
fn build_cargo_project(
crate_metadata: &CrateMetadata,
verbosity: Option<Verbosity>,
Expand Down Expand Up @@ -160,7 +163,7 @@ fn build_cargo_project(
.with_root_package_manifest(|manifest| {
manifest
.with_removed_crate_type("rlib")?
.with_profile_release_lto(true)?;
.with_profile_release_defaults(Profile::default_contract_release())?;
Ok(())
})?
.using_temp(xbuild)?;
Expand Down
193 changes: 186 additions & 7 deletions src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub struct Manifest {
}

impl Manifest {
/// Create new CargoToml for the given manifest path.
/// Create new Manifest for the given manifest path.
///
/// The path *must* be to a `Cargo.toml`.
pub fn new<P>(path: P) -> Result<Manifest>
Expand Down Expand Up @@ -137,6 +137,28 @@ impl Manifest {

/// Set `[profile.release]` lto flag
pub fn with_profile_release_lto(&mut self, enabled: bool) -> Result<&mut Self> {
let lto = self
.get_profile_release_table_mut()?
.entry("lto")
.or_insert(enabled.into());
*lto = enabled.into();
Ok(self)
}
ascjones marked this conversation as resolved.
Show resolved Hide resolved

/// Set preferred defaults for the `[profile.release]` section
///
/// # Note
///
/// Existing user defined settings for this section are preserved. Only if a setting is not
/// defined is the preferred default set.
pub fn with_profile_release_defaults(&mut self, defaults: Profile) -> Result<&mut Self> {
let profile_release = self.get_profile_release_table_mut()?;
defaults.merge(profile_release);
Ok(self)
}

/// Get mutable reference to `[profile.release]` section
fn get_profile_release_table_mut(&mut self) -> Result<&mut value::Table> {
let profile = self
.toml
.entry("profile")
Expand All @@ -146,13 +168,9 @@ impl Manifest {
.ok_or(anyhow::anyhow!("profile should be a table"))?
.entry("release")
.or_insert(value::Value::Table(Default::default()));
let lto = release
release
.as_table_mut()
.ok_or(anyhow::anyhow!("release should be a table"))?
.entry("lto")
.or_insert(enabled.into());
*lto = enabled.into();
Ok(self)
.ok_or(anyhow::anyhow!("release should be a table"))
}

/// Remove a value from the `[lib] crate-types = []` section
Expand Down Expand Up @@ -411,3 +429,164 @@ impl Workspace {
f(root_manifest_path)
}
}

/// Subset of cargo profile settings to configure defaults for building contracts
pub struct Profile {
opt_level: OptLevel,
lto: Lto,
// `None` means use rustc default.
codegen_units: Option<u32>,
overflow_checks: bool,
panic: PanicStrategy,
}

impl Profile {
/// The preferred set of defaults for compiling a release build of a contract
pub fn default_contract_release() -> Profile {
Profile {
opt_level: OptLevel::Z,
lto: Lto::Fat,
codegen_units: Some(1),
overflow_checks: true,
panic: PanicStrategy::Abort,
}
}

/// Set any unset profile settings from the config.
///
/// Therefore:
/// - If the user has explicitly defined a profile setting, it will not be overwritten.
/// - If a profile setting is not defined, the value from this profile instance will be added
fn merge(&self, profile: &mut value::Table) {
let mut set_value_if_vacant = |key: &'static str, value: value::Value| {
if !profile.contains_key(key) {
profile.insert(key.into(), value);
}
};
set_value_if_vacant("opt-level", self.opt_level.to_toml_value());
set_value_if_vacant("lto", self.lto.to_toml_value());
if let Some(codegen_units) = self.codegen_units {
set_value_if_vacant("codegen-units", codegen_units.into());
}
set_value_if_vacant("overflow-checks", self.overflow_checks.into());
set_value_if_vacant("panic", self.panic.to_toml_value());
}
}

/// The [`opt-level`](https://doc.rust-lang.org/cargo/reference/profiles.html#opt-level) setting
#[allow(unused)]
#[derive(Clone, Copy)]
pub enum OptLevel {
NoOptimizations,
O1,
O2,
O3,
S,
Z,
}

impl OptLevel {
fn to_toml_value(&self) -> value::Value {
match self {
OptLevel::NoOptimizations => 0.into(),
OptLevel::O1 => 1.into(),
OptLevel::O2 => 2.into(),
OptLevel::O3 => 3.into(),
OptLevel::S => "s".into(),
OptLevel::Z => "z".into(),
}
}
}

/// The [`link-time-optimization`](https://doc.rust-lang.org/cargo/reference/profiles.html#lto) setting.
#[derive(Clone, Copy)]
#[allow(unused)]
pub enum Lto {
/// Sets `lto = false`
ThinLocal,
/// Sets `lto = "fat"`, the equivalent of `lto = true`
Fat,
/// Sets `lto = "thin"`
Thin,
/// Sets `lto = "off"`
Off,
}

impl Lto {
fn to_toml_value(&self) -> value::Value {
match self {
Lto::ThinLocal => value::Value::Boolean(false),
Lto::Fat => value::Value::String("fat".into()),
Lto::Thin => value::Value::String("thin".into()),
Lto::Off => value::Value::String("off".into()),
}
}
}

/// The `panic` setting.
#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, PartialOrd, Ord)]
#[allow(unused)]
pub enum PanicStrategy {
Unwind,
Robbepop marked this conversation as resolved.
Show resolved Hide resolved
Abort,
}

impl PanicStrategy {
fn to_toml_value(&self) -> value::Value {
match self {
PanicStrategy::Unwind => "unwind".into(),
PanicStrategy::Abort => "abort".into(),
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;

#[test]
fn merge_profile_inserts_preferred_defaults() {
let profile = Profile::default_contract_release();

// no `[profile.release]` section specified
let manifest_toml = "";
let mut expected = toml::value::Table::new();
expected.insert("opt-level".into(), value::Value::String("z".into()));
expected.insert("lto".into(), value::Value::String("fat".into()));
expected.insert("codegen-units".into(), value::Value::Integer(1));
expected.insert("overflow-checks".into(), value::Value::Boolean(true));
expected.insert("panic".into(), value::Value::String("abort".into()));

let mut manifest_profile = toml::from_str(manifest_toml).unwrap();

profile.merge(&mut manifest_profile);

assert_eq!(expected, manifest_profile)
}

#[test]
fn merge_profile_preserves_user_defined_settings() {
let profile = Profile::default_contract_release();

let manifest_toml = r#"
panic = "unwind"
lto = false
opt-level = 3
overflow-checks = false
codegen-units = 256
"#;
let mut expected = toml::value::Table::new();
expected.insert("opt-level".into(), value::Value::Integer(3));
expected.insert("lto".into(), value::Value::Boolean(false));
expected.insert("codegen-units".into(), value::Value::Integer(256));
expected.insert("overflow-checks".into(), value::Value::Boolean(false));
expected.insert("panic".into(), value::Value::String("unwind".into()));

let mut manifest_profile = toml::from_str(manifest_toml).unwrap();

profile.merge(&mut manifest_profile);

assert_eq!(expected, manifest_profile)
}
}
6 changes: 0 additions & 6 deletions template/_Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,6 @@ ink-generate-abi = [
]
ink-as-dependency = []

[profile.release]
panic = "abort"
lto = true
opt-level = "z"
overflow-checks = true

[workspace]
members = [
".ink/abi_gen"
Expand Down