From 17bdd390753ed2f8e8cb5883653a8045b679b31d Mon Sep 17 00:00:00 2001 From: AlvinHon Date: Mon, 28 Aug 2023 23:42:16 +0800 Subject: [PATCH 1/7] FEATURE: add option to build with locked dependencies versions. (#2) --- src/bin/main.rs | 18 +++++++++++++++++- src/build.rs | 16 ++++++++++++---- src/cargo.rs | 13 +++++++++++-- src/config.rs | 13 ++++++++++++- src/docker.rs | 44 +++++++++++++++++++++++++++++++++++--------- tests/test.rs | 3 ++- 6 files changed, 89 insertions(+), 18 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 9ddcc84..0e16abe 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -8,7 +8,7 @@ //! in a docker environment. use clap::Parser; -use pchain_compile::{config::Config, DockerConfig, DockerOption}; +use pchain_compile::{config::Config, DockerConfig, DockerOption, BuildOptions}; use std::path::{Path, PathBuf}; #[derive(Debug, Parser)] @@ -35,6 +35,16 @@ enum PchainCompile { #[clap(long = "destination", display_order = 2, verbatim_doc_comment)] destination_path: Option, + /// Build with locked versions of the dependencies. If the source code directory contains the + /// file Cargo.lock, the building process uses the dependencies with the versions + /// specified by the file. If the file does not present, it is equivalent to a disabled `locked` + /// option, and the building process continues. + /// + /// With or without the presence of the input file, the compilation output includes the + /// file Cargo.lock which was used or generated in the building process. + #[clap(ong = "locked", display_order = 3, verbatim_doc_comment)] + locked: bool, + /// Compile contract without using docker. This option requires installation of Rust and target "wasm32-unknown-unknown". /// **Please note the compiled contracts are not always consistent with the previous compiled ones, because the building /// process happens in your local changing environment.** @@ -73,6 +83,7 @@ async fn main() { PchainCompile::Build { source_path, destination_path, + locked, dockerless, docker_image_tag, } => { @@ -82,6 +93,10 @@ async fn main() { } println!("Build process started. This could take several minutes for large contracts."); + let build_options = BuildOptions { + locked + }; + let docker_option = if dockerless { DockerOption::Dockerless } else { @@ -96,6 +111,7 @@ async fn main() { let config = Config { source_path, destination_path: destination_path.clone(), + build_options: build_options.clone(), docker_option: docker_option.clone(), }; diff --git a/src/build.rs b/src/build.rs index 277ae91..04eed89 100644 --- a/src/build.rs +++ b/src/build.rs @@ -39,13 +39,14 @@ pub async fn build_target( source_path: PathBuf, destination_path: Option, ) -> Result { - build_target_with_docker(source_path, destination_path, DockerConfig::default()).await + build_target_with_docker(source_path, destination_path, BuildOptions::default(), DockerConfig::default()).await } /// Validates inputs and trigger building process that uses docker. pub(crate) async fn build_target_with_docker( source_path: PathBuf, destination_path: Option, + options: BuildOptions, docker_config: DockerConfig, ) -> Result { // create destination directory if it does not exist. @@ -69,13 +70,14 @@ pub(crate) async fn build_target_with_docker( return Err(Error::UnkownDockerImageTag(docker_image_tag)); } - build_target_in_docker(source_path, destination_path, docker_image_tag, wasm_file).await + build_target_in_docker(source_path, destination_path, options, docker_image_tag, wasm_file).await } /// Validates inputs and trigger building process that does not use docker. pub(crate) async fn build_target_without_docker( source_path: PathBuf, destination_path: Option, + options: BuildOptions, ) -> Result { // create destination directory if it does not exist. if let Some(dst_path) = &destination_path { @@ -90,7 +92,7 @@ pub(crate) async fn build_target_without_docker( crate::manifests::package_name(&source_path).map_err(|_| Error::InvalidSourcePath)?; let wasm_file = format!("{package_name}.wasm").replace('-', "_"); - build_target_by_cargo(source_path, destination_path, wasm_file).await + build_target_by_cargo(source_path, destination_path, options, wasm_file).await } fn validated_source_path(source_path: PathBuf) -> Result { @@ -104,6 +106,7 @@ fn validated_source_path(source_path: PathBuf) -> Result { async fn build_target_in_docker( source_path: PathBuf, destination_path: Option, + options: BuildOptions, docker_image_tag: String, wasm_file: String, ) -> Result { @@ -124,6 +127,7 @@ async fn build_target_in_docker( dependencies, source_path, destination_path, + options, &wasm_file, ) .await; @@ -141,6 +145,7 @@ async fn compile_contract_in_docker_container( dependencies: impl IntoIterator, source_path: PathBuf, destination_path: Option, + options: BuildOptions, wasm_file: &str, ) -> Result<(), Error> { // Step 1. create dependency directory and copy source to docker @@ -156,6 +161,7 @@ async fn compile_contract_in_docker_container( docker, container_name, source_path.to_str().unwrap(), + options.locked, wasm_file, ) .await?; @@ -177,6 +183,7 @@ async fn compile_contract_in_docker_container( async fn build_target_by_cargo( source_path: PathBuf, destination_path: Option, + options: BuildOptions, wasm_file: String, ) -> Result { // 1. Create temporary folder as a working directory for cargo build @@ -188,6 +195,7 @@ async fn build_target_by_cargo( &temp_dir, source_path.as_path(), destination_path, + options.locked, &wasm_file, ); @@ -195,4 +203,4 @@ async fn build_target_by_cargo( let _ = std::fs::remove_dir_all(temp_dir); result.map(|_| wasm_file) -} +} \ No newline at end of file diff --git a/src/cargo.rs b/src/cargo.rs index 80f6a78..9310966 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -31,7 +31,7 @@ pub(crate) fn random_temp_dir_name() -> PathBuf { .to_path_buf() } -/// Equivalent to running following commands: +/// Equivalent to run following commands: /// 1. cargo build --target wasm32-unknown-unknown --release --quiet /// 2. wasm-opt -Oz --output temp.wasm /// 3. wasm-snip temp.wasm --output temp2.wasm --snip-rust-fmt-code --snip-rust-panicking-code @@ -40,14 +40,18 @@ pub(crate) fn build_contract( working_folder: &Path, source_path: &Path, destination_path: Option, + mut locked: bool, wasm_file: &str, ) -> Result<(), Error> { let output_path = destination_path.unwrap_or(Path::new(".").to_path_buf()); + // Disable "--locked" if the Cargo.lock file does not exist. + locked = locked && source_path.join("Cargo.lock").exists(); + // 1. cargo build --target wasm32-unknown-unknown --release --quiet let mut config = Config::default().unwrap(); config - .configure(0, true, None, false, false, false, &None, &[], &[]) + .configure(0, true, None, false, locked, false, &None, &[], &[]) .unwrap(); let mut compile_configs = CompileOptions::new(&config, cargo::util::command_prelude::CompileMode::Build).unwrap(); @@ -64,6 +68,11 @@ pub(crate) fn build_contract( cargo::ops::compile(&ws, &compile_configs) .map_err(|e| Error::BuildFailure(format!("Error in cargo build:\n\n{:?}\n", e)))?; + // Save Cargo.lock to output folder if applicable + if locked { + std::fs::copy(source_path.join("Cargo.lock"), output_path.join("Cargo.lock")); + } + // 2. wasm-opt -Oz wasm_file --output temp.wasm let temp_wasm = working_folder.join("temp.wasm"); wasm_opt::OptimizationOptions::new_optimize_for_size_aggressively() diff --git a/src/config.rs b/src/config.rs index 481a8ca..131912a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -17,10 +17,20 @@ pub struct Config { pub source_path: PathBuf, /// Path to destination folder. None if current folder should be used. pub destination_path: Option, + /// Options for building rust code. + pub build_options: BuildOptions, /// Compilation option regards to use of docker. pub docker_option: DockerOption, } +/// Options for building rust code. +#[derive(Clone, Default)] +pub struct BuildOptions { + /// Use of the Cargo.lock. It is equivalent to run Cargo build with + /// flag "--locked". + pub locked: bool +} + /// Compilation option regards to docker. #[derive(Clone)] pub enum DockerOption { @@ -49,12 +59,13 @@ impl Config { crate::build::build_target_with_docker( self.source_path, self.destination_path, + self.build_options, docker_config, ) .await } DockerOption::Dockerless => { - crate::build::build_target_without_docker(self.source_path, self.destination_path) + crate::build::build_target_without_docker(self.source_path, self.destination_path, self.build_options) .await } } diff --git a/src/docker.rs b/src/docker.rs index 0633a08..f814186 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -202,8 +202,11 @@ pub async fn build_contracts( docker: &Docker, container_name: &str, source_path: &str, + mut locked: bool, wasm_file: &str, ) -> Result { + // Disable "--locked" if the Cargo.lock file does not exist. + locked = locked && source_path.join("Cargo.lock").exists(); let source_path = source_path .replace(':', "") .replace('\\', "/") @@ -214,17 +217,30 @@ pub async fn build_contracts( let output_folder = "/result"; let output_file = format!("{output_folder}/{wasm_file}").to_string(); - let cmds = vec![ + let cmd_cargo_build = if locked { + vec![ + "cargo", + "build", + "--locked", + "--target", + "wasm32-unknown-unknown", + "--release", + "--quiet", + ] + } else { + vec![ + "cargo", + "build", + "--target", + "wasm32-unknown-unknown", + "--release", + "--quiet", + ] + }; + let mut cmds = vec![ ( &working_folder_code, - vec![ - "cargo", - "build", - "--target", - "wasm32-unknown-unknown", - "--release", - "--quiet", - ], + cmd_cargo_build, ), ( &working_folder_build, @@ -268,6 +284,16 @@ pub async fn build_contracts( ), ]; + // Save Cargo.lock to output folder if applicable + if locked { + cmds.push( + ( + &working_folder_code, + vec!["mv", "Cargo.lock", &output_folder] + ) + ); + } + for (working_dir, cmd) in cmds { execute(docker, container_name, Some(working_dir), cmd) .await diff --git a/tests/test.rs b/tests/test.rs index 940c776..dcf64ca 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -7,7 +7,7 @@ use std::path::Path; -use pchain_compile::DockerOption; +use pchain_compile::{DockerOption, BuildOptions}; #[tokio::test] async fn build_contract() { @@ -65,6 +65,7 @@ async fn build_contract_without_docker() { let wasm_name = pchain_compile::Config { source_path, destination_path: Some(destination_path.clone()), + build_options: BuildOptions::default(), docker_option: DockerOption::Dockerless, } .run() From 9f90f3bd5134a133aef422618aed6a5bfb5a8896 Mon Sep 17 00:00:00 2001 From: AlvinHon Date: Tue, 29 Aug 2023 11:11:12 +0800 Subject: [PATCH 2/7] update documentation and code refactoring for the new changes in issue (#2) --- src/bin/main.rs | 22 +++++++++++----------- src/build.rs | 8 ++++++-- src/cargo.rs | 11 +++++------ src/docker.rs | 18 +++++++++--------- tests/test.rs | 31 +++++++++++++++++++++++++++++-- 5 files changed, 60 insertions(+), 30 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 0e16abe..f89f807 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -21,7 +21,7 @@ use std::path::{Path, PathBuf}; long_about = None )] enum PchainCompile { - /// Build the source code. Please make sure: + /// Build the source code. By default, it uses docker for building the contract. Please make sure: /// 1. Docker is installed and its execution permission under current user is granted. /// 2. Internet is reachable. (for pulling the docker image from docker hub) #[clap(arg_required_else_help = true, display_order = 1, verbatim_doc_comment)] @@ -35,26 +35,26 @@ enum PchainCompile { #[clap(long = "destination", display_order = 2, verbatim_doc_comment)] destination_path: Option, - /// Build with locked versions of the dependencies. If the source code directory contains the - /// file Cargo.lock, the building process uses the dependencies with the versions - /// specified by the file. If the file does not present, it is equivalent to a disabled `locked` - /// option, and the building process continues. + /// Build with the version-locked dependencies. If the source code directory contains the file "Cargo.lock", + /// the contract will be built with the dependencies specified in the file. It is equivalent to + /// running "cargo build" with the flag "--locked". If the file does not exist, the building process continues + /// without using the version-locked dependencies. /// - /// With or without the presence of the input file, the compilation output includes the - /// file Cargo.lock which was used or generated in the building process. - #[clap(ong = "locked", display_order = 3, verbatim_doc_comment)] + /// With or without the file "Cargo.lock", the compilation output includes the file "Cargo.lock" which was used or + /// generated in the building process. + #[clap(long = "locked", display_order = 3, verbatim_doc_comment)] locked: bool, /// Compile contract without using docker. This option requires installation of Rust and target "wasm32-unknown-unknown". /// **Please note the compiled contracts are not always consistent with the previous compiled ones, because the building /// process happens in your local changing environment.** /// - /// To install target "wasm32-unknown-unkown", run the following command: + /// To install target "wasm32-unknown-unknown", run the following command: /// /// $ rustup target add wasm32-unknown-unknown #[clap( long = "dockerless", - display_order = 3, + display_order = 4, verbatim_doc_comment, group = "docker-option" )] @@ -68,7 +68,7 @@ enum PchainCompile { /// - 0.4.2 #[clap( long = "use-docker-tag", - display_order = 4, + display_order = 5, verbatim_doc_comment, group = "docker-option" )] diff --git a/src/build.rs b/src/build.rs index 04eed89..ce09274 100644 --- a/src/build.rs +++ b/src/build.rs @@ -31,10 +31,14 @@ use std::{collections::HashSet, path::PathBuf}; use std::fs; use crate::error::Error; -use crate::DockerConfig; +use crate::{DockerConfig, BuildOptions}; /// `build_target` takes the path to the cargo manifest file(s), generates an optimized WASM binary(ies) after building /// the source code and saves the binary(ies) to the designated destination_path. +/// +/// This method is equivalent to run the command: +/// +/// `pchain_compile` build --source `source_path` --destination `destination_path` pub async fn build_target( source_path: PathBuf, destination_path: Option, @@ -160,7 +164,7 @@ async fn compile_contract_in_docker_container( let result_in_docker = crate::docker::build_contracts( docker, container_name, - source_path.to_str().unwrap(), + source_path, options.locked, wasm_file, ) diff --git a/src/cargo.rs b/src/cargo.rs index 9310966..62e19c7 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -40,18 +40,17 @@ pub(crate) fn build_contract( working_folder: &Path, source_path: &Path, destination_path: Option, - mut locked: bool, + locked: bool, wasm_file: &str, ) -> Result<(), Error> { let output_path = destination_path.unwrap_or(Path::new(".").to_path_buf()); - // Disable "--locked" if the Cargo.lock file does not exist. - locked = locked && source_path.join("Cargo.lock").exists(); - // 1. cargo build --target wasm32-unknown-unknown --release --quiet + // Does not set "--locked" if the Cargo.lock file does not exist. + let use_cargo_lock = locked && source_path.join("Cargo.lock").exists(); let mut config = Config::default().unwrap(); config - .configure(0, true, None, false, locked, false, &None, &[], &[]) + .configure(0, true, None, false, use_cargo_lock, false, &None, &[], &[]) .unwrap(); let mut compile_configs = CompileOptions::new(&config, cargo::util::command_prelude::CompileMode::Build).unwrap(); @@ -70,7 +69,7 @@ pub(crate) fn build_contract( // Save Cargo.lock to output folder if applicable if locked { - std::fs::copy(source_path.join("Cargo.lock"), output_path.join("Cargo.lock")); + let _ = std::fs::copy(source_path.join("Cargo.lock"), output_path.join("Cargo.lock")); } // 2. wasm-opt -Oz wasm_file --output temp.wasm diff --git a/src/docker.rs b/src/docker.rs index f814186..c44a06b 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -201,23 +201,23 @@ pub async fn copy_files_from( pub async fn build_contracts( docker: &Docker, container_name: &str, - source_path: &str, - mut locked: bool, + source_path: PathBuf, + locked: bool, wasm_file: &str, ) -> Result { - // Disable "--locked" if the Cargo.lock file does not exist. - locked = locked && source_path.join("Cargo.lock").exists(); - let source_path = source_path + let source_path_str = source_path.to_str().unwrap() .replace(':', "") .replace('\\', "/") .replace(' ', "_"); - let working_folder_code = format!("/{source_path}").to_string(); + let working_folder_code = format!("/{source_path_str}").to_string(); let working_folder_build = - format!("/{source_path}/target/wasm32-unknown-unknown/release").to_string(); + format!("/{source_path_str}/target/wasm32-unknown-unknown/release").to_string(); let output_folder = "/result"; let output_file = format!("{output_folder}/{wasm_file}").to_string(); - let cmd_cargo_build = if locked { + // Does not set "--locked" if the Cargo.lock file does not exist. + let use_cargo_lock = locked && source_path.join("Cargo.lock").exists(); + let cmd_cargo_build = if use_cargo_lock { vec![ "cargo", "build", @@ -289,7 +289,7 @@ pub async fn build_contracts( cmds.push( ( &working_folder_code, - vec!["mv", "Cargo.lock", &output_folder] + vec!["mv", "Cargo.lock", output_folder] ) ); } diff --git a/tests/test.rs b/tests/test.rs index dcf64ca..1f96284 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -7,7 +7,7 @@ use std::path::Path; -use pchain_compile::{DockerOption, BuildOptions}; +use pchain_compile::{DockerOption, BuildOptions, DockerConfig}; #[tokio::test] async fn build_contract() { @@ -51,6 +51,32 @@ async fn build_contract_to_destination() { assert_eq!(wasm_name, "hello_contract.wasm"); } +#[tokio::test] +async fn build_contract_with_docker() { + let source_path = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests") + .join("contracts") + .join("hello_contract") + .to_path_buf(); + let destination_path = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests") + .join("contracts") + .to_path_buf(); + let wasm_name = pchain_compile::Config { + source_path, + destination_path: Some(destination_path.clone()), + build_options: BuildOptions { locked: true }, + docker_option: DockerOption::Docker(DockerConfig::default()), + } + .run() + .await + .unwrap(); + + assert!(destination_path.join("Cargo.lock").exists()); + let _ = std::fs::remove_file(destination_path.join(&wasm_name)); + assert_eq!(wasm_name, "hello_contract.wasm"); +} + #[tokio::test] async fn build_contract_without_docker() { let source_path = Path::new(env!("CARGO_MANIFEST_DIR")) @@ -65,13 +91,14 @@ async fn build_contract_without_docker() { let wasm_name = pchain_compile::Config { source_path, destination_path: Some(destination_path.clone()), - build_options: BuildOptions::default(), + build_options: BuildOptions { locked: true }, docker_option: DockerOption::Dockerless, } .run() .await .unwrap(); + assert!(destination_path.join("Cargo.lock").exists()); let _ = std::fs::remove_file(destination_path.join(&wasm_name)); assert_eq!(wasm_name, "hello_contract.wasm"); } From 1994a3b08e56b4075e53b23ed379b24abb2298aa Mon Sep 17 00:00:00 2001 From: AlvinHon Date: Tue, 29 Aug 2023 16:19:32 +0800 Subject: [PATCH 3/7] Display cargo build logs in the error message. (#5) --- src/build.rs | 3 ++- src/cargo.rs | 74 +++++++++++++++++++++++++++++++++++++++++++++++---- src/docker.rs | 62 +++++++++++++++++++++++++++--------------- src/error.rs | 6 ++++- 4 files changed, 117 insertions(+), 28 deletions(-) diff --git a/src/build.rs b/src/build.rs index ce09274..b2a68fd 100644 --- a/src/build.rs +++ b/src/build.rs @@ -161,7 +161,7 @@ async fn compile_contract_in_docker_container( crate::docker::copy_files(docker, container_name, source_path.to_str().unwrap()).await?; // Step 3: build the source code inside docker - let result_in_docker = crate::docker::build_contracts( + let (result_in_docker, build_log) = crate::docker::build_contracts( docker, container_name, source_path, @@ -176,6 +176,7 @@ async fn compile_contract_in_docker_container( container_name, &result_in_docker, destination_path.clone(), + build_log ) .await?; diff --git a/src/cargo.rs b/src/cargo.rs index 62e19c7..f2c6114 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -5,7 +5,7 @@ //! Implements the compilation process of smart contract by utilizing crates `cargo`, `wasm-opt` and `wasm-snip`. -use std::path::{Path, PathBuf}; +use std::{path::{Path, PathBuf}, io::Write, sync::{Arc, Mutex}}; use cargo::{ core::compiler::{CompileKind, CompileTarget}, @@ -48,9 +48,9 @@ pub(crate) fn build_contract( // 1. cargo build --target wasm32-unknown-unknown --release --quiet // Does not set "--locked" if the Cargo.lock file does not exist. let use_cargo_lock = locked && source_path.join("Cargo.lock").exists(); - let mut config = Config::default().unwrap(); + let mut config = CargoConfig::new(); config - .configure(0, true, None, false, use_cargo_lock, false, &None, &[], &[]) + .configure(0, false, None, false, use_cargo_lock, false, &None, &[], &[]) .unwrap(); let mut compile_configs = CompileOptions::new(&config, cargo::util::command_prelude::CompileMode::Build).unwrap(); @@ -64,8 +64,9 @@ pub(crate) fn build_contract( format!("Error in preparing workspace according to the manifest file in source path:\n\n{:?}\n", e), ) })?; - cargo::ops::compile(&ws, &compile_configs) - .map_err(|e| Error::BuildFailure(format!("Error in cargo build:\n\n{:?}\n", e)))?; + if let Err(_) = cargo::ops::compile(&ws, &compile_configs) { + return Err(Error::BuildFailureWithLogs(config.logs())) + } // Save Cargo.lock to output folder if applicable if locked { @@ -110,3 +111,66 @@ pub(crate) fn build_contract( Ok(()) } + +/// Captures the [cargo::util::Config] with custom instantiation. +pub struct CargoConfig { + /// The logs from the shell which is used by the cargo + logs: Arc>>, + /// Cargo configuration + config: Config, +} + +impl CargoConfig { + pub fn new() -> Self { + // Setup a shell that stores logs in memory. + let logs = Arc::new(Mutex::new(Vec::::new())); + let log_writter = BuildLogWritter { buffer: logs.clone() }; + let shell = cargo::core::Shell::from_write(Box::new(log_writter)); + + // Setup Cargo configuration with the custom shell. + let current_dir = std::env::current_dir().unwrap(); + let home_dir = cargo::util::homedir(¤t_dir).unwrap(); + let config = Config::new(shell, current_dir, home_dir); + Self { + logs, + config + } + } + + /// Return logs as a String + pub fn logs(&self) -> String{ + self.logs.lock().unwrap().join("") + } +} + +impl std::ops::Deref for CargoConfig { + type Target = Config; + fn deref(&self) -> &Self::Target { + &self.config + } +} + +impl std::ops::DerefMut for CargoConfig { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.config + } +} + +/// Implements [std::io::Write] and be used by Cargo. It stores the +/// output logs during cargo building process. +#[derive(Default)] +pub struct BuildLogWritter { + pub buffer: Arc>> +} + +impl Write for BuildLogWritter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + if let Ok(ref mut mutex) = self.buffer.try_lock() { + mutex.push(String::from_utf8_lossy(buf).to_string()); + } + Ok(buf.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} \ No newline at end of file diff --git a/src/docker.rs b/src/docker.rs index c44a06b..975e8a7 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -32,8 +32,8 @@ use std::fs::File; use crate::error::Error; use rand::{distributions::Alphanumeric, thread_rng, Rng}; -/// List of docker image tags that can be used. -pub(crate) const PCHAIN_COMPILE_IMAGE_TAGS: [&str; 2] = ["mainnet01", env!("CARGO_PKG_VERSION")]; +/// List of docker image tags that can be used. The first (0-indexed) is the default one. +pub(crate) const PCHAIN_COMPILE_IMAGE_TAGS: [&str; 2] = [env!("CARGO_PKG_VERSION"), "mainnet01"]; /// The repo name in Parallelchain Lab Dockerhub: https://hub.docker.com/r/parallelchainlab/pchain_compile pub(crate) const PCHAIN_COMPILE_IMAGE: &str = "parallelchainlab/pchain_compile"; @@ -164,6 +164,7 @@ pub async fn copy_files_from( container_name: &str, container_path: &str, specified_output_path: Option, + build_log: String, ) -> Result<(), Error> { let download_option = DownloadFromContainerOptions { path: container_path, @@ -179,7 +180,7 @@ pub async fn copy_files_from( let files_content = files_from_tar_gz(compressed_data)?; if files_content.is_empty() { - return Err(Error::BuildFailure("Fail to build contract. Please ensure there is no compilation error in the source code.".to_string())); + return Err(Error::BuildFailureWithLogs(build_log)); } // Save to destination @@ -198,13 +199,14 @@ pub async fn copy_files_from( } /// Build contract by executing commands in docker container, including `Cargo`, `wasm-opt` and `wasm-snip`. +/// Return the output folder path and the build logs if success. pub async fn build_contracts( docker: &Docker, container_name: &str, source_path: PathBuf, locked: bool, wasm_file: &str, -) -> Result { +) -> Result<(String, String), Error> { let source_path_str = source_path.to_str().unwrap() .replace(':', "") .replace('\\', "/") @@ -225,7 +227,6 @@ pub async fn build_contracts( "--target", "wasm32-unknown-unknown", "--release", - "--quiet", ] } else { vec![ @@ -234,14 +235,21 @@ pub async fn build_contracts( "--target", "wasm32-unknown-unknown", "--release", - "--quiet", ] }; + + let build_log = execute( + docker, + container_name, + Some(&working_folder_code), + cmd_cargo_build, + true + ) + .await + .map_err(|e| Error::BuildFailure(e.to_string()))? + .join(""); + let mut cmds = vec![ - ( - &working_folder_code, - cmd_cargo_build, - ), ( &working_folder_build, vec!["chmod", "+x", "/root/bin/wasm-opt"], @@ -295,12 +303,12 @@ pub async fn build_contracts( } for (working_dir, cmd) in cmds { - execute(docker, container_name, Some(working_dir), cmd) + execute(docker, container_name, Some(working_dir), cmd, false) .await .map_err(|e| Error::BuildFailure(e.to_string()))?; } - Ok(output_folder.to_string()) + Ok((output_folder.to_string(), build_log)) } /// Force stop and remove a container @@ -359,7 +367,8 @@ async fn execute( container_name: &str, working_dir: Option<&str>, cmd: Vec<&str>, -) -> Result<(), bollard::errors::Error> { + log_output: bool, +) -> Result, bollard::errors::Error> { let create_exec_results = docker .create_exec( container_name, @@ -383,13 +392,24 @@ async fn execute( ) .await?; - if let bollard::exec::StartExecResults::Attached { output, .. } = start_exec_results { - let _output = output.try_collect::>().await?; - } else { - return Err(bollard::errors::Error::DockerStreamError { - error: "Execution Result Not Attached".to_string(), - }); + match start_exec_results { + bollard::exec::StartExecResults::Attached { output, .. } => { + let log_outputs = + if log_output { + output.try_collect::>() + .await? + .into_iter() + .map(|output| output.to_string() ) + .collect() + } else { + Vec::new() + }; + return Ok(log_outputs) + }, + bollard::exec::StartExecResults::Detached => { + return Err(bollard::errors::Error::DockerStreamError { + error: "Execution Result Not Attached".to_string(), + }); + } } - - Ok(()) } diff --git a/src/error.rs b/src/error.rs index 39c68f5..ae0e9bb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -10,9 +10,12 @@ use thiserror::Error; /// Describes the exit status codes during building process. #[derive(Error, Debug)] pub enum Error { - #[error("The source code did not compile.")] + #[error("Failure during building process.")] BuildFailure(String), + #[error("Failure during building process.")] + BuildFailureWithLogs(String), + #[error("Docker daemon service did not respond.")] DockerDaemonFailure, @@ -43,6 +46,7 @@ impl Error { match self { Error::ArtifactRemovalFailure => "The compilation was successful, but pchain-compile failed to stop its Docker containers. Please remove them manually.".to_string(), Error::BuildFailure(e) => format!("\nDetails: {e}\nPlease rectify the errors and build your source code again."), + Error::BuildFailureWithLogs(log) => format!("There maybe some problems in the source code.\nBuilding log is as follows:\n\n{log}\n"), Error::DockerDaemonFailure => "Failed to compile.\nDetails: Docker Daemon Failure. Check if Docker is running on your machine and confirm read/write access privileges.".to_string(), Error::ManifestFailure => "Failed to compile.\nDetails: Manifest File Not Found. Check if the manifest file exists on the source code path.".to_string(), Error::InvalidSourcePath => "Failed to compile.\nDetails: Source Code Path Not Valid. Check if you have provided the correct path to your source code directory and confirm write access privileges.".to_string(), From 1de84d539cecdd6f998df40da9c026347909b213 Mon Sep 17 00:00:00 2001 From: AlvinHon Date: Tue, 29 Aug 2023 16:38:45 +0800 Subject: [PATCH 4/7] FIX: inspect docker execution to ensure the result before executing the next command --- src/docker.rs | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/docker.rs b/src/docker.rs index 975e8a7..1cf3fff 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -9,7 +9,7 @@ use std::{ io::{Read, Write}, ops::Not, - path::{Path, PathBuf}, + path::{Path, PathBuf}, time::Duration, }; use bollard::{ @@ -36,6 +36,7 @@ use rand::{distributions::Alphanumeric, thread_rng, Rng}; pub(crate) const PCHAIN_COMPILE_IMAGE_TAGS: [&str; 2] = [env!("CARGO_PKG_VERSION"), "mainnet01"]; /// The repo name in Parallelchain Lab Dockerhub: https://hub.docker.com/r/parallelchainlab/pchain_compile pub(crate) const PCHAIN_COMPILE_IMAGE: &str = "parallelchainlab/pchain_compile"; +const DOCKER_EXEC_TIME_LIMIT: u64 = 5; // secs. It is a time limit to normal docker execution (except cargo build). /// Generate a random Docker container name pub fn random_container_name() -> String { @@ -243,7 +244,8 @@ pub async fn build_contracts( container_name, Some(&working_folder_code), cmd_cargo_build, - true + true, + None ) .await .map_err(|e| Error::BuildFailure(e.to_string()))? @@ -303,9 +305,16 @@ pub async fn build_contracts( } for (working_dir, cmd) in cmds { - execute(docker, container_name, Some(working_dir), cmd, false) - .await - .map_err(|e| Error::BuildFailure(e.to_string()))?; + execute( + docker, + container_name, + Some(working_dir), + cmd, + false, + Some(DOCKER_EXEC_TIME_LIMIT) + ) + .await + .map_err(|e| Error::BuildFailure(e.to_string()))?; } Ok((output_folder.to_string(), build_log)) @@ -368,6 +377,7 @@ async fn execute( working_dir: Option<&str>, cmd: Vec<&str>, log_output: bool, + timeout_secs: Option ) -> Result, bollard::errors::Error> { let create_exec_results = docker .create_exec( @@ -404,6 +414,25 @@ async fn execute( } else { Vec::new() }; + + // Wait until the execution finishes. + if let Some(timeout) = timeout_secs { + let _ = tokio::time::timeout(Duration::from_secs(timeout), async { + loop { + if let Ok(inspect_result) = docker.inspect_exec(&create_exec_results.id).await { + if inspect_result.running != Some(true) { + break; + } + // Continue to check if the execution finishes. + } else { + // Fail to inspect. The loop should be terminated. + break; + } + } + }) + .await; + } + return Ok(log_outputs) }, bollard::exec::StartExecResults::Detached => { From 19dee933273200fd0da370af5d21335a2336af28 Mon Sep 17 00:00:00 2001 From: AlvinHon Date: Thu, 31 Aug 2023 08:55:49 +0800 Subject: [PATCH 5/7] fix typo BuildLogWriter and update comment for Cargo.lock --- src/cargo.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/cargo.rs b/src/cargo.rs index f2c6114..0dcefd3 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -68,7 +68,9 @@ pub(crate) fn build_contract( return Err(Error::BuildFailureWithLogs(config.logs())) } - // Save Cargo.lock to output folder if applicable + // Save Cargo.lock to output folder: If option '--locked' is enabled, the Cargo.lock file + // is the file provided by user, otherwise, the Cargo.lock file is the one generated during + // "cargo build". if locked { let _ = std::fs::copy(source_path.join("Cargo.lock"), output_path.join("Cargo.lock")); } @@ -124,8 +126,8 @@ impl CargoConfig { pub fn new() -> Self { // Setup a shell that stores logs in memory. let logs = Arc::new(Mutex::new(Vec::::new())); - let log_writter = BuildLogWritter { buffer: logs.clone() }; - let shell = cargo::core::Shell::from_write(Box::new(log_writter)); + let log_writer = BuildLogWriter { buffer: logs.clone() }; + let shell = cargo::core::Shell::from_write(Box::new(log_writer)); // Setup Cargo configuration with the custom shell. let current_dir = std::env::current_dir().unwrap(); @@ -159,11 +161,11 @@ impl std::ops::DerefMut for CargoConfig { /// Implements [std::io::Write] and be used by Cargo. It stores the /// output logs during cargo building process. #[derive(Default)] -pub struct BuildLogWritter { +pub struct BuildLogWriter { pub buffer: Arc>> } -impl Write for BuildLogWritter { +impl Write for BuildLogWriter { fn write(&mut self, buf: &[u8]) -> std::io::Result { if let Ok(ref mut mutex) = self.buffer.try_lock() { mutex.push(String::from_utf8_lossy(buf).to_string()); From 682d9fe9267e2920d779c17d9ef853b0fa736346 Mon Sep 17 00:00:00 2001 From: AlvinHon Date: Thu, 31 Aug 2023 09:41:47 +0800 Subject: [PATCH 6/7] add build timeout error message --- src/docker.rs | 35 ++++++++++++++++++++--------------- src/error.rs | 4 ++++ 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/docker.rs b/src/docker.rs index 1cf3fff..e4e8b13 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -36,7 +36,7 @@ use rand::{distributions::Alphanumeric, thread_rng, Rng}; pub(crate) const PCHAIN_COMPILE_IMAGE_TAGS: [&str; 2] = [env!("CARGO_PKG_VERSION"), "mainnet01"]; /// The repo name in Parallelchain Lab Dockerhub: https://hub.docker.com/r/parallelchainlab/pchain_compile pub(crate) const PCHAIN_COMPILE_IMAGE: &str = "parallelchainlab/pchain_compile"; -const DOCKER_EXEC_TIME_LIMIT: u64 = 5; // secs. It is a time limit to normal docker execution (except cargo build). +const DOCKER_EXEC_TIME_LIMIT: u64 = 15; // secs. It is a time limit to normal docker execution (except cargo build). /// Generate a random Docker container name pub fn random_container_name() -> String { @@ -248,8 +248,7 @@ pub async fn build_contracts( None ) .await - .map_err(|e| Error::BuildFailure(e.to_string()))? - .join(""); + .map_err(|e| Error::BuildFailure(e.to_string()))?; let mut cmds = vec![ ( @@ -378,7 +377,7 @@ async fn execute( cmd: Vec<&str>, log_output: bool, timeout_secs: Option -) -> Result, bollard::errors::Error> { +) -> Result { let create_exec_results = docker .create_exec( container_name, @@ -390,7 +389,8 @@ async fn execute( ..Default::default() }, ) - .await?; + .await + .map_err(|e| Error::BuildFailure(e.to_string()))?; let start_exec_results = docker .start_exec( @@ -400,45 +400,50 @@ async fn execute( ..Default::default() }), ) - .await?; + .await + .map_err(|e| Error::BuildFailure(e.to_string()))?; match start_exec_results { bollard::exec::StartExecResults::Attached { output, .. } => { let log_outputs = if log_output { output.try_collect::>() - .await? + .await + .map_err(|e| Error::BuildFailure(e.to_string()))? .into_iter() .map(|output| output.to_string() ) .collect() } else { Vec::new() - }; + } + .join(""); // Wait until the execution finishes. if let Some(timeout) = timeout_secs { - let _ = tokio::time::timeout(Duration::from_secs(timeout), async { + let is_inspect_ok = tokio::time::timeout(Duration::from_secs(timeout), async { loop { if let Ok(inspect_result) = docker.inspect_exec(&create_exec_results.id).await { if inspect_result.running != Some(true) { - break; + return true } // Continue to check if the execution finishes. } else { // Fail to inspect. The loop should be terminated. - break; + return false } } }) - .await; + .await + .map_err(|_| Error::BuildTimeout)?; + if !is_inspect_ok { + return Err(Error::BuildFailureWithLogs(log_outputs)) + } } return Ok(log_outputs) }, bollard::exec::StartExecResults::Detached => { - return Err(bollard::errors::Error::DockerStreamError { - error: "Execution Result Not Attached".to_string(), - }); + return Err(Error::BuildFailure("Execution Result Not Attached".to_string())); } } } diff --git a/src/error.rs b/src/error.rs index ae0e9bb..b621cf6 100644 --- a/src/error.rs +++ b/src/error.rs @@ -16,6 +16,9 @@ pub enum Error { #[error("Failure during building process.")] BuildFailureWithLogs(String), + #[error("The building process took too long.")] + BuildTimeout, + #[error("Docker daemon service did not respond.")] DockerDaemonFailure, @@ -47,6 +50,7 @@ impl Error { Error::ArtifactRemovalFailure => "The compilation was successful, but pchain-compile failed to stop its Docker containers. Please remove them manually.".to_string(), Error::BuildFailure(e) => format!("\nDetails: {e}\nPlease rectify the errors and build your source code again."), Error::BuildFailureWithLogs(log) => format!("There maybe some problems in the source code.\nBuilding log is as follows:\n\n{log}\n"), + Error::BuildTimeout => format!("The time used in the building process is abnormal. It is possible that the contract code is extraordinarily large, or there is something wrong in your building environment (e.g. docker)."), Error::DockerDaemonFailure => "Failed to compile.\nDetails: Docker Daemon Failure. Check if Docker is running on your machine and confirm read/write access privileges.".to_string(), Error::ManifestFailure => "Failed to compile.\nDetails: Manifest File Not Found. Check if the manifest file exists on the source code path.".to_string(), Error::InvalidSourcePath => "Failed to compile.\nDetails: Source Code Path Not Valid. Check if you have provided the correct path to your source code directory and confirm write access privileges.".to_string(), From 7e47451978f6bd5cf796e28a50db90c1b1bc07b5 Mon Sep 17 00:00:00 2001 From: AlvinHon Date: Thu, 31 Aug 2023 17:06:53 +0800 Subject: [PATCH 7/7] add sleep to avoid hitting docker endpoint frequently --- src/docker.rs | 2 ++ tests/test.rs | 15 +++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/docker.rs b/src/docker.rs index e4e8b13..a7ae395 100644 --- a/src/docker.rs +++ b/src/docker.rs @@ -431,6 +431,8 @@ async fn execute( // Fail to inspect. The loop should be terminated. return false } + // A small delay to avoid hitting docker endpoint immediately. + tokio::time::sleep(Duration::from_millis(20)).await; } }) .await diff --git a/tests/test.rs b/tests/test.rs index 1f96284..fb17d0c 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -88,15 +88,22 @@ async fn build_contract_without_docker() { .join("tests") .join("contracts") .to_path_buf(); - let wasm_name = pchain_compile::Config { + let run_result = pchain_compile::Config { source_path, destination_path: Some(destination_path.clone()), build_options: BuildOptions { locked: true }, docker_option: DockerOption::Dockerless, } - .run() - .await - .unwrap(); + .run() + .await; + + let wasm_name = match run_result { + Ok(wasm_name) => wasm_name, + Err(e) => { + println!("{:?}", e); + panic!("Note: This test require installation of target 'wasm32-unknown-unknown'. It can be installed by 'rustup add wasm32-unknown-unknown'"); + } + }; assert!(destination_path.join("Cargo.lock").exists()); let _ = std::fs::remove_file(destination_path.join(&wasm_name));