Re: Cleaning up multiply-defined-symbol warnings on OS X - Mailing list pgsql-patches

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.copen.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.copen.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@postgresql.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. +