[build] Fixed support for --libddir under Ubuntu #7094
Conversation
Signed-off-by: Gregoire Bellon-Gervais <greggbg@gmail.com>
Update commit message This reverts commit cb3ba8f.
Signed-off-by: Gregoire Bellon-Gervais <gbellongervais.ext@orange.com>
| %global _libdir /%{_lib} | ||
| %else | ||
| %global _libdir /lib | ||
| %endif |
There was a problem hiding this comment.
Thanks for the PR! One comment. I believe you're going to have to add --with-libdir=%{_libdir} to the %configure macro later on in the file.
There was a problem hiding this comment.
Actually, never mind. The %configure macro will pick it up automatically. My mistake.
There was a problem hiding this comment.
Yes, exactly, it's automatically added, so no need to add it
| @@ -1,5 +1,13 @@ | |||
| %global _sbindir /sbin | |||
| %global _libdir /%{_lib} | |||
There was a problem hiding this comment.
@albundy83 so I think adding a way to override the default libdir is entirely reasonable. But it's not clear to me why this doesn't already default to the right values on Ubuntu. If you take a look at the /usr/lib/rpm/macros file from Ubuntu 16.04 you'll see that both _lib and _libdir are defined correctly for the distribution. Do you know where exactly it's getting the wrong default values from?
%_prefix /usr
%_exec_prefix %{_prefix}
%_bindir %{_exec_prefix}/bin
%_sbindir %{_exec_prefix}/sbin
%_libexecdir %{_exec_prefix}/libexec
%_datadir %{_prefix}/share
%_sysconfdir /etc
%_sharedstatedir %{_prefix}/com
%_localstatedir /var
%_lib lib
%_libdir %{_exec_prefix}/%{_lib}
%_includedir %{_prefix}/include
%_infodir %{_datadir}/info
%_mandir %{_datadir}/man
There was a problem hiding this comment.
Thanks for this, I did not even know that this file exists :)
I will check on my side.
By the way, when I saw the content of this file, it seems that libdir default is more /usr/lib but according to this for exemple:
https://packages.ubuntu.com/bionic/amd64/libzfs2linux/filelist
libdir is defined in /lib so it seems correct to leave it to /lib no ?
There was a problem hiding this comment.
Traditionally the packaging defines _prefix to be / so it should default to /lib. Incidentally, if you look down a few lines you'll also see where the %configure macro gets defined.
There was a problem hiding this comment.
So, here an update about rpm macros files:
albundy@albundy:~$ rpm --eval '%{_lib}'
lib64
albundy@albundy:~$ rpm --macros=/usr/lib/rpm/macros --eval '%{_lib}'
libSo, it should work, but when running strace rpm --eval '%{_lib}', we saw (I shorten the output):
...............
open("/usr/lib/rpm/macros", O_RDONLY) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=37400, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4bf7a87000
...............And later:
...............
open("/usr/lib/rpm/platform/x86_64-linux/macros", O_RDONLY) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=2121, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f4bf7a87000
read(3, "# Per-platform rpm configuration"..., 4096) = 2121
read(3, "", 4096) = 0
close(3
...............And the file /usr/lib/rpm/platform/x86_64-linux/macros contain the following entries:
%_lib lib64
%_libdir %{_prefix}/lib64So it explains why I fallback to /lib64 by default and not /lib
There was a problem hiding this comment.
That makes sense. Then the root cause is that the /usr/lib/rpm/platform/x86_64-linux/macros provided by Debian/Ubuntu is incorrect and specifies the wrong install location for x86_64. Then how about this
diff --git a/config/zfs-build.m4 b/config/zfs-build.m4
index adc99ed..c9a17dc 100644
--- a/config/zfs-build.m4
+++ b/config/zfs-build.m4
@@ -192,6 +192,13 @@ AC_DEFUN([ZFS_AC_RPM], [
RPM_DEFINE_UTIL+=' $(DEFINE_INITRAMFS)'
RPM_DEFINE_UTIL+=' $(DEFINE_SYSTEMD)'
+ dnl # Override default lib directory on x86_64 Debian/Ubuntu systems.
+ dnl # The provided /usr/lib/rpm/platform/x86_64-linux/macros file
+ dnl # defaults to 'lib64' instead of 'lib/x86_64-linux-gnu'.
+ AS_IF([test "$DEFAULT_PACKAGE" = "deb" -a "$target_cpu" = "x86_64" ], [
+ RPM_DEFINE_UTIL+=' --define "_lib lib/x86_64-linux-gnu"'
+ ])
+
RPM_DEFINE_KMOD='--define "kernels $(LINUX_VERSION)"'
RPM_DEFINE_KMOD+=' --define "require_spldir $(SPL)"'
RPM_DEFINE_KMOD+=' --define "require_splobj $(SPL_OBJ)"'note: I'm presuming it's set correctly for other arches. This is primarily due to a difference in the way that Debian and Redhat respectively decided to handle the multiarch/multilib issue.
This way it should just do the right thing. One possible concern / advantage is that the updated libraries will now be correctly installed in the same location as the distribution provided ones.
@loli10K can we get your thoughts on this.
There was a problem hiding this comment.
This is from an armhf Debian8 install:
root@linux:~# grep -H _lib /usr/lib/rpm/platform/*86*/macros
/usr/lib/rpm/platform/i386-linux-gnueabihf/macros:%_libexecdir %{_prefix}/lib/arm-linux-gnueabihf
/usr/lib/rpm/platform/i386-linux-gnueabihf/macros:%_lib lib
/usr/lib/rpm/platform/i386-linux-gnueabihf/macros:%_libdir %{_prefix}/lib
/usr/lib/rpm/platform/i486-linux-gnueabihf/macros:%_libexecdir %{_prefix}/lib/arm-linux-gnueabihf
/usr/lib/rpm/platform/i486-linux-gnueabihf/macros:%_lib lib
/usr/lib/rpm/platform/i486-linux-gnueabihf/macros:%_libdir %{_prefix}/lib
/usr/lib/rpm/platform/i586-linux-gnueabihf/macros:%_libexecdir %{_prefix}/lib/arm-linux-gnueabihf
/usr/lib/rpm/platform/i586-linux-gnueabihf/macros:%_lib lib
/usr/lib/rpm/platform/i586-linux-gnueabihf/macros:%_libdir %{_prefix}/lib
/usr/lib/rpm/platform/i686-linux-gnueabihf/macros:%_libexecdir %{_prefix}/lib/arm-linux-gnueabihf
/usr/lib/rpm/platform/i686-linux-gnueabihf/macros:%_lib lib
/usr/lib/rpm/platform/i686-linux-gnueabihf/macros:%_libdir %{_prefix}/lib
/usr/lib/rpm/platform/x86_64-linux-gnueabihf/macros:%_libexecdir %{_prefix}/lib/arm-linux-gnueabihf
/usr/lib/rpm/platform/x86_64-linux-gnueabihf/macros:%_lib lib
/usr/lib/rpm/platform/x86_64-linux-gnueabihf/macros:%_libdir %{_prefix}/lib
Unless i am mistaking the contents of those platform files they seem wrong and i agree this seems a bug in Debian/Ubuntu-provided macros.
I can probably do more testing to see how the change in zfs-build.m4, which looks good to me, plays with "native" Debian packaging this weekend.
There was a problem hiding this comment.
I'm not sure that defining lib to lib/x86_64-linux-gnu is the correct value, it should be lib no ?
There was a problem hiding this comment.
Here the way Debian package it:
https://packages.debian.org/sid/amd64/libzfs2linux/filelist
and for Ubuntu:
https://packages.ubuntu.com/bionic/amd64/libzfs2linux/filelist
There was a problem hiding this comment.
So I opted to put them in lib/x86_64-linux-gnu since that's where all the other shared libraries are installed. I'm not sure why they opted to put the zfs libraries under lib, it looks to me like a mistake since they are the only ones installed there.
Codecov Report
@@ Coverage Diff @@
## master #7094 +/- ##
=========================================
- Coverage 75.71% 75.41% -0.3%
=========================================
Files 296 296
Lines 95661 95661
=========================================
- Hits 72428 72145 -283
- Misses 23233 23516 +283
Continue to review full report at Codecov.
|
|
The pull request done here #7101 is better, I will close this one. |
Signed-off-by: Gregoire Bellon-Gervais greggbg@gmail.com
Under Ubuntu --libdir is not correctly reflected when building rpm file and installation happens in /lib64
Description
In file
config/zfs-build.m4, added support for libdir in RPM_DEFINE_UTILIn file
rpm/generic/zfs.spec.in, assign defined value if necessaryMotivation and Context
If we don't do this, zfs libraries are installed in /lib64 under Ubuntu which is not correct
#7083
How Has This Been Tested?
Tested under Ubuntu 14.04 (kernel 3.13.0-137 and 4.4.0-104) and 17.04 (default kernel)
Types of changes
Checklist:
Signed-off-by.