From 91f9d50b9ecc8dce4e6b6ae84708ee1cfc5f12df Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Tue, 30 Mar 2021 10:30:50 +0200 Subject: [PATCH 1/6] Improve `wasm-opt` not found error message --- src/cmd/build.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index fac09452..1eb2a5bd 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -393,11 +393,18 @@ fn do_optimization( // check `wasm-opt` is installed if which::which("wasm-opt").is_err() { anyhow::bail!( - "{}", - "wasm-opt is not installed. Install this tool on your system in order to \n\ - reduce the size of your contract's Wasm binary. \n\ - See https://github.com/WebAssembly/binaryen#tools" - .bright_yellow() + "wasm-opt not found!\n\ + We use this tool to optimize the size of your contract's Wasm binary.\n\n\ + wasm-opt is part of the binaryen package. You can find detailed\n\ + installation instructions on https://github.com/WebAssembly/binaryen#tools.\n\n\ + + There are also ready-to-install packages for many platforms:\n\ + * Debian/Ubuntu: https://tracker.debian.org/pkg/binaryen\n\ + * Homebrew: https://formulae.brew.sh/formula/binaryen\n\ + * Arch Linux: https://archlinux.org/packages/community/x86_64/binaryen\n\ + * Windows: binary releases are available at https://github.com/WebAssembly/binaryen/releases" + .to_string() + .bright_yellow() ); } -- GitLab From 808cfd67af6151ac4dcaa400b53bd7b0652e3f87 Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Tue, 30 Mar 2021 11:41:51 +0200 Subject: [PATCH 2/6] Improve error message + check `wasm-opt` compatibility --- src/cmd/build.rs | 140 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 131 insertions(+), 9 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 1eb2a5bd..664933dd 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -22,8 +22,8 @@ use std::{ io::{Read, Write}, }; -#[cfg(not(feature = "binaryen-as-dependency"))] -use std::{process::Command, str}; +#[cfg(any(test, not(feature = "binaryen-as-dependency")))] +use std::{path::Path, process::Command, str}; use crate::{ crate_metadata::CrateMetadata, @@ -391,7 +391,8 @@ fn do_optimization( optimization_level: OptimizationPasses, ) -> Result<()> { // check `wasm-opt` is installed - if which::which("wasm-opt").is_err() { + let which = which::which("wasm-opt"); + if which.is_err() { anyhow::bail!( "wasm-opt not found!\n\ We use this tool to optimize the size of your contract's Wasm binary.\n\n\ @@ -407,12 +408,17 @@ fn do_optimization( .bright_yellow() ); } + let wasm_opt_path = which + .as_ref() + .expect("we just checked if which returned an err; qed") + .as_path(); + let _ = check_wasm_opt_version_compatibility(wasm_opt_path)?; log::info!( "Optimization level passed to wasm-opt: {}", optimization_level ); - let output = Command::new("wasm-opt") + let output = Command::new(wasm_opt_path) .arg(dest_wasm) .arg(format!("-O{}", optimization_level)) .arg("-o") @@ -421,15 +427,15 @@ fn do_optimization( // the memory is initialized to zeroes, otherwise it won't run the // memory-packing pre-pass. .arg("--zero-filled-memory") - .output()?; + .output() + .map_err(|err| anyhow::anyhow!("Executing {:?} failed with {:?}", wasm_opt_path, err))?; if !output.status.success() { let err = str::from_utf8(&output.stderr) .expect("cannot convert stderr output of wasm-opt to string") .trim(); anyhow::bail!( - "The wasm-opt optimization failed.\n\ - A possible source of error could be that you have an outdated binaryen package installed.\n\n\ + "The wasm-opt optimization failed.\n\n\ The error which wasm-opt returned was: \n{}", err ); @@ -437,6 +443,68 @@ fn do_optimization( Ok(()) } +/// Checks if the wasm-opt binary under `wasm_opt_path` returns a version +/// compatible with `cargo-contract`. +/// +/// Currently this must be a version >= 99. +#[cfg(any(test, not(feature = "binaryen-as-dependency")))] +fn check_wasm_opt_version_compatibility(wasm_opt_path: &Path) -> Result<()> { + let cmd = Command::new(wasm_opt_path) + .arg("--version") + .output() + .map_err(|err| { + anyhow::anyhow!( + "Executing `{:?} --version` failed with {:?}", + wasm_opt_path, + err + ) + })?; + if !cmd.status.success() { + let err = str::from_utf8(&cmd.stderr) + .expect("Cannot convert stderr output of wasm-opt to string") + .trim(); + anyhow::bail!( + "Getting version information from wasm-opt failed.\n\ + The error which wasm-opt returned was: \n{}", + err + ); + } + + // ```sh + // $ wasm-opt --version + // wasm-opt version 99 (version_99-79-gc12cc3f50) + // ``` + let version_stdout = + str::from_utf8(&cmd.stdout).expect("cannot convert stdout output of wasm-opt to string"); + let mut version_iter = version_stdout.split_whitespace(); + let _ = version_iter + .next() + .ok_or_else(|| anyhow::anyhow!("Cannot get program identifier for {:?}", version_stdout))?; + let _ = version_iter + .next() + .ok_or_else(|| anyhow::anyhow!("Cannot get version label for {:?}", version_stdout))?; + let version_number: u32 = version_iter + .next() + .ok_or_else(|| anyhow::anyhow!("Cannot get version information for {:?}", version_stdout))? + .parse() + .map_err(|err| { + anyhow::anyhow!( + "Parsing version number failed with {:?} for {:?}", + err, + version_stdout + ) + })?; + + log::info!("The wasm-opt version is \"{}\"", version_stdout); + if version_number < 99 { + anyhow::bail!( + "Your wasm-opt version is {}, but we require a version >= 99.", + version_number + ); + } + Ok(()) +} + /// Asserts that the contract's dependencies are compatible to the ones used in ink!. /// /// This function utilizes `cargo tree`, which takes semver into consideration. @@ -545,7 +613,7 @@ pub(crate) fn execute( #[cfg(feature = "test-ci-only")] #[cfg(test)] mod tests_ci_only { - use super::assert_compatible_ink_dependencies; + use super::{assert_compatible_ink_dependencies, check_wasm_opt_version_compatibility}; use crate::{ cmd::{self, BuildCommand}, util::tests::with_tmp_dir, @@ -553,7 +621,7 @@ mod tests_ci_only { BuildArtifacts, ManifestPath, OptimizationPasses, UnstableFlags, UnstableOptions, Verbosity, VerbosityFlags, }; - use std::path::PathBuf; + use std::{io::Write, os::unix::fs::PermissionsExt, path::PathBuf}; /// Modifies the `Cargo.toml` under the supplied `cargo_toml_path` by /// setting `optimization-passes` in `[package.metadata.contract]` to `passes`. @@ -764,4 +832,58 @@ mod tests_ci_only { Ok(()) }) } + + #[test] + fn incompatible_wasm_opt_version_must_be_detected() { + with_tmp_dir(|path| { + // given + let path = path.join("wasm-opt-mocked"); + { + let mut file = std::fs::File::create(&path).unwrap(); + file.write_all( + b"#!/bin/sh\necho \"wasm-opt version 13 (version_13-79-gc12cc3f50)\"", + ) + .expect("writing wasm-opt-mocked failed"); + } + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o777)) + .expect("setting permissions failed"); + + // when + let res = check_wasm_opt_version_compatibility(&path); + + // then + assert!(res.is_err()); + assert_eq!( + format!("{:?}", res), + "Err(Your wasm-opt version is 13, but we require a version >= 99.)" + ); + + Ok(()) + }) + } + + #[test] + fn compatible_wasm_opt_version_must_be_detected() { + with_tmp_dir(|path| { + // given + let path = path.join("wasm-opt-mocked"); + { + let mut file = std::fs::File::create(&path).unwrap(); + file.write_all( + b"#!/bin/sh\necho \"wasm-opt version 99 (version_99-79-gc12cc3f50)\"", + ) + .expect("writing wasm-opt-mocked failed"); + } + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o777)) + .expect("setting permissions failed"); + + // when + let res = check_wasm_opt_version_compatibility(&path); + + // then + assert!(res.is_ok()); + + Ok(()) + }) + } } -- GitLab From e91f71cfd11987637f8d611e554f1faf87e563fe Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Tue, 30 Mar 2021 12:41:00 +0200 Subject: [PATCH 3/6] Use `regex` for parsing `wasm-opt --version` --- Cargo.lock | 1 + Cargo.toml | 1 + src/cmd/build.rs | 24 ++++++++++++++---------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 021fe632..15568651 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -510,6 +510,7 @@ dependencies = [ "parity-wasm 0.42.2", "pretty_assertions", "pwasm-utils", + "regex", "rustc_version 0.3.3", "semver 0.11.0", "serde", diff --git a/Cargo.toml b/Cargo.toml index 262d965a..b41239e7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ tempfile = "3.2.0" url = { version = "2.2.1", features = ["serde"] } binaryen = { version = "0.12.0", optional = true } impl-serde = "0.3.1" +regex = "1.4" # dependencies for optional extrinsics feature async-std = { version = "1.9.0", optional = true } diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 664933dd..409165d5 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with cargo-contract. If not, see . +use regex::Regex; use std::{convert::TryFrom, ffi::OsStr, fs::metadata, path::PathBuf}; #[cfg(feature = "binaryen-as-dependency")] @@ -476,16 +477,19 @@ fn check_wasm_opt_version_compatibility(wasm_opt_path: &Path) -> Result<()> { // ``` let version_stdout = str::from_utf8(&cmd.stdout).expect("cannot convert stdout output of wasm-opt to string"); - let mut version_iter = version_stdout.split_whitespace(); - let _ = version_iter - .next() - .ok_or_else(|| anyhow::anyhow!("Cannot get program identifier for {:?}", version_stdout))?; - let _ = version_iter - .next() - .ok_or_else(|| anyhow::anyhow!("Cannot get version label for {:?}", version_stdout))?; - let version_number: u32 = version_iter - .next() - .ok_or_else(|| anyhow::anyhow!("Cannot get version information for {:?}", version_stdout))? + let re = Regex::new(r"wasm-opt version (\d+)\s+").unwrap(); + let captures = re.captures(version_stdout).ok_or_else(|| { + anyhow::anyhow!( + "Unable to extract version information from {:?}", + version_stdout + ) + })?; + let version_number: u32 = captures + .get(1) // first capture group is at index 1 + .ok_or_else(|| { + anyhow::anyhow!("Unable to extract version number from {:?}", version_stdout) + })? + .as_str() .parse() .map_err(|err| { anyhow::anyhow!( -- GitLab From 80d54299531bb22f7a0ce0b8c89d7b4d28efcb40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20M=C3=BCller?= Date: Tue, 30 Mar 2021 13:32:26 +0200 Subject: [PATCH 4/6] Apply suggestions from code review Co-authored-by: Andrew Jones --- src/cmd/build.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 409165d5..fb20494b 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -429,7 +429,7 @@ fn do_optimization( // memory-packing pre-pass. .arg("--zero-filled-memory") .output() - .map_err(|err| anyhow::anyhow!("Executing {:?} failed with {:?}", wasm_opt_path, err))?; + .map_err(|err| anyhow::anyhow!("Executing {} failed with {:?}", wasm_opt_path.display(), err))?; if !output.status.success() { let err = str::from_utf8(&output.stderr) @@ -456,7 +456,7 @@ fn check_wasm_opt_version_compatibility(wasm_opt_path: &Path) -> Result<()> { .map_err(|err| { anyhow::anyhow!( "Executing `{:?} --version` failed with {:?}", - wasm_opt_path, + wasm_opt_path.display(), err ) })?; @@ -845,7 +845,7 @@ mod tests_ci_only { { let mut file = std::fs::File::create(&path).unwrap(); file.write_all( - b"#!/bin/sh\necho \"wasm-opt version 13 (version_13-79-gc12cc3f50)\"", + b"#!/bin/sh\necho \"wasm-opt version 98 (version_13-79-gc12cc3f50)\"", ) .expect("writing wasm-opt-mocked failed"); } @@ -859,7 +859,7 @@ mod tests_ci_only { assert!(res.is_err()); assert_eq!( format!("{:?}", res), - "Err(Your wasm-opt version is 13, but we require a version >= 99.)" + "Err(Your wasm-opt version is 98, but we require a version >= 99.)" ); Ok(()) -- GitLab From 5d8709d5466ebd94d893f6c6ad6aaa1166f4661e Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Tue, 30 Mar 2021 13:35:17 +0200 Subject: [PATCH 5/6] Implement comments --- src/cmd/build.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index fb20494b..81b2c74a 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -395,7 +395,7 @@ fn do_optimization( let which = which::which("wasm-opt"); if which.is_err() { anyhow::bail!( - "wasm-opt not found!\n\ + "wasm-opt not found! Make sure the binary is in your PATH environment.\n\ We use this tool to optimize the size of your contract's Wasm binary.\n\n\ wasm-opt is part of the binaryen package. You can find detailed\n\ installation instructions on https://github.com/WebAssembly/binaryen#tools.\n\n\ @@ -413,6 +413,11 @@ fn do_optimization( .as_ref() .expect("we just checked if which returned an err; qed") .as_path(); + log::info!( + "Path to wasm-opt executable: {}", + wasm_opt_path.display() + ); + let _ = check_wasm_opt_version_compatibility(wasm_opt_path)?; log::info!( -- GitLab From 2233137cea069b7ea39d5cba1fe881c74dbe619a Mon Sep 17 00:00:00 2001 From: Michael Mueller Date: Tue, 30 Mar 2021 13:35:47 +0200 Subject: [PATCH 6/6] Apply cargo fmt --- src/cmd/build.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/cmd/build.rs b/src/cmd/build.rs index 81b2c74a..114eb702 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -413,10 +413,7 @@ fn do_optimization( .as_ref() .expect("we just checked if which returned an err; qed") .as_path(); - log::info!( - "Path to wasm-opt executable: {}", - wasm_opt_path.display() - ); + log::info!("Path to wasm-opt executable: {}", wasm_opt_path.display()); let _ = check_wasm_opt_version_compatibility(wasm_opt_path)?; @@ -434,7 +431,13 @@ fn do_optimization( // memory-packing pre-pass. .arg("--zero-filled-memory") .output() - .map_err(|err| anyhow::anyhow!("Executing {} failed with {:?}", wasm_opt_path.display(), err))?; + .map_err(|err| { + anyhow::anyhow!( + "Executing {} failed with {:?}", + wasm_opt_path.display(), + err + ) + })?; if !output.status.success() { let err = str::from_utf8(&output.stderr) -- GitLab