Quality of Life improvements (#211)

* chore: Refactor CLI code.

This is a general refactoring without logic changes; just restructures
command files into a module hierarchy to better accomodate addition of
future commands and more clearly distinguish common utilities from
commands. It also updates imports to be more consistent and compact.

Signed-off-by: Andrew Lilley Brinker <[email protected]>

* feat: improve logging and provide verbosity control

Add the ability to control logging verbosity in the CLI, and
simplify the format of log messages to be more compact.

Signed-off-by: Andrew Lilley Brinker <[email protected]>

* chore: Remove explicit panics.

This replaces explicit panics with error logging.

Signed-off-by: Andrew Lilley Brinker <[email protected]>

* fix: Update test snapshots for verbosity flags.

This fixes the snapshot tests to reflect the addition of verbosity
flags.

Signed-off-by: Andrew Lilley Brinker <[email protected]>

* chore: Refactored output mechanics.

This introduces a new `CommandOutput` trait, which ensures that any distinct
output message supports all formats.

Signed-off-by: Andrew Lilley Brinker <[email protected]>

---------

Signed-off-by: Andrew Lilley Brinker <[email protected]>

omnibor-cli/Cargo.toml

@@ -28,14 +28,16 @@ path = "src/main.rs"
 
 [dependencies]
 
-anyhow = "1.0.80"
 async-walkdir = "1.0.0"
 clap = { version = "4.5.1", features = ["derive", "env"] }
+clap-verbosity-flag = "2.2.2"
 dirs = "5.0.1"
+dyn-clone = "1.0.17"
 futures-lite = "2.2.0"
 omnibor = { version = "0.6.0", path = "../omnibor" }
 pathbuf = "1.0.0"
 serde_json = "1.0.114"
+thiserror = "2.0.3"
 tokio = { version = "1.36.0", features = [
     "fs",
     "io-std",

omnibor-cli/src/cli.rs

@@ -1,19 +1,16 @@
 //! Defines the Command Line Interface.
 
-#![allow(unused)]
-
-use anyhow::anyhow;
-use omnibor::hashes::Sha256;
-use omnibor::ArtifactId;
-use omnibor::IntoArtifactId;
+use crate::error::Error;
+use clap_verbosity_flag::{InfoLevel, Verbosity};
+use omnibor::{hashes::Sha256, ArtifactId, IntoArtifactId};
 use pathbuf::pathbuf;
-use std::default::Default;
-use std::fmt::Display;
-use std::fmt::Formatter;
-use std::path::Path;
-use std::path::PathBuf;
-use std::str::FromStr;
-use std::sync::OnceLock;
+use std::{
+    default::Default,
+    fmt::{Display, Formatter},
+    path::{Path, PathBuf},
+    str::FromStr,
+    sync::OnceLock,
+};
 
 // The default root directory for OmniBOR.
 // We use a `static` here to make sure we can safely give out
@@ -71,6 +68,9 @@ pub struct Config {
     )]
     dir: Option<PathBuf>,
 
+    #[command(flatten)]
+    pub verbose: Verbosity<InfoLevel>,
+
     #[command(subcommand)]
     command: Option<Command>,
 }
@@ -234,13 +234,13 @@ pub enum IdentifiableArg {
 }
 
 impl FromStr for IdentifiableArg {
-    type Err = anyhow::Error;
+    type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match (ArtifactId::from_str(s), PathBuf::from_str(s)) {
             (Ok(aid), _) => Ok(IdentifiableArg::ArtifactId(aid)),
             (_, Ok(path)) => Ok(IdentifiableArg::Path(path)),
-            (Err(_), Err(_)) => Err(anyhow!("input not recognized as Artifact ID or file path")),
+            (Err(_), Err(_)) => Err(Error::NotIdentifiable(s.to_string())),
         }
     }
 }

omnibor-cli/src/artifact_find.rs → omnibor-cli/src/cmd/artifact/find.rs

@@ -1,11 +1,11 @@
 //! The `artifact find` command, which finds files by ID.
 
-use crate::cli::Config;
-use crate::cli::FindArgs;
-use crate::cli::SelectedHash;
-use crate::fs::*;
-use crate::print::PrinterCmd;
-use anyhow::Result;
+use crate::{
+    cli::{Config, FindArgs, SelectedHash},
+    error::{Error, Result},
+    fs::*,
+    print::{error::ErrorMsg, find_file::FindFileMsg, PrinterCmd},
+};
 use async_walkdir::WalkDir;
 use futures_lite::stream::StreamExt as _;
 use tokio::sync::mpsc::Sender;
@@ -21,7 +21,16 @@ pub async fn run(tx: &Sender<PrinterCmd>, config: &Config, args: &FindArgs) -> R
     loop {
         match entries.next().await {
             None => break,
-            Some(Err(e)) => tx.send(PrinterCmd::error(e, config.format())).await?,
+            Some(Err(source)) => tx
+                .send(PrinterCmd::msg(
+                    ErrorMsg::new(Error::WalkDirFailed {
+                        path: path.to_path_buf(),
+                        source,
+                    }),
+                    config.format(),
+                ))
+                .await
+                .map_err(|_| Error::PrintChannelClose)?,
             Some(Ok(entry)) => {
                 let path = &entry.path();
 
@@ -33,8 +42,15 @@ pub async fn run(tx: &Sender<PrinterCmd>, config: &Config, args: &FindArgs) -> R
                 let file_url = hash_file(SelectedHash::Sha256, &mut file, path).await?;
 
                 if url == file_url {
-                    tx.send(PrinterCmd::find(path, &url, config.format()))
-                        .await?;
+                    tx.send(PrinterCmd::msg(
+                        FindFileMsg {
+                            path: path.to_path_buf(),
+                            id: url.clone(),
+                        },
+                        config.format(),
+                    ))
+                    .await
+                    .map_err(|_| Error::PrintChannelClose)?;
                 }
             }
         }

omnibor-cli/src/artifact_id.rs → omnibor-cli/src/cmd/artifact/id.rs

@@ -1,17 +1,18 @@
 //! The `artifact id` command, which identifies files.
 
-use crate::cli::Config;
-use crate::cli::IdArgs;
-use crate::fs::*;
-use crate::print::PrinterCmd;
-use anyhow::Result;
+use crate::{
+    cli::{Config, IdArgs},
+    error::Result,
+    fs::*,
+    print::PrinterCmd,
+};
 use tokio::sync::mpsc::Sender;
 
 /// Run the `artifact id` subcommand.
 pub async fn run(tx: &Sender<PrinterCmd>, config: &Config, args: &IdArgs) -> Result<()> {
     let mut file = open_async_file(&args.path).await?;
 
-    if file_is_dir(&file).await? {
+    if file_is_dir(&file, &args.path).await? {
         id_directory(tx, &args.path, config.format(), config.hash()).await?;
     } else {
         id_file(tx, &mut file, &args.path, config.format(), config.hash()).await?;

omnibor-cli/src/cmd/artifact/mod.rs

@@ -0,0 +1,2 @@
+pub mod find;
+pub mod id;

omnibor-cli/src/cmd/debug/config.rs

@@ -0,0 +1,19 @@
+//! The `debug config` command, which helps debug the CLI configuration.
+
+use crate::{
+    cli::Config,
+    error::{Error, Result},
+    print::{root_dir::RootDirMsg, PrinterCmd},
+};
+use tokio::sync::mpsc::Sender;
+
+/// Run the `debug config` subcommand.
+pub async fn run(tx: &Sender<PrinterCmd>, config: &Config) -> Result<()> {
+    let root = config.dir().ok_or(Error::NoRoot)?.to_path_buf();
+
+    tx.send(PrinterCmd::msg(RootDirMsg { path: root }, config.format()))
+        .await
+        .map_err(|_| Error::PrintChannelClose)?;
+
+    Ok(())
+}

omnibor-cli/src/cmd/debug/mod.rs

@@ -0,0 +1 @@
+pub mod config;

omnibor-cli/src/manifest_add.rs → omnibor-cli/src/cmd/manifest/add.rs

@@ -1,9 +1,10 @@
 //! The `manifest add` command, which adds manifests.
 
-use crate::cli::Config;
-use crate::cli::ManifestAddArgs;
-use crate::print::PrinterCmd;
-use anyhow::Result;
+use crate::{
+    cli::{Config, ManifestAddArgs},
+    error::Result,
+    print::PrinterCmd,
+};
 use tokio::sync::mpsc::Sender;
 
 /// Run the `manifest add` subcommand.

omnibor-cli/src/cmd/manifest/create.rs

@@ -0,0 +1,51 @@
+//! The `manifest create` command, which creates manifests.
+
+use crate::{
+    cli::{Config, ManifestCreateArgs},
+    error::{Error, Result},
+    print::PrinterCmd,
+};
+use omnibor::{
+    embedding::NoEmbed, hashes::Sha256, storage::FileSystemStorage, InputManifestBuilder,
+    IntoArtifactId, RelationKind,
+};
+use tokio::sync::mpsc::Sender;
+use tracing::info;
+
+/// Run the `manifest create` subcommand.
+pub async fn run(
+    _tx: &Sender<PrinterCmd>,
+    config: &Config,
+    args: &ManifestCreateArgs,
+) -> Result<()> {
+    let root = config.dir().ok_or_else(|| Error::NoRoot)?;
+
+    info!(root = %root.display());
+
+    let storage = FileSystemStorage::new(root).map_err(Error::StorageInitFailed)?;
+
+    let mut builder = InputManifestBuilder::<Sha256, NoEmbed, _>::with_storage(storage);
+
+    for input in &args.inputs {
+        let aid = input.clone().into_artifact_id().map_err(Error::IdFailed)?;
+        builder
+            .add_relation(RelationKind::Input, aid)
+            .map_err(Error::AddRelationFailed)?;
+    }
+
+    if let Some(built_by) = &args.built_by {
+        let aid = built_by
+            .clone()
+            .into_artifact_id()
+            .map_err(Error::IdFailed)?;
+        builder
+            .add_relation(RelationKind::BuiltBy, aid)
+            .map_err(Error::AddRelationFailed)?;
+    }
+
+    builder
+        .finish(&args.target)
+        .map_err(Error::ManifestBuildFailed)?;
+
+    Ok(())
+}

omnibor-cli/src/cmd/manifest/mod.rs

@@ -0,0 +1,3 @@
+pub mod add;
+pub mod create;
+pub mod remove;

omnibor-cli/src/manifest_remove.rs → omnibor-cli/src/cmd/manifest/remove.rs

@@ -1,9 +1,10 @@
 //! The `manifest remove` command, which removes manifests.
 
-use crate::cli::Config;
-use crate::cli::ManifestRemoveArgs;
-use crate::print::PrinterCmd;
-use anyhow::Result;
+use crate::{
+    cli::{Config, ManifestRemoveArgs},
+    error::Result,
+    print::PrinterCmd,
+};
 use tokio::sync::mpsc::Sender;
 
 /// Run the `manifest remove` subcommand.

omnibor-cli/src/cmd/mod.rs

@@ -0,0 +1,5 @@
+//! Defines individual subcommands.
+
+pub mod artifact;
+pub mod debug;
+pub mod manifest;

omnibor-cli/src/error.rs

@@ -0,0 +1,67 @@
+//! Error types.
+
+use omnibor::Error as OmniborError;
+use std::{io::Error as IoError, path::PathBuf, result::Result as StdResult};
+
+#[derive(Debug, thiserror::Error)]
+pub enum Error {
+    #[error("could not identify '{0}'")]
+    NotIdentifiable(String),
+
+    #[error("could not find root directory")]
+    NoRoot,
+
+    #[error("failed to initialize file system storage")]
+    StorageInitFailed(#[source] OmniborError),
+
+    #[error("failed to generate Artifact ID")]
+    IdFailed(#[source] OmniborError),
+
+    #[error("failed to add relation to Input Manifest")]
+    AddRelationFailed(#[source] OmniborError),
+
+    #[error("failed to build Input Manifest")]
+    ManifestBuildFailed(#[source] OmniborError),
+
+    #[error("failed to write to stdout")]
+    StdoutWriteFailed(#[source] IoError),
+
+    #[error("failed to write to stderr")]
+    StderrWriteFailed(#[source] IoError),
+
+    #[error("failed walking under directory '{}'", path.display())]
+    WalkDirFailed { path: PathBuf, source: IoError },
+
+    #[error("unable to identify file type for '{}'", path.display())]
+    UnknownFileType {
+        path: PathBuf,
+        #[source]
+        source: IoError,
+    },
+
+    #[error("failed to open file '{}'", path.display())]
+    FileFailedToOpen {
+        path: PathBuf,
+        #[source]
+        source: IoError,
+    },
+
+    #[error("failed to get file metadata '{}'", path.display())]
+    FileFailedMetadata {
+        path: PathBuf,
+        #[source]
+        source: IoError,
+    },
+
+    #[error("failed to make Artifact ID for '{}'", path.display())]
+    FileFailedToId {
+        path: PathBuf,
+        #[source]
+        source: OmniborError,
+    },
+
+    #[error("print channel closed")]
+    PrintChannelClose,
+}
+
+pub type Result<T> = StdResult<T, Error>;