From d8e0d76c9cb033d25d929265a1818faf711b70ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 12 Jul 2023 08:11:00 +0100 Subject: [PATCH 1/6] Add an exit code to the CLI --- rust/agama-cli/src/main.rs | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/rust/agama-cli/src/main.rs b/rust/agama-cli/src/main.rs index e066e04015..1b4159d9ff 100644 --- a/rust/agama-cli/src/main.rs +++ b/rust/agama-cli/src/main.rs @@ -17,9 +17,12 @@ use config::run as run_config_cmd; use printers::Format; use profile::run as run_profile_cmd; use progress::InstallerProgress; -use std::error::Error; -use std::thread::sleep; -use std::time::Duration; +use std::{ + error::Error, + process::{ExitCode, Termination}, + thread::sleep, + time::Duration, +}; #[derive(Parser)] #[command(name = "agama", version, about, long_about = None)] @@ -130,13 +133,27 @@ async fn run_command(cli: Cli) -> Result<(), Box> { } } +/// Represents the result of execution. +pub enum CliResult { + /// Successful execution. + Ok = 0, + /// Something went wrong. + Error = 1, +} + +impl Termination for CliResult { + fn report(self) -> ExitCode { + ExitCode::from(self as u8) + } +} + #[async_std::main] -async fn main() -> Result<(), Box> { +async fn main() -> CliResult { let cli = Cli::parse(); if let Err(error) = run_command(cli).await { eprintln!("{}", error); - return Err(error); + return CliResult::Error; } - Ok(()) + CliResult::Ok } From d5cbbb3098afdd12a344d9f95eb9fa1dcd932582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 12 Jul 2023 09:06:09 +0100 Subject: [PATCH 2/6] Stop using Box in agama-lib --- rust/agama-lib/src/error.rs | 12 ++++-------- rust/agama-lib/src/network/store.rs | 5 ++--- rust/agama-lib/src/progress.rs | 6 +----- rust/agama-lib/src/software/store.rs | 12 ++++-------- rust/agama-lib/src/storage/store.rs | 5 ++--- rust/agama-lib/src/store.rs | 5 ++--- rust/agama-lib/src/users/store.rs | 15 +++++++-------- 7 files changed, 22 insertions(+), 38 deletions(-) diff --git a/rust/agama-lib/src/error.rs b/rust/agama-lib/src/error.rs index da828763c0..9cf8e9fc60 100644 --- a/rust/agama-lib/src/error.rs +++ b/rust/agama-lib/src/error.rs @@ -8,10 +8,14 @@ use zbus; pub enum ServiceError { #[error("D-Bus service error: {0}")] DBus(#[from] zbus::Error), + #[error("Unknown product '{0}'. Available products: '{1:?}'")] + UnknownProduct(String, Vec), // it's fine to say only "Error" because the original // specific error will be printed too #[error("Error: {0}")] Anyhow(#[from] anyhow::Error), + #[error("Wrong user parameters: '{0:?}'")] + WrongUser(Vec), } #[derive(Error, Debug)] @@ -27,11 +31,3 @@ pub enum ProfileError { #[error("The profile is not a valid JSON file")] FormatError(#[from] serde_json::Error), } - -#[derive(Error, Debug)] -pub enum WrongParameter { - #[error("Unknown product '{0}'. Available products: '{1:?}'")] - UnknownProduct(String, Vec), - #[error("Wrong user parameters: '{0:?}'")] - WrongUser(Vec), -} diff --git a/rust/agama-lib/src/network/store.rs b/rust/agama-lib/src/network/store.rs index 3315eb05b9..73b5146c5e 100644 --- a/rust/agama-lib/src/network/store.rs +++ b/rust/agama-lib/src/network/store.rs @@ -1,6 +1,5 @@ use crate::error::ServiceError; use crate::network::{NetworkClient, NetworkSettings}; -use std::error::Error; use zbus::Connection; /// Loads and stores the network settings from/to the D-Bus service. @@ -16,7 +15,7 @@ impl<'a> NetworkStore<'a> { } // TODO: read the settings from the service - pub async fn load(&self) -> Result> { + pub async fn load(&self) -> Result { let connections = self.network_client.connections().await?; Ok(NetworkSettings { @@ -25,7 +24,7 @@ impl<'a> NetworkStore<'a> { }) } - pub async fn store(&self, settings: &NetworkSettings) -> Result<(), Box> { + pub async fn store(&self, settings: &NetworkSettings) -> Result<(), ServiceError> { for conn in &settings.connections { self.network_client.add_or_update_connection(&conn).await?; } diff --git a/rust/agama-lib/src/progress.rs b/rust/agama-lib/src/progress.rs index 150e1a2fa1..22a13ca952 100644 --- a/rust/agama-lib/src/progress.rs +++ b/rust/agama-lib/src/progress.rs @@ -46,7 +46,6 @@ use crate::error::ServiceError; use crate::proxies::ProgressProxy; use futures::stream::{SelectAll, StreamExt}; use futures_util::{future::try_join3, Stream}; -use std::error::Error; use zbus::Connection; /// Represents the progress for an Agama service. @@ -107,10 +106,7 @@ impl<'a> ProgressMonitor<'a> { } /// Runs the monitor until the current operation finishes. - pub async fn run( - &mut self, - mut presenter: impl ProgressPresenter, - ) -> Result<(), Box> { + pub async fn run(&mut self, mut presenter: impl ProgressPresenter) -> Result<(), ServiceError> { presenter.start(&self.main_progress().await); let mut changes = self.build_stream().await; diff --git a/rust/agama-lib/src/software/store.rs b/rust/agama-lib/src/software/store.rs index 609aed4536..996ee05ebd 100644 --- a/rust/agama-lib/src/software/store.rs +++ b/rust/agama-lib/src/software/store.rs @@ -1,8 +1,7 @@ //! Implements the store for the storage settings. use super::{SoftwareClient, SoftwareSettings}; -use crate::error::{ServiceError, WrongParameter}; -use std::error::Error; +use crate::error::ServiceError; use zbus::Connection; /// Loads and stores the software settings from/to the D-Bus service. @@ -17,7 +16,7 @@ impl<'a> SoftwareStore<'a> { }) } - pub async fn load(&self) -> Result> { + pub async fn load(&self) -> Result { let product = self.software_client.product().await?; Ok(SoftwareSettings { @@ -25,17 +24,14 @@ impl<'a> SoftwareStore<'a> { }) } - pub async fn store(&self, settings: &SoftwareSettings) -> Result<(), Box> { + pub async fn store(&self, settings: &SoftwareSettings) -> Result<(), ServiceError> { if let Some(product) = &settings.product { let products = self.software_client.products().await?; let ids: Vec = products.into_iter().map(|p| p.id).collect(); if ids.contains(&product) { self.software_client.select_product(product).await?; } else { - return Err(Box::new(WrongParameter::UnknownProduct( - product.clone(), - ids, - ))); + return Err(ServiceError::UnknownProduct(product.clone(), ids)); } } Ok(()) diff --git a/rust/agama-lib/src/storage/store.rs b/rust/agama-lib/src/storage/store.rs index 10c013cbdc..d7d50a9bdb 100644 --- a/rust/agama-lib/src/storage/store.rs +++ b/rust/agama-lib/src/storage/store.rs @@ -3,7 +3,6 @@ use super::{StorageClient, StorageSettings}; use crate::error::ServiceError; use std::default::Default; -use std::error::Error; use zbus::Connection; /// Loads and stores the storage settings from/to the D-Bus service. @@ -19,11 +18,11 @@ impl<'a> StorageStore<'a> { } // TODO: read the settings from the service - pub async fn load(&self) -> Result> { + pub async fn load(&self) -> Result { Ok(Default::default()) } - pub async fn store(&self, settings: &StorageSettings) -> Result<(), Box> { + pub async fn store(&self, settings: &StorageSettings) -> Result<(), ServiceError> { self.storage_client .calculate( settings.devices.iter().map(|d| d.name.clone()).collect(), diff --git a/rust/agama-lib/src/store.rs b/rust/agama-lib/src/store.rs index 65ebef8c09..ae3ee8f082 100644 --- a/rust/agama-lib/src/store.rs +++ b/rust/agama-lib/src/store.rs @@ -5,7 +5,6 @@ use crate::install_settings::{InstallSettings, Scope}; use crate::{ network::NetworkStore, software::SoftwareStore, storage::StorageStore, users::UsersStore, }; -use std::error::Error; use zbus::Connection; /// Struct that loads/stores the settings from/to the D-Bus services. @@ -32,7 +31,7 @@ impl<'a> Store<'a> { } /// Loads the installation settings from the D-Bus service - pub async fn load(&self, only: Option>) -> Result> { + pub async fn load(&self, only: Option>) -> Result { let scopes = match only { Some(scopes) => scopes, None => Scope::all().to_vec(), @@ -59,7 +58,7 @@ impl<'a> Store<'a> { } /// Stores the given installation settings in the D-Bus service - pub async fn store(&self, settings: &InstallSettings) -> Result<(), Box> { + pub async fn store(&self, settings: &InstallSettings) -> Result<(), ServiceError> { if let Some(network) = &settings.network { self.network.store(network).await?; } diff --git a/rust/agama-lib/src/users/store.rs b/rust/agama-lib/src/users/store.rs index 3c8d28b5cc..6c34f0b2a1 100644 --- a/rust/agama-lib/src/users/store.rs +++ b/rust/agama-lib/src/users/store.rs @@ -1,6 +1,5 @@ use super::{FirstUser, FirstUserSettings, RootUserSettings, UserSettings, UsersClient}; -use crate::error::WrongParameter; -use std::error::Error; +use crate::error::ServiceError; use zbus::Connection; /// Loads and stores the users settings from/to the D-Bus service. @@ -9,13 +8,13 @@ pub struct UsersStore<'a> { } impl<'a> UsersStore<'a> { - pub async fn new(connection: Connection) -> Result, zbus::Error> { + pub async fn new(connection: Connection) -> Result, ServiceError> { Ok(Self { users_client: UsersClient::new(connection).await?, }) } - pub async fn load(&self) -> Result> { + pub async fn load(&self) -> Result { let first_user = self.users_client.first_user().await?; let first_user = FirstUserSettings { user_name: Some(first_user.user_name), @@ -34,7 +33,7 @@ impl<'a> UsersStore<'a> { }) } - pub async fn store(&self, settings: &UserSettings) -> Result<(), Box> { + pub async fn store(&self, settings: &UserSettings) -> Result<(), ServiceError> { // fixme: improve if let Some(settings) = &settings.first_user { self.store_first_user(settings).await?; @@ -46,7 +45,7 @@ impl<'a> UsersStore<'a> { Ok(()) } - async fn store_first_user(&self, settings: &FirstUserSettings) -> Result<(), Box> { + async fn store_first_user(&self, settings: &FirstUserSettings) -> Result<(), ServiceError> { let first_user = FirstUser { user_name: settings.user_name.clone().unwrap_or_default(), full_name: settings.full_name.clone().unwrap_or_default(), @@ -56,12 +55,12 @@ impl<'a> UsersStore<'a> { }; let (success, issues) = self.users_client.set_first_user(&first_user).await?; if !success { - return Err(Box::new(WrongParameter::WrongUser(issues))); + return Err(ServiceError::WrongUser(issues)); } Ok(()) } - async fn store_root_user(&self, settings: &RootUserSettings) -> Result<(), Box> { + async fn store_root_user(&self, settings: &RootUserSettings) -> Result<(), ServiceError> { if let Some(root_password) = &settings.password { self.users_client .set_root_password(root_password, false) From 0e29dcbb6793021c4630b586f8d0c30631db4751 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 12 Jul 2023 09:48:57 +0100 Subject: [PATCH 3/6] Use an error instead of a &str in the settings module --- rust/agama-derive/src/lib.rs | 8 +++--- rust/agama-lib/src/install_settings.rs | 10 +++---- rust/agama-lib/src/network/settings.rs | 10 +++---- rust/agama-lib/src/settings.rs | 38 +++++++++++++++++++------ rust/agama-lib/src/software/settings.rs | 2 +- rust/agama-lib/src/storage/settings.rs | 6 ++-- rust/agama-lib/src/users/client.rs | 6 ++-- rust/agama-lib/src/users/settings.rs | 6 ++-- 8 files changed, 53 insertions(+), 33 deletions(-) diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs index fa41a4bc81..b44c9c4ffe 100644 --- a/rust/agama-derive/src/lib.rs +++ b/rust/agama-derive/src/lib.rs @@ -52,10 +52,10 @@ fn expand_set_fn(field_name: &Vec) -> TokenStream2 { } quote! { - fn set(&mut self, attr: &str, value: crate::settings::SettingValue) -> Result<(), &'static str> { + fn set(&mut self, attr: &str, value: crate::settings::SettingValue) -> Result<(), crate::settings::SettingsError> { match attr { #(stringify!(#field_name) => self.#field_name = value.try_into()?,)* - _ => return Err("unknown attribute") + _ => return Err(SettingsError::UnknownAttribute(attr.to_string())) }; Ok(()) } @@ -85,10 +85,10 @@ fn expand_add_fn(field_name: &Vec) -> TokenStream2 { } quote! { - fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), &'static str> { + fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), crate::settings::SettingsError> { match attr { #(stringify!(#field_name) => self.#field_name.push(value.try_into()?),)* - _ => return Err("unknown attribute") + _ => return Err(SettingsError::UnknownCollection(attr.to_string())) }; Ok(()) } diff --git a/rust/agama-lib/src/install_settings.rs b/rust/agama-lib/src/install_settings.rs index bd1323f185..9e5f0285d6 100644 --- a/rust/agama-lib/src/install_settings.rs +++ b/rust/agama-lib/src/install_settings.rs @@ -1,7 +1,7 @@ //! Configuration settings handling //! //! This module implements the mechanisms to load and store the installation settings. -use crate::settings::{SettingObject, SettingValue, Settings}; +use crate::settings::{SettingObject, SettingValue, Settings, SettingsError}; use crate::{ network::NetworkSettings, software::SoftwareSettings, storage::StorageSettings, users::UserSettings, @@ -93,7 +93,7 @@ impl InstallSettings { } impl Settings for InstallSettings { - fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), &'static str> { + fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), SettingsError> { if let Some((ns, id)) = attr.split_once('.') { match ns { "network" => { @@ -112,13 +112,13 @@ impl Settings for InstallSettings { let storage = self.storage.get_or_insert(Default::default()); storage.add(id, value)? } - _ => return Err("unknown attribute"), + _ => return Err(SettingsError::UnknownCollection(attr.to_string())), } } Ok(()) } - fn set(&mut self, attr: &str, value: SettingValue) -> Result<(), &'static str> { + fn set(&mut self, attr: &str, value: SettingValue) -> Result<(), SettingsError> { if let Some((ns, id)) = attr.split_once('.') { match ns { "network" => { @@ -143,7 +143,7 @@ impl Settings for InstallSettings { let storage = self.storage.get_or_insert(Default::default()); storage.set(id, value)? } - _ => return Err("unknown attribute"), + _ => return Err(SettingsError::UnknownAttribute(attr.to_string())), } } Ok(()) diff --git a/rust/agama-lib/src/network/settings.rs b/rust/agama-lib/src/network/settings.rs index 69a615bc95..c044173346 100644 --- a/rust/agama-lib/src/network/settings.rs +++ b/rust/agama-lib/src/network/settings.rs @@ -1,7 +1,7 @@ //! Representation of the network settings use super::types::DeviceType; -use crate::settings::{SettingObject, SettingValue, Settings}; +use crate::settings::{SettingObject, SettingValue, Settings, SettingsError}; use serde::{Deserialize, Serialize}; use std::convert::TryFrom; use std::default::Default; @@ -15,10 +15,10 @@ pub struct NetworkSettings { } impl Settings for NetworkSettings { - fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), &'static str> { + fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), SettingsError> { match attr { "connections" => self.connections.push(value.try_into()?), - _ => return Err("unknown attribute"), + _ => return Err(SettingsError::UnknownAttribute(attr.to_string())), }; Ok(()) } @@ -70,11 +70,11 @@ impl NetworkConnection { } impl TryFrom for NetworkConnection { - type Error = &'static str; + type Error = SettingsError; fn try_from(value: SettingObject) -> Result { let Some(id) = value.get("id") else { - return Err("The 'id' key is missing"); + return Err(SettingsError::MissingKey("id".to_string())); }; let default_method = SettingValue("disabled".to_string()); diff --git a/rust/agama-lib/src/settings.rs b/rust/agama-lib/src/settings.rs index e1c03e1e86..13ad97e3c9 100644 --- a/rust/agama-lib/src/settings.rs +++ b/rust/agama-lib/src/settings.rs @@ -17,6 +17,8 @@ use std::collections::HashMap; /// /// TODO: derive for top-level structs too use std::convert::TryFrom; +use std::fmt::Display; +use thiserror::Error; /// Implements support for easily settings attributes values given an ID (`"users.name"`) and a /// string value (`"Foo bar"`). @@ -58,12 +60,12 @@ use std::convert::TryFrom; /// assert_eq!(&settings.user.name.unwrap(), "foo.bar"); /// ``` pub trait Settings { - fn add(&mut self, _attr: &str, _value: SettingObject) -> Result<(), &'static str> { - Err("unknown collection") + fn add(&mut self, attr: &str, _value: SettingObject) -> Result<(), SettingsError> { + Err(SettingsError::UnknownCollection(attr.to_string())) } - fn set(&mut self, _attr: &str, _value: SettingValue) -> Result<(), &'static str> { - Err("unknown attribute") + fn set(&mut self, attr: &str, _value: SettingValue) -> Result<(), SettingsError> { + Err(SettingsError::UnknownAttribute(attr.to_string())) } fn merge(&mut self, _other: &Self) @@ -89,6 +91,12 @@ pub trait Settings { #[derive(Clone, Debug)] pub struct SettingValue(pub String); +impl Display for SettingValue { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + /// Represents a string-based collection and allows converting to other types /// /// It wraps a hash which uses String as key and SettingValue as value. @@ -115,19 +123,19 @@ impl From> for SettingObject { } impl TryFrom for bool { - type Error = &'static str; + type Error = SettingsError; fn try_from(value: SettingValue) -> Result { match value.0.to_lowercase().as_str() { "true" | "yes" | "t" => Ok(true), "false" | "no" | "f" => Ok(false), - _ => Err("not a valid boolean"), + _ => Err(SettingsError::InvalidValue(value.to_string())), } } } impl TryFrom for Option { - type Error = &'static str; + type Error = SettingsError; fn try_from(value: SettingValue) -> Result { Ok(Some(value.try_into()?)) @@ -135,7 +143,7 @@ impl TryFrom for Option { } impl TryFrom for String { - type Error = &'static str; + type Error = SettingsError; fn try_from(value: SettingValue) -> Result { Ok(value.0) @@ -143,7 +151,7 @@ impl TryFrom for String { } impl TryFrom for Option { - type Error = &'static str; + type Error = SettingsError; fn try_from(value: SettingValue) -> Result { Ok(Some(value.try_into()?)) @@ -172,3 +180,15 @@ mod tests { assert_eq!(value, "some value"); } } + +#[derive(Error, Debug)] +pub enum SettingsError { + #[error("Unknown attribute '{0}'")] + UnknownAttribute(String), + #[error("Unknown collection '{0}'")] + UnknownCollection(String), + #[error("Invalid value '{0}'")] + InvalidValue(String), + #[error("Missing key '{0}'")] + MissingKey(String), +} diff --git a/rust/agama-lib/src/software/settings.rs b/rust/agama-lib/src/software/settings.rs index 776e414312..c345aea578 100644 --- a/rust/agama-lib/src/software/settings.rs +++ b/rust/agama-lib/src/software/settings.rs @@ -1,6 +1,6 @@ //! Representation of the software settings -use crate::settings::Settings; +use crate::settings::{Settings, SettingsError}; use agama_derive::Settings; use serde::{Deserialize, Serialize}; diff --git a/rust/agama-lib/src/storage/settings.rs b/rust/agama-lib/src/storage/settings.rs index 1e208be46b..44a6aecb0b 100644 --- a/rust/agama-lib/src/storage/settings.rs +++ b/rust/agama-lib/src/storage/settings.rs @@ -1,6 +1,6 @@ //! Representation of the storage settings -use crate::settings::{SettingObject, Settings}; +use crate::settings::{SettingObject, Settings, SettingsError}; use agama_derive::Settings; use serde::{Deserialize, Serialize}; @@ -26,14 +26,14 @@ pub struct Device { } impl TryFrom for Device { - type Error = &'static str; + type Error = SettingsError; fn try_from(value: SettingObject) -> Result { match value.get("name") { Some(name) => Ok(Device { name: name.clone().try_into()?, }), - None => Err("'name' key not found"), + _ => return Err(SettingsError::MissingKey("name".to_string())), } } } diff --git a/rust/agama-lib/src/users/client.rs b/rust/agama-lib/src/users/client.rs index 9985ffda1d..0c22a5e398 100644 --- a/rust/agama-lib/src/users/client.rs +++ b/rust/agama-lib/src/users/client.rs @@ -2,7 +2,7 @@ use super::proxies::Users1Proxy; use crate::error::ServiceError; -use crate::settings::{SettingValue, Settings}; +use crate::settings::{SettingValue, Settings, SettingsError}; use serde::Serialize; use zbus::Connection; @@ -43,13 +43,13 @@ impl FirstUser { } impl Settings for FirstUser { - fn set(&mut self, attr: &str, value: SettingValue) -> Result<(), &'static str> { + fn set(&mut self, attr: &str, value: SettingValue) -> Result<(), SettingsError> { match attr { "full_name" => self.full_name = value.try_into()?, "user_name" => self.user_name = value.try_into()?, "password" => self.password = value.try_into()?, "autologin" => self.autologin = value.try_into()?, - _ => return Err("unknown attribute"), + _ => return Err(SettingsError::UnknownAttribute(attr.to_string())), } Ok(()) } diff --git a/rust/agama-lib/src/users/settings.rs b/rust/agama-lib/src/users/settings.rs index f14b9bb0d9..b7eff8aa5a 100644 --- a/rust/agama-lib/src/users/settings.rs +++ b/rust/agama-lib/src/users/settings.rs @@ -1,4 +1,4 @@ -use crate::settings::{SettingValue, Settings}; +use crate::settings::{SettingValue, Settings, SettingsError}; use agama_derive::Settings; use serde::{Deserialize, Serialize}; @@ -14,7 +14,7 @@ pub struct UserSettings { } impl Settings for UserSettings { - fn set(&mut self, attr: &str, value: SettingValue) -> Result<(), &'static str> { + fn set(&mut self, attr: &str, value: SettingValue) -> Result<(), SettingsError> { if let Some((ns, id)) = attr.split_once('.') { match ns { "user" => { @@ -25,7 +25,7 @@ impl Settings for UserSettings { let root_user = self.root.get_or_insert(Default::default()); root_user.set(id, value)? } - _ => return Err("unknown attribute"), + _ => return Err(SettingsError::UnknownAttribute(attr.to_string())), } } Ok(()) From eb201798add44ecc8e998ab75844cf7b92f4fa99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 12 Jul 2023 11:07:39 +0100 Subject: [PATCH 4/6] Add anyhow to agama-cli --- rust/Cargo.lock | 1 + rust/agama-cli/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 0e85cf8a37..0b77eada04 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -13,6 +13,7 @@ name = "agama-cli" version = "1.0.0" dependencies = [ "agama-lib", + "anyhow", "async-std", "clap", "console", diff --git a/rust/agama-cli/Cargo.toml b/rust/agama-cli/Cargo.toml index 29164e354a..4b3078034b 100644 --- a/rust/agama-cli/Cargo.toml +++ b/rust/agama-cli/Cargo.toml @@ -16,6 +16,7 @@ async-std = { version ="1.12.0", features = ["attributes"] } thiserror = "1.0.39" convert_case = "0.6.0" console = "0.15.7" +anyhow = "1.0.71" [[bin]] name = "agama" From dbaa54eb4534c9e25a5b34ccb9a8da15e4f7bc17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 12 Jul 2023 11:08:53 +0100 Subject: [PATCH 5/6] Use anyhow in the CLI --- rust/agama-cli/src/config.rs | 8 ++++---- rust/agama-cli/src/main.rs | 17 ++++++++--------- rust/agama-cli/src/printers.rs | 12 ++++++------ rust/agama-cli/src/profile.rs | 12 ++++++------ 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/rust/agama-cli/src/config.rs b/rust/agama-cli/src/config.rs index dbeab414f0..ce517106f9 100644 --- a/rust/agama-cli/src/config.rs +++ b/rust/agama-cli/src/config.rs @@ -31,7 +31,7 @@ pub enum ConfigAction { Load(String), } -pub async fn run(subcommand: ConfigCommands, format: Format) -> Result<(), Box> { +pub async fn run(subcommand: ConfigCommands, format: Format) -> anyhow::Result<()> { let store = SettingsStore::new(connection().await?).await?; match parse_config_command(subcommand) { @@ -44,7 +44,7 @@ pub async fn run(subcommand: ConfigCommands, format: Format) -> Result<(), Box { let model = store.load(None).await?; @@ -55,7 +55,7 @@ pub async fn run(subcommand: ConfigCommands, format: Format) -> Result<(), Box { let contents = std::fs::read_to_string(path)?; @@ -63,7 +63,7 @@ pub async fn run(subcommand: ConfigCommands, format: Format) -> Result<(), Box Result<(), Box> { +async fn probe() -> anyhow::Result<()> { let another_manager = build_manager().await?; let probe = task::spawn(async move { another_manager.probe().await }); show_progress().await?; @@ -48,13 +47,13 @@ async fn probe() -> Result<(), Box> { /// Before starting, it makes sure that the manager is idle. /// /// * `manager`: the manager client. -async fn install(manager: &ManagerClient<'_>, max_attempts: u8) -> Result<(), Box> { +async fn install(manager: &ManagerClient<'_>, max_attempts: u8) -> anyhow::Result<()> { if manager.is_busy().await { println!("Agama's manager is busy. Waiting until it is ready..."); } if !manager.can_install().await? { - return Err(Box::new(CliError::ValidationError)); + return Err(CliError::ValidationError)?; } // Display the progress (if needed) and makes sure that the manager is ready @@ -75,7 +74,7 @@ async fn install(manager: &ManagerClient<'_>, max_attempts: u8) -> Result<(), Bo } if attempts == max_attempts { eprintln!("Giving up."); - return Err(Box::new(CliError::InstallationError)); + return Err(CliError::InstallationError)?; } attempts += 1; sleep(Duration::from_secs(1)); @@ -97,7 +96,7 @@ async fn show_progress() -> Result<(), ServiceError> { Ok(()) } -async fn wait_for_services(manager: &ManagerClient<'_>) -> Result<(), Box> { +async fn wait_for_services(manager: &ManagerClient<'_>) -> Result<(), ServiceError> { let services = manager.busy_services().await?; // TODO: having it optional if !services.is_empty() { @@ -107,12 +106,12 @@ async fn wait_for_services(manager: &ManagerClient<'_>) -> Result<(), Box() -> Result, Box> { +async fn build_manager<'a>() -> anyhow::Result> { let conn = agama_lib::connection().await?; Ok(ManagerClient::new(conn).await?) } -async fn run_command(cli: Cli) -> Result<(), Box> { +async fn run_command(cli: Cli) -> anyhow::Result<()> { match cli.command { Commands::Config(subcommand) => { let manager = build_manager().await?; @@ -152,7 +151,7 @@ async fn main() -> CliResult { let cli = Cli::parse(); if let Err(error) = run_command(cli).await { - eprintln!("{}", error); + eprintln!("{:?}", error); return CliResult::Error; } CliResult::Ok diff --git a/rust/agama-cli/src/printers.rs b/rust/agama-cli/src/printers.rs index 53a6b22d5d..5541439b31 100644 --- a/rust/agama-cli/src/printers.rs +++ b/rust/agama-cli/src/printers.rs @@ -1,5 +1,5 @@ +use anyhow; use serde::Serialize; -use std::error; use std::fmt::Debug; use std::io::Write; @@ -16,7 +16,7 @@ use std::io::Write; /// print(user, io::stdout(), Some(Format::Json)) /// .expect("Something went wrong!") /// ``` -pub fn print(content: T, writer: W, format: Format) -> Result<(), Box> +pub fn print(content: T, writer: W, format: Format) -> anyhow::Result<()> where T: serde::Serialize + Debug, W: Write, @@ -38,7 +38,7 @@ pub enum Format { } pub trait Printer { - fn print(self: Box) -> Result<(), Box>; + fn print(self: Box) -> anyhow::Result<()>; } pub struct JsonPrinter { @@ -47,7 +47,7 @@ pub struct JsonPrinter { } impl Printer for JsonPrinter { - fn print(self: Box) -> Result<(), Box> { + fn print(self: Box) -> anyhow::Result<()> { Ok(serde_json::to_writer(self.writer, &self.content)?) } } @@ -57,7 +57,7 @@ pub struct TextPrinter { } impl Printer for TextPrinter { - fn print(mut self: Box) -> Result<(), Box> { + fn print(mut self: Box) -> anyhow::Result<()> { Ok(write!(self.writer, "{:?}", &self.content)?) } } @@ -68,7 +68,7 @@ pub struct YamlPrinter { } impl Printer for YamlPrinter { - fn print(self: Box) -> Result<(), Box> { + fn print(self: Box) -> anyhow::Result<()> { Ok(serde_yaml::to_writer(self.writer, &self.content)?) } } diff --git a/rust/agama-cli/src/profile.rs b/rust/agama-cli/src/profile.rs index 35eddf2b24..2ba93c12d6 100644 --- a/rust/agama-cli/src/profile.rs +++ b/rust/agama-cli/src/profile.rs @@ -1,5 +1,5 @@ -use agama_lib::error::ProfileError; use agama_lib::profile::{download, ProfileEvaluator, ProfileValidator, ValidationResult}; +use anyhow::{anyhow, Context}; use clap::Subcommand; use std::path::Path; @@ -15,7 +15,7 @@ pub enum ProfileCommands { Evaluate { path: String }, } -fn validate(path: String) -> Result<(), ProfileError> { +fn validate(path: String) -> anyhow::Result<()> { let validator = ProfileValidator::default_schema()?; let path = Path::new(&path); let result = validator.validate_file(path)?; @@ -33,14 +33,14 @@ fn validate(path: String) -> Result<(), ProfileError> { Ok(()) } -fn evaluate(path: String) -> Result<(), ProfileError> { +fn evaluate(path: String) -> anyhow::Result<()> { let evaluator = ProfileEvaluator {}; - evaluator.evaluate(Path::new(&path)) + Ok(evaluator.evaluate(Path::new(&path))?) } -pub fn run(subcommand: ProfileCommands) -> Result<(), ProfileError> { +pub fn run(subcommand: ProfileCommands) -> anyhow::Result<()> { match subcommand { - ProfileCommands::Download { url } => download(&url), + ProfileCommands::Download { url } => Ok(download(&url)?), ProfileCommands::Validate { path } => validate(path), ProfileCommands::Evaluate { path } => evaluate(path), } From e0d3ba6cfa1c66b1286eba481447c384107def0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 12 Jul 2023 15:31:18 +0100 Subject: [PATCH 6/6] Improve reporting of profile handling errors --- rust/agama-cli/src/profile.rs | 13 +++++++++---- rust/agama-lib/src/error.rs | 10 ++++------ rust/agama-lib/src/profile.rs | 17 ++++++++++++----- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/rust/agama-cli/src/profile.rs b/rust/agama-cli/src/profile.rs index 2ba93c12d6..818aaf32db 100644 --- a/rust/agama-cli/src/profile.rs +++ b/rust/agama-cli/src/profile.rs @@ -1,5 +1,5 @@ use agama_lib::profile::{download, ProfileEvaluator, ProfileValidator, ValidationResult}; -use anyhow::{anyhow, Context}; +use anyhow::Context; use clap::Subcommand; use std::path::Path; @@ -18,13 +18,15 @@ pub enum ProfileCommands { fn validate(path: String) -> anyhow::Result<()> { let validator = ProfileValidator::default_schema()?; let path = Path::new(&path); - let result = validator.validate_file(path)?; + let result = validator + .validate_file(path) + .context("Could not validate the profile")?; match result { ValidationResult::Valid => { println!("The profile is valid") } ValidationResult::NotValid(errors) => { - println!("The profile is not valid. Please, check the following errors:\n"); + eprintln!("The profile is not valid. Please, check the following errors:\n"); for error in errors { println!("* {error}") } @@ -35,7 +37,10 @@ fn validate(path: String) -> anyhow::Result<()> { fn evaluate(path: String) -> anyhow::Result<()> { let evaluator = ProfileEvaluator {}; - Ok(evaluator.evaluate(Path::new(&path))?) + evaluator + .evaluate(Path::new(&path)) + .context(format!("Could not evaluate the profile"))?; + Ok(()) } pub fn run(subcommand: ProfileCommands) -> anyhow::Result<()> { diff --git a/rust/agama-lib/src/error.rs b/rust/agama-lib/src/error.rs index 9cf8e9fc60..6eb77748c8 100644 --- a/rust/agama-lib/src/error.rs +++ b/rust/agama-lib/src/error.rs @@ -20,13 +20,11 @@ pub enum ServiceError { #[derive(Error, Debug)] pub enum ProfileError { - #[error("Cannot read the profile '{0}'")] + #[error("Could not read the profile")] Unreachable(#[from] curl::Error), - #[error("No hardware information available: '{0}'")] - NoHardwareInfo(io::Error), - #[error("Could not evaluate the profile: '{0}'")] - EvaluationError(io::Error), - #[error("Input/output error: '{0}'")] + #[error("Jsonnet evaluation failed:\n{0}")] + EvaluationError(String), + #[error("I/O error: '{0}'")] InputOutputError(#[from] io::Error), #[error("The profile is not a valid JSON file")] FormatError(#[from] serde_json::Error), diff --git a/rust/agama-lib/src/profile.rs b/rust/agama-lib/src/profile.rs index 0f17953f88..6405482654 100644 --- a/rust/agama-lib/src/profile.rs +++ b/rust/agama-lib/src/profile.rs @@ -1,4 +1,5 @@ use crate::error::ProfileError; +use anyhow::Context; use curl::easy::Easy; use jsonschema::JSONSchema; use log::info; @@ -101,7 +102,7 @@ impl ProfileValidator { pub struct ProfileEvaluator {} impl ProfileEvaluator { - pub fn evaluate(&self, profile_path: &Path) -> Result<(), ProfileError> { + pub fn evaluate(&self, profile_path: &Path) -> anyhow::Result<()> { let dir = tempdir()?; let working_path = dir.path().join("profile.jsonnet"); @@ -109,13 +110,18 @@ impl ProfileEvaluator { let hwinfo_path = dir.path().join("hw.libsonnet"); self.write_hwinfo(&hwinfo_path) - .map_err(ProfileError::NoHardwareInfo)?; + .context("Failed to read system's hardware information")?; let result = Command::new("/usr/bin/jsonnet") .arg("profile.jsonnet") .current_dir(&dir) .output() - .map_err(ProfileError::EvaluationError)?; + .context("Failed to run jsonnet")?; + if !result.status.success() { + let message = + String::from_utf8(result.stderr).context("Invalid UTF-8 sequence from jsonnet")?; + return Err(ProfileError::EvaluationError(message).into()); + } io::stdout().write_all(&result.stdout)?; Ok(()) } @@ -124,10 +130,11 @@ impl ProfileEvaluator { // // TODO: we need a better way to generate this information, as lshw and hwinfo are not usable // out of the box. - fn write_hwinfo(&self, path: &Path) -> Result<(), io::Error> { + fn write_hwinfo(&self, path: &Path) -> anyhow::Result<()> { let result = Command::new("/usr/sbin/lshw") .args(["-json", "-class", "disk"]) - .output()?; + .output() + .context("Failed to run lshw")?; let mut file = fs::File::create(path)?; file.write(b"{ \"disks\":\n")?; file.write_all(&result.stdout)?;