Consider emitting a diagnostic message when multiple incompatible log level features are used #481
Comments
|
Cargo suppresses all warnings produced by dependencies, so even if we emitted a warning you wouldn't be able to see it in normal compilation contexts. |
|
How about a compile error then, instead? I mean multiple max_level_* features set, suggests misuse, doesn't it? |
|
It is a bit of an unfortunate (mis)use of Cargo features, isn't it. Ideally I'd like to get away from the |
EFanZh
pushed a commit
to EFanZh/log
that referenced
this issue
Jul 23, 2023
…ang#481) Extraction wasn't cancellable by `cancel_on_user_sig_term` used in `entry` since it calls `block_in_place`. This PR adds cancellation support to it by adding a `static` variable `OnceCell` to `wait_on_cancellation_signal` so that once it returns `Ok(())`, all other calls to it after that point also returns `Ok(())` immediately. `StreamReadable`, which is used in cancellation process, then stores a boxed future of `wait_on_cancellation_signal` and polled it in `BufReader::fill_buf`. Note that for zip, the extraction process takes `File` instead of `StreamReadable` due to `io::Seek` requirement, so it cancelling during extraction for zip is still not possible. This PR also optimized `extract_bin` and `extract_zip` by using `StreamReadable::copy` introduced to this PR instead of `io::copy`, which allocates an internal buffer on stack, which imposes extra copy. It also fixed `StreamReadable::fill_buf` by ensuring that empty buffer is only returned on eof. * Make `wait_on_cancellation_signal` pub * Enable feature `parking_lot` of dep tokio * Mod `wait_on_cancellation_signal`: Use `OnceCell` internally to archive the effect that once call to it return `Ok(())`, all calls to it after that also returns `Ok(())`. * Impl `From<BinstallError>` for `io::Error` * Impl cancellation on user signal in `StreamReadable` * Fix err msg when cancelling during extraction in `ops::resolve` * Optimize: Impl & use `StreamReadable::copy` which is same as `io::copy` but does not allocate any internal buffer since `StreamReadable` is buffered. * Fix `next_stream`: Return non-empty bytes on `Ok(Some(bytes))` Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
So today I learned the hard way that crate features are additive. If a program directly or indirectly through other dependencies, depends on a crate of the same version multiples times with different features, it will use the union of features. It does not introduce multiple copies of the crate like what happens with a version mismatch. This is pretty logical behavior I suppose, but it's a bit annoying when you have features that are mutually incompatible (like static max output level in logs).
In my specific case, I had some far flung dependency setting
release_max_level_info, when I was trying to do trace level logging.The real problem here was a dependency library setting log level features. I'll sort that out with that library, but when I was trying to figure out what was going on, I hadn't considered that multiple features were active simultaneously until I did a bunch of digging (verbose builds, grepping through
.cargoetc.). I could see legitimate use cases for multiple log levels perhaps, so I wouldn't make this an error. A compiler warning or message saying something to the effect of "hey you've got bothrelease_max_level_infoandrelease_max_level_trace, we're going to go withinfo" (but obviously more generic than that) could be very helpful.The text was updated successfully, but these errors were encountered: