feat(env_filter): support no_std#404
Conversation
Co-authored-by: WolverinDEV <git@did.science>
cagatay-y
changed the title
|
FYI I expect PRs to be presented for how they should be reviewed and merged and not for how they were developed. Please clean up the commit history |
cagatay-y
force-pushed
the
feature/filter-no_std
branch
from
b754c66 to
364726a
Compare
|
|
||
| impl Error for ParseError {} | ||
| #[cfg(feature = "std")] | ||
| #[allow(clippy::std_instead_of_core)] |
There was a problem hiding this comment.
This allow stopped working starting from Rust 1.91. I could not find anything in the release notes that would affect it.
|
@epage, could you take another look at the PR? |
| run: | | ||
| cargo hack check --each-feature --locked --rust-version --ignore-private --package env_logger --all-targets --keep-going | ||
| cargo hack check --each-feature --locked --version-range 1.81..=1.81 --ignore-private --package env_filter --all-targets --keep-going |
There was a problem hiding this comment.
Even though it is not possible to declare a MSRV for env_filter in its Cargo.toml (see the comment on crates/env_filter/Cargo.toml) , I thought it may be useful to have a check in the CI to prevent accidental further bumps for the env_filter MSRV.
| #[cfg(not(feature = "std"))] | ||
| log::warn!("{error}, ignoring it"); |
There was a problem hiding this comment.
This is for creating the logger in the first place, so I don't think it is too meaningful to log at this point.
There was a problem hiding this comment.
At least in my use case, the filter member of the logger is a OnceCell<env_filter::Filter> and until the filter is fully initialised, the logger is returned a Option<&Filter>::None when trying to get it. When it receives the None, it simply lets the log message through, so the user is able to see the warning.
| repository.workspace = true | ||
| license.workspace = true | ||
| edition.workspace = true | ||
| rust-version.workspace = true |
There was a problem hiding this comment.
Why does this now have no MSRV?
There was a problem hiding this comment.
error::Error under core, which is used when the std feature is not enabled, only got stabilised in Rust 1.81, so the MSRV check fails with the workspace MSRV of 1.71. On the other hand, providing a env_filter MSRV of 1.81 causes the env_logger MSRV check to fail, even though it enables the std feature of env_filter and thus is not affected by the absence of core::error::Error. I thought that increasing the MSRV for both to 1.81 would unnecessarily negatively affect the users of env_logger.
There was a problem hiding this comment.
This effectively raises the MSRV of env_logger without anything specifically saying so. We need to find an alternative route or bump the MSRV.
Co-authored-by: WolverinDEV <git@did.science>
cagatay-y
force-pushed
the
feature/filter-no_std
branch
from
364726a to
2a7ae94
Compare
2 participants
Based on #378, this PR adds a new default feature
stdthat allows the crate to be used in no_std crates. I tried to incorporate the feedback that was given to the previous PR. Hopefully, I didn't miss anything.The PR is still a breaking change for crates that have the default features disabled and use
Builder::from_env, or expectBuilder::parseto print warnings to stderr and don't have the log target set to stderr.It may make sense to squash 9c70eaf and 663c9c2 (and potentially b754c66). I kept them separate to make sure I don't misattribute the work of the author of the base PR.