baoyachi / shadow-rs

A build-time information stored in your rust project.(binary,lib,cdylib,dylib)
https://docs.rs/shadow-rs
MIT License
447 stars 41 forks source link

Use tzdb to support Windows hosts, too #107

Closed Kijewski closed 2 years ago

Kijewski commented 2 years ago

tz-rs's TimeZone::local() is only supported for Linux hosts, but will fail e.g. on Windows. Shadow-rs will fallback to use UTC dates, so it will still be usable on Window.

tzdb's local_tz() bundles the Timezone Database, and works on Linux, Windows, MacOS, FreeBSD, and NetBSD. While having local datetimes on these host platforms is probably not particularly important, I still think it's a better user experience.

tzdb itself uses tz-rs to make use of the timezone data.

baoyachi commented 2 years ago

Thx your PR. @Kijewski check github action ci error:

Run cargo clippy --all-targets --all-features -- -D warnings
error: unnecessary closure used to substitute value for `Option::None`
  --> src/data_time.rs:53:31
   |
53 |         let time_zone_local = tzdb::local_tz().ok_or_else(|| "Could not get local timezone")?;
   |                               ^^^^^^^^^^^^^^^^^---------------------------------------------
   |                                                |
   |                                                help: use `ok_or(..)` instead: `ok_or("Could not get local timezone")`
   |
   = note: `-D clippy::unnecessary-lazy-evaluations` implied by `-D warnings`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
...
Kijewski commented 2 years ago

Oops, fixed.

baoyachi commented 2 years ago

Woh, so quick. đź‘Ť

baoyachi commented 2 years ago

Thx again. @Kijewski

Kijewski commented 2 years ago

I was just checking Github before going to bed. :) Thank your for the library, too. It helps a lot to have that info handy in the logs, etc.

appaquet commented 2 years ago

It seems like tzdb can’t compile on iOS making shadow-rs fail to compile on aarch64-apple-ios-sim:

Compiling iana-time-zone v0.1.35
[39](https://github.com/appaquet/exocore/runs/7457038365?check_suite_focus=true#step:6:40)
error[E0433]: failed to resolve: use of undeclared crate or module `platform`
[40](https://github.com/appaquet/exocore/runs/7457038365?check_suite_focus=true#step:6:41)
  --> /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/iana-time-zone-0.1.35/src/lib.rs:88:5
[41](https://github.com/appaquet/exocore/runs/7457038365?check_suite_focus=true#step:6:42)
   |
[42](https://github.com/appaquet/exocore/runs/7457038365?check_suite_focus=true#step:6:43)
88 |     platform::get_timezone_inner()
[43](https://github.com/appaquet/exocore/runs/7457038365?check_suite_focus=true#step:6:44)
   |     ^^^^^^^^ use of undeclared crate or module `platform`

It’s probably a matter of adding iOS as a target in iana-time-zone: https://github.com/strawlab/iana-time-zone/blob/7b3b806de4bff14d569a82f95a20d6bceb41a1f6/Cargo.toml#L13

I can open another issue if needed @baoyachi.

baoyachi commented 2 years ago

@appaquet Thx your report. I'll take a look later

astraw commented 2 years ago

iana-time-zone 0.1.37 now includes iOS support. Thanks everyone.

astraw commented 2 years ago

iana-time-zone 0.1.37 now includes iOS support. Thanks everyone.