On Monday, April 9, 2018 11:04:33 PM CEST Tom Lane wrote:
> Stephen Frost <sfr...@snowman.net> writes:
> > * Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote:
> >> On 4/5/18 02:04, Pavel Raiskup wrote:
> >>> As a followup thought; there are probably two major obstacles ATM
> >>> - the DSOs' symbols are not yet versioned, and
> >>> - the build-system doesn't seem to know how to -lpq link against
> >>> external libpq.so
> 
> > I've not looked but neither of those strike me as terribly difficult to
> > overcome, assuming they need to be overcome.
> 
> I'm not excited about introducing YA cross-platform difference in our
> library/linking behavior without fairly strong evidence that it's
> necessary.  The available evidence points in the other direction.

Well, but I believe it is really needed (in our case at least) - it's so
important that we think about doing the symbol versioning downstream-only.
I wouldn't bother you much with this, but it's worth keeping you at least
well informed since - if we moved that direction - there would soon exist
binaries in the wild linked against versioned PG shared libraries, and
those would raise ugly runtime linking warnings if used on systems where
are non-versioned PG libs (it's no problem vice-versa).

As I said, we'd like to provide multiple major PG versions, but only one
set of PG libraries.  But as the time will continue, supporting newer PG
major versions will require updating the system's default 'libpq.so.5',
potentially for the newly added symbols.  If we had in our RPMs versioned
symbols, RPM would for free guard us against wrong package installations
(IOW RPM would guarantee that users won't install packages depending on
newer symbols unless also newer 'libpq.so.5' is installed).  Without
lib symbol versions, there's no **easy** way around this.

Debian folks claim they don't have a problem with symbol versioning even
though they have the same installation pattern since ever, but IMO that's
because (as far as I understand how all of this is done on Debian):

- Debian doesn't have that long support life cycle, so new symbols are
  to occur less likely

- Debian actually provides officially only one version of PG stack
  (including libraries), the rest is provided through postgresql.org
  repositories (one could say "third party", even though those are
  probably the same maintainers).  So for Debian, it's really unlikely to
  build system package against updated 'libpq.so.5' which comes from
  the "third party" repo.

I mean, worthless saying that Debian packaging would benefit from
versioned symbols too (== it would be safe to update system-wide package),
but it would be really awesome to have consistent (upstream blessed) way
to do the versioning.

As for how it would be done downstream-only:  Have a look at the
'symbol-versioning' patch attached.  Sorry for me being verbose here and
there..  It's pretty narrow patch luckily; because the buildsystem is
already GNU ld friendly.  I implemented the new 'exports.txt' parser in
relatively portable /bin/sh, but I can (probably less portably) implement
that in awk too.  Or anything, please tell.

> As for linking against an external .so, commit dddfc4cb2 just went to
> some lengths to make sure that that *wouldn't* happen.  But as long
> as all the builds expect libpq.so to end up in the same place after
> installation, I doubt it matters much what happens at build time.
> You just need to control which build actually installs it.

Agreed, but we can build-time link against system's libpq.so pretty easily
too.  E.g. by something like the attached 'no-libs' patch.  Could we have
something like this as upstream ./configure opt-in?

---
Overall, what are your feelings?  I'd be really glad to go through more
formal patch submission, those attachments are here just to have something more
concrete in hand for the discussion purposes.

Pavel

>                       regards, tom lane
> 

Shared library symbol versioning (PORTNAME=linux)

Even though we pay attention to never add a new shlib symbol by
minor version update, this change makes the life easier for some
PG distributors - for those who aim to support multiple major
versions of PostgreSQL stacks - all built against one set of
shared PG libraries (in other words, unless there's some SO
version bump each PG library exists on such system only in one
instance).

For example (without symbol versioning) it might easily happen
that some system's binary using PQencryptPasswordConn symbol (new
in v10) is run against 'libpq.so.5' coming from v9.6.  This is not
terribly hard to imagine since at build-time it is likely that
the binary is linked against proper (v10) version of the
libpq.so.5.  Such case would end up with linking issues at
runtime.  This problem is very likely to happen on systems with
rather long term support (e.g. 10+ years) where many updates of
the shared library are likely to happen during lifetime.

Shlib symbol versioning then helps the package-tooling to always
guarantee that sufficiently new shlib is installed on such systems
(e.g. that 'libpq.so.5' providing PQencryptPasswordConn symbol is
installed, when required by any other package).

Luckily, the build system is already using the GNU ld option
--version-script on linux port, so adding symbol versions there
shouldn't really cause any new build failures.

diff --git a/config/Makefile b/config/Makefile
index 67e7998f55..86612a42c3 100644
*** a/config/Makefile
--- b/config/Makefile
***************
*** 8,13 **** include $(top_builddir)/src/Makefile.global
--- 8,14 ----
  install: all installdirs
  	$(INSTALL_SCRIPT) $(srcdir)/install-sh '$(DESTDIR)$(pgxsdir)/config/install-sh'
  	$(INSTALL_SCRIPT) $(srcdir)/missing '$(DESTDIR)$(pgxsdir)/config/missing'
+ 	$(INSTALL_SCRIPT) $(srcdir)/build-exports-gnu-ld '$(DESTDIR)$(pgxsdir)/config/build-exports-gnu-ld'
  
  installdirs:
  	$(MKDIR_P) '$(DESTDIR)$(pgxsdir)/config'
diff --git a/config/bunew file mode 100755
index 0000000000..042d9e9342
*** /dev/null
--- b/config/build-exports-gnu-ld
***************
*** 0 ****
--- 1,45 ----
+ #! /bin/sh
+ 
+ # by default use PG_ prefix
+ : "${SYMBOL_VERSION_PREFIX=PG_}"
+ 
+ # we started symbol versioning since v12
+ : "${SYMBOL_VERSION_START=9.6}"
+ 
+ version=$SYMBOL_VERSION_START
+ version_prev=
+ first=:
+ 
+ open_block ()
+ {
+ 	$first || echo
+ 	first=false
+ 	echo "${SYMBOL_VERSION_PREFIX}$version {"
+ 	echo "global:"
+ }
+ 
+ close_block ()
+ {
+ 	if test -n "$version_prev"; then
+ 		echo "} $SYMBOL_VERSION_PREFIX$version_prev;"
+ 	else
+ 		echo "};"
+ 	fi
+ 	version_prev=$version
+ 	version=$1
+ }
+ 
+ open_block
+ while read -r symbol _ new_version
+ do
+ 	case $symbol in '#'*) continue ;; esac
+ 	if test -n "$new_version" && test "$new_version" != "$version"; then
+ 		close_block "$new_version"
+ 		open_block
+ 	fi
+ 	echo "	$symbol;"
+ done
+ 
+ echo "local:"
+ echo "	*;"
+ close_block
diff --git a/src/Makefile.shlib b/index 95b82a6dea..a7065d02a4 100644
*** a/src/Makefile.shlib
--- b/src/Makefile.shlib
***************
*** 221,227 **** ifeq ($(PORTNAME), linux)
    ifdef soname
      LINK.shared		+= -Wl,-soname,$(soname)
    endif
!   BUILD.exports		= ( echo '{ global:'; $(AWK) '/^[^\#]/ {printf "%s;\n",$$1}' $<; echo ' local: *; };' ) >$@
    exports_file		= $(SHLIB_EXPORTS:%.txt=%.list)
    ifneq (,$(exports_file))
      LINK.shared		+= -Wl,--version-script=$(exports_file)
--- 221,227 ----
    ifdef soname
      LINK.shared		+= -Wl,-soname,$(soname)
    endif
!   BUILD.exports		= $(SHELL) $(top_srcdir)/config/build-exports-gnu-ld < $< > $@
    exports_file		= $(SHLIB_EXPORTS:%.txt=%.list)
    ifneq (,$(exports_file))
      LINK.shared		+= -Wl,--version-script=$(exports_file)
diff --git a/src/interfacindex d6a38d0df8..29bebeac7e 100644
*** a/src/interfaces/libpq/exports.txt
--- b/src/interfaces/libpq/exports.txt
***************
*** 171,174 **** PQsslAttributeNames       168
  PQsslAttribute            169
  PQsetErrorContextVisibility 170
  PQresultVerboseErrorMessage 171
! PQencryptPasswordConn     172
--- 171,174 ----
  PQsslAttribute            169
  PQsetErrorContextVisibility 170
  PQresultVerboseErrorMessage 171
! PQencryptPasswordConn     172     10
diff --git a/src/Makefile b/src/Makefile
index 977f80b..3d3b679 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -20,7 +20,6 @@ SUBDIRS = \
 	backend/utils/mb/conversion_procs \
 	backend/snowball \
 	include \
-	interfaces \
 	backend/replication/libpqwalreceiver \
 	fe_utils \
 	bin \
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index d280a2d..724f3cf 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -454,7 +454,7 @@ endif
 
 # This macro is for use by libraries linking to libpq.  (Because libpgport
 # isn't created with the same link flags as libpq, it can't be used.)
-libpq = -L$(libpq_builddir) -lpq
+libpq = -lpq
 
 # This macro is for use by client executables (not libraries) that use libpq.
 # We force clients to pull symbols from the non-shared libraries libpgport
@@ -480,7 +480,6 @@ endif
 # Commonly used submake targets
 
 submake-libpq:
-	$(MAKE) -C $(libpq_builddir) all
 
 submake-libpgport:
 	$(MAKE) -C $(top_builddir)/src/port all

Reply via email to