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 :|