[libvirt PATCH v3] meson: Improve RPATH handling

Andrea Bolognani posted 1 patch 3 years, 7 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200819142931.46760-1-abologna@redhat.com
meson.build       | 10 ++++++++++
meson_options.txt |  1 +
src/meson.build   | 12 ++++++------
tools/meson.build |  8 ++++----
4 files changed, 21 insertions(+), 10 deletions(-)
[libvirt PATCH v3] meson: Improve RPATH handling
Posted by Andrea Bolognani 3 years, 7 months ago
Right now we're unconditionally adding RPATH information to the
installed binaries and libraries, but that's not always desired.

autotools seem to be smart enough to only include that information
when targeting a non-standard prefix, so most distro packages
don't actually contain it; moreover, both Debian and Fedora have
wiki pages encouraging packagers to avoid setting RPATH:

  https://wiki.debian.org/RpathIssue
  https://fedoraproject.org/wiki/RPath_Packaging_Draft

Implement RPATH logic that Does The Right Thing™ in the most
common cases, while still offering users the ability to override
the default behavior if they have specific needs.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 meson.build       | 10 ++++++++++
 meson_options.txt |  1 +
 src/meson.build   | 12 ++++++------
 tools/meson.build |  8 ++++----
 4 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/meson.build b/meson.build
index a72d0c0e85..d81aceae7a 100644
--- a/meson.build
+++ b/meson.build
@@ -156,6 +156,16 @@ if rc.returncode() == 0
 endif
 
 
+# Add RPATH information when building for a non-standard prefix, or
+# when explicitly requested to do so
+
+if prefix == '/usr' and not get_option('rpath').enabled()
+  libvirt_rpath = ''
+else
+  libvirt_rpath = libdir
+endif
+
+
 # figure out libvirt version strings
 
 arr_version = meson.project_version().split('.')
diff --git a/meson_options.txt b/meson_options.txt
index c538d323c1..79554c3186 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -6,6 +6,7 @@ option('runstatedir', type: 'string', value: '', description: 'State directory f
 option('expensive_tests', type: 'feature', value: 'auto', description: 'set the default for enabling expensive tests (long timeouts), use VIR_TEST_EXPENSIVE to override')
 option('test_coverage', type: 'boolean', value: false, description: 'turn on code coverage instrumentation')
 option('git_werror', type: 'feature', value: 'auto', description: 'use -Werror if building from GIT')
+option('rpath', type: 'feature', value: 'auto', description: 'whether to include rpath information in installed binaries and libraries')
 
 
 # build dependencies options
diff --git a/src/meson.build b/src/meson.build
index 73ac99f01e..d058cec2e4 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -454,7 +454,7 @@ libvirt_qemu_lib = shared_library(
     libvirt_qemu_syms_file,
   ],
   install: true,
-  install_rpath: libdir,
+  install_rpath: libvirt_rpath,
   version: libvirt_lib_version,
   soversion: libvirt_so_version,
 )
@@ -510,7 +510,7 @@ libvirt_lxc_lib = shared_library(
     libvirt_lxc_syms_file,
   ],
   install: true,
-  install_rpath: libdir,
+  install_rpath: libvirt_rpath,
   version: libvirt_lib_version,
   soversion: libvirt_so_version,
 )
@@ -554,7 +554,7 @@ libvirt_admin_lib = shared_library(
     libvirt_admin_syms_file,
   ],
   install: true,
-  install_rpath: libdir,
+  install_rpath: libvirt_rpath,
   version: libvirt_lib_version,
   soversion: libvirt_so_version,
 )
@@ -588,7 +588,7 @@ foreach module : virt_modules
     ],
     install: true,
     install_dir: module.get('install_dir', libdir / 'libvirt' / 'connection-driver'),
-    install_rpath: libdir,
+    install_rpath: libvirt_rpath,
   )
   set_variable('@0@_module'.format(module['name'].underscorify()), mod)
 endforeach
@@ -633,7 +633,7 @@ foreach daemon : virt_daemons
     ],
     install: true,
     install_dir: sbindir,
-    install_rpath: libdir,
+    install_rpath: libvirt_rpath,
   )
 endforeach
 
@@ -661,7 +661,7 @@ foreach helper : virt_helpers
     ],
     install: true,
     install_dir: helper.get('install_dir', libexecdir),
-    install_rpath: libdir,
+    install_rpath: libvirt_rpath,
   )
 endforeach
 
diff --git a/tools/meson.build b/tools/meson.build
index 090179470a..e18adfa7f2 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -75,7 +75,7 @@ if conf.has('WITH_HOST_VALIDATE')
     ],
     install: true,
     install_dir: bindir,
-    install_rpath: libdir,
+    install_rpath: libvirt_rpath,
   )
 endif
 
@@ -112,7 +112,7 @@ if conf.has('WITH_LOGIN_SHELL')
     ],
     install: true,
     install_dir: libexecdir,
-    install_rpath: libdir,
+    install_rpath: libvirt_rpath,
   )
 
   install_data('virt-login-shell.conf', install_dir: sysconfdir / 'libvirt')
@@ -197,7 +197,7 @@ executable(
   ],
   install: true,
   install_dir: bindir,
-  install_rpath: libdir,
+  install_rpath: libvirt_rpath,
 )
 
 executable(
@@ -219,7 +219,7 @@ executable(
   ],
   install: true,
   install_dir: bindir,
-  install_rpath: libdir,
+  install_rpath: libvirt_rpath,
 )
 
 tools_conf = configuration_data()
-- 
2.26.2

Re: [libvirt PATCH v3] meson: Improve RPATH handling
Posted by Andrea Bolognani 3 years, 7 months ago
On Wed, 2020-08-19 at 16:29 +0200, Andrea Bolognani wrote:
> Right now we're unconditionally adding RPATH information to the
> installed binaries and libraries, but that's not always desired.
> 
> autotools seem to be smart enough to only include that information
> when targeting a non-standard prefix, so most distro packages
> don't actually contain it; moreover, both Debian and Fedora have
> wiki pages encouraging packagers to avoid setting RPATH:
> 
>   https://wiki.debian.org/RpathIssue
>   https://fedoraproject.org/wiki/RPath_Packaging_Draft
> 
> Implement RPATH logic that Does The Right Thing™ in the most
> common cases, while still offering users the ability to override
> the default behavior if they have specific needs.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  meson.build       | 10 ++++++++++
>  meson_options.txt |  1 +
>  src/meson.build   | 12 ++++++------
>  tools/meson.build |  8 ++++----
>  4 files changed, 21 insertions(+), 10 deletions(-)

A polite ping.

We should really make sure 6.7.0 includes this, because dealing with
the RPATH situation will be pretty annoying for distros otherwise.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH v3] meson: Improve RPATH handling
Posted by Pavel Hrdina 3 years, 7 months ago
On Wed, Aug 19, 2020 at 04:29:31PM +0200, Andrea Bolognani wrote:
> Right now we're unconditionally adding RPATH information to the
> installed binaries and libraries, but that's not always desired.
> 
> autotools seem to be smart enough to only include that information
> when targeting a non-standard prefix, so most distro packages
> don't actually contain it; moreover, both Debian and Fedora have
> wiki pages encouraging packagers to avoid setting RPATH:
> 
>   https://wiki.debian.org/RpathIssue
>   https://fedoraproject.org/wiki/RPath_Packaging_Draft
> 
> Implement RPATH logic that Does The Right Thing™ in the most
> common cases, while still offering users the ability to override
> the default behavior if they have specific needs.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  meson.build       | 10 ++++++++++
>  meson_options.txt |  1 +
>  src/meson.build   | 12 ++++++------
>  tools/meson.build |  8 ++++----
>  4 files changed, 21 insertions(+), 10 deletions(-)

Looks reasonable to not add RPATH when using /usr as prefix.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>