Обсуждение: Rethinking -L switch handling and construction of LDFLAGS

I noticed that if I build with --with-libxml on my Mac platforms,
"make installcheck" stops working for certain contrib modules such
as postgres_fdw.  I finally got around to diagnosing the reason why,
and it goes like this:

1. --with-libxml causes configure to include
    -L/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/lib
in the LDFLAGS value put into Makefile.global.  That's because
"xml2-config --libs" emits that, and we do need it if we want to link
to the platform-supplied libxml2.

2. However, that directory also contains a symlink to the
platform-supplied libpq.

3. When we go to build postgres_fdw.so, the link command line looks like

ccache gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute-Wformat-security -fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -g -O2
-bundle-multiply_defined suppress -o postgres_fdw.so postgres_fdw.o option.o deparse.o connection.o shippable.o
-L../../src/port-L../../src/common
-L/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/lib
-L/usr/local/ssl/lib-Wl,-dead_strip_dylibs   -L../../src/interfaces/libpq -lpq -bundle_loader
../../src/backend/postgres

The details of this might vary depending on your configure options,
but the key point is that the -L/Applications/... switch is before the
-L../../src/interfaces/libpq one.  This means that the linker resolves
"-lpq" to the platform-supplied libpq, not the one in the build tree.
We can confirm that with

$ otool -L postgres_fdw.so
postgres_fdw.so:
        /usr/lib/libpq.5.dylib (compatibility version 5.0.0, current version 5.6.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.50.4)

So, quite aside from any problems stemming from using a 9.3-vintage libpq
with HEAD client code, we are stuck with a libpq that uses Apple's idea of
the default socket location, rather than what the rest of our build uses.
That explains the failures seen in "make installcheck", which look like

2018-04-01 13:09:48.744 EDT [10758] ERROR:  could not connect to server "loopback"
2018-04-01 13:09:48.744 EDT [10758] DETAIL:  could not connect to server: No such file or directory
                Is the server running locally and accepting
                connections on Unix domain socket "/var/pgsql_socket/.s.PGSQL.5432"?

Of course, /var/pgsql_socket is *not* where my postmaster is putting
its socket.

In short, we need to deal more honestly with the positioning of -L
switches in link commands.  Somebody's idea that we could embed
both -L and -l into $(libpq), and then pay basically no attention to
where that ends up in the final link command, is just too simplistic.

I think that we want to establish an ironclad rule that -L switches
referencing directories in our own build tree must appear before -L
switches referencing external libraries.

I don't have a concrete patch to propose yet, but the design idea
I have in mind is to split LDFLAGS into two or more parts, so that
-L switches for the build tree are supposed to be put in the first
part and external -L switches in the second.  It'd be sufficient
to have Makefile.global do something like

ifdef PGXS
  LDFLAGS_INTERNAL = -L$(libdir)
else
  LDFLAGS_INTERNAL = -L$(top_builddir)/src/port -L$(top_builddir)/src/common
endif
LDFLAGS = $(LDFLAGS_INTERNAL) @LDFLAGS@

and then teach relevant places that they need to add $(libpq) to
LDFLAGS_INTERNAL not LDFLAGS.  (Perhaps "BUILD" would be a better keyword
than "INTERNAL" here?)  Not sure how that would play exactly with
Makefile.shlib's SHLIB_LINK, but maybe we need SHLIB_LINK_INTERNAL along
with SHLIB_LINK.  I'd also like to try to clean up the mess that is
$(libpq_pgport), though I'm not sure just how yet.

Or we could try to create a full separation between -L and -l switches,
ending up with three or more parts for LDFLAGS not just two.  But I'm
not sure if that gains anything.

I have no idea whether the MSVC build infrastructure has comparable
problems, and would not be willing to fix it myself if it does.
But I am willing to try to fix this in the gmake infrastructure.

Comments, better ideas?

            regards, tom lane

Hi,

On 2018-04-01 13:38:15 -0400, Tom Lane wrote:
> In short, we need to deal more honestly with the positioning of -L
> switches in link commands.  Somebody's idea that we could embed
> both -L and -l into $(libpq), and then pay basically no attention to
> where that ends up in the final link command, is just too simplistic.

Sounds right.


> I don't have a concrete patch to propose yet, but the design idea
> I have in mind is to split LDFLAGS into two or more parts, so that
> -L switches for the build tree are supposed to be put in the first
> part and external -L switches in the second.  It'd be sufficient
> to have Makefile.global do something like
> 
> ifdef PGXS
>   LDFLAGS_INTERNAL = -L$(libdir)
> else
>   LDFLAGS_INTERNAL = -L$(top_builddir)/src/port -L$(top_builddir)/src/common
> endif
> LDFLAGS = $(LDFLAGS_INTERNAL) @LDFLAGS@

I'm not sure I like doing this in Makefile.global. We've various files
that extend LDFLAGS in other places, and that's going to be hard if it's
already mushed together again. We end up re-building it from parts in
those files too.

Why don't we change the link commands to reference LDFLAGS_INTERNAL
explicitly?  That seems like it'd be cleaner.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2018-04-01 13:38:15 -0400, Tom Lane wrote:
>> I don't have a concrete patch to propose yet, but the design idea
>> I have in mind is to split LDFLAGS into two or more parts, so that
>> -L switches for the build tree are supposed to be put in the first
>> part and external -L switches in the second.

> I'm not sure I like doing this in Makefile.global. We've various files
> that extend LDFLAGS in other places, and that's going to be hard if it's
> already mushed together again. We end up re-building it from parts in
> those files too.

Yeah, one of the things I'd like to fix is that some of the makefiles,
eg psql's, do

override LDFLAGS := -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(LDFLAGS)

which goes *directly* against this commandment in Makefile.global:

# We want -L for libpgport.a and libpgcommon.a to be first in LDFLAGS.  We
# also need LDFLAGS to be a "recursively expanded" variable, else adjustments
# to rpathdir don't work right.  So we must NOT do LDFLAGS := something,
# meaning this has to be done first and elsewhere we must only do LDFLAGS +=
# something.

It's a bit surprising that rpath works at all for these makefiles.
But with what I'm imagining here, I think we could replace that with

LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)

and thereby preserve the recursively-expanded virginity of both
LDFLAGS_INTERNAL and LDFLAGS.  But I've not tried to test anything yet.

> Why don't we change the link commands to reference LDFLAGS_INTERNAL
> explicitly?  That seems like it'd be cleaner.

I'm hesitant to do that because LDFLAGS is a name known to make's
default rules, and I don't want to bet that we're not relying on
those default rules anywhere.  I also disagree with the idea that using
"$(LDFLAGS_INTERNAL) $(LDFLAGS)" in every link command we have is better
or less error-prone than just "$(LDFLAGS)".  Especially not if we end up
with more than two parts.

            regards, tom lane

Hi,

On 2018-04-01 13:55:05 -0400, Tom Lane wrote:
> > Why don't we change the link commands to reference LDFLAGS_INTERNAL
> > explicitly?  That seems like it'd be cleaner.
> 
> I'm hesitant to do that because LDFLAGS is a name known to make's
> default rules, and I don't want to bet that we're not relying on
> those default rules anywhere.

FWIW, postgres builds cleanly with -r -R in MAKELAGS.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2018-04-01 13:55:05 -0400, Tom Lane wrote:
>> I'm hesitant to do that because LDFLAGS is a name known to make's
>> default rules, and I don't want to bet that we're not relying on
>> those default rules anywhere.

> FWIW, postgres builds cleanly with -r -R in MAKELAGS.

That's pretty hard to believe.  Why would we bother to override every
default rule?  Even if it's true today, I would not accept it as project
policy that we must do so.  Perhaps more to the point, I would strongly
object to any design in which the standard Make variables don't mean
what the default rules expect them to mean.  That's just a recipe for
confusing people and creating hard-to-spot bugs.

            regards, tom lane

So here's a draft patch for this.  It fixes the originally complained-of
issue about --with-libxml breaking the build on Macs.  I've not tested
it very far beyond that plus a sanity cross-check on Linux, but I doubt
there's any point in further manual testing; we might as well just
throw it at the buildfarm.

I'm pretty happy with the way this turned out.  It's not a complicated
change, and we're no longer breaking our own rules about how to manage
LDFLAGS adjustments.

Some notes:

* I ended up adding not only LDFLAGS_INTERNAL and SHLIB_LINK_INTERNAL,
but also PG_LIBS_INTERNAL.  As things stood, the only uses of PG_LIBS
were for within-tree library references, so that it was fine that the
make rule that uses this variable put it before LDFLAGS.  But if anyone
ever tried to use PG_LIBS for the seemingly obvious purpose of referencing
an external library, we'd be right back in hazard land.  So that variable
needs to be split as well.

* I documented SHLIB_LINK_INTERNAL and PG_LIBS_INTERNAL in the comments
in pgxs.mk, but not in the user-facing documentation about using PGXS.
This is on the theory that only within-tree modules need either of those
variables, so users don't need them.  I could be convinced otherwise,
though.

* Some of these changes aren't really essential, eg the change in
hstore_plperl/Makefile to use SHLIB_LINK_INTERNAL instead of SHLIB_LINK;
since there's no -L switch there, it'd work either way.  But I thought
it best to maintain a consistent rule of using the _INTERNAL variable
for within-tree references.

* The change to STD_LDFLAGS in src/common/Makefile is because I noticed
that "pg_config --ldflags" has been printing "-L../../../src/common",
which it surely should not.  Apparently we forgot to fix this rule when
we rearranged things to create src/common/.  I'm tempted to make the
rule be

STD_LDFLAGS := $(filter-out $(LDFLAGS_INTERNAL),$(LDFLAGS))

in hopes of preventing a repetition of such silliness, but desisted
for the moment, because I'm not quite sure if this is a good idea,
or if it risks taking too much out.  Anybody have an opinion about it?

            regards, tom lane

diff --git a/contrib/dblink/Makefile b/contrib/dblink/Makefile
index 5189758..b1a5e06 100644
--- a/contrib/dblink/Makefile
+++ b/contrib/dblink/Makefile
@@ -3,7 +3,7 @@
 MODULE_big = dblink
 OBJS    = dblink.o $(WIN32RES)
 PG_CPPFLAGS = -I$(libpq_srcdir)
-SHLIB_LINK = $(libpq)
+SHLIB_LINK_INTERNAL = $(libpq)

 EXTENSION = dblink
 DATA = dblink--1.2.sql dblink--1.1--1.2.sql dblink--1.0--1.1.sql \
diff --git a/contrib/hstore_plperl/Makefile b/contrib/hstore_plperl/Makefile
index c0906db..f63cba2 100644
--- a/contrib/hstore_plperl/Makefile
+++ b/contrib/hstore_plperl/Makefile
@@ -28,7 +28,7 @@ ifeq ($(PORTNAME), win32)
 # these settings are the same as for plperl
 override CPPFLAGS += -DPLPERL_HAVE_UID_GID -Wno-comment
 # ... see silliness in plperl Makefile ...
-SHLIB_LINK += $(sort $(wildcard ../../src/pl/plperl/libperl*.a))
+SHLIB_LINK_INTERNAL += $(sort $(wildcard ../../src/pl/plperl/libperl*.a))
 else
 rpathdir = $(perl_archlibexp)/CORE
 SHLIB_LINK += $(perl_embed_ldflags)
diff --git a/contrib/hstore_plpython/Makefile b/contrib/hstore_plpython/Makefile
index 7ff787a..b81735a 100644
--- a/contrib/hstore_plpython/Makefile
+++ b/contrib/hstore_plpython/Makefile
@@ -26,7 +26,7 @@ endif
 # We must link libpython explicitly
 ifeq ($(PORTNAME), win32)
 # ... see silliness in plpython Makefile ...
-SHLIB_LINK += $(sort $(wildcard ../../src/pl/plpython/libpython*.a))
+SHLIB_LINK_INTERNAL += $(sort $(wildcard ../../src/pl/plpython/libpython*.a))
 else
 rpathdir = $(python_libdir)
 SHLIB_LINK += $(python_libspec) $(python_additional_libs)
diff --git a/contrib/jsonb_plpython/Makefile b/contrib/jsonb_plpython/Makefile
index 8c7090a..b3c98e6 100644
--- a/contrib/jsonb_plpython/Makefile
+++ b/contrib/jsonb_plpython/Makefile
@@ -26,7 +26,7 @@ endif
 # We must link libpython explicitly
 ifeq ($(PORTNAME), win32)
 # ... see silliness in plpython Makefile ...
-SHLIB_LINK += $(sort $(wildcard ../../src/pl/plpython/libpython*.a))
+SHLIB_LINK_INTERNAL += $(sort $(wildcard ../../src/pl/plpython/libpython*.a))
 else
 rpathdir = $(python_libdir)
 SHLIB_LINK += $(python_libspec) $(python_additional_libs)
diff --git a/contrib/ltree_plpython/Makefile b/contrib/ltree_plpython/Makefile
index bc7502b..7e988c7 100644
--- a/contrib/ltree_plpython/Makefile
+++ b/contrib/ltree_plpython/Makefile
@@ -26,7 +26,7 @@ endif
 # We must link libpython explicitly
 ifeq ($(PORTNAME), win32)
 # ... see silliness in plpython Makefile ...
-SHLIB_LINK += $(sort $(wildcard ../../src/pl/plpython/libpython*.a))
+SHLIB_LINK_INTERNAL += $(sort $(wildcard ../../src/pl/plpython/libpython*.a))
 else
 rpathdir = $(python_libdir)
 SHLIB_LINK += $(python_libspec) $(python_additional_libs)
diff --git a/contrib/oid2name/Makefile b/contrib/oid2name/Makefile
index 3414b4a..3eef8f6 100644
--- a/contrib/oid2name/Makefile
+++ b/contrib/oid2name/Makefile
@@ -7,7 +7,7 @@ PROGRAM = oid2name
 OBJS    = oid2name.o $(WIN32RES)

 PG_CPPFLAGS = -I$(libpq_srcdir)
-PG_LIBS = $(libpq_pgport)
+PG_LIBS_INTERNAL = $(libpq_pgport)

 ifdef USE_PGXS
 PG_CONFIG = pg_config
diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile
index 3543312..85394b4 100644
--- a/contrib/postgres_fdw/Makefile
+++ b/contrib/postgres_fdw/Makefile
@@ -5,7 +5,7 @@ OBJS = postgres_fdw.o option.o deparse.o connection.o shippable.o $(WIN32RES)
 PGFILEDESC = "postgres_fdw - foreign data wrapper for PostgreSQL"

 PG_CPPFLAGS = -I$(libpq_srcdir)
-SHLIB_LINK = $(libpq)
+SHLIB_LINK_INTERNAL = $(libpq)

 EXTENSION = postgres_fdw
 DATA = postgres_fdw--1.0.sql
diff --git a/contrib/spi/Makefile b/contrib/spi/Makefile
index 10ab5bb..42aa374 100644
--- a/contrib/spi/Makefile
+++ b/contrib/spi/Makefile
@@ -17,8 +17,6 @@ DOCS = $(addsuffix .example, $(MODULES))
 # comment out if you want a quieter refint package for other uses
 PG_CPPFLAGS = -DREFINT_VERBOSE

-LDFLAGS_SL += -L$(top_builddir)/src/port -lpgport
-
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/contrib/vacuumlo/Makefile b/contrib/vacuumlo/Makefile
index b4ba896..71106ff 100644
--- a/contrib/vacuumlo/Makefile
+++ b/contrib/vacuumlo/Makefile
@@ -7,7 +7,7 @@ PROGRAM = vacuumlo
 OBJS    = vacuumlo.o $(WIN32RES)

 PG_CPPFLAGS = -I$(libpq_srcdir)
-PG_LIBS = $(libpq_pgport)
+PG_LIBS_INTERNAL = $(libpq_pgport)

 ifdef USE_PGXS
 PG_CONFIG = pg_config
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 04cace1..2dac3ff 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -290,17 +290,26 @@ LLVM_LIBS=@LLVM_LIBS@
 LD = @LD@
 with_gnu_ld = @with_gnu_ld@

-# We want -L for libpgport.a and libpgcommon.a to be first in LDFLAGS.  We
-# also need LDFLAGS to be a "recursively expanded" variable, else adjustments
-# to rpathdir don't work right.  So we must NOT do LDFLAGS := something,
-# meaning this has to be done first and elsewhere we must only do LDFLAGS +=
-# something.
+# It's critical that within LDFLAGS, all -L switches pointing to build-tree
+# directories come before any -L switches pointing to external directories.
+# Otherwise it's possible for, e.g., a platform-provided copy of libpq.so
+# to get linked in place of the one we've built.  Therefore we adopt the
+# convention that the first component of LDFLAGS is an extra variable
+# LDFLAGS_INTERNAL, and -L and -l switches for PG's own libraries must be
+# put into LDFLAGS_INTERNAL, so they will appear ahead of those for external
+# libraries.
+#
+# We need LDFLAGS and LDFLAGS_INTERNAL to be "recursively expanded" variables,
+# else adjustments to, e.g., rpathdir don't work right.  So we must NOT do
+# "LDFLAGS := something" anywhere, ditto for LDFLAGS_INTERNAL.
+# These initial assignments must be "=" type, and elsewhere we must only do
+# "LDFLAGS += something" or "LDFLAGS_INTERNAL += something".
 ifdef PGXS
-  LDFLAGS = -L$(libdir)
+  LDFLAGS_INTERNAL = -L$(libdir)
 else
-  LDFLAGS = -L$(top_builddir)/src/port -L$(top_builddir)/src/common
+  LDFLAGS_INTERNAL = -L$(top_builddir)/src/port -L$(top_builddir)/src/common
 endif
-LDFLAGS += @LDFLAGS@
+LDFLAGS = $(LDFLAGS_INTERNAL) @LDFLAGS@

 LDFLAGS_EX = @LDFLAGS_EX@
 # LDFLAGS_SL might have already been assigned by calling makefile
diff --git a/src/Makefile.shlib b/src/Makefile.shlib
index 74d48c5..95b82a6 100644
--- a/src/Makefile.shlib
+++ b/src/Makefile.shlib
@@ -20,12 +20,16 @@
 #
 # NAME                  Name of library to build (no suffix nor "lib" prefix)
 # OBJS                  List of object files to include in library
-# SHLIB_LINK            If shared library relies on other libraries,
-#                       additional stuff to put in its link command
+# SHLIB_LINK            Stuff to append to library's link command
+#                       (typically, -L and -l switches for external libraries)
+# SHLIB_LINK_INTERNAL   -L and -l switches for Postgres-supplied libraries
 # SHLIB_PREREQS         Order-only prerequisites for library build target
 # SHLIB_EXPORTS         (optional) Name of file containing list of symbols to
 #                       export, in the format "function_name  number"
 #
+# Don't use SHLIB_LINK for references to files in the build tree, or the
+# wrong things will happen --- use SHLIB_LINK_INTERNAL for those!
+#
 # When building a shared library, the following version information
 # must also be set.  It should be omitted when building a dynamically
 # loadable module.
@@ -70,6 +74,8 @@
 COMPILER = $(CC) $(CFLAGS)
 LINK.static = $(AR) $(AROPT)

+LDFLAGS_INTERNAL += $(SHLIB_LINK_INTERNAL)
+


 ifdef SO_MAJOR_VERSION
diff --git a/src/backend/replication/libpqwalreceiver/Makefile b/src/backend/replication/libpqwalreceiver/Makefile
index a7a5fe1..75b0e2b 100644
--- a/src/backend/replication/libpqwalreceiver/Makefile
+++ b/src/backend/replication/libpqwalreceiver/Makefile
@@ -15,7 +15,8 @@ include $(top_builddir)/src/Makefile.global
 override CPPFLAGS := -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)

 OBJS = libpqwalreceiver.o $(WIN32RES)
-SHLIB_LINK = $(libpq) $(filter -lintl, $(LIBS))
+SHLIB_LINK_INTERNAL = $(libpq)
+SHLIB_LINK = $(filter -lintl, $(LIBS))
 SHLIB_PREREQS = submake-libpq
 PGFILEDESC = "libpqwalreceiver - receive WAL during streaming replication"
 NAME = libpqwalreceiver
diff --git a/src/bin/initdb/Makefile b/src/bin/initdb/Makefile
index dae3daf..8c23941 100644
--- a/src/bin/initdb/Makefile
+++ b/src/bin/initdb/Makefile
@@ -19,7 +19,7 @@ include $(top_builddir)/src/Makefile.global
 override CPPFLAGS := -DFRONTEND -I$(libpq_srcdir) -I$(top_srcdir)/src/timezone $(CPPFLAGS)

 # note: we need libpq only because fe_utils does
-override LDFLAGS := -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(LDFLAGS)
+LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)

 # use system timezone data?
 ifneq (,$(with_system_tzdata))
diff --git a/src/bin/pg_basebackup/Makefile b/src/bin/pg_basebackup/Makefile
index 54f915e..74dfb7a 100644
--- a/src/bin/pg_basebackup/Makefile
+++ b/src/bin/pg_basebackup/Makefile
@@ -19,7 +19,7 @@ top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global

 override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
-override LDFLAGS := -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(LDFLAGS)
+LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)

 OBJS=receivelog.o streamutil.o walmethods.o $(WIN32RES)

diff --git a/src/bin/pg_ctl/Makefile b/src/bin/pg_ctl/Makefile
index 4afc791..0202853 100644
--- a/src/bin/pg_ctl/Makefile
+++ b/src/bin/pg_ctl/Makefile
@@ -20,7 +20,7 @@ include $(top_builddir)/src/Makefile.global
 # but let's not pull that in on platforms where we don't need it.
 ifeq ($(PORTNAME), win32)
 override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
-override LDFLAGS := $(libpq_pgport) $(LDFLAGS)
+LDFLAGS_INTERNAL += $(libpq_pgport)
 SUBMAKE_LIBPQ := submake-libpq
 endif

diff --git a/src/bin/pg_dump/Makefile b/src/bin/pg_dump/Makefile
index e3bfc95..c46804a 100644
--- a/src/bin/pg_dump/Makefile
+++ b/src/bin/pg_dump/Makefile
@@ -17,7 +17,7 @@ top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global

 override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
-override LDFLAGS := -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(LDFLAGS)
+LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)

 OBJS=    pg_backup_archiver.o pg_backup_db.o pg_backup_custom.o \
     pg_backup_null.o pg_backup_tar.o pg_backup_directory.o \
diff --git a/src/bin/pg_rewind/Makefile b/src/bin/pg_rewind/Makefile
index 422c3ee..2bcfcc6 100644
--- a/src/bin/pg_rewind/Makefile
+++ b/src/bin/pg_rewind/Makefile
@@ -16,7 +16,7 @@ top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global

 override CPPFLAGS := -I$(libpq_srcdir) -DFRONTEND $(CPPFLAGS)
-override LDFLAGS := $(libpq_pgport) $(LDFLAGS)
+LDFLAGS_INTERNAL += $(libpq_pgport)

 OBJS    = pg_rewind.o parsexlog.o xlogreader.o datapagemap.o timeline.o \
     fetch.o file_ops.o copy_fetch.o libpq_fetch.o filemap.o logging.o \
diff --git a/src/bin/pg_upgrade/Makefile b/src/bin/pg_upgrade/Makefile
index 1d6ee70..adb0d5d 100644
--- a/src/bin/pg_upgrade/Makefile
+++ b/src/bin/pg_upgrade/Makefile
@@ -12,7 +12,7 @@ OBJS = check.o controldata.o dump.o exec.o file.o function.o info.o \
        tablespace.o util.o version.o $(WIN32RES)

 override CPPFLAGS := -DDLSUFFIX=\"$(DLSUFFIX)\" -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
-override LDFLAGS := -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(LDFLAGS)
+LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)


 all: pg_upgrade
diff --git a/src/bin/pgbench/Makefile b/src/bin/pgbench/Makefile
index 8a8e516..25abd0a 100644
--- a/src/bin/pgbench/Makefile
+++ b/src/bin/pgbench/Makefile
@@ -10,7 +10,7 @@ include $(top_builddir)/src/Makefile.global
 OBJS = pgbench.o exprparse.o $(WIN32RES)

 override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
-override LDFLAGS := -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(LDFLAGS)
+LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)

 ifneq ($(PORTNAME), win32)
 override CFLAGS += $(PTHREAD_CFLAGS)
diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile
index 2ad73d0..cd1b1b6 100644
--- a/src/bin/psql/Makefile
+++ b/src/bin/psql/Makefile
@@ -19,7 +19,7 @@ include $(top_builddir)/src/Makefile.global
 REFDOCDIR= $(top_srcdir)/doc/src/sgml/ref

 override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
-override LDFLAGS := -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(LDFLAGS)
+LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)

 OBJS=    command.o common.o copy.o crosstabview.o \
     describe.o help.o input.o large_obj.o mainloop.o \
diff --git a/src/bin/scripts/Makefile b/src/bin/scripts/Makefile
index 0cc528e..4c6e4b9 100644
--- a/src/bin/scripts/Makefile
+++ b/src/bin/scripts/Makefile
@@ -19,7 +19,7 @@ include $(top_builddir)/src/Makefile.global
 PROGRAMS = createdb createuser dropdb dropuser clusterdb vacuumdb reindexdb pg_isready

 override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
-override LDFLAGS := -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(LDFLAGS)
+LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)

 all: $(PROGRAMS)

diff --git a/src/common/Makefile b/src/common/Makefile
index 80e78d7..9bbd86d 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -29,7 +29,7 @@ LIBS += $(PTHREAD_LIBS)

 # don't include subdirectory-path-dependent -I and -L switches
 STD_CPPFLAGS := $(filter-out -I$(top_srcdir)/src/include -I$(top_builddir)/src/include,$(CPPFLAGS))
-STD_LDFLAGS := $(filter-out -L$(top_builddir)/src/port,$(LDFLAGS))
+STD_LDFLAGS := $(filter-out -L$(top_builddir)/src/common -L$(top_builddir)/src/port,$(LDFLAGS))
 override CPPFLAGS += -DVAL_CONFIGURE="\"$(configure_args)\""
 override CPPFLAGS += -DVAL_CC="\"$(CC)\""
 override CPPFLAGS += -DVAL_CPPFLAGS="\"$(STD_CPPFLAGS)\""
diff --git a/src/interfaces/ecpg/compatlib/Makefile b/src/interfaces/ecpg/compatlib/Makefile
index cd03af5..ebfd895 100644
--- a/src/interfaces/ecpg/compatlib/Makefile
+++ b/src/interfaces/ecpg/compatlib/Makefile
@@ -22,8 +22,8 @@ override CPPFLAGS := -I../include -I$(top_srcdir)/src/interfaces/ecpg/include \
     -I$(libpq_srcdir) -DFRONTEND $(CPPFLAGS)
 override CFLAGS += $(PTHREAD_CFLAGS)

-SHLIB_LINK = -L../ecpglib -lecpg -L../pgtypeslib -lpgtypes $(libpq) \
-    $(filter -lintl -lm, $(LIBS)) $(PTHREAD_LIBS)
+SHLIB_LINK_INTERNAL = -L../ecpglib -lecpg -L../pgtypeslib -lpgtypes $(libpq)
+SHLIB_LINK = $(filter -lintl -lm, $(LIBS)) $(PTHREAD_LIBS)
 SHLIB_PREREQS = submake-ecpglib submake-pgtypeslib

 SHLIB_EXPORTS = exports.txt
diff --git a/src/interfaces/ecpg/ecpglib/Makefile b/src/interfaces/ecpg/ecpglib/Makefile
index bc20cf7..d25d248 100644
--- a/src/interfaces/ecpg/ecpglib/Makefile
+++ b/src/interfaces/ecpg/ecpglib/Makefile
@@ -35,7 +35,8 @@ ifneq ($(PORTNAME), win32)
 OBJS += thread.o
 endif

-SHLIB_LINK = -L../pgtypeslib -lpgtypes $(libpq) $(filter -lintl -lm, $(LIBS)) $(PTHREAD_LIBS)
+SHLIB_LINK_INTERNAL = -L../pgtypeslib -lpgtypes $(libpq)
+SHLIB_LINK = $(filter -lintl -lm, $(LIBS)) $(PTHREAD_LIBS)
 SHLIB_PREREQS = submake-libpq submake-pgtypeslib

 SHLIB_EXPORTS = exports.txt
diff --git a/src/interfaces/ecpg/pgtypeslib/Makefile b/src/interfaces/ecpg/pgtypeslib/Makefile
index 0945f3d..29264ce 100644
--- a/src/interfaces/ecpg/pgtypeslib/Makefile
+++ b/src/interfaces/ecpg/pgtypeslib/Makefile
@@ -25,7 +25,7 @@ override CFLAGS += $(PTHREAD_CFLAGS)
 # Need to recompile any libpgport object files
 LIBS := $(filter-out -lpgport, $(LIBS))

-SHLIB_LINK += -lm
+SHLIB_LINK += $(filter -lm, $(LIBS))

 SHLIB_EXPORTS = exports.txt

diff --git a/src/interfaces/ecpg/test/Makefile.regress b/src/interfaces/ecpg/test/Makefile.regress
index b3d7c1e..06c0461 100644
--- a/src/interfaces/ecpg/test/Makefile.regress
+++ b/src/interfaces/ecpg/test/Makefile.regress
@@ -5,8 +5,9 @@ override CPPFLAGS := -I../../include -I$(top_srcdir)/src/interfaces/ecpg/include
     -I$(libpq_srcdir) $(CPPFLAGS)
 override CFLAGS += $(PTHREAD_CFLAGS)

-override LDFLAGS := -L../../ecpglib -L../../pgtypeslib $(filter-out -l%, $(libpq)) $(LDFLAGS)
-override LIBS := -lecpg -lpgtypes $(filter -l%, $(libpq)) $(LIBS) $(PTHREAD_LIBS)
+LDFLAGS_INTERNAL += -L../../ecpglib -lecpg -L../../pgtypeslib -lpgtypes $(libpq)
+
+override LIBS += $(PTHREAD_LIBS)

 # Standard way to invoke the ecpg preprocessor
 ECPG = ../../preproc/ecpg --regression -I$(srcdir)/../../include -I$(srcdir)
diff --git a/src/interfaces/ecpg/test/compat_informix/Makefile b/src/interfaces/ecpg/test/compat_informix/Makefile
index 8a5e854..d50fdc2 100644
--- a/src/interfaces/ecpg/test/compat_informix/Makefile
+++ b/src/interfaces/ecpg/test/compat_informix/Makefile
@@ -6,8 +6,7 @@ include $(top_srcdir)/$(subdir)/../Makefile.regress
 # Use special informix compatibility switch for all tests in this directory
 ECPG += -C INFORMIX

-override LDFLAGS := -L../../compatlib $(LDFLAGS)
-override LIBS := -lecpg_compat $(LIBS)
+LDFLAGS_INTERNAL += -L../../compatlib -lecpg_compat

 TESTS = test_informix test_informix.c \
         test_informix2 test_informix2.c \
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index 5beb1e6..c038836 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -45,7 +45,9 @@
 #   EXTRA_CLEAN -- extra files to remove in 'make clean'
 #   PG_CPPFLAGS -- will be added to CPPFLAGS
 #   PG_LIBS -- will be added to PROGRAM link line
+#   PG_LIBS_INTERNAL -- same, for references to libraries within build tree
 #   SHLIB_LINK -- will be added to MODULE_big link line
+#   SHLIB_LINK_INTERNAL -- same, for references to libraries within build tree
 #   PG_CONFIG -- path to pg_config program for the PostgreSQL installation
 #     to build against (typically just "pg_config" to use the first one in
 #     your PATH)
@@ -315,5 +317,5 @@ endif

 ifdef PROGRAM
 $(PROGRAM): $(OBJS)
-    $(CC) $(CFLAGS) $(OBJS) $(PG_LIBS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
+    $(CC) $(CFLAGS) $(OBJS) $(PG_LIBS_INTERNAL) $(LDFLAGS) $(LDFLAGS_EX) $(PG_LIBS) $(LIBS) -o $@$(X)
 endif
diff --git a/src/tools/findoidjoins/Makefile b/src/tools/findoidjoins/Makefile
index a3462b9..1af0a93 100644
--- a/src/tools/findoidjoins/Makefile
+++ b/src/tools/findoidjoins/Makefile
@@ -13,7 +13,7 @@ top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global

 override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
-override LDFLAGS := $(libpq_pgport) $(LDFLAGS)
+LDFLAGS_INTERNAL += $(libpq_pgport)

 OBJS= findoidjoins.o