Thread: [FIX] Fix for pgxs on darwin and other platforms...

pgxs on darwin is currently broken.  The behavior before was to use
-bundle_loader with a bogus directory that didn't exist.  It should be
using $(DESTDIR)$(bindir) instead of something based off of $(topdir),
which appears to not be set correctly either (but that's a different
problem).

> gcc -no-cpp-precomp -O2 -Wall -Wmissing-prototypes -Wpointer-arith
> -Wendif-labels -fno-strict-aliasing -g  -bundle  pgmemcache.o
> -L/usr/local/pgsql/lib -L/sw/lib -L/usr/local/lib -lmemcache
> -bundle_loader
> /usr/local/pgsql/lib/pgxs/src/makefiles/../../src/backend/postgres  -o
> libpgmemcache.0.0.so
> ld: can't open:
> /usr/local/pgsql/lib/pgxs/src/makefiles/../../src/backend/postgres (No
> such file or directory, errno = 2)
> gnumake: *** [libpgmemcache.0.0.so] Error 1
> Exit 2

Post patch:

> gcc -no-cpp-precomp -O2 -Wall -Wmissing-prototypes -Wpointer-arith
> -Wendif-labels -fno-strict-aliasing -g  -bundle  pgmemcache.o
> -L/usr/local/pgsql/lib -L/sw/lib -L/usr/local/lib -lmemcache
> -bundle_loader /usr/local/pgsql/bin/postgres  -o libpgmemcache.0.0.so
> rm -f libpgmemcache.0.so
> ln -s libpgmemcache.0.0.so libpgmemcache.0.so
> rm -f libpgmemcache.so
> ln -s libpgmemcache.0.0.so libpgmemcache.so

Which is correct.  Can someone please get this into the 8.X release
that way pgxs is usable for folks on the darwin platform?  It doesn't
look like this is broken on other major OSes, but some of the oddball
platforms (BeOS, cygwin, and win32) don't seem to be correct either.
I've included a fixes for them in this patch, but can't promise they'll
work.  At the very least what I'm supplying is more correct than what
they're using now.  :-/  It looks like pgxs on the platforms in
question has an assumption that pgxs gets called from within the
PostgreSQL src tree, which isn't correct after the fact.  Thanks.  The
darwin patch is included below.  Thanks in advance.  -sc



--
Sean Chittenden

Sean Chittenden <sean@chittenden.org> writes:
> Which is correct.  Can someone please get this into the 8.X release
> that way pgxs is usable for folks on the darwin platform?  It doesn't
> look like this is broken on other major OSes, but some of the oddball
> platforms (BeOS, cygwin, and win32) don't seem to be correct either.
> I've included a fixes for them in this patch, but can't promise they'll
> work.  At the very least what I'm supplying is more correct than what
> they're using now.

I don't think so :-(.  Didn't you just break building things on a
machine where Postgres isn't already installed?

            regards, tom lane

>> Which is correct.  Can someone please get this into the 8.X release
>> that way pgxs is usable for folks on the darwin platform?  It doesn't
>> look like this is broken on other major OSes, but some of the oddball
>> platforms (BeOS, cygwin, and win32) don't seem to be correct either.
>> I've included a fixes for them in this patch, but can't promise
>> they'll
>> work.  At the very least what I'm supplying is more correct than what
>> they're using now.
>
> I don't think so :-(.  Didn't you just break building things on a
> machine where Postgres isn't already installed?

Wait, are you suggesting that pgxs is used pre-install?  I sure hope
not.  If it is, what's the purpose of the internal Makefiles?  Having
pgxs used during the building of the main tree seems backwards++.  I'd
wager that the purpose of this in contrib was to accommodate modules
being built independently of the source tree.  If that's not the case,
why carry this baggage around?

ifdef USE_PGXS
PGXS = $(shell pg_config --pgxs)
include $(PGXS)
else
subdir = contrib/pgcrypto
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
include $(top_srcdir)/contrib/contrib-global.mk
endif

# to make ws2_32.lib the last library (must occur after definition of
PORTNAME)
ifeq ($(PORTNAME),win32)
SHLIB_LINK += -lwsock32 -lws2_32
endif

If not, then we need two versions of pgxs, one referenced in the src
tree, the other used for post-installation for module building.  As
things stand, pgxs is fantastically broken on darwin.  :(  -sc

--
Sean Chittenden

Sean Chittenden <sean@chittenden.org> writes:
>> I don't think so :-(.  Didn't you just break building things on a
>> machine where Postgres isn't already installed?

> Wait, are you suggesting that pgxs is used pre-install?  I sure hope
> not.

It isn't, but your patch changes make variables (to wit, BE_DLLLIBS)
that are used during the standard build process.

            regards, tom lane

Sean Chittenden <sean@chittenden.org> writes:
>   I don't have a patch yet for this yet, but if you'd like, I can fix
> this problem globally and correctly.  The solution is as follows:

> *) contrib/ should *not* use pgxs and instead should depend on the
> global makefiles

Nope.  If we can't use contrib as a test case for pgxs, the odds that
pgxs will ever be non-broken are minimal.  It does seem though that we
need two different values for BE_DLLLIBS depending on whether we are
building in-tree or not.  I think you can handle that with "ifdef PGXS"
but I'm not totally sure that that's the right test ...

            regards, tom lane

>>> I don't think so :-(.  Didn't you just break building things on a
>>> machine where Postgres isn't already installed?
>
>> Wait, are you suggesting that pgxs is used pre-install?  I sure hope
>> not.
>
> It isn't, but your patch changes make variables (to wit, BE_DLLLIBS)
> that are used during the standard build process.

Attached is the easiest and least intrusive fix.  This fixes all
scenarios as far as I can tell.  -sc



--
Sean Chittenden

Sean Chittenden <sean@chittenden.org> writes:
> Attached is the easiest and least intrusive fix.  This fixes all
> scenarios as far as I can tell.  -sc

Ok, that looks more promising.  Patch applied.

            regards, tom lane