[libvirt PATCH v2 0/1] meson: Drop RPATH usage
Changes since [v1] * drop RPATH usage completely instead of making it conditional. [v1] https://www.redhat.com/archives/libvir-list/2020-August/msg00599.html Andrea Bolognani (1): meson: Drop RPATH usage src/meson.build | 6 ------ tools/meson.build | 4 ---- 2 files changed, 10 deletions(-) -- 2.26.2
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. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/meson.build | 6 ------ tools/meson.build | 4 ---- 2 files changed, 10 deletions(-) diff --git a/src/meson.build b/src/meson.build index 73ac99f01e..84d9ab9741 100644 --- a/src/meson.build +++ b/src/meson.build @@ -454,7 +454,6 @@ libvirt_qemu_lib = shared_library( libvirt_qemu_syms_file, ], install: true, - install_rpath: libdir, version: libvirt_lib_version, soversion: libvirt_so_version, ) @@ -510,7 +509,6 @@ libvirt_lxc_lib = shared_library( libvirt_lxc_syms_file, ], install: true, - install_rpath: libdir, version: libvirt_lib_version, soversion: libvirt_so_version, ) @@ -554,7 +552,6 @@ libvirt_admin_lib = shared_library( libvirt_admin_syms_file, ], install: true, - install_rpath: libdir, version: libvirt_lib_version, soversion: libvirt_so_version, ) @@ -588,7 +585,6 @@ foreach module : virt_modules ], install: true, install_dir: module.get('install_dir', libdir / 'libvirt' / 'connection-driver'), - install_rpath: libdir, ) set_variable('@0@_module'.format(module['name'].underscorify()), mod) endforeach @@ -633,7 +629,6 @@ foreach daemon : virt_daemons ], install: true, install_dir: sbindir, - install_rpath: libdir, ) endforeach @@ -661,7 +656,6 @@ foreach helper : virt_helpers ], install: true, install_dir: helper.get('install_dir', libexecdir), - install_rpath: libdir, ) endforeach diff --git a/tools/meson.build b/tools/meson.build index 090179470a..db6a61bad7 100644 --- a/tools/meson.build +++ b/tools/meson.build @@ -75,7 +75,6 @@ if conf.has('WITH_HOST_VALIDATE') ], install: true, install_dir: bindir, - install_rpath: libdir, ) endif @@ -112,7 +111,6 @@ if conf.has('WITH_LOGIN_SHELL') ], install: true, install_dir: libexecdir, - install_rpath: libdir, ) install_data('virt-login-shell.conf', install_dir: sysconfdir / 'libvirt') @@ -197,7 +195,6 @@ executable( ], install: true, install_dir: bindir, - install_rpath: libdir, ) executable( @@ -219,7 +216,6 @@ executable( ], install: true, install_dir: bindir, - install_rpath: libdir, ) tools_conf = configuration_data() -- 2.26.2
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
On Wed, 2020-08-19 at 13:22 +0200, Pavel Hrdina wrote:
On Wed, Aug 19, 2020 at 12:47:40PM +0200, Andrea Bolognani wrote:
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.
Sounds good, thanks! -- Andrea Bolognani / Red Hat / Virtualization
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. 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. Pavel
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 :|
On Wed, Aug 19, 2020 at 01:16:17PM +0100, Daniel P. Berrangé wrote:
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.
Agreed and since the fix is fairly simple I guess we can keep using RPATH to not surprise and break some users. Otherwise I would also say that these users should start using LD_LIBRARY_PATH. Pavel
On Wed, 2020-08-19 at 13:16 +0100, Daniel P. Berrangé wrote:
On Wed, Aug 19, 2020 at 02:10:53PM +0200, Pavel Hrdina wrote:
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`.
I checked again and it looks like that option does indeed not exist. The Debian package was using it, but it was just ignored I guess.
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.
I tested with v6.6.0 and using $ ../autogen.sh --prefix=/some/dir results in RPATH being added to the various binaries, but $ ../autogen.sh --prefix=/usr doesn't. So I guess autotools are smart enough to only add RPATH when you're actually installing outside of the standard library search paths, and skip it otherwise. That's why it never showed up in distro packages. We could reasonably implement the same logic in Meson ourselves, however...
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.
... I don't think this would work: using install_rpath: '' will result in an empty RPATH being set, not in RPATH being missing. So it's a different behavior than the one we want. Maybe there's a better way to do it, but right now I'm sort of inclined to just declare that people who install libvirt under custom prefixes will have to take care of adding the corresponding paths to /etc/ld.so.conf or use $LD_LIBRARY_PATH. -- Andrea Bolognani / Red Hat / Virtualization
On Wed, Aug 19, 2020 at 02:58:07PM +0200, Andrea Bolognani wrote:
On Wed, 2020-08-19 at 13:16 +0100, Daniel P. Berrangé wrote:
On Wed, Aug 19, 2020 at 02:10:53PM +0200, Pavel Hrdina wrote:
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`.
I checked again and it looks like that option does indeed not exist. The Debian package was using it, but it was just ignored I guess.
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.
I tested with v6.6.0 and using
$ ../autogen.sh --prefix=/some/dir
results in RPATH being added to the various binaries, but
$ ../autogen.sh --prefix=/usr
doesn't. So I guess autotools are smart enough to only add RPATH when you're actually installing outside of the standard library search paths, and skip it otherwise. That's why it never showed up in distro packages.
We could reasonably implement the same logic in Meson ourselves, however...
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.
... I don't think this would work: using
install_rpath: ''
will result in an empty RPATH being set, not in RPATH being missing. So it's a different behavior than the one we want.
I specifically tried this to make sure it works and also this is from meson code: self.install_rpath = kwargs.get('install_rpath', '') so yes it works.
Maybe there's a better way to do it, but right now I'm sort of inclined to just declare that people who install libvirt under custom prefixes will have to take care of adding the corresponding paths to /etc/ld.so.conf or use $LD_LIBRARY_PATH.
-- Andrea Bolognani / Red Hat / Virtualization
On Wed, 2020-08-19 at 15:10 +0200, Pavel Hrdina wrote:
On Wed, Aug 19, 2020 at 02:58:07PM +0200, Andrea Bolognani wrote:
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.
... I don't think this would work: using
install_rpath: ''
will result in an empty RPATH being set, not in RPATH being missing. So it's a different behavior than the one we want.
I specifically tried this to make sure it works and also this is from meson code:
self.install_rpath = kwargs.get('install_rpath', '')
so yes it works.
I tried again and I stand corrected :) I'll send a new patch in a minute. -- Andrea Bolognani / Red Hat / Virtualization
On Wed, Aug 19, 2020 at 10:30 AM Andrea Bolognani <abologna@redhat.com> wrote:
On Wed, 2020-08-19 at 15:10 +0200, Pavel Hrdina wrote:
On Wed, Aug 19, 2020 at 02:58:07PM +0200, Andrea Bolognani wrote:
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.
... I don't think this would work: using
install_rpath: ''
will result in an empty RPATH being set, not in RPATH being missing. So it's a different behavior than the one we want.
I specifically tried this to make sure it works and also this is from meson code:
self.install_rpath = kwargs.get('install_rpath', '')
so yes it works.
I tried again and I stand corrected :)
I'll send a new patch in a minute.
Fedora's libtool is also slightly tweaked to minimize RPATH stuff being set. Across the board, generally RPATH values are set to "" (CMake, libtool, etc.), though I think Meson doesn't have an equivalent to set globally at the %meson macro level... -- 真実はいつも一つ！/ Always, there's only one truth!
participants (4)
-
Andrea Bolognani -
Daniel P. Berrangé -
Neal Gompa -
Pavel Hrdina