refactor(iroh-cli)!: Use config and logging from iroh-node-utils (#2953)

## Description Move things like figuring out the configuration and data directory into iroh-node-utils as well, so it can be used from other binaries than iroh-cli. E.g. doctor, or a pure blobs bin, or for testing. ## Breaking Changes This affects iroh-cli, which is a bin crate. But nevertheless, here are the changes: - module iroh_cli::logging has moved to iroh-node-util to make it available for other people building nodes - iroh_config_root, iroh_data_root and iroh_cache_root in iroh-cli have been replaced with generic config_root, data_root and cache_root in iroh-node-util::config ## Notes & open questions  ## Change checklist - [ ] Self-review. - [ ] Documentation updates following the [style guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text), if relevant. - [ ] Tests if relevant. - [ ] All breaking changes documented. --------- Co-authored-by: dignifiedquire <[email protected]> Co-authored-by: Diva M <[email protected]>
n0-computer · Nov 20, 2024 · 3ff914d · 3ff914d
1 parent f174c8e
commit 3ff914d
Show file tree

Hide file tree

Showing 11 changed files with 174 additions and 143 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/iroh-cli/Cargo.toml b/iroh-cli/Cargo.toml
@@ -33,7 +33,6 @@ console = "0.15.5"
 crossterm = "0.27.0"
 derive_more = { version = "1.0.0", features = ["display"] }
 dialoguer = { version = "0.11.0", default-features = false }
-dirs-next = "2.0.0"
 futures-buffered = "0.2.4"
 futures-lite = "2.3"
 futures-util = { version = "0.3.30", features = ["futures-sink"] }
@@ -45,6 +44,7 @@ iroh-blobs = { version = "0.28.1", features = ["cli"] }
 iroh-docs = { version = "0.28.0", features = ["cli"] }
 iroh-gossip = { version = "0.28.1", features = ["cli"] }
 iroh-metrics = { version = "0.28.0" }
+iroh-node-util = { path = "../iroh-node-util", features = ["config", "logging"] }
 parking_lot = "0.12.1"
 pkarr = { version = "2.2.0", default-features = false }
 portable-atomic = "1"
@@ -56,7 +56,6 @@ ratatui = "0.26.2"
 reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] }
 rustyline = "12.0.0"
 serde = { version = "1.0.197", features = ["derive"] }
-serde_with = "3.7.0"
 shell-words = "1.1.0"
 shellexpand = "3.1.0"
 strum = { version = "0.26.2", features = ["derive"] }
@@ -67,8 +66,6 @@ tokio = { version = "1.36.0", features = ["full"] }
 tokio-util = { version = "0.7.12", features = ["rt"] }
 toml = { version = "0.8.12", features = ["preserve_order"] }
 tracing = "0.1.40"
-tracing-appender = "0.2.3"
-tracing-subscriber = { version = "0.3", features = ["env-filter"] }
 
 [dev-dependencies]
 duct = "0.13.6"

diff --git a/iroh-cli/src/commands.rs b/iroh-cli/src/commands.rs
@@ -128,7 +128,7 @@ impl Cli {
                     )
                     .await
                 } else {
-                    crate::logging::init_terminal_logging()?;
+                    iroh_node_util::logging::init_terminal_logging()?;
                     let iroh = if let Some(addr) = self.rpc_addr {
                         Iroh::connect_addr(addr).await.context("rpc connect")?
                     } else {
@@ -154,7 +154,7 @@ impl Cli {
                     )
                     .await
                 } else {
-                    crate::logging::init_terminal_logging()?;
+                    iroh_node_util::logging::init_terminal_logging()?;
                     let iroh = if let Some(addr) = self.rpc_addr {
                         Iroh::connect_addr(addr).await.context("rpc connect")?
                     } else {

diff --git a/iroh-cli/src/commands/doctor.rs b/iroh-cli/src/commands/doctor.rs
@@ -44,6 +44,7 @@ use iroh::{
     util::{path::IrohPaths, progress::ProgressWriter},
 };
 use iroh_metrics::core::Core;
+use iroh_node_util::config::data_root;
 use portable_atomic::AtomicU64;
 use postcard::experimental::max_size::MaxSize;
 use rand::Rng;
@@ -53,7 +54,7 @@ use tokio::{io::AsyncWriteExt, sync};
 use tokio_util::task::AbortOnDropHandle;
 use tracing::warn;
 
-use crate::config::{iroh_data_root, NodeConfig};
+use crate::config::NodeConfig;
 
 /// Options for the secret key usage.
 #[derive(Debug, Clone, derive_more::Display)]
@@ -1016,7 +1017,7 @@ fn create_secret_key(secret_key: SecretKeyOption) -> anyhow::Result<SecretKey> {
             SecretKey::try_from(&bytes[..])?
         }
         SecretKeyOption::Local => {
-            let path = IrohPaths::SecretKey.with_root(iroh_data_root()?);
+            let path = IrohPaths::SecretKey.with_root(data_root("iroh")?);
             if path.exists() {
                 let bytes = std::fs::read(&path)?;
                 SecretKey::try_from_openssh(bytes)?
@@ -1108,8 +1109,9 @@ fn inspect_ticket(ticket: &str, zbase32: bool) -> anyhow::Result<()> {
 
 /// Runs the doctor commands.
 pub async fn run(command: Commands, config: &NodeConfig) -> anyhow::Result<()> {
-    let data_dir = iroh_data_root()?;
-    let _guard = crate::logging::init_terminal_and_file_logging(&config.file_logs, &data_dir)?;
+    let data_dir = data_root("iroh")?;
+    let _guard =
+        iroh_node_util::logging::init_terminal_and_file_logging(&config.file_logs, &data_dir)?;
     let metrics_fut = super::start::start_metrics_server(config.metrics_addr);
     let cmd_res = match command {
         Commands::Report {

diff --git a/iroh-cli/src/commands/start.rs b/iroh-cli/src/commands/start.rs
@@ -46,7 +46,8 @@ where
     F: FnOnce(iroh::client::Iroh) -> T + Send + 'static,
     T: Future<Output = Result<()>> + 'static,
 {
-    let _guard = crate::logging::init_terminal_and_file_logging(&config.file_logs, iroh_data_root)?;
+    let _guard =
+        iroh_node_util::logging::init_terminal_and_file_logging(&config.file_logs, iroh_data_root)?;
     let metrics_fut = start_metrics_server(config.metrics_addr);
     let metrics_dumper_fut =
         start_metrics_dumper(config.metrics_dump_path.clone(), Duration::from_millis(100));

diff --git a/iroh-cli/src/config.rs b/iroh-cli/src/config.rs
@@ -1,23 +1,23 @@
 //! Configuration for the iroh CLI.
 
 use std::{
-    env,
     net::SocketAddr,
     path::{Path, PathBuf},
-    str::FromStr,
     sync::Arc,
     time::Duration,
 };
 
-use anyhow::{anyhow, Result};
+use anyhow::Result;
 use iroh::{
     net::{RelayMap, RelayNode},
     node::GcPolicy,
 };
+use iroh_node_util::{config::config_root, logging::env_file_rust_log};
 use serde::Deserialize;
 
-const ENV_CONFIG_DIR: &str = "IROH_CONFIG_DIR";
-const ENV_FILE_RUST_LOG: &str = "IROH_FILE_RUST_LOG";
+/// BIN_NAME is the name of the binary. This is used in various places, e.g. for the home directory
+/// and for environment variables.
+pub(crate) const BIN_NAME: &str = "iroh";
 
 /// CONFIG_FILE_NAME is the name of the optional config file located in the iroh home directory
 pub(crate) const CONFIG_FILE_NAME: &str = "iroh.config.toml";
@@ -52,7 +52,7 @@ pub(crate) struct NodeConfig {
     /// Bind address on which to serve Prometheus metrics
     pub(crate) metrics_addr: Option<SocketAddr>,
     /// Configuration for the logfile.
-    pub(crate) file_logs: super::logging::FileLogging,
+    pub(crate) file_logs: iroh_node_util::logging::FileLogging,
     /// Path to dump metrics to in CSV format.
     pub(crate) metrics_dump_path: Option<PathBuf>,
 }
@@ -81,7 +81,7 @@ impl NodeConfig {
     /// default config file will be loaded.  If that is not present the default config will
     /// be used.
     pub(crate) async fn load(file: Option<&Path>) -> Result<NodeConfig> {
-        let default_config = iroh_config_path(CONFIG_FILE_NAME)?;
+        let default_config = config_root(BIN_NAME)?.join(CONFIG_FILE_NAME);
 
         let config_file = match file {
             Some(file) => Some(file),
@@ -101,7 +101,7 @@ impl NodeConfig {
         };
 
         // override from env var
-        if let Some(env_filter) = env_file_rust_log().transpose()? {
+        if let Some(env_filter) = env_file_rust_log(BIN_NAME).transpose()? {
             config.file_logs.rust_log = env_filter;
         }
         Ok(config)
@@ -144,112 +144,17 @@ impl From<GcPolicyConfig> for GcPolicy {
     }
 }
 
-/// Parse [`ENV_FILE_RUST_LOG`] as [`tracing_subscriber::EnvFilter`]. Returns `None` if not
-/// present.
-fn env_file_rust_log() -> Option<Result<crate::logging::EnvFilter>> {
-    match env::var(ENV_FILE_RUST_LOG) {
-        Ok(s) => Some(crate::logging::EnvFilter::from_str(&s).map_err(Into::into)),
-        Err(e) => match e {
-            env::VarError::NotPresent => None,
-            e @ env::VarError::NotUnicode(_) => Some(Err(e.into())),
-        },
-    }
-}
-
-/// Name of directory that wraps all iroh files in a given application directory
-const IROH_DIR: &str = "iroh";
-
-/// Returns the path to the user's iroh config directory.
-///
-/// If the `IROH_CONFIG_DIR` environment variable is set it will be used unconditionally.
-/// Otherwise the returned value depends on the operating system according to the following
-/// table.
-///
-/// | Platform | Value                                 | Example                          |
-/// | -------- | ------------------------------------- | -------------------------------- |
-/// | Linux    | `$XDG_CONFIG_HOME` or `$HOME`/.config/iroh | /home/alice/.config/iroh              |
-/// | macOS    | `$HOME`/Library/Application Support/iroh   | /Users/Alice/Library/Application Support/iroh |
-/// | Windows  | `{FOLDERID_RoamingAppData}`/iroh           | C:\Users\Alice\AppData\Roaming\iroh   |
-pub(crate) fn iroh_config_root() -> Result<PathBuf> {
-    if let Some(val) = env::var_os(ENV_CONFIG_DIR) {
-        return Ok(PathBuf::from(val));
-    }
-    let cfg = dirs_next::config_dir()
-        .ok_or_else(|| anyhow!("operating environment provides no directory for configuration"))?;
-    Ok(cfg.join(IROH_DIR))
-}
-
-/// Path that leads to a file in the iroh config directory.
-pub(crate) fn iroh_config_path(file_name: impl AsRef<Path>) -> Result<PathBuf> {
-    let path = iroh_config_root()?.join(file_name);
-    Ok(path)
-}
-
-/// Returns the path to the user's iroh data directory.
-///
-/// If the `IROH_DATA_DIR` environment variable is set it will be used unconditionally.
-/// Otherwise the returned value depends on the operating system according to the following
-/// table.
-///
-/// | Platform | Value                                         | Example                                  |
-/// | -------- | --------------------------------------------- | ---------------------------------------- |
-/// | Linux    | `$XDG_DATA_HOME`/iroh or `$HOME`/.local/share/iroh | /home/alice/.local/share/iroh                 |
-/// | macOS    | `$HOME`/Library/Application Support/iroh      | /Users/Alice/Library/Application Support/iroh |
-/// | Windows  | `{FOLDERID_RoamingAppData}/iroh`              | C:\Users\Alice\AppData\Roaming\iroh           |
-pub(crate) fn iroh_data_root() -> Result<PathBuf> {
-    let path = if let Some(val) = env::var_os("IROH_DATA_DIR") {
-        PathBuf::from(val)
-    } else {
-        let path = dirs_next::data_dir().ok_or_else(|| {
-            anyhow!("operating environment provides no directory for application data")
-        })?;
-        path.join(IROH_DIR)
-    };
-    let path = if !path.is_absolute() {
-        std::env::current_dir()?.join(path)
-    } else {
-        path
-    };
-    Ok(path)
-}
-
-/// Returns the path to the user's iroh cache directory.
-///
-/// If the `IROH_CACHE_DIR` environment variable is set it will be used unconditionally.
-/// Otherwise the returned value depends on the operating system according to the following
-/// table.
-///
-/// | Platform | Value                                         | Example                                  |
-/// | -------- | --------------------------------------------- | ---------------------------------------- |
-/// | Linux    | `$XDG_CACHE_HOME`/iroh or `$HOME`/.cache/iroh | /home/.cache/iroh                        |
-/// | macOS    | `$HOME`/Library/Caches/iroh                   | /Users/Alice/Library/Caches/iroh         |
-/// | Windows  | `{FOLDERID_LocalAppData}/iroh`                | C:\Users\Alice\AppData\Roaming\iroh      |
-#[allow(dead_code)]
-pub(crate) fn iroh_cache_root() -> Result<PathBuf> {
-    if let Some(val) = env::var_os("IROH_CACHE_DIR") {
-        return Ok(PathBuf::from(val));
-    }
-    let path = dirs_next::cache_dir().ok_or_else(|| {
-        anyhow!("operating environment provides no directory for application data")
-    })?;
-    Ok(path.join(IROH_DIR))
-}
-
-/// Path that leads to a file in the iroh cache directory.
-#[allow(dead_code)]
-pub(crate) fn iroh_cache_path(file_name: &Path) -> Result<PathBuf> {
-    let path = iroh_cache_root()?.join(file_name);
-    Ok(path)
-}
-
 #[cfg(test)]
 mod tests {
-    use std::net::{Ipv4Addr, Ipv6Addr};
+    use std::{
+        net::{Ipv4Addr, Ipv6Addr},
+        str::FromStr,
+    };
 
+    use iroh_node_util::logging::{EnvFilter, Rotation};
     use url::Url;
 
     use super::*;
-    use crate::logging::{EnvFilter, Rotation};
 
     #[test]
     fn test_toml_invalid_field() {

diff --git a/iroh-cli/src/main.rs b/iroh-cli/src/main.rs
@@ -2,10 +2,11 @@ use std::time::Duration;
 
 use anyhow::Result;
 use clap::Parser;
+use config::BIN_NAME;
+use iroh_node_util::config::data_root;
 
 mod commands;
 mod config;
-mod logging;
 
 use crate::commands::Cli;
 
@@ -24,7 +25,7 @@ fn main() -> Result<()> {
 }
 
 async fn main_impl() -> Result<()> {
-    let data_dir = config::iroh_data_root()?;
+    let data_dir = data_root(BIN_NAME)?;
     let cli = Cli::parse();
     cli.run(&data_dir).await
 }
diff --git a/iroh-node-util/Cargo.toml b/iroh-node-util/Cargo.toml
@@ -28,3 +28,18 @@ serde = "1"
 serde-error = "0.1.3"
 futures-lite = "2.5.0"
 tracing = "0.1.40"
+
+derive_more = { version = "1.0.0", features = ["display"], optional = true }
+dirs-next = { version = "2.0.0", optional = true }
+rustyline = { version = "12.0.0", optional = true }
+serde_with = { version = "3.7.0", optional = true }
+tracing-appender = { version = "0.2.3", optional = true }
+tracing-subscriber = { version = "0.3", features = ["env-filter"], optional = true }
+
+[features]
+logging = ["dep:derive_more", "dep:serde_with", "dep:rustyline", "dep:tracing-appender", "dep:tracing-subscriber"]
+config = ["dep:dirs-next"]
+
+[package.metadata.docs.rs]
+all-features = true
+rustdoc-args = ["--cfg", "iroh_docsrs"]