From be54f1071ac65b11e8bcc105747dd66c74dc2997 Mon Sep 17 00:00:00 2001 From: Pavel Ivanov Date: Sun, 5 May 2024 01:17:37 +0200 Subject: [PATCH] fix: fixed unexpected error when configuration file is missing in default path (#245) --- .github/workflows/ci.yml | 2 + Cargo.lock | 4 +- Cargo.toml | 2 +- README.md | 2 +- build/ci/coverage.sh | 10 +++++ src/cli.rs | 18 ++++---- src/config.rs | 54 ++++++++++++++++++++++-- src/main.rs | 2 +- src/settings.rs | 90 ++++++++++++++++++++++++++-------------- 9 files changed, 133 insertions(+), 51 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dc482964..706b73af 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,3 +19,5 @@ jobs: run: cargo build --verbose --benches - name: Run tests run: cargo test --verbose + - name: Run executable + run: cargo run diff --git a/Cargo.lock b/Cargo.lock index 91e480db..eadf82d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -611,7 +611,7 @@ checksum = "edd0f118536f44f5ccd48bcb8b111bdc3de888b58c74639dfb034a357d0f206d" [[package]] name = "encstr" -version = "0.29.0" +version = "0.29.1" [[package]] name = "enum-map" @@ -768,7 +768,7 @@ checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" [[package]] name = "hl" -version = "0.29.0" +version = "0.29.1" dependencies = [ "atoi", "bincode", diff --git a/Cargo.toml b/Cargo.toml index a0a3d329..cb96e861 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = [".", "crate/encstr"] [workspace.package] repository = "https://github.com/pamburus/hl" authors = ["Pavel Ivanov "] -version = "0.29.0" +version = "0.29.1" edition = "2021" license = "MIT" diff --git a/README.md b/README.md index 1bccb4b0..83ecd724 100644 --- a/README.md +++ b/README.md @@ -466,7 +466,7 @@ Arguments: [FILE]... Files to process Options: - --config Configuration file path [env: HL_CONFIG=] [default: ~/.config/hl/config.yaml] + --config Configuration file path [env: HL_CONFIG=] -s, --sort Sort messages chronologically -F, --follow Follow input streams and sort messages chronologically during time frame set by --sync-interval-ms option --tail Number of last messages to preload from each file in --follow mode [default: 10] diff --git a/build/ci/coverage.sh b/build/ci/coverage.sh index 9bfe115e..e5e43fc9 100755 --- a/build/ci/coverage.sh +++ b/build/ci/coverage.sh @@ -5,6 +5,7 @@ set -e export RUSTFLAGS="-C instrument-coverage" export CARGO_TARGET_DIR="target/coverage" export LLVM_PROFILE_FILE="target/coverage/test-%m-%p.profraw" +export MAIN_EXECUTABLE="target/coverage/debug/hl" LLVM_BIN=$(rustc --print sysroot)/lib/rustlib/$(rustc -vV | sed -n 's|host: ||p')/bin @@ -19,6 +20,7 @@ IGNORE=( ) function executables() { + echo ${MAIN_EXECUTABLE:?} cargo test --tests --no-run --message-format=json \ | jq -r 'select(.profile.test == true) | .filenames[]' \ | grep -v dSYM - @@ -36,6 +38,14 @@ function clean() { function test() { cargo test --tests + cargo build + ${MAIN_EXECUTABLE:?} > /dev/null + ${MAIN_EXECUTABLE:?} --config= --help > /dev/null + ${MAIN_EXECUTABLE:?} --config=etc/defaults/config-k8s.yaml > /dev/null + ${MAIN_EXECUTABLE:?} --config=etc/defaults/config-ecs.yaml > /dev/null + ${MAIN_EXECUTABLE:?} --shell-completions bash > /dev/null + ${MAIN_EXECUTABLE:?} --list-themes > /dev/null + echo "" | ${MAIN_EXECUTABLE:?} --concurrency 4 > /dev/null } function merge() { diff --git a/src/cli.rs b/src/cli.rs index 536fcae8..81532f68 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -19,8 +19,14 @@ use crate::{ #[derive(Args)] pub struct BootstrapArgs { /// Configuration file path. - #[arg(long, overrides_with = "config", value_name = "FILE", env = "HL_CONFIG", default_value = default_config_path(), num_args=1)] - pub config: String, + #[arg( + long, + overrides_with = "config", + value_name = "FILE", + env = "HL_CONFIG", + num_args = 1 + )] + pub config: Option, } /// JSON and logfmt log converter to human readable representation. @@ -474,11 +480,3 @@ fn parse_non_zero_size(s: &str) -> std::result::Result clap::builder::OsStr { - if let Some(dirs) = config::app_dirs() { - dirs.config_dir.join("config.yaml").into_os_string().into() - } else { - "".into() - } -} diff --git a/src/config.rs b/src/config.rs index a76c859c..ce5d72bc 100644 --- a/src/config.rs +++ b/src/config.rs @@ -6,7 +6,10 @@ use once_cell::sync::Lazy; use platform_dirs::AppDirs; // local imports -use crate::{error::Result, settings::Settings}; +use crate::{ + error::Result, + settings::{Settings, SourceFile}, +}; // --- @@ -18,12 +21,25 @@ pub fn default() -> &'static Settings { } /// Load settings from the given file or the default configuration file per platform. -pub fn load(path: String) -> Result { - if path.is_empty() { +pub fn load(path: Option<&str>) -> Result { + let mut default = None; + let (filename, required) = path.map(|p| (p, true)).unwrap_or_else(|| { + ( + if let Some(dirs) = app_dirs() { + default = Some(dirs.config_dir.join("config.yaml").to_string_lossy().to_string()); + default.as_deref().unwrap() + } else { + "" + }, + false, + ) + }); + + if filename.is_empty() { return Ok(Default::default()); } - Settings::load(&path) + Settings::load(SourceFile::new(filename).required(required).into()) } /// Get the application platform-specific directories. @@ -49,3 +65,33 @@ pub mod global { &RESOLVED } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::settings::Settings; + + #[test] + fn test_default() { + assert_eq!(default().theme, "universal"); + } + + #[test] + fn test_load_empty_filename() { + let settings = super::load(Some("")).unwrap(); + assert_eq!(settings, Settings::default()); + } + + #[test] + fn test_load_k8s() { + let settings = super::load(Some("etc/defaults/config-k8s.yaml")).unwrap(); + assert_eq!(settings.fields.predefined.time.0.names, &["ts"]); + assert_eq!(settings.fields.predefined.message.0.names, &["msg"]); + assert_eq!(settings.fields.predefined.level.variants.len(), 2); + } + + #[test] + fn test_load_auto() { + super::load(None).unwrap(); + } +} diff --git a/src/main.rs b/src/main.rs index 3b6bb599..dfbe7ef5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -31,7 +31,7 @@ use hl::{ // --- fn run() -> Result<()> { - let settings = config::load(cli::BootstrapOpt::parse().args.config)?; + let settings = config::load(cli::BootstrapOpt::parse().args.config.as_deref())?; config::global::initialize(settings.clone()); let opt = cli::Opt::parse(); diff --git a/src/settings.rs b/src/settings.rs index 8b9c1c35..84ab32e1 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -17,11 +17,11 @@ use crate::level::Level; // --- static DEFAULT_SETTINGS_RAW: &str = include_str!("../etc/defaults/config.yaml"); -static DEFAULT_SETTINGS: Lazy = Lazy::new(|| Settings::load_from_str("", FileFormat::Yaml)); +static DEFAULT_SETTINGS: Lazy = Lazy::new(|| Settings::load(Source::Str("", FileFormat::Yaml)).unwrap()); // --- -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, PartialEq, Eq)] #[serde(rename_all = "kebab-case")] pub struct Settings { pub fields: Fields, @@ -33,22 +33,15 @@ pub struct Settings { } impl Settings { - pub fn load(filename: &str) -> Result { - Ok(Config::builder() - .add_source(File::from_str(DEFAULT_SETTINGS_RAW, FileFormat::Yaml)) - .add_source(File::with_name(filename)) - .build()? - .try_deserialize()?) - } - - pub fn load_from_str(value: &str, format: FileFormat) -> Self { - Config::builder() - .add_source(File::from_str(DEFAULT_SETTINGS_RAW, FileFormat::Yaml)) - .add_source(File::from_str(value, format)) - .build() - .unwrap() - .try_deserialize() - .unwrap() + pub fn load(source: Source) -> Result { + let builder = Config::builder().add_source(File::from_str(DEFAULT_SETTINGS_RAW, FileFormat::Yaml)); + let builder = match source { + Source::File(SourceFile { filename, required }) => { + builder.add_source(File::with_name(filename).required(required)) + } + Source::Str(value, format) => builder.add_source(File::from_str(value, format)), + }; + Ok(builder.build()?.try_deserialize()?) } } @@ -66,7 +59,40 @@ impl Default for &'static Settings { // --- -#[derive(Debug, Serialize, Deserialize, Clone, Default)] +pub enum Source<'a> { + File(SourceFile<'a>), + Str(&'a str, FileFormat), +} + +impl<'a> From> for Source<'a> { + fn from(file: SourceFile<'a>) -> Self { + Self::File(file) + } +} + +// --- + +pub struct SourceFile<'a> { + filename: &'a str, + required: bool, +} + +impl<'a> SourceFile<'a> { + pub fn new(filename: &'a str) -> Self { + Self { + filename, + required: true, + } + } + + pub fn required(self, required: bool) -> Self { + Self { required, ..self } + } +} + +// --- + +#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq)] pub struct Fields { pub predefined: PredefinedFields, pub ignore: Vec, @@ -75,7 +101,7 @@ pub struct Fields { // --- -#[derive(Debug, Serialize, Deserialize, Default, Clone)] +#[derive(Debug, Serialize, Deserialize, Default, Clone, PartialEq, Eq)] #[serde(rename_all = "kebab-case")] pub struct PredefinedFields { pub time: TimeField, @@ -100,7 +126,7 @@ impl Default for TimeField { // --- -#[derive(Debug, Serialize, Deserialize, Clone)] +#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] pub struct LevelField { pub show: FieldShowOption, pub variants: Vec, @@ -123,7 +149,7 @@ impl Default for LevelField { // --- -#[derive(Debug, Serialize, Deserialize, Clone)] +#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] pub struct LevelFieldVariant { pub names: Vec, #[serde(default, serialize_with = "ordered_map_serialize")] @@ -133,8 +159,8 @@ pub struct LevelFieldVariant { // --- -#[derive(Debug, Serialize, Deserialize, Deref, Clone)] -pub struct MessageField(Field); +#[derive(Debug, Serialize, Deserialize, Deref, Clone, PartialEq, Eq)] +pub struct MessageField(pub Field); impl Default for MessageField { fn default() -> Self { @@ -144,7 +170,7 @@ impl Default for MessageField { // --- -#[derive(Debug, Serialize, Deserialize, Deref, Clone)] +#[derive(Debug, Serialize, Deserialize, Deref, Clone, PartialEq, Eq)] pub struct LoggerField(Field); impl Default for LoggerField { @@ -155,7 +181,7 @@ impl Default for LoggerField { // --- -#[derive(Debug, Serialize, Deserialize, Deref, Clone)] +#[derive(Debug, Serialize, Deserialize, Deref, Clone, PartialEq, Eq)] pub struct CallerField(Field); impl Default for CallerField { @@ -166,7 +192,7 @@ impl Default for CallerField { // --- -#[derive(Debug, Serialize, Deserialize, Deref, Clone)] +#[derive(Debug, Serialize, Deserialize, Deref, Clone, PartialEq, Eq)] pub struct CallerFileField(Field); impl Default for CallerFileField { @@ -177,7 +203,7 @@ impl Default for CallerFileField { // --- -#[derive(Debug, Serialize, Deserialize, Deref, Clone)] +#[derive(Debug, Serialize, Deserialize, Deref, Clone, PartialEq, Eq)] pub struct CallerLineField(Field); impl Default for CallerLineField { @@ -206,7 +232,7 @@ impl Field { // --- -#[derive(Clone, Debug, Default, Deserialize)] +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Eq)] #[serde(rename_all = "kebab-case")] pub struct Formatting { pub punctuation: Punctuation, @@ -215,7 +241,7 @@ pub struct Formatting { // --- -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, PartialEq, Eq)] #[serde(rename_all = "kebab-case")] pub enum FlattenOption { Never, @@ -239,7 +265,7 @@ impl Default for FieldShowOption { // --- -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, PartialEq, Eq)] #[serde(rename_all = "kebab-case")] pub struct Punctuation { pub logger_name_separator: String, @@ -338,7 +364,7 @@ mod tests { #[test] fn test_load_settings_k8s() { - let settings = Settings::load("etc/defaults/config-k8s.yaml").unwrap(); + let settings = Settings::load(SourceFile::new("etc/defaults/config-k8s.yaml").into()).unwrap(); assert_eq!( settings.fields.predefined.time, TimeField(Field {