Skip to content

Upstream libfmt >=11.0 fix; remove deprecated localtime#54

Open
nmhare wants to merge 2 commits intoproject-slippi:slippifrom
nmhare:localtime
Open

Upstream libfmt >=11.0 fix; remove deprecated localtime#54
nmhare wants to merge 2 commits intoproject-slippi:slippifrom
nmhare:localtime

Conversation

@nmhare
Copy link

@nmhare nmhare commented Jan 14, 2026

Replace deprecated fmt::localtime with common::localtime for fmt 11
Remove guipriv warning text from Qt6.10

@nmhare
Copy link
Author

nmhare commented Jan 14, 2026

This is the same fix as applied here dolphin-emu#13727

@NikhilNarayana
Copy link
Member

is there a specific reason to pull in this commit on its own? i prefer to only do this if there is a breaking issue with builds and to cherry-pick it directly into our branch from upstream

@nmhare
Copy link
Author

nmhare commented Jan 15, 2026

Yes the 12.0.0 release of libfmt (https://github.com/fmtlib/fmt/releases) deprecated the fmt::localtime() function and caused my build to fail on latest arch, with the error ‘localtime’ is not a member of ‘fmt’. This fixes that error

Let me know if you need any more information

@aurzenith
Copy link

Can I ask why your changes differ from the fixes that are merged in the upstream repo? Specifically in TimeUtil.cpp. Also adding in a random CMake warning text change on top of this is a little strange.

@nmhare
Copy link
Author

nmhare commented Jan 20, 2026

Hello, apologies for confusion, I am new to this. I promise I'm not an AI

Can I ask why your changes differ from the fixes that are merged in the upstream repo? Specifically in TimeUtil.cpp.

I did these changes manually and only changed the capitalization from Localtime to LocalTime, I simply missed TimeUtil.cpp. If pulling these changes in from upstream is proper, please do that.

Also adding in a random CMake warning text change on top of this is a little strange.

This is in response to a recent commit daae8a0 which updated for Qt6.10+. This creates the following warning:

Project MESSAGE: This project is using private headers and will therefore be tied to this specific Qt module build version.
Project MESSAGE: Running this project against other versions of the Qt modules may crash at any arbitrary point.
Project MESSAGE: This is not a bug, but a result of using Qt internals. You have been warned!

Since Slippi is not using any private headers, but only now needs to include the header file, I thought it'd be fine to suppress. I found a similar fix in the RPCS3 repo here https://github.com/RPCS3/rpcs3/pull/17589/files and thought to include this suppression. If this is not recommended or should be in a different PR, I can remove (although maybe pulling in that dolphin commit is better?).

std::optional<std::tm> LocalTime(std::time_t time)
{
std::tm local_time;
#ifdef _MSC_VER

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more changes in the original merged commit that should be added to this.

@aurzenith
Copy link

I see, the fix for CMake isn't a bad idea, but typically keep a PR focused on one thing and not add anything unrelated to it. As approving the one part, will also approve the other part, and keeps reviewing easier and cleaner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants