Fix pkg-config file for static linking

Started by Filip Gospodinovover 4 years ago7 messages
#1Filip Gospodinov
f@gospodinov.ch
1 attachment(s)

Since https://github.com/postgres/postgres/commit/ea53100d5 (or Postgres
12.0) the shipped pkg-config file is broken for statically linking libpq
because libpgcommon and libpgport are missing. This patch adds those two
missing private dependencies.

Attachments:

pkgconfig.patchtext/x-patch; charset=UTF-8; name=pkgconfig.patchDownload
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 0c4e55b6ad..fe21335d2d 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -94,7 +94,7 @@ SHLIB_PREREQS = submake-libpgport
 
 SHLIB_EXPORTS = exports.txt
 
-PKG_CONFIG_REQUIRES_PRIVATE = libssl libcrypto
+PKG_CONFIG_REQUIRES_PRIVATE = libpgcommon libpgport libssl libcrypto
 
 all: all-lib
 
#2Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Filip Gospodinov (#1)
Re: Fix pkg-config file for static linking

On 21.06.21 15:47, Filip Gospodinov wrote:

-PKG_CONFIG_REQUIRES_PRIVATE = libssl libcrypto
+PKG_CONFIG_REQUIRES_PRIVATE = libpgcommon libpgport libssl libcrypto

This doesn't work.

This patch adds libpgcommon and libpgport to Requires.private. But they
are not pkg-config names but library names, so they should be added to
Libs.private.

#3Filip Gospodinov
f@gospodinov.ch
In reply to: Peter Eisentraut (#2)
1 attachment(s)
Re: Fix pkg-config file for static linking

On 06.07.21 15:13, Peter Eisentraut wrote:

This doesn't work.

This patch adds libpgcommon and libpgport to Requires.private.  But they
are not pkg-config names but library names, so they should be added to
Libs.private.

Then, I must admit that I have misunderstood this patch at first
https://github.com/postgres/postgres/commit/beff361bc1edc24ee5f8b2073a1e5e4c92ea66eb
.

My impression was that PKG_CONFIG_REQUIRES_PRIVATE ends up in
Libs.private because of this line
https://github.com/postgres/postgres/blob/d9809bf8694c17e05537c5dd96cde3e67c02d52a/src/Makefile.shlib#L403
.

After taking a second look, I've noticed that
PKG_CONFIG_REQUIRES_PRIVATE is *filtered out*. But unfortunately, this
currently doesn't work as intended because PKG_CONFIG_REQUIRES_PRIVATE
seems to be unset in Makefile.shlib which leaves Requires.private empty
and doesn't filter out -lcrypto and -lssl for Libs.private.
That must be also the reason why I first believed that
PKG_CONFIG_REQUIRES_PRIVATE is used to populate Libs.private.

Anyway, this issue is orthogonal to my original patch. I'm proposing to
hardcode -lpgcommon and -lpgport in Libs.private instead. Patch is attached.

Attachments:

pkgconfig-v2.patchtext/x-patch; charset=UTF-8; name=pkgconfig-v2.patchDownload
diff --git a/src/Makefile.shlib b/src/Makefile.shlib
index 29a7f6d38c..d643473807 100644
--- a/src/Makefile.shlib
+++ b/src/Makefile.shlib
@@ -400,7 +400,7 @@ endif # PORTNAME == cygwin || PORTNAME == win32
 # those that point inside the build or source tree.  Use sort to
 # remove duplicates.  Also record the -l flags necessary for static
 # linking, but not those already covered by Requires.private.
-	echo 'Libs.private: $(sort $(filter-out -L.% -L$(top_srcdir)/%,$(filter -L%,$(LDFLAGS) $(SHLIB_LINK)))) $(filter-out $(PKG_CONFIG_REQUIRES_PRIVATE:lib%=-l%),$(filter -l%,$(SHLIB_LINK)))' >>$@
+	echo 'Libs.private: $(sort $(filter-out -L.% -L$(top_srcdir)/%,$(filter -L%,$(LDFLAGS) $(SHLIB_LINK)))) -lpgcommon -lpgport $(filter-out $(PKG_CONFIG_REQUIRES_PRIVATE:lib%=-l%),$(filter -l%,$(SHLIB_LINK)))' >>$@
 
 
 ##
#4Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Filip Gospodinov (#3)
1 attachment(s)
Re: Fix pkg-config file for static linking

On 20.07.21 22:04, Filip Gospodinov wrote:

Anyway, this issue is orthogonal to my original patch. I'm proposing to
hardcode -lpgcommon and -lpgport in Libs.private instead. Patch is
attached.

Makes sense. I think we could do it without hardcoding those library
names, as in the attached patch. But it comes out to the same result
AFAICT.

Attachments:

pkgconfig-v3.patchtext/plain; charset=UTF-8; name=pkgconfig-v3.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/src/Makefile.shlib b/src/Makefile.shlib
index 29a7f6d38c..4b2a62fa14 100644
--- a/src/Makefile.shlib
+++ b/src/Makefile.shlib
@@ -400,7 +400,7 @@ endif # PORTNAME == cygwin || PORTNAME == win32
 # those that point inside the build or source tree.  Use sort to
 # remove duplicates.  Also record the -l flags necessary for static
 # linking, but not those already covered by Requires.private.
-	echo 'Libs.private: $(sort $(filter-out -L.% -L$(top_srcdir)/%,$(filter -L%,$(LDFLAGS) $(SHLIB_LINK)))) $(filter-out $(PKG_CONFIG_REQUIRES_PRIVATE:lib%=-l%),$(filter -l%,$(SHLIB_LINK)))' >>$@
+	echo 'Libs.private: $(sort $(filter-out -L.% -L$(top_srcdir)/%,$(filter -L%,$(LDFLAGS) $(SHLIB_LINK)))) $(filter-out $(PKG_CONFIG_REQUIRES_PRIVATE:lib%=-l%),$(filter -l%,$(SHLIB_LINK_INTERNAL) $(SHLIB_LINK)))' >>$@
 
 
 ##
#5Filip Gospodinov
f@gospodinov.ch
In reply to: Peter Eisentraut (#4)
Re: Fix pkg-config file for static linking

On 02.09.21 13:07, Peter Eisentraut wrote:

On 20.07.21 22:04, Filip Gospodinov wrote:

Anyway, this issue is orthogonal to my original patch. I'm proposing
to hardcode -lpgcommon and -lpgport in Libs.private instead. Patch is
attached.

Makes sense.  I think we could do it without hardcoding those library
names, as in the attached patch.  But it comes out to the same result
AFAICT.

That is my impression too. Enumerating them in a variable would just
lead to an indirection. Please let me know if you'd still prefer a
solution without hardcoding.

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#4)
Re: Fix pkg-config file for static linking

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

Makes sense. I think we could do it without hardcoding those library
names, as in the attached patch. But it comes out to the same result
AFAICT.

This has been pushed, but the CF entry is still open, which is
making the cfbot unhappy. Were you leaving it open pending
pushing to back branches as well? I'm not sure what the point
of waiting is --- the buildfarm isn't going to exercise the
troublesome scenario.

regards, tom lane

#7Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Tom Lane (#6)
Re: Fix pkg-config file for static linking

On 05.09.21 21:57, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

Makes sense. I think we could do it without hardcoding those library
names, as in the attached patch. But it comes out to the same result
AFAICT.

This has been pushed, but the CF entry is still open, which is
making the cfbot unhappy. Were you leaving it open pending
pushing to back branches as well? I'm not sure what the point
of waiting is --- the buildfarm isn't going to exercise the
troublesome scenario.

I noticed another fix that was required and didn't get to it until now.
It's all done and backpatched now. CF entry is closed.