Leverage cargo features to selectively enable logging levels

[kevinaboos]

Hi @rafalh, thanks for your excellent work creating this crate!

Background

I've been experimenting with adding support for FAT32 filesystems in Theseus, my pure-Rust OS, and have successfully used this crate to do so. However, my OS uses the log crate copiously, with logging statements being emitted by a large variety of different sources; all of the other crates allow toggling of log statements for convenience in debugging.

I thought about just taking the simple approach of adding one coarse-grained feature that disables all logging, but then I figured it'd be nice to have warn-level or error-level log statements continue to be emitted by fatfs while bypassing the verbose trace-level logging that is currently emitted.

New feature

Thus, this PR adds support for selectively enabling logging levels using cargo features. I tried a few different approaches and found that this was the best -- it effectively provides overridden versions of the key macros from the log crate that are used transparently across the rest of the fatfs crate. The cargo features are inspired by those exposed by the log crate itself.

The default features have been augmented to enable all log statements from the trace level and up, which corresponds to the current default behavior of the crate.

Kindly let me know if you'd like me to make any changes, I'm happy to do so.

[kevinaboos]

Hi @rafalh, friendly ping in case you missed this. 😃

[rafalh]

Hello @kevinaboos . Sorry for long delay.
I'd like to understand why this change is important. AFAIK you can already disable logging levels from being compiled into binary by using log crate features (see https://docs.rs/log/0.4.14/log/#compile-time-filters). I always assumed that is enough for most use cases. It wouldn't work only if you wanted to keep logging in one crate and disable it in the other, e.g. when debugging your own crate, but in such case in my opinion disabling it in runtime should be enough. Please describe your use case.
Proposed solution requires developer of every library to copy-paste logging macros into crate source code and I don't really like that code duplication and additional complexity. Ideally logging crate should handle all that stuff so developers of other crates could focus on code related to their crate intended purpose.
Could you please show me other crates that implement logging in the same way as in this PR?

[kevinaboos]

Hi @rafalh , thanks for the reply and the great questions!

It wouldn't work only if you wanted to keep logging in one crate and disable it in the other
Please describe your use case.

Yes, the motivation for me introducing this changeset is to get around this problem, which is an issue stemming from how cargo performs feature resolution. If I disable it for one crate, e.g., fatfs, it will disable all other log statements across the entire workspace, which means I get no log statements from all other crates.
This is a problem for our large OS project called Theseus, which consists of several hundreds of crates, many of which emit log statements through the log crate's macros. I envision that other large projects would suffer from this issue if they chose to use fatfs too.

Notably, there are a few alternatives to this design. One easy choice is to make the log crate an optional dependency, meaning that a dependent crate could do something like this:

[dependencies.fatfs]
version = "..."
default-features = false
features = [ ..., "log" ]

That works and is very simple, but it's also quite coarse-grained, as I mentioned in my original post. In the event of something going wrong, I'd personally love to see error and warning-level logs printed by the fatfs crate, but not the trace-level ones.
I figured my solution offers the most flexibility, as other users could elect to see more or fewer log statements depending on the needs of their particular projects. I also feel the design is quite clean and easy to understand, with no impact on the rest of the fatfs codebase.

Could you please show me other crates that implement logging in the same way as in this PR?

There are quite a few, but none are quite as flexible as what I've presented in this PR. I think one of the core issues is that most other third-party crates I've come across don't use as many log statements as fatfs does (not that I would like you to use less -- the more the better!).

For example, the smoltcp crate does something similar but with its own net_log statements as seen here, which is slightly less flexible than my solution here and also requires changes to the actual code that uses the log macros.
The postgres crate also uses a similar solution but is less featureful, and only applies for a couple specific levels used in their project.
I'm sure I could find more examples but I'm low on time.

Let me know what you think. Thanks!

[rafalh]

Please add info about new features in README.md and in CHANGELOG.md

[Xiretza]

Isn't the point of the log crate that the logging backend can filter log messages based on the crate that emitted them? Why don't you just configure your log backend to ignore fatfs' messages?

[kevinaboos]

Sorry for the delay in my response! @rafalh I will add some details shortly when I get the chance.

@Xiretza, yes, you're correct that the logging backend could be configured to skip arbitrary messages. However, that filtering behavior would occur at runtime and would still result in log calls being emitted in the binary, which would cause unnecessary overhead. In contrast, the changes I've proposed here allows one to disable logging levels statically at compile time, which is preferable IMO.

[kevinaboos]

@rafalh I have made the requested changes, and also merged in the latest changes from the master branch to ensure no conflicts arose. Let me know if you'd like to see any other changes.

Thanks again for this excellent crate!

[rafalh]

Thanks for your contribution!