Use the interpreted program's TZ variable in localtime_r #3523
Conversation
saethlin
force-pushed
the
localtime_r-env
branch
from
0f50e32 to
e4b2973
Compare
Cargo.toml
Outdated
| @@ -25,6 +25,7 @@ aes = { version = "0.8.3", features = ["hazmat"] } | |||
| measureme = "11" | |||
| ctrlc = "3.2.5" | |||
| chrono = { version = "0.4.38", default-features = false, features = ["clock"] } | |||
| chrono-tz = "0.4" | |||
There was a problem hiding this comment.
Why did you add such an ancient version of chrono-tz? The current version is 0.9.
There was a problem hiding this comment.
Probably because 4 looks like 9.
src/shims/time.rs
Outdated
| let tz = this.get_var(OsStr::new("TZ"))?.unwrap_or_else(|| OsString::from("UTC")); | ||
| let tz = match tz.into_string() { | ||
| Ok(tz) => Tz::from_str(&tz).unwrap_or(Tz::Zulu), | ||
| _ => Tz::Zulu, |
There was a problem hiding this comment.
Why is Zulu the fallback, not UTC...?
src/shims/time.rs
Outdated
| @@ -146,17 +156,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { | |||
|
|
|||
| // tm_zone represents the timezone value in the form of: +0730, +08, -0730 or -08. | |||
| // This may not be consistent with libc::localtime_r's result. | |||
| let offset_in_second = Local::now().offset().local_minus_utc(); | |||
| let tm_gmtoff = offset_in_second; | |||
| let offset_in_seconds = Utc::now().with_timezone(&tz).offset().fix().local_minus_utc(); | |||
There was a problem hiding this comment.
Can't dt be used for this, why do we need Utc::now?
Cargo.toml
Outdated
| @@ -25,6 +25,7 @@ aes = { version = "0.8.3", features = ["hazmat"] } | |||
| measureme = "11" | |||
| ctrlc = "3.2.5" | |||
| chrono = { version = "0.4.38", default-features = false, features = ["clock"] } | |||
There was a problem hiding this comment.
If I understand the docs correctly, we don't need the clock feature any more?
saethlin
force-pushed
the
localtime_r-env
branch
from
11639d6 to
69ed236
Compare
src/shims/unix/env.rs
Outdated
| Size::from_bytes(u64::try_from(name.len()).unwrap().checked_add(1).unwrap()), | ||
| ecx, | ||
| )?; | ||
| ecx.read_os_str_from_c_str(var_ptr).map(|s| Some(s.to_owned())) |
There was a problem hiding this comment.
This duplicates a bunch of code in getenv, doesn't it? Can the code be shared, e.g. by having getenv call this?
There was a problem hiding this comment.
It does duplicate some code. getenv can't call it as-written but with some rearrangement it's possible to share the logic.
src/shims/env.rs
Outdated
| pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { | ||
| /// Try to get an environment variable from the interpreted program's environment. This is | ||
| /// useful for implementing shims which are documented to read from the environment. | ||
| fn get_var(&mut self, name: &OsStr) -> InterpResult<'tcx, Option<OsString>> { |
There was a problem hiding this comment.
get_env_var may be better since this is a global namespace.
|
@bors r+ |
|
☀️ Test successful - checks-actions |
This requires a bit of wiring and a new dependency, but the tests should correctly pass now regardless of what the host's time zone is.
Fixes #3522