on linux if TZ env var set and starts with ':', use as timezone #144
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lopopolo
reviewed
Nov 24, 2024
| @@ -1,6 +1,17 @@ | |||
| use std::fs::{read_link, read_to_string}; | |||
|
|
|||
| pub(crate) fn get_timezone_inner() -> Result<String, crate::GetTimezoneError> { | |||
| if let Some(tz) = std::env::var_os("TZ") { | |||
There was a problem hiding this comment.
Given that env::var_os is due to be made unsafe in Rust 2024, I'm not comfortable with this. If we do support reading from the TX env var, I'd rather it be gated by a feature and exposed via a new entry point.
At least in Artichoke, I'd not use this as is since my application must run its own hooks on env accesses and e.g. allow disabling all env accesses via a feature for sandboxing.
There was a problem hiding this comment.
I'd make the same argument. Interacting with environment variables is IMHO out-of-scope of this library.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an idea to address #118 on linux. What do folks think? @lopopolo @Kijewski