From 7eb7c3e8d2c4dbb4c7f6d239aac0eb7569f1fd37 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 27 Mar 2020 09:34:35 +0000 Subject: [PATCH 1/8] Add option to build with unmodified original manifest --- src/cmd/build.rs | 30 +++++++++++++++++++++--------- src/main.rs | 7 +++++-- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 0537b19a..b82021b4 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -94,7 +94,14 @@ pub fn collect_crate_metadata(manifest_path: &ManifestPath) -> Result) -> Result<()> { +/// +/// # Cargo.toml optimizations +/// +/// 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 set `original_manifest` to true. +fn build_cargo_project(crate_metadata: &CrateMetadata, verbosity: Option, original_manifest: bool) -> Result<()> { util::assert_channel()?; // set RUSTFLAGS, read from environment var by cargo-xbuild @@ -136,12 +143,16 @@ fn build_cargo_project(crate_metadata: &CrateMetadata, verbosity: Option Result<()> { pub(crate) fn execute_build( manifest_path: ManifestPath, verbosity: Option, + original_manifest: bool, ) -> Result { println!( " {} {}", @@ -283,7 +295,7 @@ pub(crate) fn execute_build( "[2/4]".bold(), "Building cargo project".bright_green().bold() ); - build_cargo_project(&crate_metadata, verbosity)?; + build_cargo_project(&crate_metadata, verbosity, original_manifest)?; println!( " {} {}", "[3/4]".bold(), @@ -314,7 +326,7 @@ mod tests { execute_new("new_project", Some(path)).expect("new project creation failed"); let manifest_path = ManifestPath::new(&path.join("new_project").join("Cargo.toml")).unwrap(); - super::execute_build(manifest_path, None).expect("build failed"); + super::execute_build(manifest_path, None, false).expect("build failed"); }); } } diff --git a/src/main.rs b/src/main.rs index 7bd4cf92..e4233d38 100644 --- a/src/main.rs +++ b/src/main.rs @@ -131,6 +131,9 @@ enum Command { Build { #[structopt(flatten)] verbosity: VerbosityFlags, + /// Use the original manifest (Cargo.toml), do not modify for build optimizations + #[structopt(long)] + original_manifest: bool, }, /// Generate contract metadata artifacts #[structopt(name = "generate-metadata")] @@ -197,8 +200,8 @@ fn main() { fn exec(cmd: Command) -> Result { match &cmd { Command::New { name, target_dir } => cmd::execute_new(name, target_dir.as_ref()), - Command::Build { verbosity } => { - cmd::execute_build(Default::default(), verbosity.try_into()?) + Command::Build { verbosity, original_manifest } => { + cmd::execute_build(Default::default(), verbosity.try_into()?, *original_manifest) } Command::GenerateMetadata { verbosity } => { cmd::execute_generate_metadata(Default::default(), verbosity.try_into()?) -- GitLab From e33086bc6973e732fde7d46ba2ee176c7c38487b Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 27 Mar 2020 09:45:08 +0000 Subject: [PATCH 2/8] Fmt --- src/cmd/build.rs | 6 +++++- src/main.rs | 11 ++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index b82021b4..a7cca841 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -101,7 +101,11 @@ pub fn collect_crate_metadata(manifest_path: &ManifestPath) -> Result, original_manifest: bool) -> Result<()> { +fn build_cargo_project( + crate_metadata: &CrateMetadata, + verbosity: Option, + original_manifest: bool, +) -> Result<()> { util::assert_channel()?; // set RUSTFLAGS, read from environment var by cargo-xbuild diff --git a/src/main.rs b/src/main.rs index e4233d38..60615514 100644 --- a/src/main.rs +++ b/src/main.rs @@ -200,9 +200,14 @@ fn main() { fn exec(cmd: Command) -> Result { match &cmd { Command::New { name, target_dir } => cmd::execute_new(name, target_dir.as_ref()), - Command::Build { verbosity, original_manifest } => { - cmd::execute_build(Default::default(), verbosity.try_into()?, *original_manifest) - } + Command::Build { + verbosity, + original_manifest, + } => cmd::execute_build( + Default::default(), + verbosity.try_into()?, + *original_manifest, + ), Command::GenerateMetadata { verbosity } => { cmd::execute_generate_metadata(Default::default(), verbosity.try_into()?) } -- GitLab From 7b774ebd36500b5c5895b31351dd8c8d53fcefcf Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 22 May 2020 08:13:23 +0100 Subject: [PATCH 3/8] Add unstable-options --- src/cmd/build.rs | 18 +++++++----------- src/cmd/metadata.rs | 29 ++++++++++++++++++----------- src/main.rs | 45 ++++++++++++++++++++++++++++++++++++++------- 3 files changed, 63 insertions(+), 29 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 6d1c10b0..71dd3c9c 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -21,11 +21,7 @@ use std::{ process::Command, }; -use crate::{ - util, - workspace::{ManifestPath, Workspace}, - Verbosity, -}; +use crate::{util, workspace::{ManifestPath, Workspace}, Verbosity, UnstableFlags}; use anyhow::{Context, Result}; use cargo_metadata::Package; use colored::Colorize; @@ -104,7 +100,7 @@ pub fn collect_crate_metadata(manifest_path: &ManifestPath) -> Result, - original_manifest: bool, + unstable_options: UnstableFlags, ) -> Result<()> { util::assert_channel()?; @@ -147,7 +143,7 @@ fn build_cargo_project( Ok(()) }; - if original_manifest { + if unstable_options.original_manifest { xbuild(&crate_metadata.manifest_path)?; } else { Workspace::new(&crate_metadata.cargo_meta, &crate_metadata.root_package.id)? @@ -288,7 +284,7 @@ fn optimize_wasm(crate_metadata: &CrateMetadata) -> Result<()> { pub(crate) fn execute_build( manifest_path: ManifestPath, verbosity: Option, - original_manifest: bool, + unstable_options: UnstableFlags, ) -> Result { println!( " {} {}", @@ -301,7 +297,7 @@ pub(crate) fn execute_build( "[2/4]".bold(), "Building cargo project".bright_green().bold() ); - build_cargo_project(&crate_metadata, verbosity, original_manifest)?; + build_cargo_project(&crate_metadata, verbosity, unstable_options)?; println!( " {} {}", "[3/4]".bold(), @@ -324,7 +320,7 @@ pub(crate) fn execute_build( #[cfg(feature = "test-ci-only")] #[cfg(test)] mod tests { - use crate::{cmd::execute_new, util::tests::with_tmp_dir, workspace::ManifestPath}; + use crate::{cmd::execute_new, util::tests::with_tmp_dir, workspace::ManifestPath, UnstableFlags}; #[test] fn build_template() { @@ -332,7 +328,7 @@ mod tests { execute_new("new_project", Some(path)).expect("new project creation failed"); let manifest_path = ManifestPath::new(&path.join("new_project").join("Cargo.toml")).unwrap(); - super::execute_build(manifest_path, None, false).expect("build failed"); + super::execute_build(manifest_path, None, UnstableFlags::default()).expect("build failed"); }); } } diff --git a/src/cmd/metadata.rs b/src/cmd/metadata.rs index 98803c77..846b3dea 100644 --- a/src/cmd/metadata.rs +++ b/src/cmd/metadata.rs @@ -18,6 +18,7 @@ use crate::{ util, workspace::{ManifestPath, Workspace}, Verbosity, + UnstableFlags, }; use anyhow::Result; @@ -29,6 +30,7 @@ const METADATA_FILE: &str = "metadata.json"; pub(crate) fn execute_generate_metadata( manifest_path: ManifestPath, verbosity: Option, + unstable_options: UnstableFlags, ) -> Result { util::assert_channel()?; println!(" Generating metadata"); @@ -40,14 +42,14 @@ pub(crate) fn execute_generate_metadata( let target_dir = metadata.target_directory.clone(); - let generate_metadata = move |tmp_manifest_path: &ManifestPath| -> Result<()> { + let generate_metadata = move |manifest_path: &ManifestPath| -> Result<()> { let target_dir_arg = format!("--target-dir={}", target_dir.to_string_lossy()); util::invoke_cargo( "run", &[ "--package", "abi-gen", - &tmp_manifest_path.cargo_arg(), + &manifest_path.cargo_arg(), &target_dir_arg, "--release", // "--no-default-features", // Breaks builds for MacOS (linker errors), we should investigate this issue asap! @@ -57,14 +59,18 @@ pub(crate) fn execute_generate_metadata( ) }; - Workspace::new(&metadata, &root_package_id)? - .with_root_package_manifest(|manifest| { - manifest - .with_added_crate_type("rlib")? - .with_profile_release_lto(false)?; - Ok(()) - })? - .using_temp(generate_metadata)?; + if unstable_options.original_manifest { + generate_metadata(&manifest_path)?; + } else { + Workspace::new(&metadata, &root_package_id)? + .with_root_package_manifest(|manifest| { + manifest + .with_added_crate_type("rlib")? + .with_profile_release_lto(false)?; + Ok(()) + })? + .using_temp(generate_metadata)?; + } Ok(format!( "Your metadata file is ready.\nYou can find it here:\n{}", @@ -79,6 +85,7 @@ mod tests { cmd::{execute_generate_metadata, execute_new}, util::tests::with_tmp_dir, workspace::ManifestPath, + UnstableFlags, }; #[test] @@ -89,7 +96,7 @@ mod tests { let working_dir = path.join("new_project"); let manifest_path = ManifestPath::new(working_dir.join("Cargo.toml")).unwrap(); let message = - execute_generate_metadata(manifest_path, None).expect("generate metadata failed"); + execute_generate_metadata(manifest_path, None, UnstableFlags::default()).expect("generate metadata failed"); println!("{}", message); let mut abi_file = working_dir; diff --git a/src/main.rs b/src/main.rs index 547353a4..96bc00b3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -112,6 +112,36 @@ impl TryFrom<&VerbosityFlags> for Option { } } +#[derive(Debug, StructOpt)] +struct UnstableOptions { + /// Use the original manifest (Cargo.toml), do not modify for build optimizations + #[structopt(long = "unstable-options", short = "Z", number_of_values = 1)] + options: Vec, +} + +#[derive(Default)] +struct UnstableFlags { + original_manifest: bool, +} + +impl TryFrom<&UnstableOptions> for UnstableFlags { + type Error = Error; + + fn try_from(value: &UnstableOptions) -> Result { + let valid_flags = ["original-manifest"]; + let invalid_flags = value.options + .iter() + .filter(|o| !valid_flags.contains(&o.as_str())) + .collect::>(); + if !invalid_flags.is_empty() { + anyhow::bail!("Unknown unstable-options flag(s): {:?}", invalid_flags) + } + Ok(UnstableFlags { + original_manifest: value.options.contains(&"original-manifest".to_owned()) + }) + } +} + #[derive(Debug, StructOpt)] enum Command { /// Setup and create a new smart contract project @@ -128,15 +158,16 @@ enum Command { Build { #[structopt(flatten)] verbosity: VerbosityFlags, - /// Use the original manifest (Cargo.toml), do not modify for build optimizations - #[structopt(long)] - original_manifest: bool, + #[structopt(flatten)] + unstable_options: UnstableOptions, }, /// Generate contract metadata artifacts #[structopt(name = "generate-metadata")] GenerateMetadata { #[structopt(flatten)] verbosity: VerbosityFlags, + #[structopt(flatten)] + unstable_options: UnstableOptions, }, /// Test the smart contract off-chain #[structopt(name = "test")] @@ -202,14 +233,14 @@ fn exec(cmd: Command) -> Result { Command::New { name, target_dir } => cmd::execute_new(name, target_dir.as_ref()), Command::Build { verbosity, - original_manifest, + unstable_options, } => cmd::execute_build( Default::default(), verbosity.try_into()?, - *original_manifest, + unstable_options.try_into()?, ), - Command::GenerateMetadata { verbosity } => { - cmd::execute_generate_metadata(Default::default(), verbosity.try_into()?) + Command::GenerateMetadata { verbosity, unstable_options } => { + cmd::execute_generate_metadata(Default::default(), verbosity.try_into()?, unstable_options.try_into()?) } Command::Test {} => Err(anyhow::anyhow!("Command unimplemented")), #[cfg(feature = "extrinsics")] -- GitLab From 26fdefd8a6b1621dac9ea2d7e944b02352e2c46c Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 22 May 2020 08:13:42 +0100 Subject: [PATCH 4/8] Fmt --- src/cmd/build.rs | 13 ++++++++++--- src/cmd/metadata.rs | 7 +++---- src/main.rs | 16 +++++++++++----- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 71dd3c9c..f4a84618 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -21,7 +21,11 @@ use std::{ process::Command, }; -use crate::{util, workspace::{ManifestPath, Workspace}, Verbosity, UnstableFlags}; +use crate::{ + util, + workspace::{ManifestPath, Workspace}, + UnstableFlags, Verbosity, +}; use anyhow::{Context, Result}; use cargo_metadata::Package; use colored::Colorize; @@ -320,7 +324,9 @@ pub(crate) fn execute_build( #[cfg(feature = "test-ci-only")] #[cfg(test)] mod tests { - use crate::{cmd::execute_new, util::tests::with_tmp_dir, workspace::ManifestPath, UnstableFlags}; + use crate::{ + cmd::execute_new, util::tests::with_tmp_dir, workspace::ManifestPath, UnstableFlags, + }; #[test] fn build_template() { @@ -328,7 +334,8 @@ mod tests { execute_new("new_project", Some(path)).expect("new project creation failed"); let manifest_path = ManifestPath::new(&path.join("new_project").join("Cargo.toml")).unwrap(); - super::execute_build(manifest_path, None, UnstableFlags::default()).expect("build failed"); + super::execute_build(manifest_path, None, UnstableFlags::default()) + .expect("build failed"); }); } } diff --git a/src/cmd/metadata.rs b/src/cmd/metadata.rs index 846b3dea..f1538ccc 100644 --- a/src/cmd/metadata.rs +++ b/src/cmd/metadata.rs @@ -17,8 +17,7 @@ use crate::{ util, workspace::{ManifestPath, Workspace}, - Verbosity, - UnstableFlags, + UnstableFlags, Verbosity, }; use anyhow::Result; @@ -95,8 +94,8 @@ mod tests { execute_new("new_project", Some(path)).expect("new project creation failed"); let working_dir = path.join("new_project"); let manifest_path = ManifestPath::new(working_dir.join("Cargo.toml")).unwrap(); - let message = - execute_generate_metadata(manifest_path, None, UnstableFlags::default()).expect("generate metadata failed"); + let message = execute_generate_metadata(manifest_path, None, UnstableFlags::default()) + .expect("generate metadata failed"); println!("{}", message); let mut abi_file = working_dir; diff --git a/src/main.rs b/src/main.rs index 96bc00b3..3af7a284 100644 --- a/src/main.rs +++ b/src/main.rs @@ -129,7 +129,8 @@ impl TryFrom<&UnstableOptions> for UnstableFlags { fn try_from(value: &UnstableOptions) -> Result { let valid_flags = ["original-manifest"]; - let invalid_flags = value.options + let invalid_flags = value + .options .iter() .filter(|o| !valid_flags.contains(&o.as_str())) .collect::>(); @@ -137,7 +138,7 @@ impl TryFrom<&UnstableOptions> for UnstableFlags { anyhow::bail!("Unknown unstable-options flag(s): {:?}", invalid_flags) } Ok(UnstableFlags { - original_manifest: value.options.contains(&"original-manifest".to_owned()) + original_manifest: value.options.contains(&"original-manifest".to_owned()), }) } } @@ -239,9 +240,14 @@ fn exec(cmd: Command) -> Result { verbosity.try_into()?, unstable_options.try_into()?, ), - Command::GenerateMetadata { verbosity, unstable_options } => { - cmd::execute_generate_metadata(Default::default(), verbosity.try_into()?, unstable_options.try_into()?) - } + Command::GenerateMetadata { + verbosity, + unstable_options, + } => cmd::execute_generate_metadata( + Default::default(), + verbosity.try_into()?, + unstable_options.try_into()?, + ), Command::Test {} => Err(anyhow::anyhow!("Command unimplemented")), #[cfg(feature = "extrinsics")] Command::Deploy { -- GitLab From 808ee0ff560327234fcfc8cab913395798a7653e Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 22 May 2020 08:24:44 +0100 Subject: [PATCH 5/8] Warn during build if original-manifest enabled --- src/cmd/build.rs | 5 +++++ src/main.rs | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index f4a84618..c6cf9f81 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -148,6 +148,11 @@ fn build_cargo_project( }; if unstable_options.original_manifest { + println!( + "{} {}", + "warning:".yellow().bold(), + "with 'original-manifest' enabled, the contract binary may not be of optimal size.".bold() + ); xbuild(&crate_metadata.manifest_path)?; } else { Workspace::new(&crate_metadata.cargo_meta, &crate_metadata.root_package.id)? diff --git a/src/main.rs b/src/main.rs index 3af7a284..adeae4d3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -135,7 +135,7 @@ impl TryFrom<&UnstableOptions> for UnstableFlags { .filter(|o| !valid_flags.contains(&o.as_str())) .collect::>(); if !invalid_flags.is_empty() { - anyhow::bail!("Unknown unstable-options flag(s): {:?}", invalid_flags) + anyhow::bail!("Unknown unstable-options {:?}", invalid_flags) } Ok(UnstableFlags { original_manifest: value.options.contains(&"original-manifest".to_owned()), -- GitLab From cc7cd66373c5750946d9fc8bff426fed1328aa87 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 22 May 2020 08:29:43 +0100 Subject: [PATCH 6/8] Fmt --- src/cmd/build.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index c6cf9f81..4e71c15c 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -151,7 +151,8 @@ fn build_cargo_project( println!( "{} {}", "warning:".yellow().bold(), - "with 'original-manifest' enabled, the contract binary may not be of optimal size.".bold() + "with 'original-manifest' enabled, the contract binary may not be of optimal size." + .bold() ); xbuild(&crate_metadata.manifest_path)?; } else { -- GitLab From 956b06ea686f1de366818a4f84f72c4353481331 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 22 May 2020 08:31:07 +0100 Subject: [PATCH 7/8] Update comment --- src/cmd/build.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 4e71c15c..ef100e4f 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -100,7 +100,7 @@ pub fn collect_crate_metadata(manifest_path: &ManifestPath) -> Result, -- GitLab From 4fcdd57545171d08cc289f80973a5e705c8ee086 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 22 May 2020 11:55:11 +0100 Subject: [PATCH 8/8] Fix metadata generation --- src/cmd/metadata.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cmd/metadata.rs b/src/cmd/metadata.rs index f1538ccc..ca70f176 100644 --- a/src/cmd/metadata.rs +++ b/src/cmd/metadata.rs @@ -27,21 +27,21 @@ const METADATA_FILE: &str = "metadata.json"; /// /// It does so by invoking build by cargo and then post processing the final binary. pub(crate) fn execute_generate_metadata( - manifest_path: ManifestPath, + original_manifest_path: ManifestPath, verbosity: Option, unstable_options: UnstableFlags, ) -> Result { util::assert_channel()?; println!(" Generating metadata"); - let (metadata, root_package_id) = crate::util::get_cargo_metadata(&manifest_path)?; + let (metadata, root_package_id) = crate::util::get_cargo_metadata(&original_manifest_path)?; let out_path = metadata.target_directory.join(METADATA_FILE); let out_path_display = format!("{}", out_path.display()); let target_dir = metadata.target_directory.clone(); - let generate_metadata = move |manifest_path: &ManifestPath| -> Result<()> { + let generate_metadata = |manifest_path: &ManifestPath| -> Result<()> { let target_dir_arg = format!("--target-dir={}", target_dir.to_string_lossy()); util::invoke_cargo( "run", @@ -53,13 +53,13 @@ pub(crate) fn execute_generate_metadata( "--release", // "--no-default-features", // Breaks builds for MacOS (linker errors), we should investigate this issue asap! ], - manifest_path.directory(), + original_manifest_path.directory(), verbosity, ) }; if unstable_options.original_manifest { - generate_metadata(&manifest_path)?; + generate_metadata(&original_manifest_path)?; } else { Workspace::new(&metadata, &root_package_id)? .with_root_package_manifest(|manifest| { -- GitLab