search

mmtk / mmtk-core

Memory Management ToolKit
https://www.mmtk.io
Other
379 stars 69 forks source link

Make env_logger an optional dependency #1226

Closed wks closed 2 weeks ago

wks commented 2 weeks ago

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

wks commented 2 weeks ago

JikesRVM failed once with "Failing instruction starting at f7cbd615 wasn't in RVM address space", a known bug.

k-sareen commented 2 weeks ago

I just have a quick question about this. Let's say in the future we add a new feature to the list of default features, is there a way to selectively disable builtin_env_logger? or will the binding implementer have to disable the entire default feature?

wks commented 2 weeks ago

I just have a quick question about this. Let's say in the future we add a new feature to the list of default features, is there a way to selectively disable builtin_env_logger? or will the binding implementer have to disable the entire default feature?

Unfortunately there is no way to selectively disable some default features for now. There is an issue (https://github.com/rust-lang/cargo/issues/3126) for Cargo, and it is still open now. But the user can always copy the list of default features and explicitly enable some of them.

k-sareen commented 2 weeks ago

In that case, maybe it is better to make it a negation? As in no_builtin_env_logger. Then we can remove it from the default list and since the feature is disabled by default, we get the built-in env_logger.