[libvirt] [PATCH 0/3] spec file improvements
Our spec file is non-deterministic if you have this in your ~/.rpmmacros: %_without_udev 1 because it then depends on whether you have systemd-devel installed rather than being explicitly stated in the spec file. [At this point, I'm sending the series to get the review started on the two trivial parts; I'm still working on patch 3, including coming up with a formula for proving that the unpatched build was non-deterministic, rather than just my claim above] Eric Blake (2): build: fix spelling of configure --with-systemd-daemon spec: explicitly avoid bhyve on Linux libvirt.spec.in | 2 ++ m4/virt-lib.m4 | 20 +++++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) -- 1.8.5.3
Commit 68954fb added a configure option --with-systemd_daemon, which violates the conventions of configure files preferring dash in all option names. This fixes it, before we hit a release where the tarball is baked with an awkward name. * m4/virt-lib.m4 (LIBVIRT_CHECK_LIB, LIBVIRT_CHECK_LIB_ALT) (LIBVIRT_CHECK_PKG): Favor - over _ in configure option names. Signed-off-by: Eric Blake <eblake@redhat.com> --- m4/virt-lib.m4 | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/m4/virt-lib.m4 b/m4/virt-lib.m4 index cff4fbb..75b9b1d 100644 --- a/m4/virt-lib.m4 +++ b/m4/virt-lib.m4 @@ -1,7 +1,7 @@ dnl dnl virt-lib.m4: Helper macros for checking for libraries dnl -dnl Copyright (C) 2012-2013 Red Hat, Inc. +dnl Copyright (C) 2012-2014 Red Hat, Inc. dnl dnl This library is free software; you can redistribute it and/or dnl modify it under the terms of the GNU Lesser General Public @@ -54,16 +54,17 @@ AC_DEFUN([LIBVIRT_CHECK_LIB],[ m4_pushdef([header_name], [$4]) m4_pushdef([check_name_lc], m4_tolower(check_name)) + m4_pushdef([check_name_dash], m4_translit(check_name_lc, [_], [-])) m4_pushdef([config_var], [WITH_]check_name) m4_pushdef([make_var], [WITH_]check_name) m4_pushdef([cflags_var], check_name[_CFLAGS]) m4_pushdef([libs_var], check_name[_LIBS]) - m4_pushdef([arg_var], [with-]check_name_lc) + m4_pushdef([arg_var], [with-]check_name_dash) m4_pushdef([with_var], [with_]check_name_lc) m4_divert_text([DEFAULTS], [with_var][=check]) - AC_ARG_WITH(check_name_lc, + AC_ARG_WITH(check_name_dash, [AS_HELP_STRING([--arg_var], [with lib]]m4_dquote(library_name)[[ support @<:@default=check@:>@])]) @@ -126,6 +127,7 @@ AC_DEFUN([LIBVIRT_CHECK_LIB],[ m4_popdef([make_var]) m4_popdef([config_var]) + m4_popdef([check_name_dash]) m4_popdef([check_name_lc]) m4_popdef([header_name]) @@ -182,18 +184,19 @@ AC_DEFUN([LIBVIRT_CHECK_LIB_ALT],[ m4_pushdef([header_name_alt], [$8]) m4_pushdef([check_name_lc], m4_tolower(check_name)) + m4_pushdef([check_name_dash], m4_translit(check_name_lc, [_], [-])) m4_pushdef([config_var], [WITH_]check_name) m4_pushdef([make_var], [WITH_]check_name) m4_pushdef([cflags_var], check_name[_CFLAGS]) m4_pushdef([libs_var], check_name[_LIBS]) - m4_pushdef([arg_var], [with-]check_name_lc) + m4_pushdef([arg_var], [with-]check_name_dash) m4_pushdef([with_var], [with_]check_name_lc) m4_pushdef([config_var_alt], [WITH_]check_name_alt) m4_pushdef([make_var_alt], [WITH_]check_name_alt) m4_divert_text([DEFAULTS], [with_var][=check]) - AC_ARG_WITH(check_name_lc, + AC_ARG_WITH(check_name_dash, [AS_HELP_STRING([--arg_var], [with lib]]m4_dquote(library_name)[[ support @<:@default=check@:>@])]) @@ -273,6 +276,7 @@ AC_DEFUN([LIBVIRT_CHECK_LIB_ALT],[ m4_popdef([config_var]) m4_popdef([check_name_lc]) + m4_popdef([check_name_dash]) m4_popdef([header_name_alt]) m4_popdef([function_name_alt]) @@ -310,16 +314,17 @@ AC_DEFUN([LIBVIRT_CHECK_PKG],[ m4_pushdef([pc_version], [$3]) m4_pushdef([check_name_lc], m4_tolower(check_name)) + m4_pushdef([check_name_dash], m4_translit(check_name_lc, [_], [-])) m4_pushdef([config_var], [WITH_]check_name) m4_pushdef([make_var], [WITH_]check_name) m4_pushdef([cflags_var], check_name[_CFLAGS]) m4_pushdef([libs_var], check_name[_LIBS]) - m4_pushdef([arg_var], [with-]check_name_lc) + m4_pushdef([arg_var], [with-]check_name_dash) m4_pushdef([with_var], [with_]check_name_lc) m4_divert_text([DEFAULTS], [with_var][=check]) - AC_ARG_WITH(check_name_lc, + AC_ARG_WITH(check_name_dash, [AS_HELP_STRING([--arg_var], [with ]]m4_dquote(pc_name)[[ (>= ]]m4_dquote(pc_version)[[) support @<:@default=check@:>@])]) @@ -353,6 +358,7 @@ AC_DEFUN([LIBVIRT_CHECK_PKG],[ m4_popdef([config_var]) m4_popdef([check_name_lc]) + m4_popdef([check_name_dash]) m4_popdef([pc_version]) m4_popdef([pc_name]) -- 1.8.5.3
Hey, There is a missing 'e' in "configure" in the subject line. Christophe On Tue, Feb 25, 2014 at 10:52:24AM -0700, Eric Blake wrote:
Commit 68954fb added a configure option --with-systemd_daemon, which violates the conventions of configure files preferring dash in all option names. This fixes it, before we hit a release where the tarball is baked with an awkward name.
* m4/virt-lib.m4 (LIBVIRT_CHECK_LIB, LIBVIRT_CHECK_LIB_ALT) (LIBVIRT_CHECK_PKG): Favor - over _ in configure option names.
Signed-off-by: Eric Blake <eblake@redhat.com> --- m4/virt-lib.m4 | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/m4/virt-lib.m4 b/m4/virt-lib.m4 index cff4fbb..75b9b1d 100644 --- a/m4/virt-lib.m4 +++ b/m4/virt-lib.m4 @@ -1,7 +1,7 @@ dnl dnl virt-lib.m4: Helper macros for checking for libraries dnl -dnl Copyright (C) 2012-2013 Red Hat, Inc. +dnl Copyright (C) 2012-2014 Red Hat, Inc. dnl dnl This library is free software; you can redistribute it and/or dnl modify it under the terms of the GNU Lesser General Public @@ -54,16 +54,17 @@ AC_DEFUN([LIBVIRT_CHECK_LIB],[ m4_pushdef([header_name], [$4])
m4_pushdef([check_name_lc], m4_tolower(check_name)) + m4_pushdef([check_name_dash], m4_translit(check_name_lc, [_], [-]))
m4_pushdef([config_var], [WITH_]check_name) m4_pushdef([make_var], [WITH_]check_name) m4_pushdef([cflags_var], check_name[_CFLAGS]) m4_pushdef([libs_var], check_name[_LIBS]) - m4_pushdef([arg_var], [with-]check_name_lc) + m4_pushdef([arg_var], [with-]check_name_dash) m4_pushdef([with_var], [with_]check_name_lc)
m4_divert_text([DEFAULTS], [with_var][=check]) - AC_ARG_WITH(check_name_lc, + AC_ARG_WITH(check_name_dash, [AS_HELP_STRING([--arg_var], [with lib]]m4_dquote(library_name)[[ support @<:@default=check@:>@])])
@@ -126,6 +127,7 @@ AC_DEFUN([LIBVIRT_CHECK_LIB],[ m4_popdef([make_var]) m4_popdef([config_var])
+ m4_popdef([check_name_dash]) m4_popdef([check_name_lc])
m4_popdef([header_name]) @@ -182,18 +184,19 @@ AC_DEFUN([LIBVIRT_CHECK_LIB_ALT],[ m4_pushdef([header_name_alt], [$8])
m4_pushdef([check_name_lc], m4_tolower(check_name)) + m4_pushdef([check_name_dash], m4_translit(check_name_lc, [_], [-]))
m4_pushdef([config_var], [WITH_]check_name) m4_pushdef([make_var], [WITH_]check_name) m4_pushdef([cflags_var], check_name[_CFLAGS]) m4_pushdef([libs_var], check_name[_LIBS]) - m4_pushdef([arg_var], [with-]check_name_lc) + m4_pushdef([arg_var], [with-]check_name_dash) m4_pushdef([with_var], [with_]check_name_lc) m4_pushdef([config_var_alt], [WITH_]check_name_alt) m4_pushdef([make_var_alt], [WITH_]check_name_alt)
m4_divert_text([DEFAULTS], [with_var][=check]) - AC_ARG_WITH(check_name_lc, + AC_ARG_WITH(check_name_dash, [AS_HELP_STRING([--arg_var], [with lib]]m4_dquote(library_name)[[ support @<:@default=check@:>@])])
@@ -273,6 +276,7 @@ AC_DEFUN([LIBVIRT_CHECK_LIB_ALT],[ m4_popdef([config_var])
m4_popdef([check_name_lc]) + m4_popdef([check_name_dash])
m4_popdef([header_name_alt]) m4_popdef([function_name_alt]) @@ -310,16 +314,17 @@ AC_DEFUN([LIBVIRT_CHECK_PKG],[ m4_pushdef([pc_version], [$3])
m4_pushdef([check_name_lc], m4_tolower(check_name)) + m4_pushdef([check_name_dash], m4_translit(check_name_lc, [_], [-]))
m4_pushdef([config_var], [WITH_]check_name) m4_pushdef([make_var], [WITH_]check_name) m4_pushdef([cflags_var], check_name[_CFLAGS]) m4_pushdef([libs_var], check_name[_LIBS]) - m4_pushdef([arg_var], [with-]check_name_lc) + m4_pushdef([arg_var], [with-]check_name_dash) m4_pushdef([with_var], [with_]check_name_lc)
m4_divert_text([DEFAULTS], [with_var][=check]) - AC_ARG_WITH(check_name_lc, + AC_ARG_WITH(check_name_dash, [AS_HELP_STRING([--arg_var], [with ]]m4_dquote(pc_name)[[ (>= ]]m4_dquote(pc_version)[[) support @<:@default=check@:>@])])
@@ -353,6 +358,7 @@ AC_DEFUN([LIBVIRT_CHECK_PKG],[ m4_popdef([config_var])
m4_popdef([check_name_lc]) + m4_popdef([check_name_dash])
m4_popdef([pc_version]) m4_popdef([pc_name]) -- 1.8.5.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 02/26/2014 03:02 AM, Christophe Fergeau wrote:
Hey,
There is a missing 'e' in "configure" in the subject line.
Thanks; fixed (actually, reworded): build: use --with-systemd-daemon as configure option . On 02/26/2014 02:59 AM, Michal Privoznik wrote:
On 25.02.2014 18:52, Eric Blake wrote:
Our spec file is non-deterministic if you have this in your ~/.rpmmacros: %_without_udev 1
ACK series and the release material too.
Thanks; pushed. (Missed rc2, but that's no real loss). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Generally, we try to make the spec file tweakable via user variables, so that they can select a different subset of sub-rpms to build. We also try to explicitly list all driver config options, rather than leaving the chance that the rpm build may be non-deterministic based on what the user had installed locally. But in the case of the recent bhyve hypervisor driver, there is no port of bhyve to Linux, so it is easier to just blindly disable it for now. If someone ever does try to port bhyve to Fedora, we can make the spec file conditional at that point. * libvirt.spec.in (%configure): Don't try to build bhyve. Signed-off-by: Eric Blake <eblake@redhat.com> --- libvirt.spec.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 3d5a69e..1e1e5cc 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -79,6 +79,7 @@ %define with_hyperv 0%{!?_without_hyperv:1} %define with_xenapi 0%{!?_without_xenapi:1} %define with_parallels 0%{!?_without_parallels:1} +# No test for bhyve, because it does not build on Linux # Then the secondary host drivers, which run inside libvirtd %define with_interface 0%{!?_without_interface:%{server_drivers}} @@ -1416,6 +1417,7 @@ driver %{?_without_hyperv} \ %{?_without_vmware} \ %{?_without_parallels} \ + --without-bhyve \ %{?_without_interface} \ %{?_without_network} \ %{?_with_rhel5_api} \ -- 1.8.5.3
On Fedora 20, with the following in my ~/.rpmmacros: %_without_udev 1 %_without_storage_mpath 1 and with device-mapper-devel uninstalled, 'make rpm' fails with: checking for libdevmapper.h... no configure: error: You must install device-mapper-devel/libdevmapper >= 1.0.0 to compile libvirt error: Bad exit status from /var/tmp/rpm-tmp.Wo9pOG (%build) The fix is to match the logic in configure.ac on when devmapper is required (for both mpath and storage). While at it, rbd storage is not dependent on mpath. * libvirt.spec.in (BuildRequires): Fix build when mpath is disabled. Signed-off-by: Eric Blake <eblake@redhat.com> --- This has been latent and got in the way of me trying to fix the systemd_daemon issue. libvirt.spec.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 1e1e5cc..dd3906d 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -559,7 +559,7 @@ BuildRequires: parted-devel BuildRequires: e2fsprogs-devel %endif %endif -%if %{with_storage_mpath} +%if %{with_storage_mpath} || %{with_storage_disk} # For Multipath support %if 0%{?rhel} == 5 # Broken RHEL-5 packaging has header files in main RPM :-( @@ -567,9 +567,9 @@ BuildRequires: device-mapper %else BuildRequires: device-mapper-devel %endif - %if %{with_storage_rbd} +%endif +%if %{with_storage_rbd} BuildRequires: ceph-devel - %endif %endif %if %{with_storage_gluster} BuildRequires: glusterfs-api-devel >= 3.4.1 -- 1.8.5.3
On Fedora 20, I added this to my '~/.rpmmacros': %_without_udev 1 %_without_storage_mpath 1 %_without_storage_disk 1 and uninstalled systemd-devel (which also removed device-mapper-devel). Then I ran 'make rpm', and inspected the results: $ ldd ~/rpmbuild/BUILD/libvirt-1.2.2/daemon/.libs/libvirtd | grep syst $ Then I reinstalled systemd-devel, where I now see: $ ldd ~/rpmbuild/BUILD/libvirt-1.2.2/daemon/.libs/libvirtd | grep syst libsystemd-daemon.so.0 => /lib64/libsystemd-daemon.so.0 (0x00007ffb858ba000) $ Oops - the build is non-deterministic, where the final binary depends on my build environment. The fix is to require systemd-devel in all situations where the code base uses it. Now ~/.rpmmacros can contain "%define _without_systemd_daemon 1" to explicitly disable use of the library, but the library is now a strict build requirement for normal builds; if systemd-devel is not installed, the user now gets an up-front warning: $ rpmbuild -ta libvirt-1.2.2.tar.gz error: Failed build dependencies: systemd-devel is needed by libvirt-1.2.2-1.fc20.x86_64 * libvirt.spec.in (with_systemd_daemon): New variable. (BuildRequires): Require systemd-devel for more than just udev. (%configure): Make choice of systemd_daemon explicit. Signed-off-by: Eric Blake <eblake@redhat.com> --- libvirt.spec.in | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index dd3906d..5fc8c57 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -131,6 +131,7 @@ %define with_firewalld 0%{!?_without_firewalld:0} %define with_libssh2 0%{!?_without_libssh2:0} %define with_wireshark 0%{!?_without_wireshark:0} +%define with_systemd_daemon 0%{!?_without_systemd_daemon:0} # Non-server/HV driver defaults which are always enabled %define with_sasl 0%{!?_without_sasl:1} @@ -173,6 +174,7 @@ # Fedora has systemd, libvirt still used sysvinit there. %if 0%{?fedora} >= 17 || 0%{?rhel} >= 7 %define with_systemd 1 + %define with_systemd_daemon 1 %endif # Fedora 18 / RHEL-7 are first where firewalld support is enabled @@ -445,6 +447,9 @@ BuildRequires: python %if %{with_systemd} BuildRequires: systemd-units %endif +%if %{with_systemd_daemon} +BuildRequires: systemd-devel +%endif %if %{with_xen} || %{with_libxl} BuildRequires: xen-devel %endif @@ -1374,6 +1379,10 @@ driver %define _without_wireshark --without-wireshark-dissector %endif +%if ! %{with_systemd_daemon} + %define _without_systemd_daemon --without-systemd-daemon +%endif + %define when %(date +"%%F-%%T") %define where %(hostname) %define who %{?packager}%{!?packager:Unknown} @@ -1448,6 +1457,7 @@ driver %{?_without_driver_modules} \ %{?_with_firewalld} \ %{?_without_wireshark} \ + %{?_without_systemd_daemon} \ %{with_packager} \ %{with_packager_version} \ --with-qemu-user=%{qemu_user} \ -- 1.8.5.3
On 25.02.2014 18:52, Eric Blake wrote:
Our spec file is non-deterministic if you have this in your ~/.rpmmacros: %_without_udev 1 because it then depends on whether you have systemd-devel installed rather than being explicitly stated in the spec file.
[At this point, I'm sending the series to get the review started on the two trivial parts; I'm still working on patch 3, including coming up with a formula for proving that the unpatched build was non-deterministic, rather than just my claim above]
Eric Blake (2): build: fix spelling of configure --with-systemd-daemon spec: explicitly avoid bhyve on Linux
libvirt.spec.in | 2 ++ m4/virt-lib.m4 | 20 +++++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-)
ACK series and the release material too. Michal
participants (3)
-
Christophe Fergeau -
Eric Blake -
Michal Privoznik