[libvirt PATCH v2 1/1] meson: Drop RPATH usage

[Daniel P. Berrangé]

On Wed, Aug 19, 2020 at 02:10:53PM +0200, Pavel Hrdina wrote:
> On Wed, Aug 19, 2020 at 01:22:58PM +0200, Pavel Hrdina wrote:
> > On Wed, Aug 19, 2020 at 12:47:40PM +0200, Andrea Bolognani wrote:
> > > Right now we're unconditionally adding RPATH information to the
> > > installed binaries and libraries, but that's not always desired.
> > > 
> > > Debian explicitly passes --disable-rpath to configure, and while
> > > I haven't been able to find the same option in the spec file for
> > > either Fedora or RHEL, by running
> > > 
> > >   $ readelf -d /usr/bin/virsh | grep PATH
> > > 
> > > I can see that the information is not present, so I assume they
> > > also strip it somehow.
> > > 
> > > 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
> > > 
> > > Given the above, it looks like it's actually better to not go
> > > out of our way to include that information in the first place.
> > 
> > I need to look into this because I remember adding the rpath there as
> > a result that something was not working correctly but now I don't
> > remember what was it. Originally I did not have it there.
> > 
> > Pavel
> 
> So I managed to remember what was the issue. If you run install libvirt
> into custom directory like this:
> 
>     meson build --prefix /my/custom/dir
>     ninja -C build install
> 
> and after that running:
> 
>     /my/custom/dir/bin/virsh
> 
> will fail with:
> 
>     /lib64/libvirt.so.0: version `LIBVIRT_PRIVATE_6.7.0' not found (required by ./bin/bin/virsh)
> 
> This is what autotools did by default as well and I did not know that
> there is an option --disable-rpath as it's not in output of
> `./configure --help`.
> 
> If we don't care about the use case of installing libvirt into custom
> prefix and breaking it it should be OK to remove this from meson but my
> guess is that we should not do it.

So it is only broken if you failed to set LD_LIBRARY_PATH.

rpath is basically hardcoding the association between the virsh
binary and the library so that it doesn't need LD_LIBRARY_PATH.

I'm ambivalent on whether that's important or not, though admittedly
it can avoid surprises for users not used to LD_LIBRARY_PATH type
issues.

IIUC, meson  sets rpath automatically for the binaries so you can
run from the non-installed build dir. When you run install, it
then strips the rpath for the build dir, optionally replacing it
with the "install_rpath" value.


> We can add an option like it was proposed in V1 but with the following
> changes. In meson.build we would have this:
> 
>     if get_option('rpath')
>       libvirt_rpath = libdir
>     else
>       libvirt_rpath = ''
>     endif
> 
> and all places with install_rpath would use libvirt_rpath instead of
> libdir directly and we would not have to have the craze if-else.

Yeah that would be simpler.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|