From 12ab3397f19b30a976d130ed88aa44210ffa41bc Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Wed, 17 Jan 2024 15:58:00 +0100 Subject: [PATCH 1/5] fix collision between client lib & client cli bin doc in built doc --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2a9bae291d5..46a46211538 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -648,7 +648,9 @@ jobs: - name: Generate cargo doc run: | - cargo doc --no-deps -p mithril-stm -p mithril-common -p mithril-aggregator \ + # Force `--lib` to avoid a collision between the client lib and the client cli binary who share + # the same name (we only want to document those anyway) + cargo doc --no-deps --lib -p mithril-stm -p mithril-common -p mithril-aggregator \ -p mithril-signer -p mithril-client -p mithril-client-cli \ --all-features --message-format=json \ | clippy-sarif | tee rust-cargo-doc-results.sarif | sarif-fmt From d18ca49eb887320e8cfd8ed13856671f78fe200c Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Wed, 17 Jan 2024 16:32:37 +0100 Subject: [PATCH 2/5] Reinstate `warn(missing_docs)` on client cli and fix most warnings --- mithril-client-cli/src/lib.rs | 9 +++++++++ mithril-client-cli/src/utils/expander.rs | 1 + mithril-client-cli/src/utils/mod.rs | 2 +- mithril-client-cli/src/utils/progress_reporter.rs | 5 ++++- 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/mithril-client-cli/src/lib.rs b/mithril-client-cli/src/lib.rs index e3830564c28..0d6052486da 100644 --- a/mithril-client-cli/src/lib.rs +++ b/mithril-client-cli/src/lib.rs @@ -1,2 +1,11 @@ +#![warn(missing_docs)] + +//! A command line interface that uses the [Mithril Client Library](https://mithril.network/doc/manual/developer-docs/nodes/mithril-client-library) +//! to manipulate Mithril certified types from a Mithril Aggregator: +//! * Snapshots: List, Show, download and verify +//! * Mithril Stake Distribution: List, download and verify +// +//! You can find more information on how it works reading the [documentation website](https://mithril.network/doc/mithril/mithril-network/client). + pub mod configuration; pub mod utils; diff --git a/mithril-client-cli/src/utils/expander.rs b/mithril-client-cli/src/utils/expander.rs index 422b3174df7..16893be9d80 100644 --- a/mithril-client-cli/src/utils/expander.rs +++ b/mithril-client-cli/src/utils/expander.rs @@ -2,6 +2,7 @@ use anyhow::anyhow; use futures::Future; use mithril_common::StdResult; +/// Utilities to expand aliases into their associated ids. pub struct ExpanderUtils; impl ExpanderUtils { diff --git a/mithril-client-cli/src/utils/mod.rs b/mithril-client-cli/src/utils/mod.rs index 8193bec5dff..969fa60d471 100644 --- a/mithril-client-cli/src/utils/mod.rs +++ b/mithril-client-cli/src/utils/mod.rs @@ -1,5 +1,5 @@ //! Utilities module -//! This module contains tools needed mostly in services layers. +//! This module contains tools needed for the commands layer. mod expander; mod feedback_receiver; diff --git a/mithril-client-cli/src/utils/progress_reporter.rs b/mithril-client-cli/src/utils/progress_reporter.rs index 56d4efbfdac..d7dca0bc0b2 100644 --- a/mithril-client-cli/src/utils/progress_reporter.rs +++ b/mithril-client-cli/src/utils/progress_reporter.rs @@ -72,9 +72,11 @@ impl Deref for ProgressPrinter { } } +/// Utility to format a [ProgressBar] status as json pub struct ProgressBarJsonFormatter; impl ProgressBarJsonFormatter { + /// Get a json formatted string given the progress bar status pub fn format(progress_bar: &ProgressBar) -> String { format!( r#"{{"timestamp": "{}", "bytes_downloaded": {}, "bytes_total": {}, "seconds_left": {}.{:0>3}, "seconds_elapsed": {}.{:0>3}}}"#, @@ -97,7 +99,7 @@ pub struct DownloadProgressReporter { } impl DownloadProgressReporter { - /// Instanciate a new progress reporter + /// Instantiate a new progress reporter pub fn new(progress_bar: ProgressBar, output_type: ProgressOutputType) -> Self { Self { progress_bar, @@ -129,6 +131,7 @@ impl DownloadProgressReporter { }; } + /// Report that the current download is finished and print the given message. pub fn finish(&self, message: &str) { self.progress_bar.finish_with_message(message.to_string()); } From bdcc4a8bac79d9c667469161315d42ccbbf34a90 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Wed, 17 Jan 2024 16:38:19 +0100 Subject: [PATCH 3/5] Move client cli commands module declaration to its lib.rs Reverting a change that was made when we worked on the first client library version to remove the lib api the code only needed by the binary. --- .../src/commands/mithril_stake_distribution/download.rs | 2 +- .../src/commands/mithril_stake_distribution/list.rs | 2 +- mithril-client-cli/src/commands/snapshot/download.rs | 8 ++++---- mithril-client-cli/src/commands/snapshot/list.rs | 2 +- mithril-client-cli/src/commands/snapshot/show.rs | 2 +- mithril-client-cli/src/lib.rs | 5 +++-- mithril-client-cli/src/main.rs | 4 +--- 7 files changed, 12 insertions(+), 13 deletions(-) diff --git a/mithril-client-cli/src/commands/mithril_stake_distribution/download.rs b/mithril-client-cli/src/commands/mithril_stake_distribution/download.rs index 594b76222e0..23fabbd192c 100644 --- a/mithril-client-cli/src/commands/mithril_stake_distribution/download.rs +++ b/mithril-client-cli/src/commands/mithril_stake_distribution/download.rs @@ -8,8 +8,8 @@ use std::{ sync::Arc, }; +use crate::{configuration::ConfigParameters, utils::ExpanderUtils}; use mithril_client::{ClientBuilder, MessageBuilder}; -use mithril_client_cli::{configuration::ConfigParameters, utils::ExpanderUtils}; use mithril_common::StdResult; /// Download and verify a Mithril Stake Distribution information. If the diff --git a/mithril-client-cli/src/commands/mithril_stake_distribution/list.rs b/mithril-client-cli/src/commands/mithril_stake_distribution/list.rs index 3fb8d26bb58..3031800109a 100644 --- a/mithril-client-cli/src/commands/mithril_stake_distribution/list.rs +++ b/mithril-client-cli/src/commands/mithril_stake_distribution/list.rs @@ -4,8 +4,8 @@ use config::{builder::DefaultState, ConfigBuilder}; use slog_scope::logger; use std::{collections::HashMap, sync::Arc}; +use crate::configuration::ConfigParameters; use mithril_client::ClientBuilder; -use mithril_client_cli::configuration::ConfigParameters; use mithril_common::{test_utils::fake_keys, StdResult}; /// Mithril stake distribution LIST command diff --git a/mithril-client-cli/src/commands/snapshot/download.rs b/mithril-client-cli/src/commands/snapshot/download.rs index f699f870e34..64f8b8fb693 100644 --- a/mithril-client-cli/src/commands/snapshot/download.rs +++ b/mithril-client-cli/src/commands/snapshot/download.rs @@ -10,16 +10,16 @@ use std::{ sync::Arc, }; -use mithril_client::{ - common::ProtocolMessage, Client, ClientBuilder, MessageBuilder, MithrilCertificate, Snapshot, -}; -use mithril_client_cli::{ +use crate::{ configuration::ConfigParameters, utils::{ ExpanderUtils, IndicatifFeedbackReceiver, ProgressOutputType, ProgressPrinter, SnapshotUnpacker, SnapshotUtils, }, }; +use mithril_client::{ + common::ProtocolMessage, Client, ClientBuilder, MessageBuilder, MithrilCertificate, Snapshot, +}; use mithril_common::StdResult; /// Clap command to download the snapshot and verify the certificate. diff --git a/mithril-client-cli/src/commands/snapshot/list.rs b/mithril-client-cli/src/commands/snapshot/list.rs index 2b20ac41520..f1697dfdf76 100644 --- a/mithril-client-cli/src/commands/snapshot/list.rs +++ b/mithril-client-cli/src/commands/snapshot/list.rs @@ -4,8 +4,8 @@ use config::{builder::DefaultState, ConfigBuilder}; use slog_scope::logger; use std::{collections::HashMap, sync::Arc}; +use crate::configuration::ConfigParameters; use mithril_client::ClientBuilder; -use mithril_client_cli::configuration::ConfigParameters; use mithril_common::{test_utils::fake_keys, StdResult}; /// Clap command to list existing snapshots diff --git a/mithril-client-cli/src/commands/snapshot/show.rs b/mithril-client-cli/src/commands/snapshot/show.rs index fc92513f5e5..65e11d611d6 100644 --- a/mithril-client-cli/src/commands/snapshot/show.rs +++ b/mithril-client-cli/src/commands/snapshot/show.rs @@ -5,8 +5,8 @@ use config::{builder::DefaultState, ConfigBuilder}; use slog_scope::logger; use std::{collections::HashMap, sync::Arc}; +use crate::{configuration::ConfigParameters, utils::ExpanderUtils}; use mithril_client::ClientBuilder; -use mithril_client_cli::{configuration::ConfigParameters, utils::ExpanderUtils}; use mithril_common::{test_utils::fake_keys, StdResult}; /// Clap command to show a given snapshot diff --git a/mithril-client-cli/src/lib.rs b/mithril-client-cli/src/lib.rs index 0d6052486da..877997e4ea0 100644 --- a/mithril-client-cli/src/lib.rs +++ b/mithril-client-cli/src/lib.rs @@ -7,5 +7,6 @@ // //! You can find more information on how it works reading the [documentation website](https://mithril.network/doc/mithril/mithril-network/client). -pub mod configuration; -pub mod utils; +pub mod commands; +mod configuration; +mod utils; diff --git a/mithril-client-cli/src/main.rs b/mithril-client-cli/src/main.rs index 1efd304d5a5..1e2946f4983 100644 --- a/mithril-client-cli/src/main.rs +++ b/mithril-client-cli/src/main.rs @@ -1,7 +1,5 @@ #![doc = include_str!("../README.md")] -mod commands; - use anyhow::Context; use clap::{Parser, Subcommand}; use config::{builder::DefaultState, ConfigBuilder, Map, Source, Value, ValueKind}; @@ -15,7 +13,7 @@ use std::{fs::File, path::PathBuf}; use mithril_common::StdResult; -use commands::{ +use mithril_client_cli::commands::{ mithril_stake_distribution::MithrilStakeDistributionCommands, snapshot::SnapshotCommands, }; From 3d29dd9798871474b1723408de18bd9f68ee3ebe Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Wed, 17 Jan 2024 16:54:32 +0100 Subject: [PATCH 4/5] Fix clippy warnings (mostly dead code) --- mithril-client-cli/src/commands/snapshot/download.rs | 2 +- mithril-client-cli/src/configuration.rs | 2 ++ mithril-client-cli/src/utils/feedback_receiver.rs | 4 ++-- mithril-client-cli/src/utils/progress_reporter.rs | 6 +++--- mithril-client-cli/src/utils/snapshot.rs | 10 ++-------- mithril-client-cli/src/utils/unpacker.rs | 11 ----------- 6 files changed, 10 insertions(+), 25 deletions(-) diff --git a/mithril-client-cli/src/commands/snapshot/download.rs b/mithril-client-cli/src/commands/snapshot/download.rs index 64f8b8fb693..9b9c57aeba6 100644 --- a/mithril-client-cli/src/commands/snapshot/download.rs +++ b/mithril-client-cli/src/commands/snapshot/download.rs @@ -59,7 +59,7 @@ impl SnapshotDownloadCommand { let progress_output_type = if self.json { ProgressOutputType::JsonReporter } else { - ProgressOutputType::TTY + ProgressOutputType::Tty }; let progress_printer = ProgressPrinter::new(progress_output_type, 5); let client = ClientBuilder::aggregator( diff --git a/mithril-client-cli/src/configuration.rs b/mithril-client-cli/src/configuration.rs index 18f042dbeaa..ac5127f6a80 100644 --- a/mithril-client-cli/src/configuration.rs +++ b/mithril-client-cli/src/configuration.rs @@ -24,6 +24,7 @@ impl ConfigParameters { } /// Useful constructor for testing + #[cfg(test)] pub fn build(parameters: &[(&str, &str)]) -> Self { let parameters = parameters .iter() @@ -34,6 +35,7 @@ impl ConfigParameters { } /// Add or replace a parameter in the holder + #[cfg(test)] pub fn add_parameter(&mut self, name: &str, value: &str) -> &mut Self { let _ = self.parameters.insert(name.to_string(), value.to_string()); diff --git a/mithril-client-cli/src/utils/feedback_receiver.rs b/mithril-client-cli/src/utils/feedback_receiver.rs index fdf0f4af0eb..992220deede 100644 --- a/mithril-client-cli/src/utils/feedback_receiver.rs +++ b/mithril-client-cli/src/utils/feedback_receiver.rs @@ -35,7 +35,7 @@ impl FeedbackReceiver for IndicatifFeedbackReceiver { download_id: _, size, } => { - let pb = if self.output_type == ProgressOutputType::TTY { + let pb = if self.output_type == ProgressOutputType::Tty { ProgressBar::new(size) } else { ProgressBar::with_draw_target(Some(size), ProgressDrawTarget::hidden()) @@ -68,7 +68,7 @@ impl FeedbackReceiver for IndicatifFeedbackReceiver { MithrilEvent::CertificateChainValidationStarted { certificate_chain_validation_id: _, } => { - let pb = if self.output_type == ProgressOutputType::TTY { + let pb = if self.output_type == ProgressOutputType::Tty { ProgressBar::new_spinner() } else { ProgressBar::hidden() diff --git a/mithril-client-cli/src/utils/progress_reporter.rs b/mithril-client-cli/src/utils/progress_reporter.rs index d7dca0bc0b2..d85b835b594 100644 --- a/mithril-client-cli/src/utils/progress_reporter.rs +++ b/mithril-client-cli/src/utils/progress_reporter.rs @@ -14,7 +14,7 @@ pub enum ProgressOutputType { /// Output to json JsonReporter, /// Output to tty - TTY, + Tty, /// No output Hidden, } @@ -23,7 +23,7 @@ impl From for ProgressDrawTarget { fn from(value: ProgressOutputType) -> Self { match value { ProgressOutputType::JsonReporter => ProgressDrawTarget::hidden(), - ProgressOutputType::TTY => ProgressDrawTarget::stdout(), + ProgressOutputType::Tty => ProgressDrawTarget::stdout(), ProgressOutputType::Hidden => ProgressDrawTarget::hidden(), } } @@ -54,7 +54,7 @@ impl ProgressPrinter { timestamp = Utc::now().to_rfc3339(), number_of_steps = self.number_of_steps, ), - ProgressOutputType::TTY => self + ProgressOutputType::Tty => self .multi_progress .println(format!("{step_number}/{} - {text}", self.number_of_steps))?, ProgressOutputType::Hidden => (), diff --git a/mithril-client-cli/src/utils/snapshot.rs b/mithril-client-cli/src/utils/snapshot.rs index 4e2224292e2..674951e7dd6 100644 --- a/mithril-client-cli/src/utils/snapshot.rs +++ b/mithril-client-cli/src/utils/snapshot.rs @@ -67,10 +67,7 @@ mod test { #[test] fn check_disk_space_error_should_return_error_if_error_is_not_error_not_enough_space() { - let error = SnapshotUnpackerError::UnpackFailed { - dirpath: PathBuf::from(""), - error: anyhow::Error::msg("Some error message"), - }; + let error = SnapshotUnpackerError::UnpackDirectoryAlreadyExists(PathBuf::new()); let error = SnapshotUtils::check_disk_space_error(anyhow!(error)) .expect_err("check_disk_space_error should fail"); @@ -78,10 +75,7 @@ mod test { assert!( matches!( error.downcast_ref::(), - Some(SnapshotUnpackerError::UnpackFailed { - dirpath: _, - error: _ - }) + Some(SnapshotUnpackerError::UnpackDirectoryAlreadyExists(_)) ), "Unexpected error: {:?}", error diff --git a/mithril-client-cli/src/utils/unpacker.rs b/mithril-client-cli/src/utils/unpacker.rs index 4d0fe391b92..1cf7bc317ac 100644 --- a/mithril-client-cli/src/utils/unpacker.rs +++ b/mithril-client-cli/src/utils/unpacker.rs @@ -38,17 +38,6 @@ pub enum SnapshotUnpackerError { /// Cannot write in the given directory. #[error("Unpack directory '{0}' is not writable.")] UnpackDirectoryIsNotWritable(PathBuf, #[source] StdError), - - /// Unpacking error - #[error("Could not unpack from streamed data snapshot to directory '{dirpath}'")] - UnpackFailed { - /// Location where the archive is to be extracted. - dirpath: PathBuf, - - /// Subsystem error - #[source] - error: StdError, - }, } impl SnapshotUnpacker { From 28eaed8f576b24f9bed8d5dc2dfd9778da4d826e Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Thu, 18 Jan 2024 09:38:14 +0100 Subject: [PATCH 5/5] Update mithril-client-cli crate version --- Cargo.lock | 2 +- mithril-client-cli/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fbeb2e73569..74af0e8799d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3270,7 +3270,7 @@ dependencies = [ [[package]] name = "mithril-client-cli" -version = "0.5.15" +version = "0.5.16" dependencies = [ "anyhow", "async-trait", diff --git a/mithril-client-cli/Cargo.toml b/mithril-client-cli/Cargo.toml index d7fbd95bcfc..225cf271d2b 100644 --- a/mithril-client-cli/Cargo.toml +++ b/mithril-client-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-client-cli" -version = "0.5.15" +version = "0.5.16" description = "A Mithril Client" authors = { workspace = true } edition = { workspace = true }