bnjbvr / cargo-machete

Remove unused Rust dependencies with this one weird trick!
MIT License
784 stars 28 forks source link

Use custom envvar `CARGO_MACHETE_LOG` as log filter #134

Closed oxalica closed 1 month ago

oxalica commented 2 months ago

This makes it more project-specific and avoid spamming unintended logs for users having RUST_LOG=debug in their development environment.

bnjbvr commented 2 months ago

Thanks for the PR, but is this common practice? I've never seen anything like this in any other tool, and I'd suspect you could tweak the RUST_LOG line easily to avoid getting spammed by cargo-machete's log (even if, arguably, this doesn't scale with the number of exclusions you want to set up):

RUST_LOG=debug,cargo_machete=warn

oxalica commented 2 months ago

is this common practice?

I didn't find any documentation explicitly saying which one is better. It would be good if you can find some existing discussion. In my own experience, the name RUST_LOG is overly generic and can easily affect wrong programs when you have many Rust-written tools.

RUST_LOG=debug,cargo_machete=warn

This mostly works, but it requires awareness of cargo-machete, thus is not suitable to commit into the repository as a default direnv setup. Another big issue is that RUST_LOG syntax may not always be compatible, for example, tracing-subscriber has a slightly extended syntax.

You could also argue that user could change to use their own env var instead. But cargo-machete is a development tool whose goal, I believe, is to ease and improve user's development experience. And users hardly really mean to enable cargo-machete's logging.

bnjbvr commented 2 months ago

(For what it's worth, started a small poll on Mastodon to get an idea of what custom renaming of RUST_LOG looks like in the community.)

bnjbvr commented 1 month ago

(The poll has now been deleted, due to my aggressive self-deletion policy, uh, but) The poll's result was that most people don't care, because they don't run cargo-machete at the same time as another program; running cargo-machete as part of another command at the same time would be a great justification to have a separate log env variable, though.

So since this is not the most common use case, let's find another way: do you think it'd make sense to have a way to configure what the env variable with a runtime switch? By default it'd use RUST_LOG as the env logger env variable, and if you pass e.g. --with-custom-log, it'd use CARGO_MACHETE_LOG?

oxalica commented 1 month ago

because they don't run cargo-machete at the same time as another program

Maybe my previous comment is not clear enough, but my use case is NOT a program running at the same time, but having RUST_LOG set for the whole development environment. Because I need them most of time during prototyping. And when occasionally run cargo machete in that environment, it unexpectedly (in my perspective) also print logs.

By default it'd use RUST_LOG as the env logger env variable, and if you pass e.g. --with-custom-log, it'd use CARGO_MACHETE_LOG?

My need is to make cargo-machete easier to NOT print logs in my use case, so this does not fix the issue for me, nor making it easier. Because I still need to cargo machete --with-custom-log NO, which is even longer than RUST_LOG= cargo machete.

If the final decision is to not changing the envvar to use, I'm fine to use my own patched version or add a shell wrapper for it, and it does not take too long the compile.

bnjbvr commented 1 month ago

Sounds good, let's keep it that way then. Thanks for opening a discussion and PR though!