Re: Use relative rpath if possible
On Mon, Jul 8, 2019 at 2:01 AM Tom Lane wrote: > I wrote: > > Peter Eisentraut writes: > >> rebased patch attached, no functionality changes > > > I poked at this a bit, and soon found that it fails check-world, > > because the isolationtester binary is built with an rpath that > > only works if it's part of the temp install tree, which it ain't. > > Oh ... just thought of another issue in the same vein: what about > modules being built out-of-tree with pgxs? (I'm imagining something > with a libpq.so dependency, like postgres_fdw.) We probably really > have to keep using the absolute rpath for that, because not only > would such modules certainly fail "make check" with a relative > rpath, but it's not really certain that they're intended to get > installed into the same installdir as the core libraries. There were a number of problems flagged up in Tom's feedback and then silence. I think this belongs in the 'Returned with feedback' box, so I've set it to that, but of course feel free to set it to 'Needs review' and thence 'Move to next CF'. -- Thomas Munro https://enterprisedb.com
Re: Use relative rpath if possible
I wrote: > Peter Eisentraut writes: >> rebased patch attached, no functionality changes > I poked at this a bit, and soon found that it fails check-world, > because the isolationtester binary is built with an rpath that > only works if it's part of the temp install tree, which it ain't. Oh ... just thought of another issue in the same vein: what about modules being built out-of-tree with pgxs? (I'm imagining something with a libpq.so dependency, like postgres_fdw.) We probably really have to keep using the absolute rpath for that, because not only would such modules certainly fail "make check" with a relative rpath, but it's not really certain that they're intended to get installed into the same installdir as the core libraries. regards, tom lane
Re: Use relative rpath if possible
Peter Eisentraut writes: > rebased patch attached, no functionality changes I poked at this a bit, and soon found that it fails check-world, because the isolationtester binary is built with an rpath that only works if it's part of the temp install tree, which it ain't. /home/postgres/pgsql/src/test/isolation/isolationtester: error while loading shared libraries: libpq.so.5: cannot open shared object file: No such file or directory (The cfbot seems to get past that, for reasons that are entirely unclear to me; but it falls over later in the ecpg tests with what's presumably the same problem.) While there might be some argument for making isolationtester part of the installed set of executables, that approach certainly doesn't scale; we can't insist that every test tool should be part of the installation. So I think we need some more-intelligent rule about when to apply the relative rpath. Which in turn seems to mean we still need to set up LD_LIBRARY_PATH in some cases. Another thing I noticed is that if, say, you build a new version of psql and try to test it out with "./psql ...", that doesn't work anymore (whereas today, it does work as long as you installed libpq earlier). That might be acceptable collateral damage, but it's not very desirable IMO. I'm also slightly concerned that this effectively mandates that every library we install be immediately in $(libdir), never subdirectories thereof; else it'd need more than one ".." in its rpath and there's no way to adjust that. That's not a showstopper problem probably, because we have no such libraries today, but I wonder if somebody would want some in the future. A possible partial solution to these issues is to make the rpath look like $ORIGIN/../lib and then the normal absolute rpath. But that doesn't fix the problem for non-installed binaries used in check-world with no pre-existing installation. >> (Yes, something for macOS would be nice, to work around SIP issues, but >> I'll leave that as a separate future item.) TBH, I think that supporting macOS with SIP enabled is really the only interesting case here. On these other platforms, changing this won't fix anything very critical, and it seems like it will make some cases worse. regards, tom lane
Re: Use relative rpath if possible
rebased patch attached, no functionality changes On 2019-06-27 13:45, Peter Eisentraut wrote: > On several popular operating systems, we can use relative rpaths, using > the $ORIGIN placeholder, so that the resulting installation is > relocatable. Then we also don't need to set LD_LIBRARY_PATH during make > check. > > This implementation will use a relative rpath if bindir and libdir are > under the same common parent directory. > > Supported platforms are: freebsd, linux, netbsd, openbsd, solaris > > Information from https://lekensteyn.nl/rpath.html > > (Yes, something for macOS would be nice, to work around SIP issues, but > I'll leave that as a separate future item.) -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services From 6a86e8c64d5bb416c3001dea2dae3f6365085e9b Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 5 Jul 2019 11:35:44 +0200 Subject: [PATCH v2] Use relative rpath if possible On several popular operating systems, we can use relative rpaths, using the $ORIGIN placeholder, so that the resulting installation is relocatable. Then we also don't need to set LD_LIBRARY_PATH during make check. This implementation will use a relative rpath if bindir and libdir are under the same common parent directory. --- src/Makefile.global.in | 2 +- src/makefiles/Makefile.freebsd | 5 + src/makefiles/Makefile.linux | 5 + src/makefiles/Makefile.netbsd | 5 + src/makefiles/Makefile.openbsd | 5 + src/makefiles/Makefile.solaris | 10 ++ 6 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 321af38b0c..69657e8ab8 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -422,7 +422,7 @@ ld_library_path_var = LD_LIBRARY_PATH # nothing. with_temp_install = \ PATH="$(abs_top_builddir)/tmp_install$(bindir):$$PATH" \ - $(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \ + $(if $(strip $(ld_library_path_var)),$(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir))) \ $(with_temp_install_extra) ifeq ($(enable_tap_tests),yes) diff --git a/src/makefiles/Makefile.freebsd b/src/makefiles/Makefile.freebsd index c462e2fd58..ae92dba5ee 100644 --- a/src/makefiles/Makefile.freebsd +++ b/src/makefiles/Makefile.freebsd @@ -1,7 +1,12 @@ AROPT = cr export_dynamic = -Wl,-export-dynamic +ifeq ($(dir $(bindir)),$(dir $(libdir))) +rpath = -Wl,-R'$(subst $(dir $(libdir)),$$ORIGIN/../,$(rpathdir))',-z,origin +ld_library_path_var = +else rpath = -Wl,-R'$(rpathdir)' +endif DLSUFFIX = .so diff --git a/src/makefiles/Makefile.linux b/src/makefiles/Makefile.linux index ac58fe45de..d967eec9de 100644 --- a/src/makefiles/Makefile.linux +++ b/src/makefiles/Makefile.linux @@ -3,7 +3,12 @@ AROPT = crs export_dynamic = -Wl,-E # Use --enable-new-dtags to generate DT_RUNPATH instead of DT_RPATH. # This allows LD_LIBRARY_PATH to still work when needed. +ifeq ($(dir $(bindir)),$(dir $(libdir))) +rpath = -Wl,-rpath,'$(subst $(dir $(libdir)),$$ORIGIN/../,$(rpathdir))',--enable-new-dtags +ld_library_path_var = +else rpath = -Wl,-rpath,'$(rpathdir)',--enable-new-dtags +endif DLSUFFIX = .so diff --git a/src/makefiles/Makefile.netbsd b/src/makefiles/Makefile.netbsd index 15695fb65c..4a96be9121 100644 --- a/src/makefiles/Makefile.netbsd +++ b/src/makefiles/Makefile.netbsd @@ -1,7 +1,12 @@ AROPT = cr export_dynamic = -Wl,-E +ifeq ($(dir $(bindir)),$(dir $(libdir))) +rpath = -Wl,-R'$(subst $(dir $(libdir)),$$ORIGIN/../,$(rpathdir))' +ld_library_path_var = +else rpath = -Wl,-R'$(rpathdir)' +endif DLSUFFIX = .so diff --git a/src/makefiles/Makefile.openbsd b/src/makefiles/Makefile.openbsd index 15695fb65c..36b7420057 100644 --- a/src/makefiles/Makefile.openbsd +++ b/src/makefiles/Makefile.openbsd @@ -1,7 +1,12 @@ AROPT = cr export_dynamic = -Wl,-E +ifeq ($(dir $(bindir)),$(dir $(libdir))) +rpath = -Wl,-R'$(subst $(dir $(libdir)),$$ORIGIN/../,$(rpathdir))',-z,origin +ld_library_path_var = +else rpath = -Wl,-R'$(rpathdir)' +endif DLSUFFIX = .so diff --git a/src/makefiles/Makefile.solaris b/src/makefiles/Makefile.solaris index a7f5652f0c..b61fdabbad 100644 --- a/src/makefiles/Makefile.solaris +++ b/src/makefiles/Makefile.solaris @@ -4,10 +4,20 @@ AROPT = crs ifeq ($(with_gnu_ld), yes) export_dynamic = -Wl,-E +ifeq ($(dir $(bindir)),$(dir $(libdir))) +rpath = -Wl,-rpath,'$(subst $(dir $(libdir)),$$ORIGIN/../,$(rpathdir))' +ld_library_path_var = +else rpath = -Wl,-rpath,'$(rpathdir)' +endif +else +ifeq ($(dir $(bindir)),$(dir $(libdir))) +rpath = -Wl,-R'$(subst $(dir $(libdir)),$$ORIGIN/../,$(rpathdir))' +ld_library_path_var = else rpath = -Wl,-R'$(rpathdir)' endif +endif DLSUFFIX = .so ifeq ($(GCC), yes) base-commit: 594df378ffb04a72b713a13cc0a7166b3bced7b7 -- 2.22.0
Use relative rpath if possible
On several popular operating systems, we can use relative rpaths, using the $ORIGIN placeholder, so that the resulting installation is relocatable. Then we also don't need to set LD_LIBRARY_PATH during make check. This implementation will use a relative rpath if bindir and libdir are under the same common parent directory. Supported platforms are: freebsd, linux, netbsd, openbsd, solaris Information from https://lekensteyn.nl/rpath.html (Yes, something for macOS would be nice, to work around SIP issues, but I'll leave that as a separate future item.) -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services From e15d15749a7b19fc9fe933f3728668299a0201f4 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Tue, 25 Jun 2019 11:54:51 + Subject: [PATCH] Use relative rpath if possible On several popular operating systems, we can use relative rpaths, using the $ORIGIN placeholder, so that the resulting installation is relocatable. Then we also don't need to set LD_LIBRARY_PATH during make check. This implementation will use a relative rpath if bindir and libdir are under the same common parent directory. --- src/Makefile.global.in | 2 +- src/makefiles/Makefile.freebsd | 5 + src/makefiles/Makefile.linux | 5 + src/makefiles/Makefile.netbsd | 5 + src/makefiles/Makefile.openbsd | 5 + src/makefiles/Makefile.solaris | 10 ++ 6 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/Makefile.global.in b/src/Makefile.global.in index b9d86ac..0b69064 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -422,7 +422,7 @@ ld_library_path_var = LD_LIBRARY_PATH # nothing. with_temp_install = \ PATH="$(abs_top_builddir)/tmp_install$(bindir):$$PATH" \ - $(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \ + $(if $(strip $(ld_library_path_var)),$(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir))) \ $(with_temp_install_extra) ifeq ($(enable_tap_tests),yes) diff --git a/src/makefiles/Makefile.freebsd b/src/makefiles/Makefile.freebsd index 98a6f50..6378551 100644 --- a/src/makefiles/Makefile.freebsd +++ b/src/makefiles/Makefile.freebsd @@ -2,8 +2,13 @@ AROPT = cr ifdef ELF_SYSTEM export_dynamic = -Wl,-export-dynamic +ifeq ($(dir $(bindir)),$(dir $(libdir))) +rpath = -Wl,-R,'$(subst $(dir $(libdir)),$$ORIGIN/../,$(rpathdir))',-z,origin +ld_library_path_var = +else rpath = -Wl,-R'$(rpathdir)' endif +endif DLSUFFIX = .so diff --git a/src/makefiles/Makefile.linux b/src/makefiles/Makefile.linux index ac58fe4..d967eec 100644 --- a/src/makefiles/Makefile.linux +++ b/src/makefiles/Makefile.linux @@ -3,7 +3,12 @@ AROPT = crs export_dynamic = -Wl,-E # Use --enable-new-dtags to generate DT_RUNPATH instead of DT_RPATH. # This allows LD_LIBRARY_PATH to still work when needed. +ifeq ($(dir $(bindir)),$(dir $(libdir))) +rpath = -Wl,-rpath,'$(subst $(dir $(libdir)),$$ORIGIN/../,$(rpathdir))',--enable-new-dtags +ld_library_path_var = +else rpath = -Wl,-rpath,'$(rpathdir)',--enable-new-dtags +endif DLSUFFIX = .so diff --git a/src/makefiles/Makefile.netbsd b/src/makefiles/Makefile.netbsd index 7bb9721..5cc152c 100644 --- a/src/makefiles/Makefile.netbsd +++ b/src/makefiles/Makefile.netbsd @@ -2,7 +2,12 @@ AROPT = cr ifdef ELF_SYSTEM export_dynamic = -Wl,-E +ifeq ($(dir $(bindir)),$(dir $(libdir))) +rpath = -Wl,-R,'$(subst $(dir $(libdir)),$$ORIGIN/../,$(rpathdir))' +ld_library_path_var = +else rpath = -Wl,-R'$(rpathdir)' +endif else rpath = -Wl,-R'$(rpathdir)' endif diff --git a/src/makefiles/Makefile.openbsd b/src/makefiles/Makefile.openbsd index eda3110..c03987d 100644 --- a/src/makefiles/Makefile.openbsd +++ b/src/makefiles/Makefile.openbsd @@ -2,8 +2,13 @@ AROPT = cr ifdef ELF_SYSTEM export_dynamic = -Wl,-E +ifeq ($(dir $(bindir)),$(dir $(libdir))) +rpath = -Wl,-R,'$(subst $(dir $(libdir)),$$ORIGIN/../,$(rpathdir))',-z,origin +ld_library_path_var = +else rpath = -Wl,-R'$(rpathdir)' endif +endif DLSUFFIX = .so diff --git a/src/makefiles/Makefile.solaris b/src/makefiles/Makefile.solaris index a7f5652..b61757f 100644 --- a/src/makefiles/Makefile.solaris +++ b/src/makefiles/Makefile.solaris @@ -4,10 +4,20 @@ AROPT = crs ifeq ($(with_gnu_ld), yes) export_dynamic = -Wl,-E +ifeq ($(dir $(bindir)),$(dir $(libdir))) +rpath = -Wl,-rpath,'$(subst $(dir $(libdir)),$$ORIGIN/../,$(rpathdir))' +ld_library_path_var = +else rpath = -Wl,-rpath,'$(rpathdir)' +endif +else +ifeq ($(dir $(bindir)),$(dir $(libdir))) +rpath = -Wl,-R,'$(subst $(dir $(libdir)),$$ORIGIN/../,$(rpathdir))' +ld_library_path_var = else rpath = -Wl,-R'$(rpathdir)' endif +endif DLSUFFIX = .so ifeq ($(GCC), yes) -- 1.8.3.1
