pkg-config files for libpq and ecpg

I'll take another stab at providing pkg-config files for the client-side
libraries.

The main reason this time around is that this works a lot better (or at
all) for multi-arch library installations.

Another is that pkg-config has become a lot smarter and flexible over
the years, and it's probably a better choice for users who are already
used to its interface. There is a lot of confusion, for example, about
what pg_config --libs really means. We often evade that by saying,
well, those are the libraries we linked with, but there is a lack of
clarity in that context about what libraries a user should link with.

The way it's implemented, it doesn't require manual maintenance, so it
should not be much of a bother.

A side issue that arose: libecpg_compat is linked with libpq, but
doesn't seem to use it. This was added many years ago in
cd75f94dafd43358305811b7576ad75d889097e3, but it doesn't appear to be
required anymore. Needs some checking.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> I'll take another stab at providing pkg-config files for the client-side
> libraries.

This bit:

> + echo 'Libs.private: $(filter-out $(PKG_CONFIG_REQUIRES_PRIVATE:lib%=-l%),$(filter-out -L..%, $(SHLIB_LINK)))' >>$@

appears to assume that SHLIB_LINK contains nothing except -L and -l
switches. I don't think I trust that a whole lot --- in fact, it
looks guaranteed to fail on HPUX because of -print-libgcc-file-name.
There might be other platform-specific bogosity on other platforms;
PTHREAD_LIBS seems like a likely source for instance.

Another point is that -L switches used at build time might not have much
to do with the environment a .pc file would be used in later. I see
that you're dropping relative -L paths, which is good as far as it
goes, but I wonder whether it goes far enough.

Perhaps the first step should be to filter in -l%, rather than
filter out -L% ?

regards, tom lane

On Tue, Jan 15, 2013 at 02:16:01PM -0500, Peter Eisentraut wrote:
> A side issue that arose: libecpg_compat is linked with libpq, but
> doesn't seem to use it. This was added many years ago in
> cd75f94dafd43358305811b7576ad75d889097e3, but it doesn't appear to be
> required anymore. Needs some checking.

libecpg_compat doesn't directly need libpq IIRC, but it does indirectly There
are a couple functions from libecpg that are used in libecpg_compat and libecpg
itself does need libpq.

Michael

--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

On 1/15/13 6:53 PM, Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
>> I'll take another stab at providing pkg-config files for the client-side
>> libraries.
>
> This bit:
>
>> + echo 'Libs.private: $(filter-out $(PKG_CONFIG_REQUIRES_PRIVATE:lib%=-l%),$(filter-out -L..%, $(SHLIB_LINK)))' >>$@
>
> appears to assume that SHLIB_LINK contains nothing except -L and -l
> switches. I don't think I trust that a whole lot --- in fact, it
> looks guaranteed to fail on HPUX because of -print-libgcc-file-name.
> There might be other platform-specific bogosity on other platforms;
> PTHREAD_LIBS seems like a likely source for instance.

Updated patch addressing this concern. Also added comments and
documentation.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> On 1/15/13 6:53 PM, Tom Lane wrote:
>> This bit:
>>> + echo 'Libs.private: $(filter-out $(PKG_CONFIG_REQUIRES_PRIVATE:lib%=-l%),$(filter-out -L..%, $(SHLIB_LINK)))' >>$@
>> appears to assume that SHLIB_LINK contains nothing except -L and -l
>> switches.

> Updated patch addressing this concern. Also added comments and
> documentation.

I experimented a bit with this version of the patch. The hunk that
removes -I$(libpq_srcdir) and $(libpq) from the ecpg/compatlib build
breaks the build for me, so I took it out. With that gone, I noted
that I got this when building in a Fedora RPM environment (ie, the
same way I would package PG for Fedora):

Name: libpq
Description: PostgreSQL libpq library
Url: http://www.postgresql.org/
Version: 9.3devel
Requires:
Requires.private:
Cflags: -I/usr/include -I/usr/include/libxml2
Libs: -L/usr/lib64 -lpq
Libs.private: -lssl -lcrypto -lkrb5 -lcom_err -lgssapi_krb5 -lcrypt -lldap_r -lpthread

The -I for libxml2 seems pretty darn bogus; can't we avoid that?
At least for the libraries we are currently proposing to pkgconfig-ify,
it seems to me that we only want a -I for where we are installing our
own headers; there is no need for anything else. That is,
echo 'Cflags: -I$(includedir)'
seems like plenty. We aren't exposing any other packages' headers
in the public header files for these libraries, so there's no need
to tell client packages about them.

Also, I'm underwhelmed by the usefulness of the automatically-generated
description lines. It might be better to ask the individual makefiles
to set a PKG_CONFIG_DESCRIPTION macro.

Lastly, I wonder whether it's worth adding a configure option or some
such to control whether pkgconfig files are built, or at least whether
they're installed. In a lot of environments this would just be adding
useless clutter to the $(libdir).

Otherwise it seems about ready from here.

regards, tom lane

On 3/24/13 1:55 PM, Tom Lane wrote:
> I experimented a bit with this version of the patch. The hunk that
> removes -I$(libpq_srcdir) and $(libpq) from the ecpg/compatlib build
> breaks the build for me, so I took it out.

What was the error message? Probably not important, but curious.

> With that gone, I noted
> that I got this when building in a Fedora RPM environment (ie, the
> same way I would package PG for Fedora):
>
> Name: libpq
> Description: PostgreSQL libpq library
> Url: http://www.postgresql.org/
> Version: 9.3devel
> Requires:
> Requires.private:
> Cflags: -I/usr/include -I/usr/include/libxml2
> Libs: -L/usr/lib64 -lpq
> Libs.private: -lssl -lcrypto -lkrb5 -lcom_err -lgssapi_krb5 -lcrypt -lldap_r -lpthread
>
> The -I for libxml2 seems pretty darn bogus; can't we avoid that?

We could, but only in a hardcoded way. As it is, it's part of the
general set of flags that we use to build. libxml isn't special in this
regard. It only shows up because you have it in a nondefault path. If
you had openssl somewhere funny, it would show up as well.

> At least for the libraries we are currently proposing to pkgconfig-ify,
> it seems to me that we only want a -I for where we are installing our
> own headers; there is no need for anything else. That is,
> echo 'Cflags: -I$(includedir)'
> seems like plenty. We aren't exposing any other packages' headers
> in the public header files for these libraries, so there's no need
> to tell client packages about them.

libpq exposes at least openssl and gssapi, so we need those at least.
So in general, it won't work so easily to trim this list intelligently.
That's something that could perhaps be tuned in the future, but I'd
rather offer a few flags too many than not enough.

One way to look at this for now is: It's not worse than what pg_config does.

> Also, I'm underwhelmed by the usefulness of the automatically-generated
> description lines. It might be better to ask the individual makefiles
> to set a PKG_CONFIG_DESCRIPTION macro.

I think nobody reads that, so it would be a waste of time to maintain
it. The URL is more important. I'd be more interested if some Windows
thing had already put in the need for a package description (see
PGFILEDESC). I'm not strictly against it, though.

> Lastly, I wonder whether it's worth adding a configure option or some
> such to control whether pkgconfig files are built, or at least whether
> they're installed. In a lot of environments this would just be adding
> useless clutter to the $(libdir).

Considering how much other clutter we install without any options, I
think it's fine as is.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> On 3/24/13 1:55 PM, Tom Lane wrote:
>> I experimented a bit with this version of the patch. The hunk that
>> removes -I$(libpq_srcdir) and $(libpq) from the ecpg/compatlib build
>> breaks the build for me, so I took it out.

> What was the error message? Probably not important, but curious.

ecpg's #include of libpq-fe.h failed. I speculate that you didn't
notice because you tested on a machine where libpq-fe.h exists in
/usr/include.

>> At least for the libraries we are currently proposing to pkgconfig-ify,
>> it seems to me that we only want a -I for where we are installing our
>> own headers; there is no need for anything else. That is,
>> echo 'Cflags: -I$(includedir)'
>> seems like plenty. We aren't exposing any other packages' headers
>> in the public header files for these libraries, so there's no need
>> to tell client packages about them.

> libpq exposes at least openssl and gssapi, so we need those at least.

No, it does not. A client might choose to #include those of its own
accord, but then it's the client's problem. Our exported headers do
not #include anything more exotic than <stdio.h>, and it's not the
business of the pkg-config switches to provide for anything beyond
allowing inclusions of our headers to succeed.

regards, tom lane

On Wed, 2013-03-27 at 17:06 -0400, Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > On 3/24/13 1:55 PM, Tom Lane wrote:
> >> I experimented a bit with this version of the patch. The hunk that
> >> removes -I$(libpq_srcdir) and $(libpq) from the ecpg/compatlib build
> >> breaks the build for me, so I took it out.
>
> > What was the error message? Probably not important, but curious.
>
> ecpg's #include of libpq-fe.h failed. I speculate that you didn't
> notice because you tested on a machine where libpq-fe.h exists in
> /usr/include.

Right, we need to keep libpq in CPPFLAGS, but we can remove it from
SHLIB_LINK.

> >> At least for the libraries we are currently proposing to pkgconfig-ify,
> >> it seems to me that we only want a -I for where we are installing our
> >> own headers; there is no need for anything else. That is,
> >> echo 'Cflags: -I$(includedir)'
> >> seems like plenty. We aren't exposing any other packages' headers
> >> in the public header files for these libraries, so there's no need
> >> to tell client packages about them.
>
> > libpq exposes at least openssl and gssapi, so we need those at least.
>
> No, it does not. A client might choose to #include those of its own
> accord, but then it's the client's problem. Our exported headers do
> not #include anything more exotic than <stdio.h>, and it's not the
> business of the pkg-config switches to provide for anything beyond
> allowing inclusions of our headers to succeed.

I was actually thinking of PQgetssl(), which is documented to require
OpenSSL, but that was actually changed a long time ago and the
documentation not updated.

So actually you are right, we don't need to provided any extra -I flags
(if we ignore libpq-int.h). We do need that whole logic for
Libs.private however.

So here is my updated patch, with the ecpg business changed as explained
above, and the extra magic removed from the Cflags lines.

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> So here is my updated patch, with the ecpg business changed as explained
> above, and the extra magic removed from the Cflags lines.

No objections to this version.

regards, tom lane

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> Right, we need to keep libpq in CPPFLAGS, but we can remove it from
> SHLIB_LINK.

Buildfarm member anole says otherwise. I suspected something like that
might happen --- if memory serves, there are platforms where you have to
list all required libraries on the link line, even if some of them are
only indirect dependencies of shlibs you are linking in.

I think you need to revert 3780fc6.

regards, tom lane