Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ jobs:
- uses: Swatinem/rust-cache@v2
- uses: taiki-e/install-action@cargo-hack
- name: Default features
run: cargo hack check --each-feature --locked --rust-version --ignore-private --workspace --all-targets --keep-going
run: |
cargo hack check --each-feature --locked --rust-version --ignore-private --package env_logger --all-targets --keep-going
cargo hack check --each-feature --locked --version-range 1.81..=1.81 --ignore-private --package env_filter --all-targets --keep-going
Comment on lines +75 to +77
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?

Copy link
Copy Markdown
Author

@cagatay-y cagatay-y May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though it is not possible to declare a MSRV for env_filter in its Cargo.toml (see the comment on crates/env_filter/Cargo.toml) , I thought it may be useful to have a check in the CI to prevent accidental further bumps for the env_filter MSRV.

minimal-versions:
name: Minimal versions
strategy:
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ unstable-kv = ["kv"]

[dependencies]
log = { version = "0.4.29", features = ["std"] }
env_filter = { version = "1.0.0", path = "crates/env_filter", default-features = false }
env_filter = { version = "1.0.0", path = "crates/env_filter", default-features = false, features = ["std"] }
jiff = { version = "0.2.22", default-features = false, features = ["std"], optional = true }
anstream = { version = "1.0.0", default-features = false, features = ["wincon"], optional = true }
anstyle = { version = "1.0.13", optional = true }
Expand Down
8 changes: 4 additions & 4 deletions crates/env_filter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ keywords = ["logging", "log", "logger"]
repository.workspace = true
license.workspace = true
edition.workspace = true
rust-version.workspace = true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this now have no MSRV?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error::Error under core, which is used when the std feature is not enabled, only got stabilised in Rust 1.81, so the MSRV check fails with the workspace MSRV of 1.71. On the other hand, providing a env_filter MSRV of 1.81 causes the env_logger MSRV check to fail, even though it enables the std feature of env_filter and thus is not affected by the absence of core::error::Error. I thought that increasing the MSRV for both to 1.81 would unnecessarily negatively affect the users of env_logger.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This effectively raises the MSRV of env_logger without anything specifically saying so. We need to find an alternative route or bump the MSRV.

include.workspace = true

[package.metadata.docs.rs]
Expand All @@ -26,12 +25,13 @@ pre-release-replacements = [
]

[features]
default = ["regex"]
default = ["std", "regex"]
regex = ["dep:regex"]
std = ["regex/std"]

[dependencies]
log = { version = "0.4.29", features = ["std"] }
regex = { version = "1.12.3", optional = true, default-features=false, features=["std", "perf"] }
log = { version = "0.4.29", default-features = false }
regex = { version = "1.12.3", optional = true, default-features=false, features=["perf"] }

[dev-dependencies]
snapbox = "1.0"
Expand Down
2 changes: 2 additions & 0 deletions crates/env_filter/src/directive.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use alloc::string::String;

use log::Level;
use log::LevelFilter;

Expand Down
13 changes: 9 additions & 4 deletions crates/env_filter/src/filter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::env;
use std::fmt;
use std::mem;
use alloc::{borrow::ToOwned, string::ToString, vec::Vec};
use core::{fmt, mem};

use log::{LevelFilter, Metadata, Record};

Expand Down Expand Up @@ -48,10 +47,11 @@ impl Builder {
}

/// Initializes the filter builder from an environment.
#[cfg(feature = "std")]
pub fn from_env(env: &str) -> Builder {
let mut builder = Builder::new();

if let Ok(s) = env::var(env) {
if let Ok(s) = std::env::var(env) {
builder.parse(&s);
}

Expand Down Expand Up @@ -108,7 +108,10 @@ impl Builder {
} = parse_spec(filters);

for error in errors {
#[cfg(feature = "std")]
eprintln!("warning: {error}, ignoring it");
#[cfg(not(feature = "std"))]
log::warn!("{error}, ignoring it");
Comment on lines +113 to +114
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for creating the logger in the first place, so I don't think it is too meaningful to log at this point.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least in my use case, the filter member of the logger is a OnceCell<env_filter::Filter> and until the filter is fully initialised, the logger is returned a Option<&Filter>::None when trying to get it. When it receives the None, it simply lets the log message through, so the user is able to see the warning.

}

self.filter = filter;
Expand Down Expand Up @@ -258,6 +261,8 @@ impl fmt::Debug for Filter {

#[cfg(test)]
mod tests {
use alloc::{borrow::ToOwned, vec, vec::Vec};

use log::{Level, LevelFilter};
use snapbox::{assert_data_eq, str};

Expand Down
5 changes: 5 additions & 0 deletions crates/env_filter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,14 @@
//! ```

#![cfg_attr(docsrs, feature(doc_cfg))]
#![cfg_attr(all(not(feature = "std"), not(test)), no_std)]
#![warn(missing_docs)]
#![warn(clippy::print_stderr)]
#![warn(clippy::print_stdout)]
Comment thread
cagatay-y marked this conversation as resolved.
#![warn(clippy::std_instead_of_core)]
#![warn(clippy::std_instead_of_alloc)]

extern crate alloc;

mod directive;
mod filter;
Expand Down
7 changes: 4 additions & 3 deletions crates/env_filter/src/op.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::fmt;
use alloc::string::{String, ToString};
use core::fmt;

#[derive(Debug, Clone)]
pub(crate) struct FilterOp {
Expand All @@ -24,13 +25,13 @@ impl FilterOp {

#[cfg(not(feature = "regex"))]
impl FilterOp {
pub fn new(spec: &str) -> Result<Self, String> {
pub(crate) fn new(spec: &str) -> Result<Self, String> {
Ok(Self {
inner: spec.to_string(),
})
}

pub fn is_match(&self, s: &str) -> bool {
pub(crate) fn is_match(&self, s: &str) -> bool {
s.contains(&self.inner)
}
}
Expand Down
16 changes: 12 additions & 4 deletions crates/env_filter/src/parser.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use alloc::{borrow::ToOwned, format, string::String, vec::Vec};
use core::fmt::{Display, Formatter};

use log::LevelFilter;
use std::error::Error;
use std::fmt::{Display, Formatter};

use crate::Directive;
use crate::FilterOp;
Expand Down Expand Up @@ -46,12 +47,17 @@ pub struct ParseError {
}

impl Display for ParseError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
write!(f, "error parsing logger filter: {}", self.details)
}
}

impl Error for ParseError {}
#[cfg(feature = "std")]
#[allow(clippy::std_instead_of_core)]
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allow stopped working starting from Rust 1.91. I could not find anything in the release notes that would affect it.

impl std::error::Error for ParseError {}

#[cfg(not(feature = "std"))]
impl core::error::Error for ParseError {}

/// Parse a logging specification string (e.g: `crate1,crate2::mod3,crate3::x=error/foo`)
/// and return a vector with log directives.
Expand Down Expand Up @@ -115,6 +121,8 @@ pub(crate) fn parse_spec(spec: &str) -> ParseResult {

#[cfg(test)]
mod tests {
use alloc::{borrow::ToOwned, string::ToString};

use crate::ParseError;
use log::LevelFilter;
use snapbox::{assert_data_eq, str, Data, IntoData};
Expand Down