improve error reporting in CLI

Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>

Author: jprendes

Cargo.lock

@@ -6,6 +6,7 @@ edition = "2024"
 [dependencies]
 anyhow = "1.0.100"
 clap = { version = "4.5", features = ["derive"] }
+console = "0.16.1"
 const_format = "0.2.34"
 glob = "0.3.3"
 libc = "0.2.176"

Cargo.toml

@@ -1,7 +1,7 @@
 use std::collections::HashMap;
 use std::env;
 use std::ffi::{OsStr, OsString};
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
 
 use anyhow::{Result, bail};
@@ -25,16 +25,36 @@ pub trait CargoCmd {
     fn checked_status(&mut self) -> Result<()>;
 }
 
-pub fn cargo_cmd() -> Result<Command> {
+#[derive(Clone, Hash)]
+pub struct CargoBinary {
+    pub path: PathBuf,
+    pub rustup_toolchain: Option<OsString>,
+}
+
+impl CargoBinary {
+    pub fn command(&self) -> Command {
+        let mut cmd = Command::new(&self.path);
+        if let Some(rustup_toolchain) = &self.rustup_toolchain {
+            cmd.env("RUSTUP_TOOLCHAIN", rustup_toolchain);
+        }
+        cmd
+    }
+}
+
+pub fn find_cargo() -> Result<CargoBinary> {
     let cargo = match env::var_os("CARGO") {
         Some(cargo) => Path::new(&cargo).canonicalize()?,
         None => which::which("cargo")?.canonicalize()?,
     };
-    let mut cmd = Command::new(cargo);
-    if let Some(rustup_toolchain) = env::var_os("RUSTUP_TOOLCHAIN") {
-        cmd.env("RUSTUP_TOOLCHAIN", rustup_toolchain);
-    }
-    Ok(cmd)
+    let rustup_toolchain = env::var_os("RUSTUP_TOOLCHAIN");
+    Ok(CargoBinary {
+        path: cargo,
+        rustup_toolchain,
+    })
+}
+
+pub fn cargo_cmd() -> Result<Command> {
+    Ok(find_cargo()?.command())
 }
 
 pub struct CheckedOutput {
@@ -152,20 +172,7 @@ impl CargoCmd for Command {
         &self,
         base: impl IntoIterator<Item = (impl AsRef<OsStr>, impl AsRef<OsStr>)>,
     ) -> HashMap<OsString, OsString> {
-        let mut envs = base
-            .into_iter()
-            .map(|(k, v)| (k.as_ref().to_owned(), v.as_ref().to_owned()))
-            .collect::<HashMap<_, _>>();
-
-        for (k, v) in self.get_envs() {
-            if let Some(v) = v {
-                envs.insert(k.to_owned(), v.to_owned());
-            } else {
-                envs.remove(k);
-            }
-        }
-
-        envs
+        merge_env(base, self.get_envs())
     }
 
     fn checked_output(&mut self) -> Result<CheckedOutput> {
@@ -204,3 +211,23 @@ fn get_env(cmd: &Command, key: &str) -> Option<OsString> {
         None => std::env::var_os(key),
     }
 }
+
+pub fn merge_env(
+    base: impl IntoIterator<Item = (impl AsRef<OsStr>, impl AsRef<OsStr>)>,
+    envs: impl IntoIterator<Item = (impl AsRef<OsStr>, Option<impl AsRef<OsStr>>)>,
+) -> HashMap<OsString, OsString> {
+    let mut base = base
+        .into_iter()
+        .map(|(k, v)| (k.as_ref().to_owned(), v.as_ref().to_owned()))
+        .collect::<HashMap<_, _>>();
+
+    for (k, v) in envs {
+        if let Some(v) = v {
+            base.insert(k.as_ref().to_owned(), v.as_ref().to_owned());
+        } else {
+            base.remove(k.as_ref());
+        }
+    }
+
+    base
+}

src/cargo_cmd.rs

@@ -1,64 +1,183 @@
 use std::collections::HashMap;
+use std::convert::Infallible;
 use std::env;
 use std::env::consts::ARCH;
 use std::ffi::OsString;
+use std::fmt::Debug;
 use std::path::PathBuf;
 
 use anyhow::{Context, Result};
 use clap::{Parser, Subcommand};
 use const_format::formatcp;
 
 use crate::cargo_cmd::{CargoCmd as _, cargo_cmd};
+use crate::toolchain;
 
 pub struct Args {
     pub manifest_path: Option<PathBuf>,
     pub target_dir: PathBuf,
     pub target: String,
     pub env: HashMap<OsString, OsString>,
     pub current_dir: PathBuf,
+    pub clang: Option<PathBuf>,
+    pub ar: Option<PathBuf>,
+}
+
+pub trait WarningLevel {
+    type Error;
+    fn warning<T: Debug>(
+        &self,
+        msg: &str,
+        err: impl Into<anyhow::Error>,
+        default: T,
+    ) -> Result<T, Self::Error>;
+}
+
+pub struct Warning;
+
+#[doc(hidden)]
+pub mod warning {
+    pub struct WarningIgnore;
+    pub struct WarningWarn;
+    #[allow(dead_code)]
+    pub struct WarningError;
+}
+
+impl Warning {
+    pub const IGNORE: warning::WarningIgnore = warning::WarningIgnore;
+    pub const WARN: warning::WarningWarn = warning::WarningWarn;
+    #[allow(dead_code)]
+    pub const ERROR: warning::WarningError = warning::WarningError;
+}
+
+impl WarningLevel for warning::WarningIgnore {
+    type Error = Infallible;
+    fn warning<T: Debug>(
+        &self,
+        _msg: &str,
+        _err: impl Into<anyhow::Error>,
+        default: T,
+    ) -> Result<T, Self::Error> {
+        Ok(default)
+    }
+}
+
+impl WarningLevel for warning::WarningWarn {
+    type Error = Infallible;
+    fn warning<T: Debug>(
+        &self,
+        msg: &str,
+        err: impl Into<anyhow::Error>,
+        default: T,
+    ) -> Result<T, Self::Error> {
+        warning(msg);
+        warning(format!("using {default:?}"));
+        println!("{:?}", err.into());
+        Ok(default)
+    }
+}
+
+impl WarningLevel for warning::WarningError {
+    type Error = anyhow::Error;
+    fn warning<T: Debug>(
+        &self,
+        msg: &str,
+        err: impl Into<anyhow::Error>,
+        _default: T,
+    ) -> Result<T, Self::Error> {
+        Err(err.into()).context(msg.to_string())
+    }
 }
 
 impl Args {
-    pub fn parse_from(
+    pub fn parse<W: WarningLevel>(
         args: impl IntoIterator<Item = impl Into<OsString> + Clone>,
         env: impl IntoIterator<Item = (impl Into<OsString>, impl Into<OsString>)>,
         cwd: Option<impl Into<PathBuf>>,
-    ) -> Result<Args> {
+        warn: W,
+    ) -> Result<Args, W::Error> {
         let mut args = ArgsImpl::parse_from(args);
         args.env = env.into_iter().map(|(k, v)| (k.into(), v.into())).collect();
-        args.current_dir = match cwd {
+        let cwd = match cwd {
             Some(cwd) => cwd.into(),
-            None => env::current_dir().context("Failed to get current directory")?,
+            None => match env::current_dir() {
+                Ok(cwd) => cwd,
+                Err(err) => {
+                    warn.warning("Could not get current directory", err, PathBuf::from("."))?
+                }
+            },
         };
-        args.try_into()
+        args.current_dir = cwd.clone();
+        Args::try_from_with_defaults(warn, args)
     }
 }
 
+fn warning(msg: impl AsRef<str>) {
+    eprintln!(
+        "{}{}{}",
+        console::style("warning").yellow().bold(),
+        console::style(": ").bold(),
+        console::style(msg.as_ref()).bold(),
+    );
+}
+
 impl TryFrom<ArgsImpl> for Args {
     type Error = anyhow::Error;
 
     fn try_from(value: ArgsImpl) -> Result<Self> {
+        Args::try_from_with_defaults(Warning::ERROR, value)
+    }
+}
+
+impl Args {
+    fn try_from_with_defaults<W: WarningLevel>(warn: W, value: ArgsImpl) -> Result<Self, W::Error> {
         let manifest_path = value.manifest_path;
 
         let target_dir = match value.target_dir {
             Some(dir) => dir,
-            None => resolve_target_dir(&manifest_path, &value.env, &value.current_dir)?,
+            None => match resolve_target_dir(&manifest_path, &value.env, &value.current_dir) {
+                Ok(dir) => dir,
+                Err(err) => warn.warning(
+                    "could not resolve target directory",
+                    err,
+                    value.current_dir.join("target"),
+                )?,
+            },
         };
 
         let target = match value.target {
             Some(triplet) => triplet,
-            None => resolve_target(&value.env, &value.current_dir)?,
+            None => match resolve_target(&value.env, &value.current_dir) {
+                Ok(triplet) => triplet,
+                Err(err) => warn.warning(
+                    "could not resolve target triple",
+                    err,
+                    DEFAULT_TARGET.to_string(),
+                )?,
+            },
+        };
+
+        let target = if target.ends_with("-hyperlight-none") {
+            target
+        } else {
+            let (arch, _) = target.split_once('-').unwrap_or((&target, ""));
+            warn.warning(
+                "requested target is not a hyperlight target",
+                anyhow::anyhow!("invalid hyperlight target: {target}"),
+                format!("{arch}-hyperlight-none"),
+            )?
         };
 
-        let cwd = env::current_dir().context("Failed to get current directory")?;
-        let target_dir = cwd.join(target_dir);
+        let target_dir = value.current_dir.join(target_dir);
 
         Ok(Args {
             manifest_path,
             target_dir,
             target,
             env: value.env,
             current_dir: value.current_dir,
+            clang: toolchain::find_cc().ok(),
+            ar: toolchain::find_ar().ok(),
         })
     }
 }