From 1f7e1b375392e67de9851944c0d96dace0e60dce Mon Sep 17 00:00:00 2001 From: Asem Abdelhady Date: Mon, 11 Dec 2023 04:26:01 +0300 Subject: [PATCH] [fix] #4090 #3858: Fix having to pass IROHA_SKIP_WASM_CHECKS env variable with true (#4096) * [fix] #4090: Fix having to pass IROHA_SKIP_WASM_CHECKS env variable with true Signed-off-by: Asem-Abdelhady * [fix]: Fix having to pass variable IROHA_SKIP_WASM_CHECKS with checking PORFILE variable to be test Signed-off-by: Asem-Abdelhady * [fix]: delete formating from the src code Signed-off-by: Asem-Abdelhady * [delete] the check needed in building for client and cli Signed-off-by: Asem-Abdelhady * [add] format and checking of smart contracts in CI Signed-off-by: Asem-Abdelhady * [add] check and format in pr for dev Signed-off-by: Asem-Abdelhady * [change] format chekck from dev and dev-pr to dev-pr-static Signed-off-by: Asem-Abdelhady * [revert] paths deletion in dev-pr && [fix] naming issues in dev-pr-static CIs Signed-off-by: Asem-Abdelhady * [change] revert double quotes to single quotes in paths && remove the checking from dev CI Signed-off-by: Asem-Abdelhady * [delete] build script from client && [delete] IROHA_SKIP_WASM_CHECKS from nix Signed-off-by: Asem-Abdelhady * [delete] unecessary print statments and stray comment from cli build.rs Signed-off-by: Asem-Abdelhady --------- Signed-off-by: Asem-Abdelhady --- .github/workflows/iroha2-dev-pr-static.yml | 20 +++++++++- cli/build.rs | 14 ------- client/build.rs | 29 -------------- flake.nix | 1 - tools/wasm_builder_cli/src/main.rs | 6 --- wasm_builder/src/lib.rs | 45 +--------------------- 6 files changed, 21 insertions(+), 94 deletions(-) delete mode 100644 client/build.rs diff --git a/.github/workflows/iroha2-dev-pr-static.yml b/.github/workflows/iroha2-dev-pr-static.yml index 7fadced88d1..c89ef6976d8 100644 --- a/.github/workflows/iroha2-dev-pr-static.yml +++ b/.github/workflows/iroha2-dev-pr-static.yml @@ -17,7 +17,25 @@ env: RUSTUP_TOOLCHAIN: nightly-2023-06-25 jobs: - analysis: + smart_contracts_analysis: + runs-on: ubuntu-latest + container: + image: hyperledger/iroha2-ci:nightly-2023-06-25 + steps: + - uses: actions/checkout@v4 + - uses: Swatinem/rust-cache@v2 + + - name: Default executor format + run: | + cd ./default_executor + mold --run cargo fmt --all -- --check + + - name: Integration tests smart contracts format + run: | + cd ./client/tests/integration/smartcontracts + mold --run cargo fmt --all -- --check + + workspace_analysis: runs-on: ubuntu-latest container: image: hyperledger/iroha2-ci:nightly-2023-06-25 diff --git a/cli/build.rs b/cli/build.rs index 17491fc4ef2..699f4559ba8 100644 --- a/cli/build.rs +++ b/cli/build.rs @@ -5,13 +5,6 @@ use eyre::{eyre, Result, WrapErr}; fn main() -> Result<()> { extract_git_hash()?; - // HACK: used by Nix, since at the moment - // the checks are a process that's hard to accomodate - // in Nix environment - if std::option_env!("IROHA_SKIP_WASM_CHECKS").is_none() { - check_default_executor()?; - } - Ok(()) } @@ -23,10 +16,3 @@ fn extract_git_hash() -> Result<()> { .map_err(|err| eyre!(Box::new(err))) .wrap_err("Failed to extract git hash") } - -/// Apply `cargo check` to the smartcontract. -fn check_default_executor() -> Result<()> { - iroha_wasm_builder::Builder::new(DEFAULT_EXECUTOR_PATH) - .format() - .check() -} diff --git a/client/build.rs b/client/build.rs deleted file mode 100644 index 4f60d4cf4b0..00000000000 --- a/client/build.rs +++ /dev/null @@ -1,29 +0,0 @@ -//! Build script which checks smartcontracts for tests -//! -//! Technically this script is used only for testing purposes, but current cargo implementation -//! doesn't allow to run build script only for tests or get info about current profile from it. -//! See [cargo issue #4001](https://github.com/rust-lang/cargo/issues/4001) -//! and [#4789](https://github.com/rust-lang/cargo/issues/4789) - -use eyre::Result; - -const TEST_SMARTCONTRACTS_DIR: &str = "tests/integration/smartcontracts"; - -fn main() -> Result<()> { - println!("cargo:rerun-if-changed={TEST_SMARTCONTRACTS_DIR}"); - - // HACK: used by Nix, since at the moment - // the checks are a process that's hard to accomodate - // in Nix environment - if std::option_env!("IROHA_SKIP_WASM_CHECKS").is_none() { - check_all_smartcontracts()?; - } - - Ok(()) -} - -fn check_all_smartcontracts() -> Result<()> { - iroha_wasm_builder::Builder::new(TEST_SMARTCONTRACTS_DIR) - .format() - .check() -} diff --git a/flake.nix b/flake.nix index 3902404f9b8..0d7a40f63bf 100755 --- a/flake.nix +++ b/flake.nix @@ -214,7 +214,6 @@ fenix'.rust-analyzer ]; - IROHA_SKIP_WASM_CHECKS = true; }; }); } diff --git a/tools/wasm_builder_cli/src/main.rs b/tools/wasm_builder_cli/src/main.rs index fdef78d3cb8..33949919e7d 100644 --- a/tools/wasm_builder_cli/src/main.rs +++ b/tools/wasm_builder_cli/src/main.rs @@ -19,10 +19,6 @@ enum Cli { Build { #[command(flatten)] common: CommonArgs, - /// Enable smartcontract formatting using `cargo fmt`. - // TODO: why it is a part of `build` in wasm_builder? - #[arg(long)] - format: bool, /// Optimize WASM output. #[arg(long)] optimize: bool, @@ -48,12 +44,10 @@ fn main() -> color_eyre::Result<()> { } Cli::Build { common: CommonArgs { path }, - format, optimize, outfile, } => { let builder = Builder::new(&path); - let builder = if format { builder.format() } else { builder }; let output = { let mut sp = spinoff::Spinner::new_with_stream( diff --git a/wasm_builder/src/lib.rs b/wasm_builder/src/lib.rs index d59d7f38ad5..81c50541e2a 100644 --- a/wasm_builder/src/lib.rs +++ b/wasm_builder/src/lib.rs @@ -27,7 +27,6 @@ const TOOLCHAIN: &str = "+nightly-2023-06-25"; /// fn main() -> Result<()> { /// let bytes = Builder::new("relative/path/to/smartcontract/") /// .out_dir("path/to/out/dir") // Optional: Set output directory -/// .format() // Optional: Enable smartcontract formatting /// .build()? // Run build /// .optimize()? // Optimize WASM output /// .into_bytes()?; // Get resulting WASM bytes @@ -44,8 +43,6 @@ pub struct Builder<'path, 'out_dir> { path: &'path Path, /// Build output directory out_dir: Option<&'out_dir Path>, - /// Flag to enable smartcontract formatting - format: bool, } impl<'path, 'out_dir> Builder<'path, 'out_dir> { @@ -59,7 +56,6 @@ impl<'path, 'out_dir> Builder<'path, 'out_dir> { Self { path: relative_path.as_ref(), out_dir: None, - format: false, } } @@ -76,19 +72,11 @@ impl<'path, 'out_dir> Builder<'path, 'out_dir> { self } - /// Enable smartcontract formatting using `cargo fmt`. - /// - /// Disabled by default. - pub fn format(mut self) -> Self { - self.format = true; - self - } - /// Apply `cargo check` to the smartcontract. /// /// # Errors /// - /// Can fail due to multiple reasons like invalid path, failed formatting, failed build, etc. + /// Can fail due to multiple reasons like invalid path, failed build, etc. pub fn check(self) -> Result<()> { self.into_internal()?.check() } @@ -97,8 +85,7 @@ impl<'path, 'out_dir> Builder<'path, 'out_dir> { /// /// # Errors /// - /// Can fail due to multiple reasons like invalid path, failed formatting, - /// failed build, etc. + /// Can fail due to multiple reasons like invalid path, failed build, etc. /// /// Will also return error if ran on workspace and not on the concrete package. pub fn build(self) -> Result { @@ -115,7 +102,6 @@ impl<'path, 'out_dir> Builder<'path, 'out_dir> { || -> Result<_> { Ok(Cow::Owned(Self::default_out_dir()?)) }, |out_dir| Ok(Cow::Borrowed(out_dir)), )?, - format: self.format, }) } @@ -168,13 +154,10 @@ mod internal { pub struct Builder<'out_dir> { pub absolute_path: PathBuf, pub out_dir: Cow<'out_dir, Path>, - pub format: bool, } impl Builder<'_> { pub fn check(self) -> Result<()> { - self.maybe_format()?; - self.check_smartcontract().wrap_err_with(|| { format!( "Failed to check the smartcontract at path: {}", @@ -184,8 +167,6 @@ mod internal { } pub fn build(self) -> Result { - self.maybe_format()?; - let absolute_path = self.absolute_path.clone(); self.build_smartcontract().wrap_err_with(|| { format!( @@ -195,18 +176,6 @@ mod internal { }) } - fn maybe_format(&self) -> Result<()> { - if self.format { - self.format_smartcontract().wrap_err_with(|| { - format!( - "Failed to format the smartcontract at path: {}", - self.absolute_path.display() - ) - })?; - } - Ok(()) - } - fn build_options() -> impl Iterator { [ "--release", @@ -222,16 +191,6 @@ mod internal { .into_iter() } - fn format_smartcontract(&self) -> Result<()> { - let command_output = cargo_command() - .current_dir(&self.absolute_path) - .arg("fmt") - .output() - .wrap_err("Failed to run `cargo fmt`")?; - - check_command_output(&command_output, "cargo fmt") - } - fn get_base_command(&self, cmd: &'static str) -> std::process::Command { let mut command = cargo_command(); command