[qemu RPM PATCH] use --with variables for build-time settings

[Eduardo Habkost]

This keeps the Fedora and RHEL defaults as-is, but makes the spec code
clearer.

I would like to get some review before committing this to the fedpkg
repository.

Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com&gt;
---
 qemu.spec |   66 +++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/qemu.spec b/qemu.spec
index 7ab15d7..e711e72 100644
--- a/qemu.spec
+++ b/qemu.spec
@@ -1,3 +1,39 @@
+# build-time settings that support --with or --without:
+#
+# = x86only =
+# Build only x86 Qemu targets.
+#
+# Disabled by default.
+#
+# = exclusive_x86_64 =
+# ExclusiveArch: x86_64
+#
+# Disabled by default, except on RHEL.
+#
+# = rbd =
+# Enable rbd support.
+#
+# Enable by default, except on RHEL.
+#
+# = fdt =
+# Enable fdt support.
+#
+# Enabled by default, except on RHEL.
+
+
+%if 0%{?rhel}
+# RHEL-specific defaults:
+%bcond_without exclusive_x86_64
+%bcond_with    rbd
+%bcond_with    fdt
+%else
+# General defaults:
+%bcond_without exclusive_x86_64
+%bcond_without rbd
+%bcond_without fdt
+%endif
+
+
 Summary: QEMU is a FAST! processor emulator
 Name: qemu
 Version: 1.0
@@ -8,14 +44,10 @@ License: GPLv2+ and LGPLv2+ and BSD
 Group: Development/Tools
 URL: http://www.qemu.org/
 # RHEL will build Qemu only on x86_64:
-%if 0%{?rhel}
+%if %{with exclusive_x86_64}
 ExclusiveArch: x86_64
 %endif
 
-# Allow one off builds to be minimalized without foreign
-# architecture support (--with x86only):
-%define with_x86only  %{?_with_x86only:     1} %{?!_with_x86only:     0}
-
 # OOM killer breaks builds with parallel make on s390(x)
 %ifarch s390 s390x
 %define _smp_mflags %{nil}
@@ -137,7 +169,7 @@ BuildRequires: spice-server-devel >= 0.9.0
 %endif
 # For network block driver
 BuildRequires: libcurl-devel
-%if !0%{?rhel}
+%if %{with rbd}
 # For rbd block driver
 BuildRequires: ceph-devel
 %endif
@@ -158,7 +190,7 @@ BuildRequires: libuuid-devel
 BuildRequires: bluez-libs-devel
 # For Braille device support
 BuildRequires: brlapi-devel
-%if !0%{?rhel}
+%if %{with fdt}
 # For FDT device tree support
 BuildRequires: libfdt-devel
 %endif
@@ -303,7 +335,7 @@ This package provides the system emulator for x86. When being run in a
x86
 machine that supports it, this package also provides the KVM virtualization
 platform.
 
-%if !%{with_x86only}
+%if %{without x86only}
 %package system-arm
 Summary: QEMU system emulator for arm
 Group: Development/Tools
@@ -445,7 +477,7 @@ such as kvm_stat.
 %build
 # By default we build everything, but allow x86 to build a minimal version
 # with only similar arch target support
-%if %{with_x86only}
+%if %{with x86only}
     buildarch="i386-softmmu x86_64-softmmu i386-linux-user x86_64-linux-user"
 %else
     buildarch="i386-softmmu x86_64-softmmu arm-softmmu cris-softmmu m68k-softmmu \
@@ -479,8 +511,10 @@ sed -i.debug 's/"-g $CFLAGS"/"$CFLAGS"/g'
configure
             --extra-ldflags="$extraldflags -pie -Wl,-z,relro -Wl,-z,now" \
             --extra-cflags="%{optflags} -fPIE -DPIE" \
             --enable-spice \
-%if 0%{?rhel}
+%if %{without rbd}
             --disable-rbd \
+%endif
+%if %{without fdt}
             --disable-fdt \
 %endif
             --enable-trace-backend=dtrace \
@@ -515,8 +549,10 @@ make clean
 %ifarch %{ix86} x86_64
     --enable-spice \
 %endif
-%if 0%{?rhel}
+%if %{without rbd}
     --disable-rbd \
+%endif
+%if %{without fdt}
     --disable-fdt \
 %endif
     --enable-trace-backend=dtrace \
@@ -613,7 +649,7 @@ for i in dummy \
 %ifnarch %{ix86} x86_64
     qemu-i386 \
 %endif
-%if !%{with_x86only}
+%if %{without x86only}
 %ifnarch arm
     qemu-arm \
 %endif
@@ -726,7 +762,7 @@ fi
 %{_exec_prefix}/lib/binfmt.d/qemu-*.conf
 %{_bindir}/qemu-i386
 %{_bindir}/qemu-x86_64
-%if !%{with_x86only}
+%if %{without x86only}
 %{_bindir}/qemu-alpha
 %{_bindir}/qemu-arm
 %{_bindir}/qemu-armeb
@@ -739,7 +775,7 @@ fi
 %endif
 %{_datadir}/systemtap/tapset/qemu-i386.stp
 %{_datadir}/systemtap/tapset/qemu-x86_64.stp
-%if !%{with_x86only}
+%if %{without x86only}
 %{_datadir}/systemtap/tapset/qemu-alpha.stp
 %{_datadir}/systemtap/tapset/qemu-arm.stp
 %{_datadir}/systemtap/tapset/qemu-armeb.stp
@@ -788,7 +824,7 @@ fi
 %{_bindir}/kvm_stat
 %endif
 
-%if !%{with_x86only}
+%if %{without x86only}
 
 %files system-arm
 %defattr(-,root,root)
-- 
1.7.3.2

[Daniel P. Berrange]

On Wed, Mar 28, 2012 at 11:30:17AM -0300, Eduardo Habkost wrote:
>  This keeps the Fedora and RHEL defaults as-is, but makes the spec
> code
>  clearer.
>  
>  I would like to get some review before committing this to the fedpkg
>  repository. 
This looks ok to me. You'll have to rebase though, since I just pushed
a couple of fixes to master, related to the x86only conditional.

>  
>  Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com&gt;
>  ---
>   qemu.spec |   66 +++++++++++++++++++++++++++++++++++++++++++++++-------------
>   1 files changed, 51 insertions(+), 15 deletions(-)
>  
>  diff --git a/qemu.spec b/qemu.spec
>  index 7ab15d7..e711e72 100644
>  --- a/qemu.spec
>  +++ b/qemu.spec
>  @@ -1,3 +1,39 @@
>  +# build-time settings that support --with or --without:
>  +#
>  +# = x86only =
>  +# Build only x86 Qemu targets.
>  +#
>  +# Disabled by default.
>  +#
>  +# = exclusive_x86_64 =
>  +# ExclusiveArch: x86_64
>  +#
>  +# Disabled by default, except on RHEL.
>  +#
>  +# = rbd =
>  +# Enable rbd support.
>  +#
>  +# Enable by default, except on RHEL.
>  +#
>  +# = fdt =
>  +# Enable fdt support.
>  +#
>  +# Enabled by default, except on RHEL. 
Fdt is only used by the PPC emulators, so if x86only is set,
we should disable fdt too.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

[Eduardo Habkost]

On Wed, Mar 28, 2012 at 07:50:52PM +0100, Daniel P. Berrange wrote:
>  On Wed, Mar 28, 2012 at 11:30:17AM -0300, Eduardo Habkost wrote:
>  > This keeps the Fedora and RHEL defaults as-is, but makes the spec code
>  > clearer.
>  > 
>  > I would like to get some review before committing this to the fedpkg
>  > repository.
>  
>  This looks ok to me. You'll have to rebase though, since I just pushed
>  a couple of fixes to master, related to the x86only conditional. 
OK, rebased, changed the new %{with_x86only} lines to use %{with
x86only}, and pushed.

>  
>  > +# = fdt =
>  > +# Enable fdt support.
>  > +#
>  > +# Enabled by default, except on RHEL.
>  
>  Fdt is only used by the PPC emulators, so if x86only is set,
>  we should disable fdt too. 
To do that, the code would have to look like:

%if 0%{?rhel}
# RHEL-specific defaults:
%bcond_without exclusive_x86_64
%bcond_with    rbd
%bcond_with    fdt
%else
# General defaults:
%bcond_without exclusive_x86_64
%bcond_without rbd
%if %{with x86only}
%bcond_with    fdt
%else
%bcond_without fdt
%endif
%endif

That looks really confusing, IMO. Considering that having the extra
BuildRequires is harmless and can be easily fixed by the build submitter
using --without fdt, it doesn't seem to be worth the effort, to me.

But if you have a simpler solution or you think the above solution is
worth it, I won't oppose it.

-- 
Eduardo

[Daniel P. Berrange]

On Wed, Mar 28, 2012 at 05:14:51PM -0300, Eduardo Habkost wrote:
>  On Wed, Mar 28, 2012 at 07:50:52PM +0100, Daniel P. Berrange wrote:
>  > On Wed, Mar 28, 2012 at 11:30:17AM -0300, Eduardo Habkost wrote:
>  > > This keeps the Fedora and RHEL defaults as-is, but makes the spec code
>  > > clearer.
>  > > 
>  > > I would like to get some review before committing this to the fedpkg
>  > > repository.
>  > 
>  > This looks ok to me. You'll have to rebase though, since I just pushed
>  > a couple of fixes to master, related to the x86only conditional.
>  
>  OK, rebased, changed the new %{with_x86only} lines to use %{with
>  x86only}, and pushed.
>  
>  > 
>  > > +# = fdt =
>  > > +# Enable fdt support.
>  > > +#
>  > > +# Enabled by default, except on RHEL.
>  > 
>  > Fdt is only used by the PPC emulators, so if x86only is set,
>  > we should disable fdt too.
>  
>  To do that, the code would have to look like:
>  
>  %if 0%{?rhel}
>  # RHEL-specific defaults:
>  %bcond_without exclusive_x86_64
>  %bcond_with    rbd
>  %bcond_with    fdt
>  %else
>  # General defaults:
>  %bcond_without exclusive_x86_64
>  %bcond_without rbd
>  %if %{with x86only}
>  %bcond_with    fdt
>  %else
>  %bcond_without fdt
>  %endif
>  %endif
>  
>  That looks really confusing, IMO. Considering that having the extra
>  BuildRequires is harmless and can be easily fixed by the build submitter
>  using --without fdt, it doesn't seem to be worth the effort, to me.
>  
>  But if you have a simpler solution or you think the above solution is
>  worth it, I won't oppose it. 
Agreed, it is a bit fugly, so lets leave it


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

[M A Young]

Are these changes why qemu-1.0-11.fc18 only has x86_64 packages and not 
i686 ones, and was this deliberate?

 	Michael Young

[Richard W.M. Jones]

On Thu, Mar 29, 2012 at 03:13:26PM +0100, M A Young wrote:
>  Are these changes why qemu-1.0-11.fc18 only has x86_64 packages and
>  not i686 ones, and was this deliberate? 
I noticed the same thing:

http://koji.fedoraproject.org/koji/buildinfo?buildID=310215

It breaks libguestfs, amongst other things.

I'm sure this is a mistake though.  It looks like the ExclusiveArch is
only intended to take effect on RHEL.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw

[Eduardo Habkost]

On Thu, Mar 29, 2012 at 03:55:27PM +0100, Richard W.M. Jones wrote:
>  On Thu, Mar 29, 2012 at 03:13:26PM +0100, M A Young wrote:
>  > Are these changes why qemu-1.0-11.fc18 only has x86_64 packages and
>  > not i686 ones, and was this deliberate?
>  
>  I noticed the same thing:
>  
>  http://koji.fedoraproject.org/koji/buildinfo?buildID=310215
>  
>  It breaks libguestfs, amongst other things.
>  
>  I'm sure this is a mistake though.  It looks like the ExclusiveArch is
>  only intended to take effect on RHEL. 
Yes, sorry! exclusive_x86_64 was supposed to be disabled by default on
Fedora, but the with/without macros confused me. I will fix it
immediately.

-- 
Eduardo