From 91df71c9ac392a6e18e5c8a128540116fdedfdff Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 8 Jul 2021 11:59:49 +0200 Subject: [PATCH 01/10] Implement `BuildMode` --- src/cmd/build.rs | 152 +++++++++++++++++++++++++++++++++++++++++--- src/cmd/metadata.rs | 3 +- src/main.rs | 51 ++++++++++++++- 3 files changed, 195 insertions(+), 11 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index fbbc6514..7def563d 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -18,13 +18,14 @@ use crate::{ crate_metadata::CrateMetadata, maybe_println, util, validate_wasm, workspace::{Manifest, ManifestPath, Profile, Workspace}, - BuildArtifacts, BuildResult, OptimizationPasses, OptimizationResult, UnstableFlags, + BuildArtifacts, BuildMode, BuildResult, OptimizationPasses, OptimizationResult, UnstableFlags, UnstableOptions, Verbosity, VerbosityFlags, }; use anyhow::{Context, Result}; use colored::Colorize; use parity_wasm::elements::{External, MemoryType, Module, Section}; use regex::Regex; +use semver::Version; use std::{ convert::TryFrom, ffi::OsStr, @@ -47,6 +48,24 @@ pub struct BuildCommand { /// Path to the Cargo.toml of the contract to build #[structopt(long, parse(from_os_str))] manifest_path: Option, + /// Which mode to build the contract in. + /// + /// - `debug`: The contract will be compiled with debug functionality + /// included. This enables the contract to output debug messages. + /// + /// This increases the contract size and the used gas. A production + /// contract should never be build in debug mode! + /// + /// - `release`: No debug functionality is compiled into the contract. + /// + /// - The default value is `debug`. + #[structopt( + long = "mode", + default_value = "debug", + value_name = "debug | release", + verbatim_doc_comment + )] + build_mode: BuildMode, /// Which build artifacts to generate. /// /// - `all`: Generate the Wasm, the metadata and a bundled `.contract` file. @@ -115,6 +134,7 @@ impl BuildCommand { execute( &manifest_path, verbosity, + self.build_mode, self.build_artifact, unstable_flags, optimization_passes, @@ -143,6 +163,7 @@ impl CheckCommand { execute( &manifest_path, verbosity, + BuildMode::Debug, BuildArtifacts::CheckOnly, unstable_flags, OptimizationPasses::Zero, @@ -169,6 +190,7 @@ impl CheckCommand { fn exec_cargo_for_wasm_target( crate_metadata: &CrateMetadata, command: &str, + build_mode: BuildMode, verbosity: Verbosity, unstable_flags: &UnstableFlags, ) -> Result<()> { @@ -183,14 +205,18 @@ fn exec_cargo_for_wasm_target( let cargo_build = |manifest_path: &ManifestPath| { let target_dir = &crate_metadata.target_directory; - let args = [ + let target_dir = format!("--target-dir={}", target_dir.to_string_lossy()); + let mut args = vec![ "--target=wasm32-unknown-unknown", "-Zbuild-std", "-Zbuild-std-features=panic_immediate_abort", "--no-default-features", "--release", - &format!("--target-dir={}", target_dir.to_string_lossy()), + &target_dir, ]; + if build_mode == BuildMode::Debug { + args.push("--features=ink_env/ink-debug"); + } util::invoke_cargo(command, &args, manifest_path.directory(), verbosity)?; Ok(()) @@ -520,12 +546,26 @@ fn assert_compatible_ink_dependencies( Ok(()) } +/// Checks whether the supplied `ink_version` already contains the debug feature. +/// +/// This feature was introduced in `3.0.0-rc4` with `ink_env/ink-debug`. +pub fn assert_debug_mode_supported(ink_version: &Version) -> anyhow::Result<()> { + log::info!("Contract version: {:?}", ink_version); + if ink_version <= &Version::parse("3.0.0-rc3").expect("parsing minimum ink! version failed") { + anyhow::bail!( + "Building the contract in debug mode requires an ink! version newer than `3.0.0-rc3`!" + ); + } + Ok(()) +} + /// Executes build of the smart-contract which produces a wasm binary that is ready for deploying. /// /// It does so by invoking `cargo build` and then post processing the final binary. pub(crate) fn execute( manifest_path: &ManifestPath, verbosity: Verbosity, + build_mode: BuildMode, build_artifact: BuildArtifacts, unstable_flags: UnstableFlags, optimization_passes: OptimizationPasses, @@ -533,6 +573,7 @@ pub(crate) fn execute( let crate_metadata = CrateMetadata::collect(manifest_path)?; assert_compatible_ink_dependencies(manifest_path, verbosity)?; + assert_debug_mode_supported(&crate_metadata.ink_version)?; let build = || -> Result { maybe_println!( @@ -541,7 +582,13 @@ pub(crate) fn execute( format!("[1/{}]", build_artifact.steps()).bold(), "Building cargo project".bright_green().bold() ); - exec_cargo_for_wasm_target(&crate_metadata, "build", verbosity, &unstable_flags)?; + exec_cargo_for_wasm_target( + &crate_metadata, + "build", + build_mode, + verbosity, + &unstable_flags, + )?; maybe_println!( verbosity, @@ -564,7 +611,13 @@ pub(crate) fn execute( let (opt_result, metadata_result) = match build_artifact { BuildArtifacts::CheckOnly => { - exec_cargo_for_wasm_target(&crate_metadata, "check", verbosity, &unstable_flags)?; + exec_cargo_for_wasm_target( + &crate_metadata, + "check", + BuildMode::Release, + verbosity, + &unstable_flags, + )?; (None, None) } BuildArtifacts::CodeOnly => { @@ -590,6 +643,7 @@ pub(crate) fn execute( metadata_result, target_directory: crate_metadata.target_directory, optimization_result: opt_result, + build_mode, build_artifact, verbosity, }) @@ -598,14 +652,18 @@ pub(crate) fn execute( #[cfg(feature = "test-ci-only")] #[cfg(test)] mod tests_ci_only { - use super::{assert_compatible_ink_dependencies, check_wasm_opt_version_compatibility}; + use super::{ + assert_compatible_ink_dependencies, assert_debug_mode_supported, + check_wasm_opt_version_compatibility, + }; use crate::{ cmd::BuildCommand, util::tests::{with_new_contract_project, with_tmp_dir}, workspace::Manifest, - BuildArtifacts, ManifestPath, OptimizationPasses, UnstableFlags, UnstableOptions, - Verbosity, VerbosityFlags, + BuildArtifacts, BuildMode, ManifestPath, OptimizationPasses, UnstableFlags, + UnstableOptions, Verbosity, VerbosityFlags, }; + use semver::Version; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; use std::{ @@ -657,6 +715,7 @@ mod tests_ci_only { let res = super::execute( &manifest_path, Verbosity::Default, + BuildMode::default(), BuildArtifacts::CodeOnly, UnstableFlags::default(), OptimizationPasses::default(), @@ -696,6 +755,7 @@ mod tests_ci_only { super::execute( &manifest_path, Verbosity::Default, + BuildMode::default(), BuildArtifacts::CheckOnly, UnstableFlags::default(), OptimizationPasses::default(), @@ -726,6 +786,7 @@ mod tests_ci_only { let cmd = BuildCommand { manifest_path: Some(manifest_path.into()), build_artifact: BuildArtifacts::All, + build_mode: BuildMode::default(), verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), @@ -766,6 +827,7 @@ mod tests_ci_only { let cmd = BuildCommand { manifest_path: Some(manifest_path.into()), build_artifact: BuildArtifacts::All, + build_mode: BuildMode::default(), verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), @@ -927,6 +989,7 @@ mod tests_ci_only { let cmd = BuildCommand { manifest_path: Some(manifest_path.into()), build_artifact: BuildArtifacts::All, + build_mode: BuildMode::default(), verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), optimization_passes: None, @@ -944,4 +1007,77 @@ mod tests_ci_only { Ok(()) }) } + + #[test] + pub fn debug_mode_must_be_compatible() { + let _ = + assert_debug_mode_supported(&Version::parse("3.0.0-rc4").expect("parsing must work")) + .expect("debug mode must be compatible"); + let _ = + assert_debug_mode_supported(&Version::parse("4.0.0-rc1").expect("parsing must work")) + .expect("debug mode must be compatible"); + let _ = assert_debug_mode_supported(&Version::parse("5.0.0").expect("parsing must work")) + .expect("debug mode must be compatible"); + } + + #[test] + pub fn debug_mode_must_be_incompatible() { + let res = + assert_debug_mode_supported(&Version::parse("3.0.0-rc3").expect("parsing must work")) + .expect_err("assertion must fail"); + assert_eq!( + res.to_string(), + "Building the contract in debug mode requires an ink! version newer than `3.0.0-rc3`!" + ); + } + + // We must ignore the test until ink! `3.0.0-rc4` is released, which will + // contain `ink_env/ink-debug`. + #[ignore] + #[test] + fn building_template_in_debug_mode_must_work() { + with_new_contract_project(|manifest_path| { + // given + let build_mode = BuildMode::Debug; + + // when + let res = super::execute( + &manifest_path, + Verbosity::Default, + build_mode, + BuildArtifacts::All, + UnstableFlags::default(), + OptimizationPasses::default(), + ); + + // then + assert!(res.is_ok(), "building template in debug mode failed!"); + Ok(()) + }) + } + + // We must ignore the test until ink! `3.0.0-rc4` is released, which will + // contain `ink_env/ink-debug`. + #[ignore] + #[test] + fn building_template_in_release_mode_must_work() { + with_new_contract_project(|manifest_path| { + // given + let build_mode = BuildMode::Release; + + // when + let res = super::execute( + &manifest_path, + Verbosity::Default, + build_mode, + BuildArtifacts::All, + UnstableFlags::default(), + OptimizationPasses::default(), + ); + + // then + assert!(res.is_ok(), "building template in release mode failed!"); + Ok(()) + }) + } } diff --git a/src/cmd/metadata.rs b/src/cmd/metadata.rs index 2a0d2e2b..cb4472cb 100644 --- a/src/cmd/metadata.rs +++ b/src/cmd/metadata.rs @@ -226,7 +226,7 @@ mod tests { use crate::cmd::metadata::blake2_hash; use crate::{ cmd, crate_metadata::CrateMetadata, util::tests::with_new_contract_project, BuildArtifacts, - ManifestPath, OptimizationPasses, UnstableFlags, Verbosity, + BuildMode, ManifestPath, OptimizationPasses, UnstableFlags, Verbosity, }; use contract_metadata::*; use serde_json::{Map, Value}; @@ -325,6 +325,7 @@ mod tests { let build_result = cmd::build::execute( &test_manifest.manifest_path, Verbosity::Default, + BuildMode::default(), BuildArtifacts::All, UnstableFlags::default(), OptimizationPasses::default(), diff --git a/src/main.rs b/src/main.rs index a74056b9..f578cc8e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -270,6 +270,44 @@ impl std::str::FromStr for BuildArtifacts { } } +/// The mode to build the contract in. +#[derive(Copy, Clone, Eq, PartialEq, Debug, StructOpt)] +#[structopt(name = "build-mode")] +pub enum BuildMode { + /// Functionality to output debug messages is build into the contract. + #[structopt(name = "debug")] + Debug, + /// The contract is build without any debugging functionality. + #[structopt(name = "release")] + Release, +} + +impl Default for BuildMode { + fn default() -> BuildMode { + BuildMode::Debug + } +} + +impl Display for BuildMode { + fn fmt(&self, f: &mut Formatter<'_>) -> DisplayResult { + match self { + Self::Debug => write!(f, "debug"), + Self::Release => write!(f, "release"), + } + } +} + +impl std::str::FromStr for BuildMode { + type Err = String; + fn from_str(artifact: &str) -> Result { + match artifact { + "debug" => Ok(BuildMode::Debug), + "release" => Ok(BuildMode::Release), + _ => Err("Could not parse build mode".to_string()), + } + } +} + /// Result of the metadata generation process. pub struct BuildResult { /// Path to the resulting Wasm file. @@ -280,6 +318,8 @@ pub struct BuildResult { pub target_directory: PathBuf, /// If existent the result of the optimization. pub optimization_result: Option, + /// The mode to build the contract in. + pub build_mode: BuildMode, /// Which build artifacts were generated. pub build_artifact: BuildArtifacts, /// The verbosity flags. @@ -309,10 +349,16 @@ impl BuildResult { "optimized file size must be greater 0" ); + let build_mode = format!( + "The contract was built in {} mode.\n\n", + format!("{}", self.build_mode).to_uppercase().bold(), + ); + if self.build_artifact == BuildArtifacts::CodeOnly { let out = format!( - "{}Your contract's code is ready. You can find it here:\n{}", + "{}{}Your contract's code is ready. You can find it here:\n{}", size_diff, + build_mode, self.dest_wasm .as_ref() .expect("wasm path must exist") @@ -324,8 +370,9 @@ impl BuildResult { }; let mut out = format!( - "{}Your contract artifacts are ready. You can find them in:\n{}\n\n", + "{}{}Your contract artifacts are ready. You can find them in:\n{}\n\n", size_diff, + build_mode, self.target_directory.display().to_string().bold(), ); if let Some(metadata_result) = self.metadata_result.as_ref() { -- GitLab From e4ed7efa4e1c544a8989280119c9392a97735691 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 8 Jul 2021 12:28:05 +0200 Subject: [PATCH 02/10] Remove `ignore` since all tests are failing anyway --- src/cmd/build.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 7def563d..8f1e8b56 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -1031,9 +1031,6 @@ mod tests_ci_only { ); } - // We must ignore the test until ink! `3.0.0-rc4` is released, which will - // contain `ink_env/ink-debug`. - #[ignore] #[test] fn building_template_in_debug_mode_must_work() { with_new_contract_project(|manifest_path| { @@ -1056,9 +1053,6 @@ mod tests_ci_only { }) } - // We must ignore the test until ink! `3.0.0-rc4` is released, which will - // contain `ink_env/ink-debug`. - #[ignore] #[test] fn building_template_in_release_mode_must_work() { with_new_contract_project(|manifest_path| { -- GitLab From 3cb01e1044f668649d9900e62a2fdeb1b17f44c8 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 8 Jul 2021 19:24:03 +0200 Subject: [PATCH 03/10] Switch to `--release` --- src/cmd/build.rs | 72 ++++++++++++++++++--------------------------- src/cmd/metadata.rs | 4 +-- src/main.rs | 48 +++++------------------------- 3 files changed, 37 insertions(+), 87 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 8f1e8b56..10f86b3e 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -18,7 +18,7 @@ use crate::{ crate_metadata::CrateMetadata, maybe_println, util, validate_wasm, workspace::{Manifest, ManifestPath, Profile, Workspace}, - BuildArtifacts, BuildMode, BuildResult, OptimizationPasses, OptimizationResult, UnstableFlags, + BuildArtifacts, BuildResult, OptimizationPasses, OptimizationResult, UnstableFlags, UnstableOptions, Verbosity, VerbosityFlags, }; use anyhow::{Context, Result}; @@ -48,24 +48,14 @@ pub struct BuildCommand { /// Path to the Cargo.toml of the contract to build #[structopt(long, parse(from_os_str))] manifest_path: Option, - /// Which mode to build the contract in. + /// By default the contract is compiled with debug functionality + /// included. This enables the contract to output debug messages, + /// but increases the contract size and the amount of gas used. /// - /// - `debug`: The contract will be compiled with debug functionality - /// included. This enables the contract to output debug messages. - /// - /// This increases the contract size and the used gas. A production - /// contract should never be build in debug mode! - /// - /// - `release`: No debug functionality is compiled into the contract. - /// - /// - The default value is `debug`. - #[structopt( - long = "mode", - default_value = "debug", - value_name = "debug | release", - verbatim_doc_comment - )] - build_mode: BuildMode, + /// A production contract should always be build in `release` mode! + /// Then no debug functionality is compiled into the contract. + #[structopt(long = "--release")] + build_release: bool, /// Which build artifacts to generate. /// /// - `all`: Generate the Wasm, the metadata and a bundled `.contract` file. @@ -134,7 +124,7 @@ impl BuildCommand { execute( &manifest_path, verbosity, - self.build_mode, + self.build_release, self.build_artifact, unstable_flags, optimization_passes, @@ -163,7 +153,7 @@ impl CheckCommand { execute( &manifest_path, verbosity, - BuildMode::Debug, + false, BuildArtifacts::CheckOnly, unstable_flags, OptimizationPasses::Zero, @@ -190,7 +180,7 @@ impl CheckCommand { fn exec_cargo_for_wasm_target( crate_metadata: &CrateMetadata, command: &str, - build_mode: BuildMode, + build_release: bool, verbosity: Verbosity, unstable_flags: &UnstableFlags, ) -> Result<()> { @@ -214,7 +204,7 @@ fn exec_cargo_for_wasm_target( "--release", &target_dir, ]; - if build_mode == BuildMode::Debug { + if !build_release { args.push("--features=ink_env/ink-debug"); } util::invoke_cargo(command, &args, manifest_path.directory(), verbosity)?; @@ -551,7 +541,7 @@ fn assert_compatible_ink_dependencies( /// This feature was introduced in `3.0.0-rc4` with `ink_env/ink-debug`. pub fn assert_debug_mode_supported(ink_version: &Version) -> anyhow::Result<()> { log::info!("Contract version: {:?}", ink_version); - if ink_version <= &Version::parse("3.0.0-rc3").expect("parsing minimum ink! version failed") { + if ink_version <= &Version::parse("3.0.0-rc2").expect("parsing minimum ink! version failed") { anyhow::bail!( "Building the contract in debug mode requires an ink! version newer than `3.0.0-rc3`!" ); @@ -565,7 +555,7 @@ pub fn assert_debug_mode_supported(ink_version: &Version) -> anyhow::Result<()> pub(crate) fn execute( manifest_path: &ManifestPath, verbosity: Verbosity, - build_mode: BuildMode, + build_release: bool, build_artifact: BuildArtifacts, unstable_flags: UnstableFlags, optimization_passes: OptimizationPasses, @@ -585,7 +575,7 @@ pub(crate) fn execute( exec_cargo_for_wasm_target( &crate_metadata, "build", - build_mode, + build_release, verbosity, &unstable_flags, )?; @@ -611,13 +601,7 @@ pub(crate) fn execute( let (opt_result, metadata_result) = match build_artifact { BuildArtifacts::CheckOnly => { - exec_cargo_for_wasm_target( - &crate_metadata, - "check", - BuildMode::Release, - verbosity, - &unstable_flags, - )?; + exec_cargo_for_wasm_target(&crate_metadata, "check", true, verbosity, &unstable_flags)?; (None, None) } BuildArtifacts::CodeOnly => { @@ -643,7 +627,7 @@ pub(crate) fn execute( metadata_result, target_directory: crate_metadata.target_directory, optimization_result: opt_result, - build_mode, + build_release, build_artifact, verbosity, }) @@ -660,8 +644,8 @@ mod tests_ci_only { cmd::BuildCommand, util::tests::{with_new_contract_project, with_tmp_dir}, workspace::Manifest, - BuildArtifacts, BuildMode, ManifestPath, OptimizationPasses, UnstableFlags, - UnstableOptions, Verbosity, VerbosityFlags, + BuildArtifacts, ManifestPath, OptimizationPasses, UnstableFlags, UnstableOptions, + Verbosity, VerbosityFlags, }; use semver::Version; #[cfg(unix)] @@ -715,7 +699,7 @@ mod tests_ci_only { let res = super::execute( &manifest_path, Verbosity::Default, - BuildMode::default(), + false, BuildArtifacts::CodeOnly, UnstableFlags::default(), OptimizationPasses::default(), @@ -755,7 +739,7 @@ mod tests_ci_only { super::execute( &manifest_path, Verbosity::Default, - BuildMode::default(), + false, BuildArtifacts::CheckOnly, UnstableFlags::default(), OptimizationPasses::default(), @@ -786,7 +770,7 @@ mod tests_ci_only { let cmd = BuildCommand { manifest_path: Some(manifest_path.into()), build_artifact: BuildArtifacts::All, - build_mode: BuildMode::default(), + build_release: false, verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), @@ -827,7 +811,7 @@ mod tests_ci_only { let cmd = BuildCommand { manifest_path: Some(manifest_path.into()), build_artifact: BuildArtifacts::All, - build_mode: BuildMode::default(), + build_release: false, verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), @@ -989,7 +973,7 @@ mod tests_ci_only { let cmd = BuildCommand { manifest_path: Some(manifest_path.into()), build_artifact: BuildArtifacts::All, - build_mode: BuildMode::default(), + build_release: false, verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), optimization_passes: None, @@ -1035,13 +1019,13 @@ mod tests_ci_only { fn building_template_in_debug_mode_must_work() { with_new_contract_project(|manifest_path| { // given - let build_mode = BuildMode::Debug; + let build_release = false; // when let res = super::execute( &manifest_path, Verbosity::Default, - build_mode, + build_release, BuildArtifacts::All, UnstableFlags::default(), OptimizationPasses::default(), @@ -1057,13 +1041,13 @@ mod tests_ci_only { fn building_template_in_release_mode_must_work() { with_new_contract_project(|manifest_path| { // given - let build_mode = BuildMode::Release; + let build_release = true; // when let res = super::execute( &manifest_path, Verbosity::Default, - build_mode, + build_release, BuildArtifacts::All, UnstableFlags::default(), OptimizationPasses::default(), diff --git a/src/cmd/metadata.rs b/src/cmd/metadata.rs index cb4472cb..474cf201 100644 --- a/src/cmd/metadata.rs +++ b/src/cmd/metadata.rs @@ -226,7 +226,7 @@ mod tests { use crate::cmd::metadata::blake2_hash; use crate::{ cmd, crate_metadata::CrateMetadata, util::tests::with_new_contract_project, BuildArtifacts, - BuildMode, ManifestPath, OptimizationPasses, UnstableFlags, Verbosity, + ManifestPath, OptimizationPasses, UnstableFlags, Verbosity, }; use contract_metadata::*; use serde_json::{Map, Value}; @@ -325,7 +325,7 @@ mod tests { let build_result = cmd::build::execute( &test_manifest.manifest_path, Verbosity::Default, - BuildMode::default(), + false, BuildArtifacts::All, UnstableFlags::default(), OptimizationPasses::default(), diff --git a/src/main.rs b/src/main.rs index f578cc8e..bdda204c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -270,44 +270,6 @@ impl std::str::FromStr for BuildArtifacts { } } -/// The mode to build the contract in. -#[derive(Copy, Clone, Eq, PartialEq, Debug, StructOpt)] -#[structopt(name = "build-mode")] -pub enum BuildMode { - /// Functionality to output debug messages is build into the contract. - #[structopt(name = "debug")] - Debug, - /// The contract is build without any debugging functionality. - #[structopt(name = "release")] - Release, -} - -impl Default for BuildMode { - fn default() -> BuildMode { - BuildMode::Debug - } -} - -impl Display for BuildMode { - fn fmt(&self, f: &mut Formatter<'_>) -> DisplayResult { - match self { - Self::Debug => write!(f, "debug"), - Self::Release => write!(f, "release"), - } - } -} - -impl std::str::FromStr for BuildMode { - type Err = String; - fn from_str(artifact: &str) -> Result { - match artifact { - "debug" => Ok(BuildMode::Debug), - "release" => Ok(BuildMode::Release), - _ => Err("Could not parse build mode".to_string()), - } - } -} - /// Result of the metadata generation process. pub struct BuildResult { /// Path to the resulting Wasm file. @@ -318,8 +280,8 @@ pub struct BuildResult { pub target_directory: PathBuf, /// If existent the result of the optimization. pub optimization_result: Option, - /// The mode to build the contract in. - pub build_mode: BuildMode, + /// If the contract was built as a release. + pub build_release: bool, /// Which build artifacts were generated. pub build_artifact: BuildArtifacts, /// The verbosity flags. @@ -349,9 +311,13 @@ impl BuildResult { "optimized file size must be greater 0" ); + let mode = match self.build_release { + true => "BUILD", + false => "DEBUG", + }; let build_mode = format!( "The contract was built in {} mode.\n\n", - format!("{}", self.build_mode).to_uppercase().bold(), + format!("{}", mode).bold(), ); if self.build_artifact == BuildArtifacts::CodeOnly { -- GitLab From ac8055054ff6087632b1bb75ca86f080e3d9400e Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 8 Jul 2021 19:24:15 +0200 Subject: [PATCH 04/10] Revert "Switch to `--release`" This reverts commit 3cb01e1044f668649d9900e62a2fdeb1b17f44c8. --- src/cmd/build.rs | 72 +++++++++++++++++++++++++++------------------ src/cmd/metadata.rs | 4 +-- src/main.rs | 48 +++++++++++++++++++++++++----- 3 files changed, 87 insertions(+), 37 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 10f86b3e..8f1e8b56 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -18,7 +18,7 @@ use crate::{ crate_metadata::CrateMetadata, maybe_println, util, validate_wasm, workspace::{Manifest, ManifestPath, Profile, Workspace}, - BuildArtifacts, BuildResult, OptimizationPasses, OptimizationResult, UnstableFlags, + BuildArtifacts, BuildMode, BuildResult, OptimizationPasses, OptimizationResult, UnstableFlags, UnstableOptions, Verbosity, VerbosityFlags, }; use anyhow::{Context, Result}; @@ -48,14 +48,24 @@ pub struct BuildCommand { /// Path to the Cargo.toml of the contract to build #[structopt(long, parse(from_os_str))] manifest_path: Option, - /// By default the contract is compiled with debug functionality - /// included. This enables the contract to output debug messages, - /// but increases the contract size and the amount of gas used. + /// Which mode to build the contract in. /// - /// A production contract should always be build in `release` mode! - /// Then no debug functionality is compiled into the contract. - #[structopt(long = "--release")] - build_release: bool, + /// - `debug`: The contract will be compiled with debug functionality + /// included. This enables the contract to output debug messages. + /// + /// This increases the contract size and the used gas. A production + /// contract should never be build in debug mode! + /// + /// - `release`: No debug functionality is compiled into the contract. + /// + /// - The default value is `debug`. + #[structopt( + long = "mode", + default_value = "debug", + value_name = "debug | release", + verbatim_doc_comment + )] + build_mode: BuildMode, /// Which build artifacts to generate. /// /// - `all`: Generate the Wasm, the metadata and a bundled `.contract` file. @@ -124,7 +134,7 @@ impl BuildCommand { execute( &manifest_path, verbosity, - self.build_release, + self.build_mode, self.build_artifact, unstable_flags, optimization_passes, @@ -153,7 +163,7 @@ impl CheckCommand { execute( &manifest_path, verbosity, - false, + BuildMode::Debug, BuildArtifacts::CheckOnly, unstable_flags, OptimizationPasses::Zero, @@ -180,7 +190,7 @@ impl CheckCommand { fn exec_cargo_for_wasm_target( crate_metadata: &CrateMetadata, command: &str, - build_release: bool, + build_mode: BuildMode, verbosity: Verbosity, unstable_flags: &UnstableFlags, ) -> Result<()> { @@ -204,7 +214,7 @@ fn exec_cargo_for_wasm_target( "--release", &target_dir, ]; - if !build_release { + if build_mode == BuildMode::Debug { args.push("--features=ink_env/ink-debug"); } util::invoke_cargo(command, &args, manifest_path.directory(), verbosity)?; @@ -541,7 +551,7 @@ fn assert_compatible_ink_dependencies( /// This feature was introduced in `3.0.0-rc4` with `ink_env/ink-debug`. pub fn assert_debug_mode_supported(ink_version: &Version) -> anyhow::Result<()> { log::info!("Contract version: {:?}", ink_version); - if ink_version <= &Version::parse("3.0.0-rc2").expect("parsing minimum ink! version failed") { + if ink_version <= &Version::parse("3.0.0-rc3").expect("parsing minimum ink! version failed") { anyhow::bail!( "Building the contract in debug mode requires an ink! version newer than `3.0.0-rc3`!" ); @@ -555,7 +565,7 @@ pub fn assert_debug_mode_supported(ink_version: &Version) -> anyhow::Result<()> pub(crate) fn execute( manifest_path: &ManifestPath, verbosity: Verbosity, - build_release: bool, + build_mode: BuildMode, build_artifact: BuildArtifacts, unstable_flags: UnstableFlags, optimization_passes: OptimizationPasses, @@ -575,7 +585,7 @@ pub(crate) fn execute( exec_cargo_for_wasm_target( &crate_metadata, "build", - build_release, + build_mode, verbosity, &unstable_flags, )?; @@ -601,7 +611,13 @@ pub(crate) fn execute( let (opt_result, metadata_result) = match build_artifact { BuildArtifacts::CheckOnly => { - exec_cargo_for_wasm_target(&crate_metadata, "check", true, verbosity, &unstable_flags)?; + exec_cargo_for_wasm_target( + &crate_metadata, + "check", + BuildMode::Release, + verbosity, + &unstable_flags, + )?; (None, None) } BuildArtifacts::CodeOnly => { @@ -627,7 +643,7 @@ pub(crate) fn execute( metadata_result, target_directory: crate_metadata.target_directory, optimization_result: opt_result, - build_release, + build_mode, build_artifact, verbosity, }) @@ -644,8 +660,8 @@ mod tests_ci_only { cmd::BuildCommand, util::tests::{with_new_contract_project, with_tmp_dir}, workspace::Manifest, - BuildArtifacts, ManifestPath, OptimizationPasses, UnstableFlags, UnstableOptions, - Verbosity, VerbosityFlags, + BuildArtifacts, BuildMode, ManifestPath, OptimizationPasses, UnstableFlags, + UnstableOptions, Verbosity, VerbosityFlags, }; use semver::Version; #[cfg(unix)] @@ -699,7 +715,7 @@ mod tests_ci_only { let res = super::execute( &manifest_path, Verbosity::Default, - false, + BuildMode::default(), BuildArtifacts::CodeOnly, UnstableFlags::default(), OptimizationPasses::default(), @@ -739,7 +755,7 @@ mod tests_ci_only { super::execute( &manifest_path, Verbosity::Default, - false, + BuildMode::default(), BuildArtifacts::CheckOnly, UnstableFlags::default(), OptimizationPasses::default(), @@ -770,7 +786,7 @@ mod tests_ci_only { let cmd = BuildCommand { manifest_path: Some(manifest_path.into()), build_artifact: BuildArtifacts::All, - build_release: false, + build_mode: BuildMode::default(), verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), @@ -811,7 +827,7 @@ mod tests_ci_only { let cmd = BuildCommand { manifest_path: Some(manifest_path.into()), build_artifact: BuildArtifacts::All, - build_release: false, + build_mode: BuildMode::default(), verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), @@ -973,7 +989,7 @@ mod tests_ci_only { let cmd = BuildCommand { manifest_path: Some(manifest_path.into()), build_artifact: BuildArtifacts::All, - build_release: false, + build_mode: BuildMode::default(), verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), optimization_passes: None, @@ -1019,13 +1035,13 @@ mod tests_ci_only { fn building_template_in_debug_mode_must_work() { with_new_contract_project(|manifest_path| { // given - let build_release = false; + let build_mode = BuildMode::Debug; // when let res = super::execute( &manifest_path, Verbosity::Default, - build_release, + build_mode, BuildArtifacts::All, UnstableFlags::default(), OptimizationPasses::default(), @@ -1041,13 +1057,13 @@ mod tests_ci_only { fn building_template_in_release_mode_must_work() { with_new_contract_project(|manifest_path| { // given - let build_release = true; + let build_mode = BuildMode::Release; // when let res = super::execute( &manifest_path, Verbosity::Default, - build_release, + build_mode, BuildArtifacts::All, UnstableFlags::default(), OptimizationPasses::default(), diff --git a/src/cmd/metadata.rs b/src/cmd/metadata.rs index 474cf201..cb4472cb 100644 --- a/src/cmd/metadata.rs +++ b/src/cmd/metadata.rs @@ -226,7 +226,7 @@ mod tests { use crate::cmd::metadata::blake2_hash; use crate::{ cmd, crate_metadata::CrateMetadata, util::tests::with_new_contract_project, BuildArtifacts, - ManifestPath, OptimizationPasses, UnstableFlags, Verbosity, + BuildMode, ManifestPath, OptimizationPasses, UnstableFlags, Verbosity, }; use contract_metadata::*; use serde_json::{Map, Value}; @@ -325,7 +325,7 @@ mod tests { let build_result = cmd::build::execute( &test_manifest.manifest_path, Verbosity::Default, - false, + BuildMode::default(), BuildArtifacts::All, UnstableFlags::default(), OptimizationPasses::default(), diff --git a/src/main.rs b/src/main.rs index bdda204c..f578cc8e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -270,6 +270,44 @@ impl std::str::FromStr for BuildArtifacts { } } +/// The mode to build the contract in. +#[derive(Copy, Clone, Eq, PartialEq, Debug, StructOpt)] +#[structopt(name = "build-mode")] +pub enum BuildMode { + /// Functionality to output debug messages is build into the contract. + #[structopt(name = "debug")] + Debug, + /// The contract is build without any debugging functionality. + #[structopt(name = "release")] + Release, +} + +impl Default for BuildMode { + fn default() -> BuildMode { + BuildMode::Debug + } +} + +impl Display for BuildMode { + fn fmt(&self, f: &mut Formatter<'_>) -> DisplayResult { + match self { + Self::Debug => write!(f, "debug"), + Self::Release => write!(f, "release"), + } + } +} + +impl std::str::FromStr for BuildMode { + type Err = String; + fn from_str(artifact: &str) -> Result { + match artifact { + "debug" => Ok(BuildMode::Debug), + "release" => Ok(BuildMode::Release), + _ => Err("Could not parse build mode".to_string()), + } + } +} + /// Result of the metadata generation process. pub struct BuildResult { /// Path to the resulting Wasm file. @@ -280,8 +318,8 @@ pub struct BuildResult { pub target_directory: PathBuf, /// If existent the result of the optimization. pub optimization_result: Option, - /// If the contract was built as a release. - pub build_release: bool, + /// The mode to build the contract in. + pub build_mode: BuildMode, /// Which build artifacts were generated. pub build_artifact: BuildArtifacts, /// The verbosity flags. @@ -311,13 +349,9 @@ impl BuildResult { "optimized file size must be greater 0" ); - let mode = match self.build_release { - true => "BUILD", - false => "DEBUG", - }; let build_mode = format!( "The contract was built in {} mode.\n\n", - format!("{}", mode).bold(), + format!("{}", self.build_mode).to_uppercase().bold(), ); if self.build_artifact == BuildArtifacts::CodeOnly { -- GitLab From e5f255a088e38c90df06a4f918934ad853d8efff Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 8 Jul 2021 19:51:50 +0200 Subject: [PATCH 05/10] Keep `BuildMode` enum --- src/cmd/build.rs | 43 ++++++++++++++++++++----------------------- src/main.rs | 16 +--------------- 2 files changed, 21 insertions(+), 38 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 8f1e8b56..4b45f208 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -48,24 +48,14 @@ pub struct BuildCommand { /// Path to the Cargo.toml of the contract to build #[structopt(long, parse(from_os_str))] manifest_path: Option, - /// Which mode to build the contract in. + /// By default the contract is compiled with debug functionality + /// included. This enables the contract to output debug messages, + /// but increases the contract size and the amount of gas used. /// - /// - `debug`: The contract will be compiled with debug functionality - /// included. This enables the contract to output debug messages. - /// - /// This increases the contract size and the used gas. A production - /// contract should never be build in debug mode! - /// - /// - `release`: No debug functionality is compiled into the contract. - /// - /// - The default value is `debug`. - #[structopt( - long = "mode", - default_value = "debug", - value_name = "debug | release", - verbatim_doc_comment - )] - build_mode: BuildMode, + /// A production contract should always be build in `release` mode! + /// Then no debug functionality is compiled into the contract. + #[structopt(long = "--release")] + build_release: bool, /// Which build artifacts to generate. /// /// - `all`: Generate the Wasm, the metadata and a bundled `.contract` file. @@ -131,10 +121,14 @@ impl BuildCommand { } }; + let build_mode = match self.build_release { + true => BuildMode::Release, + false => BuildMode::Debug, + }; execute( &manifest_path, verbosity, - self.build_mode, + build_mode, self.build_artifact, unstable_flags, optimization_passes, @@ -422,6 +416,7 @@ fn do_optimization( err ) })?; + // TODO if !output.status.success() { let err = str::from_utf8(&output.stderr) @@ -551,7 +546,7 @@ fn assert_compatible_ink_dependencies( /// This feature was introduced in `3.0.0-rc4` with `ink_env/ink-debug`. pub fn assert_debug_mode_supported(ink_version: &Version) -> anyhow::Result<()> { log::info!("Contract version: {:?}", ink_version); - if ink_version <= &Version::parse("3.0.0-rc3").expect("parsing minimum ink! version failed") { + if ink_version <= &Version::parse("3.0.0-rc2").expect("parsing minimum ink! version failed") { anyhow::bail!( "Building the contract in debug mode requires an ink! version newer than `3.0.0-rc3`!" ); @@ -573,7 +568,9 @@ pub(crate) fn execute( let crate_metadata = CrateMetadata::collect(manifest_path)?; assert_compatible_ink_dependencies(manifest_path, verbosity)?; - assert_debug_mode_supported(&crate_metadata.ink_version)?; + if build_mode == BuildMode::Debug { + assert_debug_mode_supported(&crate_metadata.ink_version)?; + } let build = || -> Result { maybe_println!( @@ -786,7 +783,7 @@ mod tests_ci_only { let cmd = BuildCommand { manifest_path: Some(manifest_path.into()), build_artifact: BuildArtifacts::All, - build_mode: BuildMode::default(), + build_release: false, verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), @@ -827,7 +824,7 @@ mod tests_ci_only { let cmd = BuildCommand { manifest_path: Some(manifest_path.into()), build_artifact: BuildArtifacts::All, - build_mode: BuildMode::default(), + build_release: false, verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), @@ -989,7 +986,7 @@ mod tests_ci_only { let cmd = BuildCommand { manifest_path: Some(manifest_path.into()), build_artifact: BuildArtifacts::All, - build_mode: BuildMode::default(), + build_release: false, verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), optimization_passes: None, diff --git a/src/main.rs b/src/main.rs index f578cc8e..4425e16f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -271,14 +271,11 @@ impl std::str::FromStr for BuildArtifacts { } /// The mode to build the contract in. -#[derive(Copy, Clone, Eq, PartialEq, Debug, StructOpt)] -#[structopt(name = "build-mode")] +#[derive(Eq, PartialEq, Copy, Clone, Debug)] pub enum BuildMode { /// Functionality to output debug messages is build into the contract. - #[structopt(name = "debug")] Debug, /// The contract is build without any debugging functionality. - #[structopt(name = "release")] Release, } @@ -297,17 +294,6 @@ impl Display for BuildMode { } } -impl std::str::FromStr for BuildMode { - type Err = String; - fn from_str(artifact: &str) -> Result { - match artifact { - "debug" => Ok(BuildMode::Debug), - "release" => Ok(BuildMode::Release), - _ => Err("Could not parse build mode".to_string()), - } - } -} - /// Result of the metadata generation process. pub struct BuildResult { /// Path to the resulting Wasm file. -- GitLab From 9e81c2ebf2ef604a6bdc13469b59ec3bda1534b6 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 8 Jul 2021 20:03:39 +0200 Subject: [PATCH 06/10] Improve readability --- 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 4b45f208..740e7642 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -546,7 +546,8 @@ fn assert_compatible_ink_dependencies( /// This feature was introduced in `3.0.0-rc4` with `ink_env/ink-debug`. pub fn assert_debug_mode_supported(ink_version: &Version) -> anyhow::Result<()> { log::info!("Contract version: {:?}", ink_version); - if ink_version <= &Version::parse("3.0.0-rc2").expect("parsing minimum ink! version failed") { + let minimum_version = Version::parse("3.0.0-rc4").expect("parsing version failed"); + if ink_version < &minimum_version { anyhow::bail!( "Building the contract in debug mode requires an ink! version newer than `3.0.0-rc3`!" ); -- GitLab From 8b23a92423d7a893896823a5b322cec091834a00 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Fri, 9 Jul 2021 11:09:55 +0200 Subject: [PATCH 07/10] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02d5b318..75a52b6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- Convenient off-chain testing through `cargo contract test` - [#283](https://github.com/paritytech/cargo-contract/pull/283) +- Build contracts in debug mode by default, add `--release` flag - [#298](https://github.com/paritytech/cargo-contract/pull/298) + ## [0.12.1] - 2021-04-25 ### Added -- GitLab From 7ebb69c79c74ec7ef7c31d12a3abaa4fd4c17e46 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Wed, 14 Jul 2021 13:56:13 +0200 Subject: [PATCH 08/10] Make `rustfmt` always report todo's and fixme's --- .rustfmt.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.rustfmt.toml b/.rustfmt.toml index 814bfd61..183ce92e 100644 --- a/.rustfmt.toml +++ b/.rustfmt.toml @@ -1 +1,3 @@ license_template_path = "FILE_HEADER" # changed +report_todo = "Always" +report_fixme = "Always" -- GitLab From 742a3d939a5667b3b505664dd474fd89b60d2839 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Wed, 14 Jul 2021 13:56:33 +0200 Subject: [PATCH 09/10] Remove todo comment --- src/cmd/build.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 740e7642..50c7b1fb 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -416,7 +416,6 @@ fn do_optimization( err ) })?; - // TODO if !output.status.success() { let err = str::from_utf8(&output.stderr) -- GitLab From 2125f5e30e1c6f1e2c4c2ae3ba9bdae8528e966b Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Thu, 22 Jul 2021 10:51:55 +0200 Subject: [PATCH 10/10] Fix tests --- src/cmd/build.rs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index d1ae92dc..cf2ef906 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -1077,6 +1077,7 @@ mod tests_ci_only { BuildArtifacts::All, UnstableFlags::default(), OptimizationPasses::default(), + Default::default(), ); // then @@ -1099,6 +1100,7 @@ mod tests_ci_only { BuildArtifacts::All, UnstableFlags::default(), OptimizationPasses::default(), + Default::default(), ); // then @@ -1107,11 +1109,34 @@ mod tests_ci_only { }) } - fn keep_debug_symbols() { + #[test] + fn keep_debug_symbols_in_debug_mode() { with_new_contract_project(|manifest_path| { let res = super::execute( &manifest_path, Verbosity::Default, + BuildMode::Debug, + BuildArtifacts::CodeOnly, + UnstableFlags::default(), + OptimizationPasses::default(), + true, + ) + .expect("build failed"); + + // we specified that debug symbols should be kept + assert!(has_debug_symbols(&res.dest_wasm.unwrap())); + + Ok(()) + }) + } + + #[test] + fn keep_debug_symbols_in_release_mode() { + with_new_contract_project(|manifest_path| { + let res = super::execute( + &manifest_path, + Verbosity::Default, + BuildMode::Release, BuildArtifacts::CodeOnly, UnstableFlags::default(), OptimizationPasses::default(), -- GitLab