From fa8ff872a178e8f4b4c0620b4bd6a18c318fbac9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Fri, 20 Oct 2023 14:59:47 +0100 Subject: [PATCH 1/4] Clarify release builds using max_level_* features Rephrase the documentation to make it clear that the max_level_* and release_max_level_* are not separate ways of setting STATIC_MAX_LEVEL for respectively debug and release builds but instead, that max_level_* are used by all builds and that release_max_level_* are just a way to override them. --- src/lib.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 46d4378af..e36d5c0ed 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -241,9 +241,7 @@ //! //! # Compile time filters //! -//! Log levels can be statically disabled at compile time via Cargo features. Log invocations at -//! disabled levels will be skipped and will not even be present in the resulting binary. -//! This level is configured separately for release and debug builds. The features are: +//! Log levels can be statically disabled at compile time by enabling one of these Cargo features: //! //! * `max_level_off` //! * `max_level_error` @@ -251,6 +249,13 @@ //! * `max_level_info` //! * `max_level_debug` //! * `max_level_trace` +//! +//! Log invocations at disabled levels will be skipped and will not even be present in the +//! resulting binary. These features control the value of the `STATIC_MAX_LEVEL` constant. The +//! logging macros check this value before logging a message. By default, no levels are disabled. +//! +//! It is possible to override this level for release builds only with the following features: +//! //! * `release_max_level_off` //! * `release_max_level_error` //! * `release_max_level_warn` @@ -258,9 +263,6 @@ //! * `release_max_level_debug` //! * `release_max_level_trace` //! -//! These features control the value of the `STATIC_MAX_LEVEL` constant. The logging macros check -//! this value before logging a message. By default, no levels are disabled. -//! //! Libraries should avoid using the max level features because they're global and can't be changed //! once they're set. //! From cc7efd8f12508eb1b41091270a6b8c219a4018a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Fri, 20 Oct 2023 10:32:14 +0100 Subject: [PATCH 2/4] Add feature-dependent tests for STATIC_MAX_LEVEL Add unit tests that check, for the current configuration of the crate features, that STATIC_MAX_LEVEL has the appropriate value. As the features are statically set (at build time), coverage of all the max_level_* and release_max_level_* value pairs must be achieved by launching cargo with different sets: levels="off debug error info trace warn" for i in '' $levels; do release=${i:+--features=release_max_level_${i}} for j in '' $levels; do debug=${j:+--features=max_level_${j}} cargo test static_max_level ${release} ${debug} cargo test static_max_level ${release} ${debug} --release done done --- src/lib.rs | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index e36d5c0ed..be5f3fd92 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -336,6 +336,7 @@ #[cfg(all(not(feature = "std"), not(test)))] extern crate core as std; +use std::cfg; #[cfg(feature = "std")] use std::error; use std::str::FromStr; @@ -1537,7 +1538,7 @@ const fn get_max_level_inner() -> LevelFilter { #[cfg(test)] mod tests { - use super::{Level, LevelFilter, ParseLevelError}; + use super::{Level, LevelFilter, ParseLevelError, STATIC_MAX_LEVEL}; #[test] fn test_levelfilter_from_str() { @@ -1650,6 +1651,54 @@ mod tests { } } + #[test] + #[cfg_attr(not(debug_assertions), ignore)] + fn test_static_max_level_debug() { + if cfg!(feature = "max_level_off") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Off); + } else if cfg!(feature = "max_level_error") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Error); + } else if cfg!(feature = "max_level_warn") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Warn); + } else if cfg!(feature = "max_level_info") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Info); + } else if cfg!(feature = "max_level_debug") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Debug); + } else { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Trace); + } + } + + #[test] + #[cfg_attr(debug_assertions, ignore)] + fn test_static_max_level_release() { + if cfg!(feature = "release_max_level_off") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Off); + } else if cfg!(feature = "release_max_level_error") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Error); + } else if cfg!(feature = "release_max_level_warn") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Warn); + } else if cfg!(feature = "release_max_level_info") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Info); + } else if cfg!(feature = "release_max_level_debug") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Debug); + } else if cfg!(feature = "release_max_level_trace") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Trace); + } else if cfg!(feature = "max_level_off") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Off); + } else if cfg!(feature = "max_level_error") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Error); + } else if cfg!(feature = "max_level_warn") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Warn); + } else if cfg!(feature = "max_level_info") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Info); + } else if cfg!(feature = "max_level_debug") { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Debug); + } else { + assert_eq!(STATIC_MAX_LEVEL, LevelFilter::Trace); + } + } + #[test] #[cfg(feature = "std")] fn test_error_trait() { From 296fa1a4de79a67514a1046a73d8ff716d6eab50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Fri, 20 Oct 2023 16:40:25 +0100 Subject: [PATCH 3/4] Simplify STATIC_MAX_LEVEL intialization Replace get_max_level_inner() with cfg!() and a const-compatible match, now that the MSRV has been bumped to 1.60.0. --- src/lib.rs | 67 ++++++++++++------------------------------------------ 1 file changed, 14 insertions(+), 53 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index be5f3fd92..77b4c8afa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1482,59 +1482,20 @@ pub mod __private_api; /// [`logger`]: fn.logger.html pub const STATIC_MAX_LEVEL: LevelFilter = MAX_LEVEL_INNER; -const MAX_LEVEL_INNER: LevelFilter = get_max_level_inner(); - -const fn get_max_level_inner() -> LevelFilter { - #[allow(unreachable_code)] - { - #[cfg(all(not(debug_assertions), feature = "release_max_level_off"))] - { - return LevelFilter::Off; - } - #[cfg(all(not(debug_assertions), feature = "release_max_level_error"))] - { - return LevelFilter::Error; - } - #[cfg(all(not(debug_assertions), feature = "release_max_level_warn"))] - { - return LevelFilter::Warn; - } - #[cfg(all(not(debug_assertions), feature = "release_max_level_info"))] - { - return LevelFilter::Info; - } - #[cfg(all(not(debug_assertions), feature = "release_max_level_debug"))] - { - return LevelFilter::Debug; - } - #[cfg(all(not(debug_assertions), feature = "release_max_level_trace"))] - { - return LevelFilter::Trace; - } - #[cfg(feature = "max_level_off")] - { - return LevelFilter::Off; - } - #[cfg(feature = "max_level_error")] - { - return LevelFilter::Error; - } - #[cfg(feature = "max_level_warn")] - { - return LevelFilter::Warn; - } - #[cfg(feature = "max_level_info")] - { - return LevelFilter::Info; - } - #[cfg(feature = "max_level_debug")] - { - return LevelFilter::Debug; - } - - LevelFilter::Trace - } -} +const MAX_LEVEL_INNER: LevelFilter = match cfg!(debug_assertions) { + false if cfg!(feature = "release_max_level_off") => LevelFilter::Off, + false if cfg!(feature = "release_max_level_error") => LevelFilter::Error, + false if cfg!(feature = "release_max_level_warn") => LevelFilter::Warn, + false if cfg!(feature = "release_max_level_info") => LevelFilter::Info, + false if cfg!(feature = "release_max_level_debug") => LevelFilter::Debug, + false if cfg!(feature = "release_max_level_trace") => LevelFilter::Trace, + _ if cfg!(feature = "max_level_off") => LevelFilter::Off, + _ if cfg!(feature = "max_level_error") => LevelFilter::Error, + _ if cfg!(feature = "max_level_warn") => LevelFilter::Warn, + _ if cfg!(feature = "max_level_info") => LevelFilter::Info, + _ if cfg!(feature = "max_level_debug") => LevelFilter::Debug, + _ => LevelFilter::Trace, +}; #[cfg(test)] mod tests { From 05d8c46c60a48cec860d2e41da5767258923f802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Fri, 20 Oct 2023 16:45:53 +0100 Subject: [PATCH 4/4] Remove intermediate MAX_LEVEL_INNER Drop the constant as it now seems unnecessary. Note: this is done in a separate commit to be easy to bisect and revert. --- src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 77b4c8afa..ec9b868b9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1480,9 +1480,7 @@ pub mod __private_api; /// should compare the level against this value. /// /// [`logger`]: fn.logger.html -pub const STATIC_MAX_LEVEL: LevelFilter = MAX_LEVEL_INNER; - -const MAX_LEVEL_INNER: LevelFilter = match cfg!(debug_assertions) { +pub const STATIC_MAX_LEVEL: LevelFilter = match cfg!(debug_assertions) { false if cfg!(feature = "release_max_level_off") => LevelFilter::Off, false if cfg!(feature = "release_max_level_error") => LevelFilter::Error, false if cfg!(feature = "release_max_level_warn") => LevelFilter::Warn,