fix: fixed unexpected error when configuration file is missing in default path (#245)

Author: pamburus

.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

Cargo.lock

@@ -4,7 +4,7 @@ members = [".", "crate/encstr"]
 [workspace.package]
 repository = "https://github.com/pamburus/hl"
 authors = ["Pavel Ivanov <[email protected]>"]
-version = "0.29.0"
+version = "0.29.1"
 edition = "2021"
 license = "MIT"
 

Cargo.toml

@@ -466,7 +466,7 @@ Arguments:
   [FILE]...  Files to process
 
 Options:
-      --config <FILE>                    Configuration file path [env: HL_CONFIG=] [default: ~/.config/hl/config.yaml]
+      --config <FILE>                    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 <N>                         Number of last messages to preload from each file in --follow mode [default: 10]

README.md

@@ -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() {

build/ci/coverage.sh

@@ -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<String>,
 }
 
 /// JSON and logfmt log converter to human readable representation.
@@ -474,11 +480,3 @@ fn parse_non_zero_size(s: &str) -> std::result::Result<NonZeroUsize, NonZeroSize
         Err(NonZeroSizeParseError::ZeroSize)
     }
 }
-
-fn default_config_path() -> clap::builder::OsStr {
-    if let Some(dirs) = config::app_dirs() {
-        dirs.config_dir.join("config.yaml").into_os_string().into()
-    } else {
-        "".into()
-    }
-}

src/cli.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<Settings> {
-    if path.is_empty() {
+pub fn load(path: Option<&str>) -> Result<Settings> {
+    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();
+    }
+}

src/config.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();

src/main.rs

@@ -17,11 +17,11 @@ use crate::level::Level;
 // ---
 
 static DEFAULT_SETTINGS_RAW: &str = include_str!("../etc/defaults/config.yaml");
-static DEFAULT_SETTINGS: Lazy<Settings> = Lazy::new(|| Settings::load_from_str("", FileFormat::Yaml));
+static DEFAULT_SETTINGS: Lazy<Settings> = 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<Self, Error> {
-        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<Self, Error> {
+        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<SourceFile<'a>> 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<String>,
@@ -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<LevelFieldVariant>,
@@ -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<String>,
     #[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 {