Can you please show how you have tested locally that your proposed change fixes the problem?

There is a reason fmt::localtime was used and not std::localtime:

Converts given time since epoch as std::time_t value into calendar time, expressed in local time. Unlike std::localtime, this function is thread-safe on most platforms.
(From the fmt docs)

This shows the mess that localtime is on various platforms in various versions. This needs to be figured out and tested on all platforms.

Can you please show how you have tested locally that your proposed change fixes the problem?

@lonvia hi Sarah, locally i tested it just by compiling it with fmt 12. see https://copr.fedorainfracloud.org/coprs/tchaikov/fmt-12/build/10289349/ . I am a maintainer of fmt library for fedora, so i need to fix all dependent packages that fail to build when bumping the packaged fmt library in the distro i am targeting. osm2pgsql is one of them. unfortunately, i don't use osm2pgsql myself, and i don't have a pgsql db around to run the test suite against. but i also use COPR/Koji build environment runs the full test suite. see https://download.copr.fedorainfracloud.org/results/tchaikov/fmt-12/fedora-rawhide-aarch64/10289349-osm2pgsql/builder-live.log

@joto i concur with you. so i've rewritten it and repushed the change to replicate the implementation used by fmt::localtime -- including the exception raised by it. hopefully, this would help to build more confidence to that this change is safe.

https://en.cppreference.com/w/c/chrono/localtime.html says localtime_r is C23, but the man page says its in POSIX.1-1196 and SUSv2, so that's plenty old enough for our use case. But it doesn't look like MSVC supports it. And, in fact, the code in fmt is much more complicated than in your commit. Looks like we need to use localtime_s for Windows.

See also fmtlib/fmt#4458 and fmtlib/fmt#4464 and other issues. We can not use std::chrono::time_zone::to_local yet, because that's C++20 (which would be okay to switch to I guess), but it might be unsupported in clang-19 which is a deal-breaker.

@joto hi Jochen, thank you for your insights. indeed, i completely missed that we had to be compatible with windows platforms and removed the MSVC bits in fmt's implementation. just updated the change to use localtime_s() to address the windows portability, as you pointed out std::chrono::time_zone::to_local() is not ready for us yet.