From 04e5edd24a957f349fff1689a719df8d7c54f402 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 6 Aug 2021 11:35:40 -0400 Subject: [PATCH 01/18] Spit out JSON results at the end of a build --- src/cmd/build.rs | 19 +++++++++++++++++-- src/cmd/metadata.rs | 1 + src/main.rs | 36 +++++++++++++++++++++++++++++++++--- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index c43f9006..d3ef10fe 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -18,8 +18,8 @@ use crate::{ crate_metadata::CrateMetadata, maybe_println, util, validate_wasm, workspace::{Manifest, ManifestPath, Profile, Workspace}, - BuildArtifacts, BuildMode, BuildResult, OptimizationPasses, OptimizationResult, UnstableFlags, - UnstableOptions, Verbosity, VerbosityFlags, + BuildArtifacts, BuildMode, BuildResult, OptimizationPasses, OptimizationResult, OutputType, + UnstableFlags, UnstableOptions, Verbosity, VerbosityFlags, }; use anyhow::{Context, Result}; use colored::Colorize; @@ -102,6 +102,10 @@ pub struct BuildCommand { /// This is useful if one wants to analyze or debug the optimized binary. #[structopt(long)] keep_debug_symbols: bool, + + /// Export the build output in JSON format. + #[structopt(long)] + output_json: bool, } impl BuildCommand { @@ -130,6 +134,12 @@ impl BuildCommand { true => BuildMode::Release, false => BuildMode::Debug, }; + + let output_type = match self.output_json { + true => OutputType::Json, + false => OutputType::HumanReadable, + }; + execute( &manifest_path, verbosity, @@ -138,6 +148,7 @@ impl BuildCommand { unstable_flags, optimization_passes, self.keep_debug_symbols, + output_type, ) } } @@ -168,6 +179,7 @@ impl CheckCommand { unstable_flags, OptimizationPasses::Zero, false, + OutputType::default(), ) } } @@ -590,6 +602,7 @@ pub(crate) fn execute( unstable_flags: UnstableFlags, optimization_passes: OptimizationPasses, keep_debug_symbols: bool, + output_type: OutputType, ) -> Result { let crate_metadata = CrateMetadata::collect(manifest_path)?; @@ -662,6 +675,7 @@ pub(crate) fn execute( } }; let dest_wasm = opt_result.as_ref().map(|r| r.dest_wasm.clone()); + Ok(BuildResult { dest_wasm, metadata_result, @@ -670,6 +684,7 @@ pub(crate) fn execute( build_mode, build_artifact, verbosity, + output_type, }) } diff --git a/src/cmd/metadata.rs b/src/cmd/metadata.rs index 720a20fc..f836d78c 100644 --- a/src/cmd/metadata.rs +++ b/src/cmd/metadata.rs @@ -38,6 +38,7 @@ use url::Url; const METADATA_FILE: &str = "metadata.json"; /// Metadata generation result. +#[derive(serde::Serialize)] pub struct MetadataResult { /// Path to the resulting metadata file. pub dest_metadata: PathBuf, diff --git a/src/main.rs b/src/main.rs index b2aaf38c..a330906c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -170,7 +170,7 @@ pub struct VerbosityFlags { } /// Denotes if output should be printed to stdout. -#[derive(Clone, Copy)] +#[derive(Clone, Copy, serde::Serialize)] pub enum Verbosity { /// Use default output Default, @@ -235,7 +235,7 @@ impl TryFrom<&UnstableOptions> for UnstableFlags { } /// Describes which artifacts to generate -#[derive(Copy, Clone, Eq, PartialEq, Debug, StructOpt)] +#[derive(Copy, Clone, Eq, PartialEq, Debug, StructOpt, serde::Serialize)] #[structopt(name = "build-artifacts")] pub enum BuildArtifacts { /// Generate the Wasm, the metadata and a bundled `.contract` file @@ -271,7 +271,7 @@ impl std::str::FromStr for BuildArtifacts { } /// The mode to build the contract in. -#[derive(Eq, PartialEq, Copy, Clone, Debug)] +#[derive(Eq, PartialEq, Copy, Clone, Debug, serde::Serialize)] pub enum BuildMode { /// Functionality to output debug messages is build into the contract. Debug, @@ -294,7 +294,21 @@ impl Display for BuildMode { } } +pub enum OutputType { + /// Output build results in a human readable format. + HumanReadable, + /// Output the build results JSON formatted. + Json, +} + +impl Default for OutputType { + fn default() -> Self { + OutputType::HumanReadable + } +} + /// Result of the metadata generation process. +#[derive(serde::Serialize)] pub struct BuildResult { /// Path to the resulting Wasm file. pub dest_wasm: Option, @@ -310,9 +324,13 @@ pub struct BuildResult { pub build_artifact: BuildArtifacts, /// The verbosity flags. pub verbosity: Verbosity, + /// The type of formatting to use for the built output. + #[serde(skip_serializing)] + pub output_type: OutputType, } /// Result of the optimization process. +#[derive(serde::Serialize)] pub struct OptimizationResult { /// The path of the optimized wasm file. pub dest_wasm: PathBuf, @@ -395,6 +413,12 @@ impl BuildResult { .expect("optimization result must exist"); (optimization.original_size, optimization.optimized_size) } + + /// Display the build results in a pretty formatted JSON string. + pub fn serialize_json(&self) -> String { + serde_json::to_string_pretty(self) + .expect("Since the built execution finished our BuiltResult must be non-empty.") + } } #[derive(Debug, StructOpt)] @@ -485,6 +509,12 @@ fn exec(cmd: Command) -> Result> { Command::New { name, target_dir } => cmd::new::execute(name, target_dir.as_ref()), Command::Build(build) => { let result = build.exec()?; + + // How should this play with the verbosity argument? + if matches!(result.output_type, OutputType::Json) { + return Ok(Some(result.serialize_json())); + } + if result.verbosity.is_verbose() { Ok(Some(result.display())) } else { -- GitLab From f14860131c06073c8a2b372d4c63b9ab6e8fa763 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 6 Aug 2021 12:21:21 -0400 Subject: [PATCH 02/18] Mark `--output_json` as conflicting with `--verbose` --- 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 d3ef10fe..6a24a028 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -104,7 +104,7 @@ pub struct BuildCommand { keep_debug_symbols: bool, /// Export the build output in JSON format. - #[structopt(long)] + #[structopt(long, conflicts_with = "verbose")] output_json: bool, } -- GitLab From a99a6ad2ef382fbbe686ea0539cc81a198fa9ce1 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 6 Aug 2021 12:21:57 -0400 Subject: [PATCH 03/18] Override verbosity to `quiet` when outputting JSON --- src/cmd/build.rs | 7 ++++++- src/main.rs | 8 +++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 6a24a028..4a059b10 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -113,7 +113,7 @@ impl BuildCommand { let manifest_path = ManifestPath::try_from(self.manifest_path.as_ref())?; let unstable_flags: UnstableFlags = TryFrom::<&UnstableOptions>::try_from(&self.unstable_options)?; - let verbosity = TryFrom::<&VerbosityFlags>::try_from(&self.verbosity)?; + let mut verbosity = TryFrom::<&VerbosityFlags>::try_from(&self.verbosity)?; // The CLI flag `optimization-passes` overwrites optimization passes which are // potentially defined in the `Cargo.toml` profile. @@ -140,6 +140,11 @@ impl BuildCommand { false => OutputType::HumanReadable, }; + // We want to ensure that the only thing in `STDOUT` is our JSON formatted string. + if matches!(output_type, OutputType::Json) { + verbosity = Verbosity::Quiet; + } + execute( &manifest_path, verbosity, diff --git a/src/main.rs b/src/main.rs index a330906c..5e5f4052 100644 --- a/src/main.rs +++ b/src/main.rs @@ -294,6 +294,7 @@ impl Display for BuildMode { } } +/// The type of output to display at the end of a build. pub enum OutputType { /// Output build results in a human readable format. HumanReadable, @@ -510,12 +511,9 @@ fn exec(cmd: Command) -> Result> { Command::Build(build) => { let result = build.exec()?; - // How should this play with the verbosity argument? if matches!(result.output_type, OutputType::Json) { - return Ok(Some(result.serialize_json())); - } - - if result.verbosity.is_verbose() { + Ok(Some(result.serialize_json())) + } else if result.verbosity.is_verbose() { Ok(Some(result.display())) } else { Ok(None) -- GitLab From 6c70a1ac8f20f5d777193d0bf2169234e2b1bc03 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 6 Aug 2021 12:50:14 -0400 Subject: [PATCH 04/18] Update tests to include `OutputType` param --- src/cmd/build.rs | 12 +++++++++++- src/cmd/metadata.rs | 3 ++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 4a059b10..25087085 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -704,7 +704,7 @@ mod tests_ci_only { cmd::{build::load_module, BuildCommand}, util::tests::{with_new_contract_project, with_tmp_dir}, workspace::Manifest, - BuildArtifacts, BuildMode, ManifestPath, OptimizationPasses, UnstableFlags, + BuildArtifacts, BuildMode, ManifestPath, OptimizationPasses, OutputType, UnstableFlags, UnstableOptions, Verbosity, VerbosityFlags, }; use semver::Version; @@ -768,6 +768,7 @@ mod tests_ci_only { UnstableFlags::default(), OptimizationPasses::default(), false, + OutputType::default(), ) .expect("build failed"); @@ -813,6 +814,7 @@ mod tests_ci_only { UnstableFlags::default(), OptimizationPasses::default(), false, + OutputType::default(), ) .expect("build failed"); @@ -847,6 +849,7 @@ mod tests_ci_only { // we choose zero optimization passes as the "cli" parameter optimization_passes: Some(OptimizationPasses::Zero), keep_debug_symbols: false, + output_json: false, }; // when @@ -886,6 +889,7 @@ mod tests_ci_only { // we choose no optimization passes as the "cli" parameter optimization_passes: None, keep_debug_symbols: false, + output_json: false, }; // when @@ -1050,6 +1054,7 @@ mod tests_ci_only { unstable_options: UnstableOptions::default(), optimization_passes: None, keep_debug_symbols: false, + output_json: false, }; let res = cmd.exec().expect("build failed"); @@ -1103,6 +1108,7 @@ mod tests_ci_only { UnstableFlags::default(), OptimizationPasses::default(), Default::default(), + Default::default(), ); // then @@ -1126,6 +1132,7 @@ mod tests_ci_only { UnstableFlags::default(), OptimizationPasses::default(), Default::default(), + Default::default(), ); // then @@ -1161,6 +1168,7 @@ mod tests_ci_only { UnstableFlags::default(), OptimizationPasses::default(), Default::default(), + Default::default(), ); // then @@ -1180,6 +1188,7 @@ mod tests_ci_only { UnstableFlags::default(), OptimizationPasses::default(), true, + OutputType::default(), ) .expect("build failed"); @@ -1201,6 +1210,7 @@ mod tests_ci_only { UnstableFlags::default(), OptimizationPasses::default(), true, + OutputType::default(), ) .expect("build failed"); diff --git a/src/cmd/metadata.rs b/src/cmd/metadata.rs index f836d78c..c7d20a2d 100644 --- a/src/cmd/metadata.rs +++ b/src/cmd/metadata.rs @@ -227,7 +227,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, + BuildMode, ManifestPath, OptimizationPasses, OutputType, UnstableFlags, Verbosity, }; use anyhow::Context; use contract_metadata::*; @@ -328,6 +328,7 @@ mod tests { UnstableFlags::default(), OptimizationPasses::default(), false, + OutputType::default(), )?; let dest_bundle = build_result .metadata_result -- GitLab From 3519d59693d38e56ae3bc3ece4bf0d53cf47bfca Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 6 Aug 2021 16:09:14 -0400 Subject: [PATCH 05/18] Temporarily please Clippy --- src/cmd/build.rs | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 25087085..9acf795c 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -599,6 +599,8 @@ pub fn assert_debug_mode_supported(ink_version: &Version) -> anyhow::Result<()> /// 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. +// TODO: Maybe turn these args into a struct +#[allow(clippy::too_many_arguments)] pub(crate) fn execute( manifest_path: &ManifestPath, verbosity: Verbosity, @@ -1220,4 +1222,44 @@ mod tests_ci_only { Ok(()) }) } + + #[test] + fn outputs_json() { + with_new_contract_project(|manifest_path| { + // given + let build_mode = BuildMode::Debug; + + let build_result = crate::BuildResult { + dest_wasm: Some(PathBuf::default()), + metadata_result: Some(crate::MetadataResult { + dest_metadata: Default::default(), + dest_bundle: Default::default(), + }), + target_directory: PathBuf::default(), + optimization_result: Some(crate::OptimizationResult::default()), + build_mode: BuildMode::Debug, + build_artifact: BuildArtifacts::All, + verbosity: Verbosity::default(), + output_type: OutputType::Json, + }; + let j = serde_json::to_string_pretty(&build_result).unwrap(); + + // when + let res = super::execute( + &manifest_path, + Verbosity::Default, + build_mode, + BuildArtifacts::All, + UnstableFlags::default(), + OptimizationPasses::default(), + Default::default(), + OutputType::Json, + ); + + // then + assert!(j, res.serialize_json()); + // assert!(res.is_ok(), "building template in debug mode failed!"); + Ok(()) + }) + } } -- GitLab From d2ae80c1caf1e5382a2f607895cffa5d47901107 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 6 Aug 2021 16:16:18 -0400 Subject: [PATCH 06/18] Add some logging This is gonna fail, but I want to see what the CI has to say. --- src/cmd/build.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 9acf795c..5dad3fb2 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -1228,6 +1228,7 @@ mod tests_ci_only { with_new_contract_project(|manifest_path| { // given let build_mode = BuildMode::Debug; + dbg!(&manifest_path); let build_result = crate::BuildResult { dest_wasm: Some(PathBuf::default()), @@ -1236,13 +1237,19 @@ mod tests_ci_only { dest_bundle: Default::default(), }), target_directory: PathBuf::default(), - optimization_result: Some(crate::OptimizationResult::default()), + optimization_result: Some(crate::OptimizationResult { + dest_wasm: Default::default(), + original_size: Default::default(), + optimized_size: Default::default(), + }), build_mode: BuildMode::Debug, build_artifact: BuildArtifacts::All, - verbosity: Verbosity::default(), + verbosity: Verbosity::Quiet, output_type: OutputType::Json, }; + let j = serde_json::to_string_pretty(&build_result).unwrap(); + dbg!(&j); // when let res = super::execute( @@ -1257,7 +1264,7 @@ mod tests_ci_only { ); // then - assert!(j, res.serialize_json()); + assert_eq!(j, res.unwrap().serialize_json()); // assert!(res.is_ok(), "building template in debug mode failed!"); Ok(()) }) -- GitLab From 2addbcea656866aff7830df6d1b247997e9d2917 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 6 Aug 2021 16:18:52 -0400 Subject: [PATCH 07/18] Remove TODO so that format CI step passes --- 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 5dad3fb2..5d44e07b 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -599,7 +599,7 @@ pub fn assert_debug_mode_supported(ink_version: &Version) -> anyhow::Result<()> /// 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. -// TODO: Maybe turn these args into a struct +// Maybe turn these args into a struct #[allow(clippy::too_many_arguments)] pub(crate) fn execute( manifest_path: &ManifestPath, -- GitLab From b87deb29449fcdae475040175971230682c3bc67 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 6 Aug 2021 17:21:05 -0400 Subject: [PATCH 08/18] Return result from `serialize_json` --- src/main.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main.rs b/src/main.rs index 5e5f4052..b8776a38 100644 --- a/src/main.rs +++ b/src/main.rs @@ -416,9 +416,8 @@ impl BuildResult { } /// Display the build results in a pretty formatted JSON string. - pub fn serialize_json(&self) -> String { - serde_json::to_string_pretty(self) - .expect("Since the built execution finished our BuiltResult must be non-empty.") + pub fn serialize_json(&self) -> Result { + Ok(serde_json::to_string_pretty(self)?) } } @@ -512,7 +511,7 @@ fn exec(cmd: Command) -> Result> { let result = build.exec()?; if matches!(result.output_type, OutputType::Json) { - Ok(Some(result.serialize_json())) + Ok(Some(result.serialize_json()?)) } else if result.verbosity.is_verbose() { Ok(Some(result.display())) } else { -- GitLab From 0045736c2835fb345124cfa039aefff961755a16 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 6 Aug 2021 17:21:21 -0400 Subject: [PATCH 09/18] Make JSON test a sanity check --- src/cmd/build.rs | 37 +++++-------------------------------- 1 file changed, 5 insertions(+), 32 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 5d44e07b..6a144d74 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -1224,48 +1224,21 @@ mod tests_ci_only { } #[test] - fn outputs_json() { + fn build_result_seralization_sanity_check() { with_new_contract_project(|manifest_path| { - // given - let build_mode = BuildMode::Debug; - dbg!(&manifest_path); - - let build_result = crate::BuildResult { - dest_wasm: Some(PathBuf::default()), - metadata_result: Some(crate::MetadataResult { - dest_metadata: Default::default(), - dest_bundle: Default::default(), - }), - target_directory: PathBuf::default(), - optimization_result: Some(crate::OptimizationResult { - dest_wasm: Default::default(), - original_size: Default::default(), - optimized_size: Default::default(), - }), - build_mode: BuildMode::Debug, - build_artifact: BuildArtifacts::All, - verbosity: Verbosity::Quiet, - output_type: OutputType::Json, - }; - - let j = serde_json::to_string_pretty(&build_result).unwrap(); - dbg!(&j); - - // when let res = super::execute( &manifest_path, Verbosity::Default, - build_mode, + BuildMode::Release, BuildArtifacts::All, UnstableFlags::default(), OptimizationPasses::default(), Default::default(), OutputType::Json, - ); + ) + .expect("build failed"); - // then - assert_eq!(j, res.unwrap().serialize_json()); - // assert!(res.is_ok(), "building template in debug mode failed!"); + assert!(res.serialize_json().is_ok()); Ok(()) }) } -- GitLab From 65b2014902ee788e82cda26164325f0351df1fd4 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 9 Aug 2021 11:37:25 -0400 Subject: [PATCH 10/18] Mention this PR in the CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4215c93..d40d7376 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- Add option for JSON formatted output [#324](https://github.com/paritytech/cargo-contract/pull/324) + ## [0.13.1] - 2021-08-03 ### Fixed -- GitLab From 1a07b5da62f2c21e51bfa8fddc293bd32342df7c Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 9 Aug 2021 12:03:32 -0400 Subject: [PATCH 11/18] Implement `Default` for a few build options --- src/main.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/main.rs b/src/main.rs index b8776a38..4f868897 100644 --- a/src/main.rs +++ b/src/main.rs @@ -180,6 +180,12 @@ pub enum Verbosity { Verbose, } +impl Default for Verbosity { + fn default() -> Self { + Verbosity::Default + } +} + impl Verbosity { /// Returns `true` if output should be printed (i.e. verbose output is set). pub(crate) fn is_verbose(&self) -> bool { @@ -270,6 +276,12 @@ impl std::str::FromStr for BuildArtifacts { } } +impl Default for BuildArtifacts { + fn default() -> Self { + BuildArtifacts::All + } +} + /// The mode to build the contract in. #[derive(Eq, PartialEq, Copy, Clone, Debug, serde::Serialize)] pub enum BuildMode { -- GitLab From 7cb5eaf5db6c1241f8e1bb30ae21029d64dc665f Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 9 Aug 2021 12:04:35 -0400 Subject: [PATCH 12/18] Address Clippy's `too_many_arguments` lint --- src/cmd/build.rs | 70 ++++++++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 6a144d74..f40377a0 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -39,6 +39,18 @@ use structopt::StructOpt; /// This is the maximum number of pages available for a contract to allocate. const MAX_MEMORY_PAGES: u32 = 16; +#[derive(Default)] +pub(crate) struct ExecuteArgs { + manifest_path: ManifestPath, + verbosity: Verbosity, + build_mode: BuildMode, + build_artifact: BuildArtifacts, + unstable_flags: UnstableFlags, + optimization_passes: OptimizationPasses, + keep_debug_symbols: bool, + output_type: OutputType, +} + /// 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. @@ -145,16 +157,18 @@ impl BuildCommand { verbosity = Verbosity::Quiet; } - execute( - &manifest_path, + let args = ExecuteArgs { + manifest_path, verbosity, build_mode, - self.build_artifact, + build_artifact: self.build_artifact, unstable_flags, optimization_passes, - self.keep_debug_symbols, + keep_debug_symbols: self.keep_debug_symbols, output_type, - ) + }; + + execute(args) } } @@ -176,16 +190,19 @@ impl CheckCommand { let unstable_flags: UnstableFlags = TryFrom::<&UnstableOptions>::try_from(&self.unstable_options)?; let verbosity: Verbosity = TryFrom::<&VerbosityFlags>::try_from(&self.verbosity)?; - execute( - &manifest_path, + + let args = ExecuteArgs { + manifest_path, verbosity, - BuildMode::Debug, - BuildArtifacts::CheckOnly, + build_mode: BuildMode::Debug, + build_artifact: BuildArtifacts::CheckOnly, unstable_flags, - OptimizationPasses::Zero, - false, - OutputType::default(), - ) + optimization_passes: OptimizationPasses::Zero, + keep_debug_symbols: false, + output_type: OutputType::default(), + }; + + execute(args) } } @@ -600,20 +617,21 @@ pub fn assert_debug_mode_supported(ink_version: &Version) -> anyhow::Result<()> /// /// It does so by invoking `cargo build` and then post processing the final binary. // Maybe turn these args into a struct -#[allow(clippy::too_many_arguments)] -pub(crate) fn execute( - manifest_path: &ManifestPath, - verbosity: Verbosity, - build_mode: BuildMode, - build_artifact: BuildArtifacts, - unstable_flags: UnstableFlags, - optimization_passes: OptimizationPasses, - keep_debug_symbols: bool, - output_type: OutputType, -) -> Result { - let crate_metadata = CrateMetadata::collect(manifest_path)?; +pub(crate) fn execute(args: ExecuteArgs) -> Result { + let ExecuteArgs { + manifest_path, + verbosity, + build_mode, + build_artifact, + unstable_flags, + optimization_passes, + keep_debug_symbols, + output_type, + } = args; + + let crate_metadata = CrateMetadata::collect(&manifest_path)?; - assert_compatible_ink_dependencies(manifest_path, verbosity)?; + assert_compatible_ink_dependencies(&manifest_path, verbosity)?; if build_mode == BuildMode::Debug { assert_debug_mode_supported(&crate_metadata.ink_version)?; } -- GitLab From 730d14a6b7f7a2ef250b3ffc23e3022f710bad89 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 9 Aug 2021 12:30:21 -0400 Subject: [PATCH 13/18] Use `ExecuteArgs` in tests --- src/cmd/build.rs | 159 +++++++++++++++++++------------------------- src/cmd/metadata.rs | 21 +++--- 2 files changed, 79 insertions(+), 101 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index f40377a0..266d3732 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -39,6 +39,7 @@ use structopt::StructOpt; /// This is the maximum number of pages available for a contract to allocate. const MAX_MEMORY_PAGES: u32 = 16; +/// Arguments to use when executing `build` or `check` commands. #[derive(Default)] pub(crate) struct ExecuteArgs { manifest_path: ManifestPath, @@ -51,6 +52,16 @@ pub(crate) struct ExecuteArgs { output_type: OutputType, } +impl ExecuteArgs { + /// Construct a default set of `ExecuteArgs` with the given `ManifestPath`. + pub(crate) fn with_manifest(manifest_path: ManifestPath) -> Self { + Self { + manifest_path, + ..Default::default() + } + } +} + /// 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. @@ -724,8 +735,8 @@ mod tests_ci_only { cmd::{build::load_module, BuildCommand}, util::tests::{with_new_contract_project, with_tmp_dir}, workspace::Manifest, - BuildArtifacts, BuildMode, ManifestPath, OptimizationPasses, OutputType, UnstableFlags, - UnstableOptions, Verbosity, VerbosityFlags, + BuildArtifacts, BuildMode, ManifestPath, OptimizationPasses, OutputType, UnstableOptions, + Verbosity, VerbosityFlags, }; use semver::Version; #[cfg(unix)] @@ -780,17 +791,13 @@ mod tests_ci_only { #[test] fn build_code_only() { with_new_contract_project(|manifest_path| { - let res = super::execute( - &manifest_path, - Verbosity::Default, - BuildMode::default(), - BuildArtifacts::CodeOnly, - UnstableFlags::default(), - OptimizationPasses::default(), - false, - OutputType::default(), - ) - .expect("build failed"); + let args = crate::cmd::build::ExecuteArgs { + manifest_path, + build_artifact: BuildArtifacts::CodeOnly, + ..Default::default() + }; + + let res = super::execute(args).expect("build failed"); // our ci has set `CARGO_TARGET_DIR` to cache artifacts. // this dir does not include `/target/` as a path, hence @@ -824,19 +831,14 @@ mod tests_ci_only { with_new_contract_project(|manifest_path| { // given let project_dir = manifest_path.directory().expect("directory must exist"); + let args = crate::cmd::build::ExecuteArgs { + manifest_path: manifest_path.clone(), + build_artifact: BuildArtifacts::CheckOnly, + ..Default::default() + }; // when - super::execute( - &manifest_path, - Verbosity::Default, - BuildMode::default(), - BuildArtifacts::CheckOnly, - UnstableFlags::default(), - OptimizationPasses::default(), - false, - OutputType::default(), - ) - .expect("build failed"); + super::execute(args).expect("build failed"); // then assert!( @@ -1117,19 +1119,14 @@ 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 args = crate::cmd::build::ExecuteArgs { + manifest_path, + build_mode: BuildMode::Debug, + ..Default::default() + }; // when - let res = super::execute( - &manifest_path, - Verbosity::Default, - build_mode, - BuildArtifacts::All, - UnstableFlags::default(), - OptimizationPasses::default(), - Default::default(), - Default::default(), - ); + let res = super::execute(args); // then assert!(res.is_ok(), "building template in debug mode failed!"); @@ -1141,19 +1138,14 @@ 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 args = crate::cmd::build::ExecuteArgs { + manifest_path, + build_mode: BuildMode::Release, + ..Default::default() + }; // when - let res = super::execute( - &manifest_path, - Verbosity::Default, - build_mode, - BuildArtifacts::All, - UnstableFlags::default(), - OptimizationPasses::default(), - Default::default(), - Default::default(), - ); + let res = super::execute(args); // then assert!(res.is_ok(), "building template in release mode failed!"); @@ -1179,17 +1171,14 @@ mod tests_ci_only { .expect("setting lib path must work"); manifest.write(&manifest_path).expect("writing must work"); + let args = crate::cmd::build::ExecuteArgs { + manifest_path, + build_artifact: BuildArtifacts::CheckOnly, + ..Default::default() + }; + // when - let res = super::execute( - &manifest_path, - Verbosity::Default, - BuildMode::default(), - BuildArtifacts::CheckOnly, - UnstableFlags::default(), - OptimizationPasses::default(), - Default::default(), - Default::default(), - ); + let res = super::execute(args); // then assert!(res.is_ok(), "building contract failed!"); @@ -1200,17 +1189,15 @@ mod tests_ci_only { #[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, - OutputType::default(), - ) - .expect("build failed"); + let args = crate::cmd::build::ExecuteArgs { + manifest_path, + build_mode: BuildMode::Debug, + build_artifact: BuildArtifacts::CodeOnly, + keep_debug_symbols: true, + ..Default::default() + }; + + let res = super::execute(args).expect("build failed"); // we specified that debug symbols should be kept assert!(has_debug_symbols(&res.dest_wasm.unwrap())); @@ -1222,17 +1209,15 @@ mod tests_ci_only { #[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(), - true, - OutputType::default(), - ) - .expect("build failed"); + let args = crate::cmd::build::ExecuteArgs { + manifest_path, + build_mode: BuildMode::Release, + build_artifact: BuildArtifacts::CodeOnly, + keep_debug_symbols: true, + ..Default::default() + }; + + let res = super::execute(args).expect("build failed"); // we specified that debug symbols should be kept assert!(has_debug_symbols(&res.dest_wasm.unwrap())); @@ -1244,17 +1229,13 @@ mod tests_ci_only { #[test] fn build_result_seralization_sanity_check() { with_new_contract_project(|manifest_path| { - let res = super::execute( - &manifest_path, - Verbosity::Default, - BuildMode::Release, - BuildArtifacts::All, - UnstableFlags::default(), - OptimizationPasses::default(), - Default::default(), - OutputType::Json, - ) - .expect("build failed"); + let args = crate::cmd::build::ExecuteArgs { + manifest_path, + output_type: OutputType::Json, + ..Default::default() + }; + + let res = super::execute(args).expect("build failed"); assert!(res.serialize_json().is_ok()); Ok(()) diff --git a/src/cmd/metadata.rs b/src/cmd/metadata.rs index c7d20a2d..5d1c16c9 100644 --- a/src/cmd/metadata.rs +++ b/src/cmd/metadata.rs @@ -226,8 +226,7 @@ fn blake2_hash(code: &[u8]) -> CodeHash { mod tests { use crate::cmd::metadata::blake2_hash; use crate::{ - cmd, crate_metadata::CrateMetadata, util::tests::with_new_contract_project, BuildArtifacts, - BuildMode, ManifestPath, OptimizationPasses, OutputType, UnstableFlags, Verbosity, + cmd, crate_metadata::CrateMetadata, util::tests::with_new_contract_project, ManifestPath, }; use anyhow::Context; use contract_metadata::*; @@ -320,16 +319,14 @@ mod tests { fs::create_dir_all(final_contract_wasm_path.parent().unwrap()).unwrap(); fs::write(final_contract_wasm_path, "TEST FINAL WASM BLOB").unwrap(); - let build_result = cmd::build::execute( - &test_manifest.manifest_path, - Verbosity::Default, - BuildMode::default(), - BuildArtifacts::All, - UnstableFlags::default(), - OptimizationPasses::default(), - false, - OutputType::default(), - )?; + let args = crate::cmd::build::ExecuteArgs::with_manifest(test_manifest.manifest_path); + // args.manifest_path = test_manifest.manifest_path; + // { + // manifest_path: test_manifest.manifest_path, + // ..Default::default() + // }; + + let build_result = cmd::build::execute(args)?; let dest_bundle = build_result .metadata_result .expect("Metadata should be generated") -- GitLab From bf971e0cd44f10bd748b78a6c616f50c360da48c Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Mon, 9 Aug 2021 12:41:05 -0400 Subject: [PATCH 14/18] Make `manifest_path` field public instead Clippy was complaining about `dead_code` when building in non-test mode, so I'm going with this instead. --- src/cmd/build.rs | 13 ++----------- src/cmd/metadata.rs | 8 ++------ 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 266d3732..0d2c418c 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -42,7 +42,8 @@ const MAX_MEMORY_PAGES: u32 = 16; /// Arguments to use when executing `build` or `check` commands. #[derive(Default)] pub(crate) struct ExecuteArgs { - manifest_path: ManifestPath, + /// The location of the Cargo manifest (`Cargo.toml`) file to use. + pub(crate) manifest_path: ManifestPath, verbosity: Verbosity, build_mode: BuildMode, build_artifact: BuildArtifacts, @@ -52,16 +53,6 @@ pub(crate) struct ExecuteArgs { output_type: OutputType, } -impl ExecuteArgs { - /// Construct a default set of `ExecuteArgs` with the given `ManifestPath`. - pub(crate) fn with_manifest(manifest_path: ManifestPath) -> Self { - Self { - manifest_path, - ..Default::default() - } - } -} - /// 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. diff --git a/src/cmd/metadata.rs b/src/cmd/metadata.rs index 5d1c16c9..92bb3c3b 100644 --- a/src/cmd/metadata.rs +++ b/src/cmd/metadata.rs @@ -319,12 +319,8 @@ mod tests { fs::create_dir_all(final_contract_wasm_path.parent().unwrap()).unwrap(); fs::write(final_contract_wasm_path, "TEST FINAL WASM BLOB").unwrap(); - let args = crate::cmd::build::ExecuteArgs::with_manifest(test_manifest.manifest_path); - // args.manifest_path = test_manifest.manifest_path; - // { - // manifest_path: test_manifest.manifest_path, - // ..Default::default() - // }; + let mut args = crate::cmd::build::ExecuteArgs::default(); + args.manifest_path = test_manifest.manifest_path; let build_result = cmd::build::execute(args)?; let dest_bundle = build_result -- GitLab From 84e85168039a5236e563674aa9521705c36244f6 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 11 Aug 2021 10:11:53 -0400 Subject: [PATCH 15/18] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michael Müller --- CHANGELOG.md | 2 +- src/cmd/build.rs | 4 +++- src/main.rs | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d40d7376..6f6ff4e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added -- Add option for JSON formatted output [#324](https://github.com/paritytech/cargo-contract/pull/324) +- Add option for JSON formatted output - [#324](https://github.com/paritytech/cargo-contract/pull/324) ## [0.13.1] - 2021-08-03 diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 0d2c418c..e85db3b3 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -618,7 +618,6 @@ pub fn assert_debug_mode_supported(ink_version: &Version) -> anyhow::Result<()> /// 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. -// Maybe turn these args into a struct pub(crate) fn execute(args: ExecuteArgs) -> Result { let ExecuteArgs { manifest_path, @@ -1220,14 +1219,17 @@ mod tests_ci_only { #[test] fn build_result_seralization_sanity_check() { with_new_contract_project(|manifest_path| { + // given let args = crate::cmd::build::ExecuteArgs { manifest_path, output_type: OutputType::Json, ..Default::default() }; + // when let res = super::execute(args).expect("build failed"); + // then assert!(res.serialize_json().is_ok()); Ok(()) }) diff --git a/src/main.rs b/src/main.rs index 4f868897..056b0148 100644 --- a/src/main.rs +++ b/src/main.rs @@ -337,7 +337,7 @@ pub struct BuildResult { pub build_artifact: BuildArtifacts, /// The verbosity flags. pub verbosity: Verbosity, - /// The type of formatting to use for the built output. + /// The type of formatting to use for the build output. #[serde(skip_serializing)] pub output_type: OutputType, } -- GitLab From f3a9518d42eff2755d4be764af466d4aff94acd7 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Wed, 11 Aug 2021 11:04:11 -0400 Subject: [PATCH 16/18] Sanity check `BuildResult'` JSON serialized form --- src/cmd/build.rs | 23 ++-------------------- src/main.rs | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index e85db3b3..88ac2372 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -725,8 +725,8 @@ mod tests_ci_only { cmd::{build::load_module, BuildCommand}, util::tests::{with_new_contract_project, with_tmp_dir}, workspace::Manifest, - BuildArtifacts, BuildMode, ManifestPath, OptimizationPasses, OutputType, UnstableOptions, - Verbosity, VerbosityFlags, + BuildArtifacts, BuildMode, ManifestPath, OptimizationPasses, UnstableOptions, Verbosity, + VerbosityFlags, }; use semver::Version; #[cfg(unix)] @@ -1215,23 +1215,4 @@ mod tests_ci_only { Ok(()) }) } - - #[test] - fn build_result_seralization_sanity_check() { - with_new_contract_project(|manifest_path| { - // given - let args = crate::cmd::build::ExecuteArgs { - manifest_path, - output_type: OutputType::Json, - ..Default::default() - }; - - // when - let res = super::execute(args).expect("build failed"); - - // then - assert!(res.serialize_json().is_ok()); - Ok(()) - }) - } } diff --git a/src/main.rs b/src/main.rs index 056b0148..b7fb083b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -579,3 +579,54 @@ fn exec(cmd: Command) -> Result> { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn build_result_seralization_sanity_check() { + // given + let raw_result = r#"{ + "dest_wasm": "/path/to/contract.wasm", + "metadata_result": { + "dest_metadata": "/path/to/metadata.json", + "dest_bundle": "/path/to/contract.contract" + }, + "target_directory": "/path/to/target", + "optimization_result": { + "dest_wasm": "/path/to/contract.wasm", + "original_size": 64.0, + "optimized_size": 32.0 + }, + "build_mode": "Debug", + "build_artifact": "All", + "verbosity": "Quiet" +}"#; + + let build_result = crate::BuildResult { + dest_wasm: Some(PathBuf::from("/path/to/contract.wasm")), + metadata_result: Some(crate::cmd::metadata::MetadataResult { + dest_metadata: PathBuf::from("/path/to/metadata.json"), + dest_bundle: PathBuf::from("/path/to/contract.contract"), + }), + target_directory: PathBuf::from("/path/to/target"), + optimization_result: Some(crate::OptimizationResult { + dest_wasm: PathBuf::from("/path/to/contract.wasm"), + original_size: 64.0, + optimized_size: 32.0, + }), + build_mode: Default::default(), + build_artifact: Default::default(), + verbosity: Verbosity::Quiet, + output_type: OutputType::Json, + }; + + // when + let serialized_result = build_result.serialize_json(); + + // then + assert!(serialized_result.is_ok()); + assert_eq!(serialized_result.unwrap(), raw_result); + } +} -- GitLab From e6d02a9e8f1735527ab3bba9ec996af1114bb7d6 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 12 Aug 2021 10:50:13 -0400 Subject: [PATCH 17/18] Add `execute()` test back --- src/cmd/build.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 88ac2372..b90e46b5 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -1215,4 +1215,23 @@ mod tests_ci_only { Ok(()) }) } + + #[test] + fn build_with_json_output_works() { + with_new_contract_project(|manifest_path| { + // given + let args = crate::cmd::build::ExecuteArgs { + manifest_path, + output_type: OutputType::Json, + ..Default::default() + }; + + // when + let res = super::execute(args).expect("build failed"); + + // then + assert!(res.serialize_json().is_ok()); + Ok(()) + }) + } } -- GitLab From f57e7aa61ec7960b4f824f98eb6878c13535fcf5 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Thu, 12 Aug 2021 10:56:41 -0400 Subject: [PATCH 18/18] Import `OutputType` in tests --- src/cmd/build.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index d3cc552d..ee465a79 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -726,8 +726,8 @@ mod tests_ci_only { cmd::{build::load_module, BuildCommand}, util::tests::{with_new_contract_project, with_tmp_dir}, workspace::Manifest, - BuildArtifacts, BuildMode, ManifestPath, OptimizationPasses, UnstableOptions, Verbosity, - VerbosityFlags, + BuildArtifacts, BuildMode, ManifestPath, OptimizationPasses, OutputType, UnstableOptions, + Verbosity, VerbosityFlags, }; use semver::Version; #[cfg(unix)] -- GitLab