Skip to content

Commit

Permalink
fix: fixed unexpected error when configuration file is missing in def…
Browse files Browse the repository at this point in the history
…ault path (#245)
  • Loading branch information
pamburus authored May 4, 2024
1 parent ad2afb9 commit be54f10
Show file tree
Hide file tree
Showing 9 changed files with 133 additions and 51 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,5 @@ jobs:
run: cargo build --verbose --benches
- name: Run tests
run: cargo test --verbose
- name: Run executable
run: cargo run
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
10 changes: 10 additions & 0 deletions build/ci/coverage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 -
Expand All @@ -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() {
Expand Down
18 changes: 8 additions & 10 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
}
}
54 changes: 50 additions & 4 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};

// ---

Expand All @@ -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.
Expand All @@ -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();
}
}
2 changes: 1 addition & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
90 changes: 58 additions & 32 deletions src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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()?)
}
}

Expand All @@ -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>,
Expand All @@ -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,
Expand All @@ -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>,
Expand All @@ -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")]
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit be54f10

Please sign in to comment.