Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[log][v2] How to convert log time to local time? #1926

Closed
ayangweb opened this issue Oct 13, 2024 · 8 comments
Closed

[log][v2] How to convert log time to local time? #1926

ayangweb opened this issue Oct 13, 2024 · 8 comments
Labels
bug Something isn't working platform: android Android specific issues platform: ios iOS specific issues platform: linux Linux specific issues platform: macos macOS specific issues plugin: log status: upstream This issue needs to be fixed in an upstream project

Comments

@ayangweb
Copy link
Contributor

ayangweb commented Oct 13, 2024

Currently using UseLocal is not local time? How do I convert them correctly?

...
.plugin(
    tauri_plugin_log::Builder::new()
        .timezone_strategy(TimezoneStrategy::UseLocal)
        .build(),
)

Log time:
iShot_2024-10-13_20 12 38

Local time:
iShot_2024-10-13_20 12 08

@FabianLars FabianLars added question Further information is requested plugin: log labels Oct 13, 2024
@ayangweb ayangweb changed the title [feat][log][v2] How to convert log time to local time? [log][v2] How to convert log time to local time? Oct 13, 2024
@Legend-Master
Copy link
Contributor

TimezoneStrategy::UseLocal works for me here on Windows 10

I wonder if error you got is because of this errored out and fall backed to use UTC time instead, do you mind trying time-rs's time::OffsetDateTime::now_local() and see it works?

OffsetDateTime::now_local().unwrap_or_else(|_| OffsetDateTime::now_utc())

@ayangweb
Copy link
Contributor Author

ayangweb commented Oct 19, 2024

Also wrong

image

@Legend-Master Legend-Master added bug Something isn't working status: upstream This issue needs to be fixed in an upstream project platform: macos macOS specific issues and removed question Further information is requested labels Oct 19, 2024
@Legend-Master
Copy link
Contributor

Legend-Master commented Oct 19, 2024

Then it's probably a bug in time-rs, I recommend you open a bug report there (I don't have a mac device so I can't help much here)

@ayangweb
Copy link
Contributor Author

@Legend-Master Hey👋, there's a reply. time-rs/time#713 (comment)

@Legend-Master
Copy link
Contributor

Oh well, after some digging, it seems like you need TZ (stands for Time Zone) environment variable on unix systems to get the time zone, but getting the environment variable is not thread safe, so time just disabled this on all multi-threaded usages, chrono seems to use std::env::var which has a lock internally but it's only thread safe if there're no C calls to modify the environment variable

Not sure what we can do here, maybe switch to chrono?

Also see time-rs/time#293 (comment)

@Legend-Master Legend-Master added platform: android Android specific issues platform: linux Linux specific issues platform: ios iOS specific issues labels Oct 19, 2024
@jhpratt
Copy link

jhpratt commented Oct 19, 2024

Incidentally, the last I checked chrono's behavior is unsound as well, just in slightly fewer situations. Even relying on only the standard library has been demonstrated to be unsound in extreme situations.

For what it's worth I am actively investigating a potential solution to make this work in multi-threaded situations on Unix while remaining sound and having the expected behavior in most cases. I think it's workable, but I should know for sure this coming week.

@ayangweb
Copy link
Contributor Author

@Legend-Master @jhpratt Ok, thank you both, I won't test chrono then, looking forward to you guys solving this problem!

@Legend-Master
Copy link
Contributor

I believe this is fixed by time v0.3.37 now, thanks @jhpratt!

Obtaining the system UTC offset on Unix-like systems should now succeed when multi-threaded. However, if the TZ environment variable is altered, the program will not be aware of this until time::util::refresh_tz or time::util::refresh_tz_unchecked is called. refresh_tz has the same soundness requirements as obtaining the system UTC offset previously did, with the requirements still being automatically enforced. refresh_tz_unchecked does not enforce these requirements at the expense of being unsafe. Most programs should not need to call either function.

Due to this change, the time::util::local_offset module has been deprecated in its entirety. The get_soundness and set_soundness functions are now no-ops.

Note that while calls should succeed, success is not guaranteed in any situation. Downstream users should always be prepared to handle the error case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working platform: android Android specific issues platform: ios iOS specific issues platform: linux Linux specific issues platform: macos macOS specific issues plugin: log status: upstream This issue needs to be fixed in an upstream project
Projects
None yet
Development

No branches or pull requests

4 participants