On Thu, Nov 21, 2024 at 8:38 PM Peter Eisentraut <pe...@eisentraut.org> wrote: > On 20.11.24 10:00, Thomas Munro wrote: > > OK, do you think these three patches tell the _configthreadlocale() > > story properly? (Then after that we can get back to getting rid of > > it...) > > Yes, this is very clear and helpful. Thanks.
I realised that there is another aspect to this: it must be impossible to build PostgreSQL with the original MinGW/MSYS project by now. I don't understand the history of the MinGW/MinGW-w64 fork, but if they're both still live projects out there adding to the general confusion about the frankenwindows multiverse, we should clarify our situation. As far as I know, we're only testing the second thing, and only the second thing can use UCRT, and only the second thing is a viable alternative toolchain for software that is primarily targeting current Visual Studio, which I think is something we can say about our project. Right? I was looking at the installation.sgml file and wondering where to write "UCRT required" in the 0002 patch. Perhaps we don't even need to mention it if it's the default on MinGW-w64, but I think we do need to remove the discussion of the original MinGW. It's even pointing to a cybersquatted URL. I also suspect that MinGW-w64 shouldn't be described as for "building 64 bit binaries" (maybe it started that way but by now it's has other goals, modern libraries/APIs etc keeping up with VC; and AFAICS it can build 32 bit binaries too, if anyone still cares about that; wow, the naming of all this stuff is so confusing to a non-Windows person, 32 and 64 get thrown around all over the place with apparently no real meaning, but I digress...). Andrew, would you be interested in updating this section to talk only about the modern installation and build steps for MinGW-w64/MSYS2 et al? There are several confusingly related projects and I'm not sure I'd get it right if I tried, I've never used any of it...
From fbe7be0c0c1d4e2b047d2dcfb0ba5ccd9ce0994a Mon Sep 17 00:00:00 2001 From: Thomas Munro <tmu...@postgresql.org> Date: Sat, 23 Nov 2024 08:44:02 +1300 Subject: [PATCH v2 1/3] Remove configure check for _configthreadlocale(). All modern Windows systems have _configthreadlocale(). It was first introduced in msvcr80.dll from Visual Studio 2005. Historically, MinGW was stuck on even older msvcrt.dll, but added its own dummy implementation of the function when using msvcrt.dll years ago anyway, effectively rendering the configure test useless. In practice we don't encounter the dummy anymore because modern MinGW uses ucrt. Reviewed-by: Peter Eisentraut <pe...@eisentraut.org> Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech --- configure | 11 ----------- configure.ac | 1 - meson.build | 1 - src/include/pg_config.h.in | 3 --- src/interfaces/ecpg/ecpglib/descriptor.c | 4 ++-- src/interfaces/ecpg/ecpglib/ecpglib_extern.h | 2 +- src/interfaces/ecpg/ecpglib/execute.c | 4 ++-- 7 files changed, 5 insertions(+), 21 deletions(-) diff --git a/configure b/configure index 28719ed30c..07dd2dec1b 100755 --- a/configure +++ b/configure @@ -15917,17 +15917,6 @@ fi # Win32 (really MinGW) support if test "$PORTNAME" = "win32"; then - for ac_func in _configthreadlocale -do : - ac_fn_c_check_func "$LINENO" "_configthreadlocale" "ac_cv_func__configthreadlocale" -if test "x$ac_cv_func__configthreadlocale" = xyes; then : - cat >>confdefs.h <<_ACEOF -#define HAVE__CONFIGTHREADLOCALE 1 -_ACEOF - -fi -done - case " $LIBOBJS " in *" dirmod.$ac_objext "* ) ;; *) LIBOBJS="$LIBOBJS dirmod.$ac_objext" diff --git a/configure.ac b/configure.ac index 533f4ab78a..184b920e3e 100644 --- a/configure.ac +++ b/configure.ac @@ -1825,7 +1825,6 @@ fi # Win32 (really MinGW) support if test "$PORTNAME" = "win32"; then - AC_CHECK_FUNCS(_configthreadlocale) AC_LIBOBJ(dirmod) AC_LIBOBJ(kill) AC_LIBOBJ(open) diff --git a/meson.build b/meson.build index b64d253fe4..a56f3311ea 100644 --- a/meson.build +++ b/meson.build @@ -2614,7 +2614,6 @@ endif # XXX: Might be worth conditioning some checks on the OS, to avoid doing # unnecessary checks over and over, particularly on windows. func_checks = [ - ['_configthreadlocale', {'skip': host_system != 'windows'}], ['backtrace_symbols', {'dependencies': [execinfo_dep]}], ['clock_gettime', {'dependencies': [rt_dep], 'define': false}], ['copyfile'], diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index a903c60a3a..e53560c87f 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -535,9 +535,6 @@ /* Define to 1 if your compiler understands __builtin_unreachable. */ #undef HAVE__BUILTIN_UNREACHABLE -/* Define to 1 if you have the `_configthreadlocale' function. */ -#undef HAVE__CONFIGTHREADLOCALE - /* Define to 1 if you have __cpuid. */ #undef HAVE__CPUID diff --git a/src/interfaces/ecpg/ecpglib/descriptor.c b/src/interfaces/ecpg/ecpglib/descriptor.c index ad279e245c..56e2bc4153 100644 --- a/src/interfaces/ecpg/ecpglib/descriptor.c +++ b/src/interfaces/ecpg/ecpglib/descriptor.c @@ -490,7 +490,7 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...) Assert(ecpg_clocale); stmt.oldlocale = uselocale(ecpg_clocale); #else -#ifdef HAVE__CONFIGTHREADLOCALE +#ifdef WIN32 stmt.oldthreadlocale = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE); #endif stmt.oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno); @@ -510,7 +510,7 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...) setlocale(LC_NUMERIC, stmt.oldlocale); ecpg_free(stmt.oldlocale); } -#ifdef HAVE__CONFIGTHREADLOCALE +#ifdef WIN32 if (stmt.oldthreadlocale != -1) (void) _configthreadlocale(stmt.oldthreadlocale); #endif diff --git a/src/interfaces/ecpg/ecpglib/ecpglib_extern.h b/src/interfaces/ecpg/ecpglib/ecpglib_extern.h index bad3cd9920..75cc68275b 100644 --- a/src/interfaces/ecpg/ecpglib/ecpglib_extern.h +++ b/src/interfaces/ecpg/ecpglib/ecpglib_extern.h @@ -77,7 +77,7 @@ struct statement locale_t oldlocale; #else char *oldlocale; -#ifdef HAVE__CONFIGTHREADLOCALE +#ifdef WIN32 int oldthreadlocale; #endif #endif diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c index c578c21cf6..466d5600f9 100644 --- a/src/interfaces/ecpg/ecpglib/execute.c +++ b/src/interfaces/ecpg/ecpglib/execute.c @@ -1995,7 +1995,7 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator, return false; } #else -#ifdef HAVE__CONFIGTHREADLOCALE +#ifdef WIN32 stmt->oldthreadlocale = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE); #endif stmt->oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno); @@ -2219,7 +2219,7 @@ ecpg_do_epilogue(struct statement *stmt) #else if (stmt->oldlocale) setlocale(LC_NUMERIC, stmt->oldlocale); -#ifdef HAVE__CONFIGTHREADLOCALE +#ifdef WIN32 /* * This is a bit trickier than it looks: if we failed partway through -- 2.47.0
From a5e4b17a10be22e20d81252783094753ff0afda3 Mon Sep 17 00:00:00 2001 From: Thomas Munro <tmu...@postgresql.org> Date: Sat, 23 Nov 2024 09:14:48 +1300 Subject: [PATCH v2 2/3] Formally require ucrt on Windows. Historically we tolerated the absence of various C runtime library features for the benefit of the MinGW tool chain, because it used ancient msvcrt.dll for a long period of time. It now uses ucrt by default (like Windows 10+, Visual Studio 2015+), and that's the only configuration we're testing. In practice, we effectively required ucrt already in PostgreSQL 17, when commit 8d9a9f03 required _create_locale etc, first available in msvcr120.dll (Visual Studio 2013, the last of the pre-ucrt series of runtimes), and for MinGW users that practically meant ucrt because it was difficult or impossible to use msvcr120.dll. That may even not have been the first such case, but old MinGW configurations had already dropped off our testing radar so we weren't paying much attention. This commit formalizes the requirement. It also removes a couple of obsolete comments that discussed msvcrt.dll limitations, and some tests of !defined(_MSC_VER) to imply msvcrt.dll. There are many more anachronisms, but it'll take some time to figure out how to remove them all. XXX Help needed to rewrite doc/src/sgml/installation.sgml, to clarify that we now only support https://en.wikipedia.org/wiki/Mingw-w64 and MSYS2, not https://en.wikipedia.org/wiki/MinGW and MSYS (no one is testing the latter, and it probably can't build PostgreSQL 17, since it can't use UCRT?) Reviewed-by: Peter Eisentraut <pe...@eisentraut.org> Discussion: https://postgr.es/m/d9e7731c-ca1b-477c-9298-fa51e135574a%40eisentraut.org --- src/backend/utils/adt/pg_locale.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c index d4e89663ec..d1b392c248 100644 --- a/src/backend/utils/adt/pg_locale.c +++ b/src/backend/utils/adt/pg_locale.c @@ -1017,13 +1017,6 @@ cache_locale_time(void) * get ISO Locale name directly by using GetLocaleInfoEx() with LCType as * LOCALE_SNAME. * - * MinGW headers declare _create_locale(), but msvcrt.dll lacks that symbol in - * releases before Windows 8. IsoLocaleName() always fails in a MinGW-built - * postgres.exe, so only Unix-style values of the lc_messages GUC can elicit - * localized messages. In particular, every lc_messages setting that initdb - * can select automatically will yield only C-locale messages. XXX This could - * be fixed by running the fully-qualified locale name through a lookup table. - * * This function returns a pointer to a static buffer bearing the converted * name or NULL if conversion fails. * @@ -1031,8 +1024,6 @@ cache_locale_time(void) * [2] https://docs.microsoft.com/en-us/windows/win32/intl/locale-names */ -#if defined(_MSC_VER) - /* * Callback function for EnumSystemLocalesEx() in get_iso_localename(). * @@ -1201,16 +1192,6 @@ IsoLocaleName(const char *winlocname) return get_iso_localename(winlocname); } -#else /* !defined(_MSC_VER) */ - -static char * -IsoLocaleName(const char *winlocname) -{ - return NULL; /* Not supported on MinGW */ -} - -#endif /* defined(_MSC_VER) */ - #endif /* WIN32 && LC_MESSAGES */ -- 2.47.0
From 4c94af123a38c8c68178497f7b59e1d948f2d9ca Mon Sep 17 00:00:00 2001 From: Thomas Munro <tmu...@postgresql.org> Date: Sat, 23 Nov 2024 09:43:16 +1300 Subject: [PATCH v2 3/3] Revert "Blind attempt to fix _configthreadlocale() failures on MinGW." This reverts commit 2cf91ccb73ce888c44e3751548fb7c77e87335f2. When using the old msvcrt.dll, MinGW would supply its own dummy version of _configthreadlocale() that just returns -1 if you try to use it. For a time we tolerated that to shut the build farm up. We would fall back to code that was enough for the tests to pass, but it would have crashed a real multithreaded program though, so it was bogus. We don't need that kludge anymore, because we require ucrt (the modern Windows C runtime). We expect the real _configthreadlocale() to be present, and the ECPG tests will now fail if it isn't. The workaround was dead code and it's time to revert it. Reviewed-by: Peter Eisentraut <pe...@eisentraut.org> Discussion: https://postgr.es/m/d9e7731c-ca1b-477c-9298-fa51e135574a%40eisentraut.org --- src/interfaces/ecpg/ecpglib/descriptor.c | 2 +- src/interfaces/ecpg/ecpglib/execute.c | 20 +++++++++----------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/interfaces/ecpg/ecpglib/descriptor.c b/src/interfaces/ecpg/ecpglib/descriptor.c index 56e2bc4153..aee888432f 100644 --- a/src/interfaces/ecpg/ecpglib/descriptor.c +++ b/src/interfaces/ecpg/ecpglib/descriptor.c @@ -512,7 +512,7 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...) } #ifdef WIN32 if (stmt.oldthreadlocale != -1) - (void) _configthreadlocale(stmt.oldthreadlocale); + _configthreadlocale(stmt.oldthreadlocale); #endif #endif } diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c index 466d5600f9..b5089eac78 100644 --- a/src/interfaces/ecpg/ecpglib/execute.c +++ b/src/interfaces/ecpg/ecpglib/execute.c @@ -1977,9 +1977,7 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator, * Make sure we do NOT honor the locale for numeric input/output since the * database wants the standard decimal point. If available, use * uselocale() for this because it's thread-safe. Windows doesn't have - * that, but it usually does have _configthreadlocale(). In some versions - * of MinGW, _configthreadlocale() exists but always returns -1 --- so - * treat that situation as if the function doesn't exist. + * that, but it does have _configthreadlocale(). */ #ifdef HAVE_USELOCALE @@ -1997,6 +1995,11 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator, #else #ifdef WIN32 stmt->oldthreadlocale = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE); + if (stmt->oldthreadlocale == -1) + { + ecpg_do_epilogue(stmt); + return false; + } #endif stmt->oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno); if (stmt->oldlocale == NULL) @@ -2218,17 +2221,12 @@ ecpg_do_epilogue(struct statement *stmt) uselocale(stmt->oldlocale); #else if (stmt->oldlocale) + { setlocale(LC_NUMERIC, stmt->oldlocale); #ifdef WIN32 - - /* - * This is a bit trickier than it looks: if we failed partway through - * statement initialization, oldthreadlocale could still be 0. But that's - * okay because a call with 0 is defined to be a no-op. - */ - if (stmt->oldthreadlocale != -1) - (void) _configthreadlocale(stmt->oldthreadlocale); + _configthreadlocale(stmt->oldthreadlocale); #endif + } #endif free_statement(stmt); -- 2.47.0
