Thread: Making pgxs builds work with a relocated installation

Making pgxs builds work with a relocated installation

From
Tom Lane
Date:
I looked at the recently noted problem that pgxs builds only work
if the installation paths recorded in Makefile.global are accurate;
which pretty much breaks our claim to supporting relocatable
installations.

What I propose we do about this is change the path setup section
of Makefile.global to look like (for each path variable)

ifdef PGXS
pkglibdir = $(shell pg_config --pkglibdir)
else
# existing code to set up pkglibdir
endif

Since a pgxs build has already assumed it could use pg_config to find
pgxs.mk, this isn't introducing any new dependency, and it will allow
the existing relocatable-path code to do its thing.

Not all of the path variables set up in Makefile.global are currently
available from pg_config; the missing ones are

prefix
exec_prefix
sbindir
mandir
localedir
libexecdir
datadir
sysconfdir
pkgincludedir
docdir

The first three of these don't seem to be directly referenced anywhere
in the Makefiles, so I propose just removing them from Makefile.global.
The other ones will need to be added to pg_config's repertoire, unless
someone can make a pretty good case that no pgxs-using module would ever
need to install into that directory.

Also note that I'm assuming the following path variables can continue to
be defined as they are, ie, relative to other path variables that will
get the pg_config treatment:

includedir_server = $(pkgincludedir)/server
includedir_internal = $(pkgincludedir)/internal
pgxsdir = $(pkglibdir)/pgxs

Comments?

The other open issue in this area was that on Windows, pg_config needs
to return space-free path names to avoid breaking the makefiles.  It was
suggested that this could be handled by passing pg_config's result path
names through GetShortPathName() on that platform.  That sounds OK to me
but I'm not in a position to write or test such a patch; can someone
take care of that?
        regards, tom lane


Re: Making pgxs builds work with a relocated installation

From
"Dave Page"
Date:

> -----Original Message-----
> From: pgsql-hackers-owner@postgresql.org
> [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Tom Lane
> Sent: 27 September 2005 01:13
> To: Peter Eisentraut
> Cc: pgsql-hackers@postgresql.org
> Subject: [HACKERS] Making pgxs builds work with a relocated
> installation
>
> The other open issue in this area was that on Windows, pg_config needs
> to return space-free path names to avoid breaking the
> makefiles.  It was
> suggested that this could be handled by passing pg_config's
> result path
> names through GetShortPathName() on that platform.  That
> sounds OK to me
> but I'm not in a position to write or test such a patch; can someone
> take care of that?

At the risk of getting squished in the rush to help, if noone else
volunteers I'll try to find some time.

:-)

/D


Re: Making pgxs builds work with a relocated installation

From
"Magnus Hagander"
Date:
> > The other open issue in this area was that on Windows,
> pg_config needs
> > to return space-free path names to avoid breaking the
> makefiles.  It
> > was suggested that this could be handled by passing
> pg_config's result
> > path names through GetShortPathName() on that platform.
> That sounds
> > OK to me but I'm not in a position to write or test such a
> patch; can
> > someone take care of that?
>
> At the risk of getting squished in the rush to help, if noone
> else volunteers I'll try to find some time.

Missed this one when it came through the first time. Note: Not
volunteering, just commenting on the how-fix :P

Using GetShortPathName() will break on any system that has disabled
short filename generatino, which IIRC is a recommended best practice
both for performance and for security in legacy apps. I don't know what
it does, but probably it will just return the same long path again.

//Magnus


Re: Making pgxs builds work with a relocated installation

From
"Dave Page"
Date:

> -----Original Message-----
> From: Magnus Hagander [mailto:mha@sollentuna.net]
> Sent: 27 September 2005 08:58
> To: Dave Page; Tom Lane; Peter Eisentraut
> Cc: pgsql-hackers@postgresql.org
> Subject: RE: [HACKERS] Making pgxs builds work with a
> relocated installation
>
> > > The other open issue in this area was that on Windows,
> > pg_config needs
> > > to return space-free path names to avoid breaking the
> > makefiles.  It
> > > was suggested that this could be handled by passing
> > pg_config's result
> > > path names through GetShortPathName() on that platform.
> > That sounds
> > > OK to me but I'm not in a position to write or test such a
> > patch; can
> > > someone take care of that?
> >
> > At the risk of getting squished in the rush to help, if noone
> > else volunteers I'll try to find some time.
>
> Missed this one when it came through the first time. Note: Not
> volunteering, just commenting on the how-fix :P
>
> Using GetShortPathName() will break on any system that has disabled
> short filename generatino, which IIRC is a recommended best practice
> both for performance and for security in legacy apps. I don't
> know what
> it does, but probably it will just return the same long path again.

If it's disabled, then they aren't going to be able to use short names
anyway, therefore we can't do much about it. If you see what I mean!

If GetShortPathName() just returns what was passed to it in such cases,
then at least we won't be any worse off than we are now.

Regards,Dave.


Re: Making pgxs builds work with a relocated installation

From
"Magnus Hagander"
Date:
> > > > The other open issue in this area was that on Windows,
> > > pg_config needs
> > > > to return space-free path names to avoid breaking the
> > > makefiles.  It
> > > > was suggested that this could be handled by passing
> > > pg_config's result
> > > > path names through GetShortPathName() on that platform.
> > > That sounds
> > > > OK to me but I'm not in a position to write or test such a
> > > patch; can
> > > > someone take care of that?
> > >
> > > At the risk of getting squished in the rush to help, if
> noone else
> > > volunteers I'll try to find some time.
> >
> > Missed this one when it came through the first time. Note: Not
> > volunteering, just commenting on the how-fix :P
> >
> > Using GetShortPathName() will break on any system that has disabled
> > short filename generatino, which IIRC is a recommended best
> practice
> > both for performance and for security in legacy apps. I don't know
> > what it does, but probably it will just return the same long path
> > again.
>
> If it's disabled, then they aren't going to be able to use
> short names anyway, therefore we can't do much about it. If
> you see what I mean!

Well, are we sure thare are no other ways? Either some funky quoting or
backslash-escaping spaces or something like that?


> If GetShortPathName() just returns what was passed to it in
> such cases, then at least we won't be any worse off than we are now.

Right. That'll have to be checked though, the API docs don't seem to
talk about that. Could be it gives you NULL or something equally evil..

//Magnus


Re: Making pgxs builds work with a relocated installation

From
"Dave Page"
Date:

> -----Original Message-----
> From: Magnus Hagander [mailto:mha@sollentuna.net]
> Sent: 27 September 2005 09:19
> To: Dave Page; Tom Lane; Peter Eisentraut
> Cc: pgsql-hackers@postgresql.org
> Subject: RE: [HACKERS] Making pgxs builds work with a
> relocated installation
>
> > If it's disabled, then they aren't going to be able to use
> > short names anyway, therefore we can't do much about it. If
> > you see what I mean!
>
> Well, are we sure thare are no other ways? Either some funky
> quoting or
> backslash-escaping spaces or something like that?

Tried quoting and '\ '. Neither work.

> > If GetShortPathName() just returns what was passed to it in
> > such cases, then at least we won't be any worse off than we are now.
>
> Right. That'll have to be checked though, the API docs don't seem to
> talk about that. Could be it gives you NULL or something
> equally evil..

Well, knowing you you have a VM setup in this way so testing should be
easy enough :-p

/D


Re: Making pgxs builds work with a relocated installation

From
Peter Eisentraut
Date:
Am Dienstag, 27. September 2005 02:12 schrieb Tom Lane:
> What I propose we do about this is change the path setup section
> of Makefile.global to look like (for each path variable)
>
> ifdef PGXS
> pkglibdir = $(shell pg_config --pkglibdir)
> else
> # existing code to set up pkglibdir
> endif

That looks right.

> Not all of the path variables set up in Makefile.global are currently
> available from pg_config; the missing ones are
>
> prefix
> exec_prefix
> sbindir
> mandir
> localedir
> libexecdir
> datadir
> sysconfdir
> pkgincludedir
> docdir
>
> The first three of these don't seem to be directly referenced anywhere
> in the Makefiles, so I propose just removing them from Makefile.global.

I see

prefix := /usr/local/pgsql
exec_prefix := ${prefix}

bindir := ${exec_prefix}/bin
sbindir := ${exec_prefix}/sbin

in the default configuration, so we need to keep the first two at least.  We 
don't need to expose them through pgxs, though.

> The other ones will need to be added to pg_config's repertoire, unless
> someone can make a pretty good case that no pgxs-using module would ever
> need to install into that directory.

pgxs only needs to expose the currently exposed variables plus sysconfdir, as 
previously discussed.  Unless someone can make a case that they need to 
access the other directories.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/


Re: Making pgxs builds work with a relocated installation

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> Am Dienstag, 27. September 2005 02:12 schrieb Tom Lane:
>> Not all of the path variables set up in Makefile.global are currently
>> available from pg_config; the missing ones are
>> 
>> prefix
>> exec_prefix
>> sbindir
>> mandir
>> localedir
>> libexecdir
>> datadir
>> sysconfdir
>> pkgincludedir
>> docdir
>> 
>> The first three of these don't seem to be directly referenced anywhere
>> in the Makefiles, so I propose just removing them from Makefile.global.

> I see
>
> prefix := /usr/local/pgsql
> exec_prefix := ${prefix}
>
> bindir := ${exec_prefix}/bin
> sbindir := ${exec_prefix}/sbin
>
> in the default configuration, so we need to keep the first two at least.  We 
> don't need to expose them through pgxs, though.

I stand corrected on those.

>> The other ones will need to be added to pg_config's repertoire, unless
>> someone can make a pretty good case that no pgxs-using module would ever
>> need to install into that directory.

> pgxs only needs to expose the currently exposed variables plus sysconfdir, as
> previously discussed.  Unless someone can make a case that they need to 
> access the other directories.

pgxs.mk itself requires access to datadir and docdir, so I don't
see how you can maintain that those aren't necessary.  The only reason
it doesn't also reference mandir and localedir is that none of our
current contrib modules have any man pages or locale support, but that
hardly seems far-fetched as a requirement for external modules.  Also,
pkgincludedir *must* be supported else we cannot set up the -I options
for includedir_server and includedir_internal.

On second look, libexecdir isn't used anywhere, so we might as well just
remove it entirely.  But all the others seem necessary to me.
        regards, tom lane


Re: Making pgxs builds work with a relocated installation

From
Peter Eisentraut
Date:
Tom Lane wrote:
> pgxs.mk itself requires access to datadir and docdir, so I don't
> see how you can maintain that those aren't necessary.  The only
> reason it doesn't also reference mandir and localedir is that none of
> our current contrib modules have any man pages or locale support,

Well, I don't support the notion that pgxs installs things there by 
default, but if it does, then I guess we have to fix it to do so 
correctly.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/


Re: Making pgxs builds work with a relocated installation

From
Tom Lane
Date:
"Magnus Hagander" <mha@sollentuna.net> writes:
> Using GetShortPathName() will break on any system that has disabled
> short filename generatino, which IIRC is a recommended best practice
> both for performance and for security in legacy apps. I don't know what
> it does, but probably it will just return the same long path again.

Yuck.  Anyone have another idea on coping with space-containing
pathnames?  I suppose we could try to quote the path variables properly
in all the makefiles, but that sure seems like a painful proposition.

Meanwhile, I believe I've fixed the relocatability issue per se.
        regards, tom lane


Re: Making pgxs builds work with a relocated installation

From
Peter Eisentraut
Date:
Tom Lane wrote:
> Yuck.  Anyone have another idea on coping with space-containing
> pathnames?

Switch to scons.  You heard it here first!

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/


Re: Making pgxs builds work with a relocated installation

From
"Dave Page"
Date:

> -----Original Message-----
> From: Peter Eisentraut [mailto:peter_e@gmx.net]
> Sent: 28 September 2005 06:37
> To: Tom Lane
> Cc: Magnus Hagander; Dave Page; pgsql-hackers@postgresql.org
> Subject: Re: [HACKERS] Making pgxs builds work with a
> relocated installation
>
> Tom Lane wrote:
> > Yuck.  Anyone have another idea on coping with space-containing
> > pathnames?
>
> Switch to scons.  You heard it here first!

Oooh, that looks nice at first glance...

/D


Re: Making pgxs builds work with a relocated installation

From
"Dave Page"
Date:

> -----Original Message-----
> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> Sent: 27 September 2005 18:57
> To: Magnus Hagander
> Cc: Dave Page; Peter Eisentraut; pgsql-hackers@postgresql.org
> Subject: Re: [HACKERS] Making pgxs builds work with a
> relocated installation
>
> "Magnus Hagander" <mha@sollentuna.net> writes:
> > Using GetShortPathName() will break on any system that has disabled
> > short filename generatino, which IIRC is a recommended best practice
> > both for performance and for security in legacy apps. I
> don't know what
> > it does, but probably it will just return the same long path again.
>
> Yuck.  Anyone have another idea on coping with space-containing
> pathnames?  I suppose we could try to quote the path
> variables properly
> in all the makefiles, but that sure seems like a painful proposition.

Actually it seems to work quite nicely - on Windows 2000 with short
names disabled it generates paths like:

C:\PROGRA~1\PostgreSQL\8.1-beta2\bin

Ie, it still fixes the spaces, but leaves the long bits, umm, long. With
short names enabled (on XP), you get:

C:\PROGRA~1\POSTGR~1\827E4~1.1-B\bin

Which is truly hideous, but works as expected in cmd.exe and Msys.

Patch attached that does this, and doubles up on the backslashes to keep
msys/make happy. Cmd.exe doesn't seem to care about the double
backslashes.

Regards, Dave

Re: Making pgxs builds work with a relocated installation

From
Alvaro Herrera
Date:
On Wed, Sep 28, 2005 at 10:05:48AM +0100, Dave Page wrote:

> > Tom Lane wrote:
> > > Yuck.  Anyone have another idea on coping with space-containing
> > > pathnames?
> > 
> > Switch to scons.  You heard it here first!
> 
> Oooh, that looks nice at first glance...

The only question is, do we want to force a Python dependency for
building Postgres?  I doubt it will be less portable than what we have
now, but it will be extra burden for the users.

Scons certainly seems nice anyway ...

-- 
Alvaro Herrera                        http://www.advogato.org/person/alvherre
"El destino baraja y nosotros jugamos" (A. Schopenhauer)


Re: Making pgxs builds work with a relocated installation

From
Tom Lane
Date:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> The only question is, do we want to force a Python dependency for
> building Postgres?

That's not happening.  Especially not now that Dave found the other
solution does work ...
        regards, tom lane