Relocatable locale
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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?
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
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
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.
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