Relocatable locale

Started by Bruce Momjianabout 22 years ago46 messagespatches
Jump to latest
#1Bruce Momjian
bruce@momjian.us

Magnus found out that LOCALEDIR wasn't being handled in a relocatable
manner.

This patch fixes that. It also adjusts the get_*_path functions to
limit values to MAXPGPATH.

I have two questions. First, setlocale() seemed to be inconsistently
set inside and outside of ENABLE_NLS. I assume the proper location is
inside. Second, libpq has a locale setting for error messages, but a
libpq program could be in any directory, so I see no way to make that
relocatable. Instead, I just use the hardcoded path. I could make it
relocatable, but that seems to error-prone, plus I would have to look up
the exec path and stuff, and it seemed too complicated.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Attachments:

/bjm/difftext/plainDownload+225-159
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#1)
Re: Relocatable locale

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I have two questions. First, setlocale() seemed to be inconsistently
set inside and outside of ENABLE_NLS. I assume the proper location is
inside.

Please do *not* go adding setlocale calls that were not there before.
You *will* break things.

! setlocale(LC_ALL, "");

^^^^^^^^^^^^^^^^^^^^^

Putting this call in the backend is a very serious mistake. It might be
okay in clients, but not in the backend.

regards, tom lane

#3Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#2)
Re: Relocatable locale

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I have two questions. First, setlocale() seemed to be inconsistently
set inside and outside of ENABLE_NLS. I assume the proper location is
inside.

Please do *not* go adding setlocale calls that were not there before.
You *will* break things.

! setlocale(LC_ALL, "");

^^^^^^^^^^^^^^^^^^^^^

Putting this call in the backend is a very serious mistake. It might be
okay in clients, but not in the backend.

OK, patch applied. Turns out it was only added for the backend
("postgres").

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Attachments:

/bjm/difftext/plainDownload+5-5
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#3)
Re: Relocatable locale

Bruce Momjian <pgman@candle.pha.pa.us> writes:

OK, patch applied. Turns out it was only added for the backend
("postgres").

Hm? Isn't that code going to be executed in postmaster, bootstrap,
checkpoint processes, etc etc?

I don't really believe that path.c has any business doing this at
all, in any program.

regards, tom lane

#5Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#4)
Re: Relocatable locale

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

OK, patch applied. Turns out it was only added for the backend
("postgres").

Hm? Isn't that code going to be executed in postmaster, bootstrap,
checkpoint processes, etc etc?

I don't really believe that path.c has any business doing this at
all, in any program.

Well, all our client apps used to do it in their own code. Now they
call set_pglocale to do it centrally and relocabably.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#5)
Re: Relocatable locale

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

I don't really believe that path.c has any business doing this at
all, in any program.

Well, all our client apps used to do it in their own code. Now they
call set_pglocale to do it centrally and relocabably.

I don't care if the clients do it. I don't want main.c doing it.

regards, tom lane

#7Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#6)
Re: Relocatable locale

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

I don't really believe that path.c has any business doing this at
all, in any program.

Well, all our client apps used to do it in their own code. Now they
call set_pglocale to do it centrally and relocabably.

I don't care if the clients do it. I don't want main.c doing it.

Well, as coded now, it will behave the same.

The old code has in main.c:

- bindtextdomain("postgres", LOCALEDIR);
- textdomain("postgres");

and now it has a call to set_pgsetlocale which does:

if (find_my_exec(argv0, my_exec_path) < 0)
return;

get_locale_path(argv0, path);
bindtextdomain(app, path);
textdomain(app);

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Attachments:

/bjm/difftext/plainDownload+2-5
#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#7)
Re: Relocatable locale

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Well, as coded now, it will behave the same.

No it won't: as you have it, the postmaster and everything else that
goes through main.c will execute a setlocale call, which was not there
before for very good reasons.

regards, tom lane

#9Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#8)
Re: Relocatable locale

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Well, as coded now, it will behave the same.

No it won't: as you have it, the postmaster and everything else that
goes through main.c will execute a setlocale call, which was not there
before for very good reasons.

I don't understand. I moved it up little in the file, but those calls
were happening in that file before, just a little lower.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#9)
Re: Relocatable locale

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I don't understand. I moved it up little in the file, but those calls
were happening in that file before, just a little lower.

No, *that* call wasn't happening at all. The calls that were there
were setting certain limited, safe LC categories. You added a call
that sets *all* LC categories, including ones we do not want changed.
See the discussion in src/backend/utils/adt/pg_locale.c.

regards, tom lane

#11Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#10)
Re: Relocatable locale

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I don't understand. I moved it up little in the file, but those calls
were happening in that file before, just a little lower.

No, *that* call wasn't happening at all. The calls that were there
were setting certain limited, safe LC categories. You added a call
that sets *all* LC categories, including ones we do not want changed.
See the discussion in src/backend/utils/adt/pg_locale.c.

But I added code to path.c to skip that if the app is "postgres". Why
doesn't that work?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#11)
Re: Relocatable locale

Bruce Momjian <pgman@candle.pha.pa.us> writes:

But I added code to path.c to skip that if the app is "postgres". Why
doesn't that work?

That will work fine ... for a standalone backend. Not so fine for
postmaster or bootstrap or other cases that go through main.c.

regards, tom lane

#13Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#12)
Re: Relocatable locale

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

But I added code to path.c to skip that if the app is "postgres". Why
doesn't that work?

That will work fine ... for a standalone backend. Not so fine for
postmaster or bootstrap or other cases that go through main.c.

But they all use the app name of "postgres". They always did.

That is not 'progname' but a hardcoded appname that is used in the
main.c call.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#13)
Re: Relocatable locale

Bruce Momjian <pgman@candle.pha.pa.us> writes:

But they all use the app name of "postgres". They always did.

Oh! Sorry, I'm barking up the wrong tree then. I was thinking you were
looking at the exec_path.

regards, tom lane

#15Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#14)
Re: Relocatable locale

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

But they all use the app name of "postgres". They always did.

Oh! Sorry, I'm barking up the wrong tree then. I was thinking you were
looking at the exec_path.

Nope, hard-coded, as they all are. I could pull from argv[0], but that
doesn't work in too many cases. The main.c call is:

set_pglocale(argv[0], "postgres");

and is the same no matter what is coming through main.c.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#16Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#1)
Re: Relocatable locale

Bruce Momjian wrote:

I have two questions. First, setlocale() seemed to be inconsistently
set inside and outside of ENABLE_NLS. I assume the proper location
is inside.

No, in case of doubt it's outside.

Second, libpq has a locale setting for error messages,
but a libpq program could be in any directory, so I see no way to
make that relocatable. Instead, I just use the hardcoded path. I
could make it relocatable, but that seems to error-prone, plus I
would have to look up the exec path and stuff, and it seemed too
complicated.

Hmm, so this says that a relocatable install isn't in fact possible?

#17Bruce Momjian
bruce@momjian.us
In reply to: Peter Eisentraut (#16)
Re: Relocatable locale

Peter Eisentraut wrote:

Bruce Momjian wrote:

I have two questions. First, setlocale() seemed to be inconsistently
set inside and outside of ENABLE_NLS. I assume the proper location
is inside.

No, in case of doubt it's outside.

OK, moved. New code is:

void
set_pglocale(const char *argv0, const char *app)
{
#ifdef ENABLE_NLS
char path[MAXPGPATH];
char my_exec_path[MAXPGPATH];
#endif

/* don't set LC_ALL in the backend */
if (strcmp(app, "postgres") != 0)
setlocale(LC_ALL, "");

#ifdef ENABLE_NLS
if (find_my_exec(argv0, my_exec_path) < 0)
return;

get_locale_path(my_exec_path, path);
bindtextdomain(app, path);
textdomain(app);
#endif
}

Second, libpq has a locale setting for error messages,
but a libpq program could be in any directory, so I see no way to
make that relocatable. Instead, I just use the hardcoded path. I
could make it relocatable, but that seems to error-prone, plus I
would have to look up the exec path and stuff, and it seemed too
complicated.

Hmm, so this says that a relocatable install isn't in fact possible?

Yep. I don't see how you can have a library know where to look for its
localized messages. The only thing I can think of is an environment
variable to override the hard-coded default. How is that?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#18Andrew Dunstan
andrew@dunslane.net
In reply to: Bruce Momjian (#17)
Re: Relocatable locale

Bruce Momjian wrote:

Yep. I don't see how you can have a library know where to look for its

localized messages. The only thing I can think of is an environment
variable to override the hard-coded default. How is that?

I'm confused. Can you explain the problem more clearly, please? Do we
need to distinguish known apps with a known (relative) message location,
from unknown libpq clients?

cheers

andrew

#19Peter Eisentraut
peter_e@gmx.net
In reply to: Andrew Dunstan (#18)
Re: Relocatable locale

Andrew Dunstan wrote:

I'm confused. Can you explain the problem more clearly, please? Do we
need to distinguish known apps with a known (relative) message
location, from unknown libpq clients?

The problem boils down to the fact that libpq needs to find its data
files (in this case locale data, but it could be anything), but it
doesn't know its own location, so it can't use a relative path
reference as has been proposed.

#20Bruce Momjian
bruce@momjian.us
In reply to: Peter Eisentraut (#19)
Re: Relocatable locale

Peter Eisentraut wrote:

Andrew Dunstan wrote:

I'm confused. Can you explain the problem more clearly, please? Do we
need to distinguish known apps with a known (relative) message
location, from unknown libpq clients?

The problem boils down to the fact that libpq needs to find its data
files (in this case locale data, but it could be anything), but it
doesn't know its own location, so it can't use a relative path
reference as has been proposed.

I am wondering if we should use environment variables, and have our apps
use putenv() to set it to the proper relative path.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#21Andrew Dunstan
andrew@dunslane.net
In reply to: Bruce Momjian (#20)
#22Bruce Momjian
bruce@momjian.us
In reply to: Andrew Dunstan (#21)
#23Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#20)
#24Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#23)
#25Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#24)
#26Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#25)
#27Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#25)
#28Bruce Momjian
bruce@momjian.us
In reply to: Andrew Dunstan (#26)
#29Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#20)
#30Peter Eisentraut
peter_e@gmx.net
In reply to: Andrew Dunstan (#26)
#31Bruce Momjian
bruce@momjian.us
In reply to: Peter Eisentraut (#29)
#32Magnus Hagander
magnus@hagander.net
In reply to: Bruce Momjian (#31)
#33Peter Eisentraut
peter_e@gmx.net
In reply to: Magnus Hagander (#32)
#34Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#31)
#35Bruce Momjian
bruce@momjian.us
In reply to: Peter Eisentraut (#33)
#36Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#35)
#37Bruce Momjian
bruce@momjian.us
In reply to: Peter Eisentraut (#36)
#38Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#37)
#39Magnus Hagander
magnus@hagander.net
In reply to: Peter Eisentraut (#38)
#40Bruce Momjian
bruce@momjian.us
In reply to: Peter Eisentraut (#38)
#41Bruce Momjian
bruce@momjian.us
In reply to: Magnus Hagander (#39)
#42Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#37)
#43Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#42)
#44Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#22)
#45Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#44)
#46Bruce Momjian
bruce@momjian.us
In reply to: Peter Eisentraut (#45)