Fix `wasm-opt --version` parsing (#248)

* Fix `wasm-opt --version` parsing

* Mark stdout output and error clearer

* Implement comments

* Update readme

* Update readme

README.md

@@ -30,8 +30,8 @@ We optimize the resulting contract Wasm using `binaryen`. You have two options f
 ## Usage
 
 ```
-cargo-contract 0.8.0
-Utilities to develop Wasm smart contracts.
+cargo-contract 0.11.0
+Utilities to develop Wasm smart contracts
 
 USAGE:
     cargo contract <SUBCOMMAND>
@@ -42,8 +42,11 @@ OPTIONS:
 
 SUBCOMMANDS:
     new                  Setup and create a new smart contract project
-    build                Compiles the contract, generates metadata, bundles both together in a '.contract' file
-    check                Check that the code builds as Wasm; does not output any build artifact to the top level `target/` directory
+    build                Compiles the contract, generates metadata, bundles
+                         both together in a `<name>.contract` file
+    generate-metadata    Command has been deprecated, use `cargo contract build` instead
+    check                Check that the code builds as Wasm; does not output any
+                         `<name>.contract` artifact to the `target/` directory
     test                 Test the smart contract off-chain
     deploy               Upload the smart contract code to the chain
     instantiate          Instantiate a deployed smart contract

src/cmd/build.rs

@@ -493,10 +493,10 @@ 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")
         .trim();
-    let re = Regex::new(r"wasm-opt version (\d+)\s+").unwrap();
+    let re = Regex::new(r"wasm-opt version (\d+)").expect("invalid regex");
     let captures = re.captures(version_stdout).ok_or_else(|| {
         anyhow::anyhow!(
-            "Unable to extract version information from {}.\n\
+            "Unable to extract version information from '{}'.\n\
             Your wasm-opt version is most probably too old. Make sure you use a version >= 99.",
             version_stdout
         )
@@ -504,19 +504,26 @@ fn check_wasm_opt_version_compatibility(wasm_opt_path: &Path) -> Result<()> {
     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)
+            anyhow::anyhow!(
+                "Unable to extract version number from '{:?}'",
+                version_stdout
+            )
         })?
         .as_str()
         .parse()
         .map_err(|err| {
             anyhow::anyhow!(
-                "Parsing version number failed with {:?} for {:?}",
+                "Parsing version number failed with '{:?}' for '{:?}'",
                 err,
                 version_stdout
             )
         })?;
 
-    log::info!("The wasm-opt version is \"{}\"", version_stdout);
+    log::info!(
+        "The wasm-opt version output is '{}', which was parsed to '{}'",
+        version_stdout,
+        version_number
+    );
     if version_number < 99 {
         anyhow::bail!(
             "Your wasm-opt version is {}, but we require a version >= 99.",
@@ -642,7 +649,11 @@ mod tests_ci_only {
         BuildArtifacts, ManifestPath, OptimizationPasses, UnstableFlags, UnstableOptions,
         Verbosity, VerbosityFlags,
     };
-    use std::{io::Write, os::unix::fs::PermissionsExt, path::PathBuf};
+    use std::{
+        io::Write,
+        os::unix::fs::PermissionsExt,
+        path::{Path, PathBuf},
+    };
 
     /// Modifies the `Cargo.toml` under the supplied `cargo_toml_path` by
     /// setting `optimization-passes` in `[package.metadata.contract]` to `passes`.
@@ -661,6 +672,23 @@ mod tests_ci_only {
             .expect("writing manifest failed");
     }
 
+    /// Creates an executable `wasm-opt-mocked` file which outputs
+    /// "wasm-opt version `version`".
+    ///
+    /// Returns the path to this file.
+    fn mock_wasm_opt_version(tmp_dir: &Path, version: &str) -> PathBuf {
+        let path = tmp_dir.join("wasm-opt-mocked");
+        {
+            let mut file = std::fs::File::create(&path).unwrap();
+            let version = format!("#!/bin/sh\necho \"wasm-opt version {}\"", version);
+            file.write_all(version.as_bytes())
+                .expect("writing wasm-opt-mocked failed");
+        }
+        std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o777))
+            .expect("setting permissions failed");
+        path
+    }
+
     #[test]
     fn build_code_only() {
         with_tmp_dir(|path| {
@@ -855,19 +883,10 @@ mod tests_ci_only {
     }
 
     #[test]
-    fn incompatible_wasm_opt_version_must_be_detected() {
+    fn incompatible_wasm_opt_version_must_be_detected_if_built_from_repo() {
         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 98 (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");
+            let path = mock_wasm_opt_version(path, "98 (version_13-79-gc12cc3f50)");
 
             // when
             let res = check_wasm_opt_version_compatibility(&path);
@@ -884,19 +903,46 @@ mod tests_ci_only {
     }
 
     #[test]
-    fn compatible_wasm_opt_version_must_be_detected() {
+    fn compatible_wasm_opt_version_must_be_detected_if_built_from_repo() {
         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");
+            let path = mock_wasm_opt_version(path, "99 (version_99-79-gc12cc3f50");
+
+            // when
+            let res = check_wasm_opt_version_compatibility(&path);
+
+            // then
+            assert!(res.is_ok());
+
+            Ok(())
+        })
+    }
+
+    #[test]
+    fn incompatible_wasm_opt_version_must_be_detected_if_installed_as_package() {
+        with_tmp_dir(|path| {
+            // given
+            let path = mock_wasm_opt_version(path, "98");
+
+            // when
+            let res = check_wasm_opt_version_compatibility(&path);
+
+            // then
+            assert!(res.is_err());
+            assert_eq!(
+                format!("{:?}", res),
+                "Err(Your wasm-opt version is 98, but we require a version >= 99.)"
+            );
+
+            Ok(())
+        })
+    }
+
+    #[test]
+    fn compatible_wasm_opt_version_must_be_detected_if_installed_as_package() {
+        with_tmp_dir(|path| {
+            // given
+            let path = mock_wasm_opt_version(path, "99");
 
             // when
             let res = check_wasm_opt_version_compatibility(&path);