Re: Cleaning up multiply-defined-symbol warnings on OS X

We have two approaches for dealing with pgport leakage. For libraries,
we remove libpgport from the link line and recompile our own object
files:

# Need to recompile any libpgport object files
LIBS := $(filter-out -lpgport, $(LIBS))

OBJS= execute.o typename.o descriptor.o data.o error.o prepare.o memory.o \
connect.o misc.o path.o exec.o \
$(filter snprintf.o, $(LIBOBJS))

The problem is that there is no testing of which object files need to be
added, but fortunately Win32 has the dllexport which makes libpq export
_only_ functions we define, so if we forget something, it shows up as a
Win32 link error.

For applications, we have them pull symbols from libpgport first, and
are not bound to specific libpgport functions being in libpq.
Makefile.global has:

# Force clients to pull symbols from the non-shared library libpgport
# rather than pulling some libpgport symbols from libpq just because
# libpq uses those functions too. This makes applications less
# dependent on changes in libpq's usage of pgport. To do this we link to
# pgport before libpq. This does cause duplicate -lpgport's to appear
# on client link lines.
ifdef PGXS
libpq_pgport = -L$(libdir) -lpgport $(libpq)
else
libpq_pgport = -L$(top_builddir)/src/port -lpgport $(libpq)
endif

That is our _portable_ way to do it.

So, technically we have everything covered, at least since 8.0.2.

The problem is that the call to thread.c::pqGetpwuid() happens in a
#ifndef WIN32 block, so the function is not seen by Win32, so we don't
get a compile error.

I am thinking your patch is a good idea because it will give us a
non-Win32 platform that has the _check_ behavior. We do have to bump
the major for ecpg libs for this, but not libpq.

---------------------------------------------------------------------------

Tom Lane wrote:
> Recent versions of Darwin spew a lot of multiply-defined-symbol warnings
> while building Postgres, mostly because the programs in src/bin import
> both libpq and libpgport, and libpq includes copies of many of the
> libpgport files. Since the libpgport routines aren't officially part of
> the libpq API, it'd be better if those symbols weren't visible from
> outside libpq. The attached patch makes this so, using the already
> existing exports.txt list as the definition of libpq's official API.
>
> I found while testing the patch that we have one API leak in current
> sources: ecpglib is depending on the libpq-exported pqGetpwuid()
> because src/port/path.c requires it but src/port/thread.c isn't included
> into ecpglib. The patch corrects this oversight. This BTW is
> sufficient reason for a libpq major version bump if we apply this patch;
> we learned that lesson last time we fixed an API leak. (Did we already
> bump libpq's major version for 8.2? I don't recall.)
>
> The reason I didn't go ahead and apply this is that I'm wondering if
> there's some more-portable solution that would work for other platforms
> besides Darwin. I seem to recall that we've discussed the point before,
> but no patch has been forthcoming.
>
> Comments?
>
> regards, tom lane
>

Content-Description: darwin-symbols.patch

> Index: src/Makefile.shlib
> ===================================================================
> RCS file: /cvsroot/pgsql/src/Makefile.shlib,v
> retrieving revision 1.103
> diff -c -r1.103 Makefile.shlib
> *** src/Makefile.shlib 19 Apr 2006 16:32:08 -0000 1.103
> --- src/Makefile.shlib 20 Apr 2006 00:56:38 -0000
> ***************
> *** 107,117 ****
> ifeq ($(DLTYPE), library)
> # linkable library
> DLSUFFIX := .dylib
> ! LINK.shared = $(COMPILER) -dynamiclib -install_name $(libdir)/lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX) $(version_link) -multiply_defined suppress
> else
> # loadable module (default case)
> DLSUFFIX := .so
> ! LINK.shared = $(COMPILER) -bundle
> endif
> shlib = lib$(NAME).$(SO_MAJOR_VERSION).$(SO_MINOR_VERSION)$(DLSUFFIX)
> shlib_major = lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX)
> --- 107,117 ----
> ifeq ($(DLTYPE), library)
> # linkable library
> DLSUFFIX := .dylib
> ! LINK.shared = $(COMPILER) -dynamiclib -install_name $(libdir)/lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX) $(version_link) $(exported_symbols_list) -multiply_defined suppress
> else
> # loadable module (default case)
> DLSUFFIX := .so
> ! LINK.shared = $(COMPILER) -bundle -multiply_defined suppress
> endif
> shlib = lib$(NAME).$(SO_MAJOR_VERSION).$(SO_MINOR_VERSION)$(DLSUFFIX)
> shlib_major = lib$(NAME).$(SO_MAJOR_VERSION)$(DLSUFFIX)
> Index: src/interfaces/libpq/Makefile
> ===================================================================
> RCS file: /cvsroot/pgsql/src/interfaces/libpq/Makefile,v
> retrieving revision 1.143
> diff -c -r1.143 Makefile
> *** src/interfaces/libpq/Makefile 11 Apr 2006 20:26:40 -0000 1.143
> --- src/interfaces/libpq/Makefile 20 Apr 2006 00:56:39 -0000
> ***************
> *** 125,130 ****
> --- 125,143 ----
> echo '; Aliases for MS compatible names' >> $@
> sed -e '/^#/d' -e 's/^\(.* \)\([0-9][0-9]*\)/ \1= _\1/' < $< | sed 's/ *$$//' >> $@
>
> + # On Darwin, restrict the symbols exported by the library to just the
> + # official list, so as to avoid multiply-defined-symbol warnings in
> + # programs that use libpgport along with libpq.
> +
> + ifeq ($(PORTNAME), darwin)
> + $(shlib): exports.darwin
> +
> + exports.darwin: exports.txt
> + $(AWK) '/^[^#]/ {printf "_%s\n",$$1}' $< >$@
> +
> + exported_symbols_list = -exported_symbols_list exports.darwin
> + endif
> +
> # depend on Makefile.global to force rebuild on re-run of configure
> $(srcdir)/libpq.rc: libpq.rc.in $(top_builddir)/src/Makefile.global
> sed -e 's/\(VERSION.*\),0 *$$/\1,'`date '+%y%j' | sed 's/^0*//'`'/' < $< > $@
> ***************
> *** 147,153 ****
> rm -f '$(DESTDIR)$(includedir)/libpq-fe.h' '$(DESTDIR)$(includedir_internal)/libpq-int.h' '$(DESTDIR)$(includedir_internal)/pqexpbuffer.h' '$(DESTDIR)$(datadir)/pg_service.conf.sample'
>
> clean distclean: clean-lib
> ! rm -f $(OBJS) pg_config_paths.h crypt.c getaddrinfo.c inet_aton.c noblock.c pgstrcasecmp.c snprintf.c strerror.c open.c thread.c md5.c ip.c encnames.c wchar.c pthread.h
> rm -f pg_config_paths.h # Might be left over from a Win32 client-only build
>
> maintainer-clean: distclean
> --- 160,166 ----
> rm -f '$(DESTDIR)$(includedir)/libpq-fe.h' '$(DESTDIR)$(includedir_internal)/libpq-int.h' '$(DESTDIR)$(includedir_internal)/pqexpbuffer.h' '$(DESTDIR)$(datadir)/pg_service.conf.sample'
>
> clean distclean: clean-lib
> ! rm -f $(OBJS) pg_config_paths.h crypt.c getaddrinfo.c inet_aton.c noblock.c pgstrcasecmp.c snprintf.c strerror.c open.c thread.c md5.c ip.c encnames.c wchar.c pthread.h exports.darwin
> rm -f pg_config_paths.h # Might be left over from a Win32 client-only build
>
> maintainer-clean: distclean
> Index: src/interfaces/ecpg/ecpglib/Makefile
> ===================================================================
> RCS file: /cvsroot/pgsql/src/interfaces/ecpg/ecpglib/Makefile,v
> retrieving revision 1.38
> diff -c -r1.38 Makefile
> *** src/interfaces/ecpg/ecpglib/Makefile 17 Jan 2006 19:49:23 -0000 1.38
> --- src/interfaces/ecpg/ecpglib/Makefile 20 Apr 2006 00:56:39 -0000
> ***************
> *** 25,31 ****
> LIBS := $(filter-out -lpgport, $(LIBS))
>
> OBJS= execute.o typename.o descriptor.o data.o error.o prepare.o memory.o \
> ! connect.o misc.o path.o exec.o \
> $(filter snprintf.o, $(LIBOBJS))
>
> SHLIB_LINK = -L../pgtypeslib -lpgtypes $(libpq) \
> --- 25,31 ----
> LIBS := $(filter-out -lpgport, $(LIBS))
>
> OBJS= execute.o typename.o descriptor.o data.o error.o prepare.o memory.o \
> ! connect.o misc.o path.o exec.o thread.o \
> $(filter snprintf.o, $(LIBOBJS))
>
> SHLIB_LINK = -L../pgtypeslib -lpgtypes $(libpq) \
> ***************
> *** 46,52 ****
> # necessarily use the same object files as the backend uses. Instead,
> # symlink the source files in here and build our own object file.
>
> ! path.c exec.c snprintf.c: % : $(top_srcdir)/src/port/%
> rm -f $@ && $(LN_S) $< .
>
> path.o: path.c $(top_builddir)/src/port/pg_config_paths.h
> --- 46,52 ----
> # necessarily use the same object files as the backend uses. Instead,
> # symlink the source files in here and build our own object file.
>
> ! path.c exec.c snprintf.c thread.c: % : $(top_srcdir)/src/port/%
> rm -f $@ && $(LN_S) $< .
>
> path.o: path.c $(top_builddir)/src/port/pg_config_paths.h
> ***************
> *** 62,68 ****
> uninstall: uninstall-lib
>
> clean distclean maintainer-clean: clean-lib
> ! rm -f $(OBJS) path.c exec.c snprintf.c
>
> depend dep:
> $(CC) -MM $(CFLAGS) *.c >depend
> --- 62,68 ----
> uninstall: uninstall-lib
>
> clean distclean maintainer-clean: clean-lib
> ! rm -f $(OBJS) path.c exec.c snprintf.c thread.c
>
> depend dep:
> $(CC) -MM $(CFLAGS) *.c >depend

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +