Remove the usage of set_var
#886
Conversation
`std::env::set_var` is marked as unsafe starting in the 2024 Edition. https://doc.rust-lang.org/edition-guide/rust-2024/newly-unsafe-functions.html
|
Can you also look for other usages in the repo and replace them as well? |
ChenRuiwei
changed the title
|
Grep all usages of |
| std::env::set_var( | ||
| "RUST_LOG", | ||
| std::env::var("RUST_LOG") | ||
| .or_else(|_| std::env::var(LOG_ENV.clone())) | ||
| .unwrap_or_else(|_| format!("{}=info", env!("CARGO_CRATE_NAME"))), | ||
| ); | ||
| unsafe { | ||
| std::env::set_var( | ||
| "RUST_LOG", | ||
| std::env::var("RUST_LOG") | ||
| .or_else(|_| std::env::var(LOG_ENV.clone())) | ||
| .unwrap_or_else(|_| format!("{}=info", env!("CARGO_CRATE_NAME"))), | ||
| ) | ||
| }; |
There was a problem hiding this comment.
It's probably better to remove the set_var call instead of making this unsafe, and configuring EnvFilter below properly.
There was a problem hiding this comment.
I'm okay with either way, we can also add a comment why this is unsafe etc.
Maybe it would be nice to show both ways?
There was a problem hiding this comment.
Not really - it's there because EnvFilter uses RUST_LOG in the call later down, but it doesn't have to. Envfilter has a way to choose the env variable to use, and it has a way to set a default directive that's used if the env variable is not set. And it has a way to return an error if the variable is not set. I.e. all the things that this code tries to do here are handled by correctly using EnvFilter::builder. The set_var call is unnecessary.
The usage of [`from_default_env`](https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html#method.from_default_env) is equivelent to ```rust use tracing_subscriber::filter::{EnvFilter, LevelFilter}; EnvFilter::builder() .with_default_directive(LevelFilter::ERROR.into()) .from_env_lossy() ``` While in this case the `with_default_directive` will never be reached because we always set a **vaild** crate level filter string to `RUST_LOG`. And [`from_env_lossy`](https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.Builder.html#method.from_env_lossy) [uses `parse_lossy` internally](https://docs.rs/tracing-subscriber/latest/src/tracing_subscriber/filter/env/builder.rs.html#168-171)
ChenRuiwei
changed the title
set_var
| .or_else(|_| std::env::var(LOG_ENV.clone())) | ||
| .unwrap_or_else(|_| format!("{}=info", env!("CARGO_CRATE_NAME"))), | ||
| ); | ||
| let filter_string = std::env::var("RUST_LOG") |
There was a problem hiding this comment.
nit:
| let filter_string = std::env::var("RUST_LOG") | |
| let log_filter = std::env::var("RUST_LOG") |
3 participants
std::env::set_varis marked as unsafe starting in the 2024 Edition.https://doc.rust-lang.org/edition-guide/rust-2024/newly-unsafe-functions.html