Remove Utc::now() and Local::now() on unsupported platforms

[ramosbugs]

std::time::SystemTime::now() panics in WASM environments other than Emscripten (i.e., wasm32-unknown-emscripten) or WASI (e.g., wasm32-wasi). Since compilation errors are preferable to unexpected runtime panics, this PR removes the Utc::now() and Local::now() functions from this crate's public interface altogether in unsupported WASM environments unless the wasmbind feature is enabled.

This catches the case in which a user of the crate forgets to enable the wasmbind feature (see ramosbugs/openidconnect-rs#127 and ramosbugs/oauth2-rs#230) in build targets that require it.

This is a breaking change: WASM targets (other than Emscripten/WASI) that referenced Utc::now() or Local::now() would previously compile (then panic at runtime). Now they will fail to compile, which is the intended purpose of this PR.

Fixes #1301.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.99%. Comparing base (332be72) to head (5848a36).

Additional details and impacted files

@@           Coverage Diff           @@
##            0.5.x    #1567   +/-   ##
=======================================
  Coverage   93.99%   93.99%           
=======================================
  Files          37       37           
  Lines       16543    16544    +1     
=======================================
+ Hits        15550    15551    +1     
  Misses        993      993           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ramosbugs]

cc: @pitdicker

[pitdicker]

I just investigated what the standard library does.

It has a platform abstraction layer in the sys::pal module. The list of supported platforms includes some I've never even heard of 😆. Any unsupported platform defaults to the sys::pal::unsupported module, and panics on SystemTime::now().
Also two platforms (wasm and zkvm) define their time module to be sys::pal::unsupported::time.

If we want to be consistent and only use SystemTime::now() on platforms where we know it doesn't panic at runtime but has an implementation maybe we should have a cfg similar to to the one in the platform abstraction layer? https://github.com/rust-lang/rust/blob/master/library/std/src/sys/pal/mod.rs#L27

I.e. only use SystemTime::now() on any(unix, windows, target_os = "solid_asp3", target_os = "hermit", target_os = "wasi", all(target_vendor = "fortanix", target_env = "sgx"), target_os = "teeos", target_os = "zkvm").

[djc]

I think unix, windows, wasi are likely to be much more common than any of the other ones, and I'm okay with waiting for issues that ask us to enable support for the rest (since there isn't really a good way we can keep our list in sync with std over time I think it makes sense to be more conservative).

[pitdicker]

@ramosbugs Do you want to update this PR to only make now() available for unix, windows, wasi and wasm with the wasmbind feature?

[ramosbugs]

@ramosbugs Do you want to update this PR to only make now() available for unix, windows, wasi and wasm with the wasmbind feature?

Sure. Do we still want emscripten support in addition to wasi, or should that be dropped as well?

[ramosbugs]

I think unix, windows, wasi are likely to be much more common than any of the other ones, and I'm okay with waiting for issues that ask us to enable support for the rest (since there isn't really a good way we can keep our list in sync with std over time I think it makes sense to be more conservative).

This does seem like it will break chrono for niche targets that work fine today and for which SystemTime::now is implemented by the standard library. I agree that it may be difficult to keep the list in sync with the standard library in the future, but I assume the standard library will only ever add support for new targets. It's hard to imagine them removing support for existing targets.

Consequently, I think it would make sense to match the current list of targets supported by the standard library, and wait for issues to get filed after support is added for other targets in the future. Why wait for issues to get filed for supporting targets that are already supported by chrono today? The only tradeoff I see is slightly more complicated #[cfg(...)] attributes.

[djc]

I don't think it's true that std will only ever add targets, but I see your point. Okay, let's add a similar expression to what std has, and please link to the current version of the std attribute so we can easily verify updates.

[ramosbugs]

I.e. only use SystemTime::now() on any(unix, windows, target_os = "solid_asp3", target_os = "hermit", target_os = "wasi", all(target_vendor = "fortanix", target_env = "sgx"), target_os = "teeos", target_os = "zkvm").

On closer inspection, SystemTime::time() is also unsupported for zkvm, but seems to be implemented for all the others in the PAL table except wasm and uefi: https://github.com/search?q=repo%3Arust-lang%2Frust%20path%3Alibrary%2Fstd%2Fsrc%2Fsys%2Fpal%20%22pub%20mod%20time%22&type=code. This includes an OS called xous.

Support for uefi was implemented in rust-lang/rust@92d4060 but has not yet been released to stable Rust (as of 1.77.2).

Working on a PR with the following config logic:

    #[cfg(any(
        unix,
        windows,
        target_os = "solid_asp3",
        target_os = "hermit",
        target_os = "wasi",
        target_os = "xous",
        all(target_vendor = "fortanix", target_env = "sgx"),
        target_os = "teeos",
        all(target_arch = "wasm32", feature = "wasmbind")
    ))]

[pitdicker]

On closer inspection, SystemTime::time() is also unsupported for zkvm, but seems to be implemented for all the others in the PAL table except wasm and uefi: https://github.com/search?q=repo%3Arust-lang%2Frust%20path%3Alibrary%2Fstd%2Fsrc%2Fsys%2Fpal%20%22pub%20mod%20time%22&type=code. This includes an OS called xous.

Oops, did I end up confusing zkvm and xous after all? 👍

Support for uefi was implemented in rust-lang/rust@92d4060 but has not yet been released to stable Rust (as of 1.77.2).

Cool! It should be part of rust 1.78. Given that it will be stable on 2024-05-02 I think you can already add it. We are definitely not going to release 0.5 before that time 🤣 (I hope this year though).

[ramosbugs]

Cool! It should be part of rust 1.78. Given that it will be stable on 2024-05-02 I think you can already add it. We are definitely not going to release 0.5 before that time 🤣 (I hope this year though).

Sounds good. I confirmed it's in the beta channel: https://github.com/rust-lang/rust/blob/beta/library/std/src/sys/pal/uefi/mod.rs#L35

[pitdicker]

Thank you @ramosbugs!