setlocale and gettext in Postgres

Hi all,

I see now the following lines in libintl.h of version
0.18.1.1 which didn't exist in 0.17 version.

/* Support for the locale chosen by the user. */
#if (defined __APPLE__ && defined __MACH__) || defined _WIN32 || defined
__WIN32__ || defined __CYGWIN__

#undef setlocale
#define setlocale libintl_setlocale
extern char *setlocale (int, const char *);
.
.

The macro may cause a trouble especially on Windows.

Though libintl_setlocale() calls setlocale() internally,
what the Mingw version of libintl library calls is the one
in msvcrt.dll not in the libraries which main programs link.
Unfortunately locale-aware functions like printf() calls
in main programs would see "C" locales not the locales set
by libintl_setlocale().

Attached is a patch to disable the macro on Windows.

regards,
Hiroshi Inoue

2011/1/27 Hiroshi Inoue <inoue(at)tpf(dot)co(dot)jp>:
> I see now the following lines in libintl.h of version
> 0.18.1.1 which didn't exist in 0.17 version.
>
> The macro may cause a trouble especially on Windows.
> Attached is a patch to disable the macro on Windows.

Can anyone test the fix?

I added the patch to the current commitfest for reminder.
https://commitfest.postgresql.org/action/patch_view?id=528

--
Itagaki Takahiro

On Tue, Feb 1, 2011 at 5:36 AM, Itagaki Takahiro
<itagaki(dot)takahiro(at)gmail(dot)com> wrote:
> 2011/1/27 Hiroshi Inoue <inoue(at)tpf(dot)co(dot)jp>:
>> I see now the following lines in libintl.h of version
>> 0.18.1.1 which didn't exist in 0.17 version.
>>
>> The macro may cause a trouble especially on Windows.
>> Attached is a patch to disable the macro on Windows.
>
> Can anyone test the fix?
>
> I added the patch to the current commitfest for reminder.
> https://commitfest.postgresql.org/action/patch_view?id=528

The QA team in EDB have tested the patch for me. It works as designed
and allows us to upgrade gettext to fix Japanese localisation on
Win64. Upgrading gettext without the patch will fix Japanese, but
break other translations (eg. French).

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Feb 1, 2011 at 09:08, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> On Tue, Feb 1, 2011 at 5:36 AM, Itagaki Takahiro
> <itagaki(dot)takahiro(at)gmail(dot)com> wrote:
>> 2011/1/27 Hiroshi Inoue <inoue(at)tpf(dot)co(dot)jp>:
>>> I see now the following lines in libintl.h of version
>>> 0.18.1.1 which didn't exist in 0.17 version.
>>>
>>> The macro may cause a trouble especially on Windows.
>>> Attached is a patch to disable the macro on Windows.
>>
>> Can anyone test the fix?
>>
>> I added the patch to the current commitfest for reminder.
>> https://commitfest.postgresql.org/action/patch_view?id=528
>
> The QA team in EDB have tested the patch for me. It works as designed
> and allows us to upgrade gettext to fix Japanese localisation on
> Win64. Upgrading gettext without the patch will fix Japanese, but
> break other translations (eg. French).

Do we need to backpatch this?

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

On Tue, Feb 1, 2011 at 8:29 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Tue, Feb 1, 2011 at 09:08, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>> On Tue, Feb 1, 2011 at 5:36 AM, Itagaki Takahiro
>> <itagaki(dot)takahiro(at)gmail(dot)com> wrote:
>>> 2011/1/27 Hiroshi Inoue <inoue(at)tpf(dot)co(dot)jp>:
>>>> I see now the following lines in libintl.h of version
>>>> 0.18.1.1 which didn't exist in 0.17 version.
>>>>
>>>> The macro may cause a trouble especially on Windows.
>>>> Attached is a patch to disable the macro on Windows.
>>>
>>> Can anyone test the fix?
>>>
>>> I added the patch to the current commitfest for reminder.
>>> https://commitfest.postgresql.org/action/patch_view?id=528
>>
>> The QA team in EDB have tested the patch for me. It works as designed
>> and allows us to upgrade gettext to fix Japanese localisation on
>> Win64. Upgrading gettext without the patch will fix Japanese, but
>> break other translations (eg. French).
>
> Do we need to backpatch this?

We've only seen the problem on 64 bit builds - but I guess it may
occur on 32 bit too, given the right version of gettext (they come
form different places for win32 vs. win64, so it's not entirely
straightforward to figure out).

So, it certainly needs to go to 9.0, and probably wouldn't hurt to put
it in 8.3/8.4 too.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 02/01/2011 03:47 AM, Dave Page wrote:
> On Tue, Feb 1, 2011 at 8:29 AM, Magnus Hagander<magnus(at)hagander(dot)net> wrote:
>> On Tue, Feb 1, 2011 at 09:08, Dave Page<dpage(at)pgadmin(dot)org> wrote:
>>> On Tue, Feb 1, 2011 at 5:36 AM, Itagaki Takahiro
>>> <itagaki(dot)takahiro(at)gmail(dot)com> wrote:
>>>> 2011/1/27 Hiroshi Inoue<inoue(at)tpf(dot)co(dot)jp>:
>>>>> I see now the following lines in libintl.h of version
>>>>> 0.18.1.1 which didn't exist in 0.17 version.
>>>>>
>>>>> The macro may cause a trouble especially on Windows.
>>>>> Attached is a patch to disable the macro on Windows.
>>>> Can anyone test the fix?
>>>>
>>>> I added the patch to the current commitfest for reminder.
>>>> https://commitfest.postgresql.org/action/patch_view?id=528
>>> The QA team in EDB have tested the patch for me. It works as designed
>>> and allows us to upgrade gettext to fix Japanese localisation on
>>> Win64. Upgrading gettext without the patch will fix Japanese, but
>>> break other translations (eg. French).
>> Do we need to backpatch this?
> We've only seen the problem on 64 bit builds - but I guess it may
> occur on 32 bit too, given the right version of gettext (they come
> form different places for win32 vs. win64, so it's not entirely
> straightforward to figure out).
>
> So, it certainly needs to go to 9.0, and probably wouldn't hurt to put
> it in 8.3/8.4 too.

Why are we only disabling the macro for WIN32 and not for the other
platforms that the macro is defined for? Do we know it's not also a
problem on Apple or Cygwin?

cheers

andrew

On Tue, Feb 1, 2011 at 10:45 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
> Why are we only disabling the macro for WIN32 and not for the other
> platforms that the macro is defined for? Do we know it's not also a problem
> on Apple or Cygwin?

No, not as far as I know.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Feb 1, 2011 at 12:08, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> On Tue, Feb 1, 2011 at 10:45 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>
>> Why are we only disabling the macro for WIN32 and not for the other
>> platforms that the macro is defined for? Do we know it's not also a problem
>> on Apple or Cygwin?
>
> No, not as far as I know.

I've applied the patch with an update to the comment that explains why
it happens more clearly, which should also make it obvious why it
won't happen on apple or cygwin (because they don't link to different
versions of the microsoft runtime..)

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/