5:47 a.m.
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
5:47 a.m.
New subject: [libvirt PATCH v2 1/1] meson: Drop RPATH usage
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(a)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(a)_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
6:22 a.m.
New subject: [libvirt PATCH v2 1/1] meson: Drop RPATH usage
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
7:05 a.m.
New subject: [libvirt PATCH v2 1/1] meson: Drop RPATH usage
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
7:10 a.m.
New subject: [libvirt PATCH v2 1/1] meson: Drop RPATH usage
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
7:16 a.m.
New subject: [libvirt PATCH v2 1/1] meson: Drop RPATH usage
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 :|
7:22 a.m.
New subject: [libvirt PATCH v2 1/1] meson: Drop RPATH usage
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
7:58 a.m.
New subject: [libvirt PATCH v2 1/1] meson: Drop RPATH usage
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
8:10 a.m.
New subject: [libvirt PATCH v2 1/1] meson: Drop RPATH usage
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
9:28 a.m.
New subject: [libvirt PATCH v2 1/1] meson: Drop RPATH usage
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
5:59 p.m.
New subject: [libvirt PATCH v2 1/1] meson: Drop RPATH usage
On Wed, Aug 19, 2020 at 10:30 AM Andrea Bolognani <abologna(a)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!
1557
days inactive
1560
days old
10 comments
4 participants
participants (4)
-
Andrea Bolognani -
Daniel P. Berrangé -
Neal Gompa -
Pavel Hrdina