Re: Thread-safe nl_langinfo() and localeconv()
Thomas Munro <thomas.munro@gmail.com>
From: Thomas Munro <thomas.munro@gmail.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-08-13T23:27:50Z
Lists: pgsql-hackers
Attachments
- 0001-Harmonize-MinGW-CODESET-lookup-with-MSVC.patch (text/x-patch) patch 0001
Here's another mystery from Windows + MinGW. Although "fairywren" is green, that is because it lacks ICU, which would activate extra tests. CI is green too, but the optional CI task "Windows - Server 2019, MinGW64 - Meson" has ICU and it is now failing if you trigger it[1] after commit 35eeea62, in initdb/001_initdb: [05:43:49.764] | 146/305 - options --locale-provider=icu --locale=und --lc-*=C: no stderr FAIL ... because it logs a warning to stderr: WARNING: no usable system locales were found I can only assume there was some extra dependency on setlocale() global state changes in the removed code. I don't quite get it, but whatever the reason, it's less than helpful to have different compilers taking different code paths on our weirdest OS that most of us don't use, so I propose to push this change to take the regular MSVC code path for MinGW too, when looking up code pages. Somehow, this fixes that, though it'd probably take someone with a local MinGW setup to dig into what exactly is happening there. (There are plenty more places where we do something different for MinGW. I suspect they are all obsolete problems. We should probably just harmonise everything and see what breaks now that we have a CI system, but that can be for another day.) That warning is from pg_import_system_locales(), which is new-ish (v16) on that OS. It was recently discovered to trigger a pre-existing problem[2]: the simple setlocale() save/restore pattern doesn't work in general on Windows, because some local names are non-ASCII, and the restore can fail (abort in the system library due to bad encoding, because the intermediate setlocale() changed the expected encoding of the locale name itself). So it's good that we aren't doing that anymore in this location; I'm just thinking out loud about whether that phenomenon could also be somehow connected to this failure, though I don't see it. Another realisation is that my pg_localeconv_r() patch, which can't avoid a thread-safe setlocale() save-and-restore on that OS (and might finish up being the only one left in the tree by the time we're done?), had better use wsetlocale() instead to defend itself against that particular madness. [1] https://cirrus-ci.com/task/5928104793735168 [2] https://www.postgresql.org/message-id/CA%2BhUKG%2BFxeRLURZ%3Dn8NPyLwgjFds_SqU_cQvE40ks6RQKUGbGg%40mail.gmail.com
Commits
-
Fix "‘static’ is not at beginning of declaration" warning
- 116e851db5b3 18.0 landed
-
Use thread-safe strftime_l() instead of strftime().
- 890fc826c91f 18.0 landed
-
Provide thread-safe pg_localeconv_r().
- b98be8a2a2a6 18.0 landed
-
Harmonize MinGW CODESET lookup with MSVC.
- bf3401fe813a 18.0 landed
-
Include <xlocale.h> for macOS, take II.
- 2488058dc356 18.0 landed
-
Use thread-safe nl_langinfo_l(), not nl_langinfo().
- 35eeea623022 18.0 landed
-
All POSIX systems have langinfo.h and CODESET.
- 14c648ff0094 18.0 landed