Re: WIN32 pg_import_system_collations

On Mon, Nov 7, 2022 at 4:08 PM Peter Eisentraut <
peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:

>
> This looks pretty good to me. The refactoring of the non-Windows parts
> makes sense. The Windows parts look reasonable on manual inspection,
> but again, I don't have access to Windows here, so someone else should
> also look it over.
>
> I was going to say that at least it is getting tested on the CI, but I
have found out that meson changes version(). That is fixed in this version.

> A small style issue: Change return (TRUE) to return TRUE.
>
> Fixed.

> The code
>
> + if (strlen(localebuf) == 5 && localebuf[2] == '-')
>
> might be too specific. At least on some POSIX systems, I have seen
> locales with a three-letter language name. Maybe you should look with
> strchr() and not be too strict about the exact position.
>
> Ok, in this version the POSIX alias is created unconditionally.

> For the test patch, why is a separate test for non-UTF8 needed on
> Windows. Does the UTF8 one not work?
>
> Windows locales will retain their CP_ACP encoding unless you change the OS
code page to UFT8, which is still experimental [1].

> + version() !~ 'Visual C\+\+'
>
> This probably won't work for MinGW.
>
> When I proposed this patch it wouldn't have worked because of the
project's Windows minimum version requirement, now it should work in MinGW.
It actually doesn't because most locales are failing with "skipping locale
with unrecognized encoding", but checking what's wrong
with pg_get_encoding_from_locale() in MiNGW is subject for another thread.

[1]
https://stackoverflow.com/questions/56419639/what-does-beta-use-unicode-utf-8-for-worldwide-language-support-actually-do

Regards,

Juan José Santamaría Flecha