From 357552182a986088057a0000cb61be6e554db397 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 6 Nov 2024 13:49:45 +0800 Subject: [PATCH] Make env_logger an optional dependency (#1226) Now the built-in `env_logger` is guarded behind a Cargo feature "builtin_env_logger". It is a default feature, but can be disabled in Cargo.toml by setting `dependencies.mmtk.default-features = false`. In this way, VM bindings that want to implement its own logger can remove the `env_logger` crate from its dependencies. Fixes: https://github.com/mmtk/mmtk-core/issues/744 --- Cargo.toml | 9 +++++++-- src/memory_manager.rs | 11 ++++------- src/util/logger.rs | 41 +++++++++++++++++++++++++++++++++++------ 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1ce4a83bec..1266d27e88 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ crossbeam = "0.8.1" delegate = "0.12.0" downcast-rs = "1.1.1" enum-map = "2.4.2" -env_logger = "0.11.3" +env_logger = { version = "0.11.3", optional = true } is-terminal = "0.4.7" itertools = "0.12.0" jemalloc-sys = { version = "0.5.3", features = ["disable_initial_exec_tls"], optional = true } @@ -66,7 +66,12 @@ name = "main" harness = false [features] -default = [] +default = ["builtin_env_logger"] + +# Built-in env_logger. This feature is enabled by default. +# The user can disable this default feature to remove `env_logger` from the dependencies. +# See `crate::util::logger` for more details. +builtin_env_logger = ["dep:env_logger"] # This feature is only supported on x86-64 for now # It's manually added to CI scripts diff --git a/src/memory_manager.rs b/src/memory_manager.rs index 9c6b30034a..1949eca8eb 100644 --- a/src/memory_manager.rs +++ b/src/memory_manager.rs @@ -38,7 +38,9 @@ use crate::vm::VMBinding; /// supported. Currently we assume a binding will only need one MMTk instance. Note that GC is enabled by default and the binding should /// implement `VMCollection::is_collection_enabled()` if it requires that the GC should be disabled at a particular time. /// -/// Note that this method will attempt to initialize a logger. If the VM would like to use its own logger, it should initialize the logger before calling this method. +/// This method will attempt to initialize the built-in `env_logger` if the Cargo feature "builtin_env_logger" is enabled (by default). +/// If the VM would like to use its own logger, it should disable the default feature "builtin_env_logger" in `Cargo.toml`. +/// /// Note that, to allow MMTk to do GC properly, `initialize_collection()` needs to be called after this call when /// the VM's thread system is ready to spawn GC workers. /// @@ -51,12 +53,7 @@ use crate::vm::VMBinding; /// Arguments: /// * `builder`: The reference to a MMTk builder. pub fn mmtk_init(builder: &MMTKBuilder) -> Box> { - match crate::util::logger::try_init() { - Ok(_) => debug!("MMTk initialized the logger."), - Err(_) => debug!( - "MMTk failed to initialize the logger. Possibly a logger has been initialized by user." - ), - } + crate::util::logger::try_init(); #[cfg(all(feature = "perf_counter", target_os = "linux"))] { use std::fs::File; diff --git a/src/util/logger.rs b/src/util/logger.rs index cc56b93786..5f7cfe2597 100644 --- a/src/util/logger.rs +++ b/src/util/logger.rs @@ -1,9 +1,38 @@ -use log::{self, SetLoggerError}; +//! This module provides a built-in logger implementation. +//! +//! The built-in logger implementation uses the `env_logger` crate. It is enabled by the Cargo +//! feature "builtin_env_logger" which is enabled by default. When enabled, it will be initialized +//! in [`crate::memory_manager::mmtk_init`] and will show logs of levels INFO or lower (the lower, +//! the more important). +//! +//! This provides convenient out-of-the-box experience for binding developers so that they can see +//! logs when using MMTk without configuration, and can easily configure log levels from environment +//! variables. Some bindings may wish to choose a different implementation, or implement their own +//! logging implementations to integrate with the existing logging frameworks of their VMs. In such +//! cases, the binding can disable the Cargo feature "builtin_env_logger" and register their own +//! implementations with the `log` crate. /// Attempt to init a env_logger for MMTk. -pub fn try_init() -> Result<(), SetLoggerError> { - env_logger::try_init_from_env( - // By default, use info level logging. - env_logger::Env::default().filter_or(env_logger::DEFAULT_FILTER_ENV, "info"), - ) +/// Does nothing if the "builtin_env_logger" feature is disabled. +pub(crate) fn try_init() { + cfg_if::cfg_if! { + if #[cfg(feature = "builtin_env_logger")] { + let result = env_logger::try_init_from_env( + // By default, show info level logging. + env_logger::Env::default().filter_or(env_logger::DEFAULT_FILTER_ENV, "info"), + ); + + match result { + Ok(()) => { + debug!("MMTk initialized the logger."); + } + Err(e) => { + // Currently `log::SetLoggerError` can only be raised for one reason: the logger has already been initialized. + debug!("MMTk failed to initialize the built-in env_logger: {e}"); + } + } + } else { + debug!("MMTk didn't initialize the built-in env_logger. The Cargo feature \"builtin_env_logger\" is not enabled."); + } + } }