On Fri, Apr 08, 2016 at 07:13:07PM -0400, Cole Robinson wrote: > Hi all, > > I wanted to float some ideas about cleaning up the RPM spec file. None of it > is urgent but I'll end up scratching the itch eventually and I want to make > sure people are on board > > Old distro bits: > > * Drop support for building on rhel5... this was ACKd previously by eblake: > http://www.redhat.com/archives/libvir-list/2015-November/msg00175.html Yep > * Drop support for end-of-life Fedora. There's lots of Fedora conditionals > going back to Fedora 13. I don't think it's really interesting to try to > support building an RPM on a distribution that is no longer receiving updates. > But we could be a bit conservative and give a couple release window, dropping > support for less than Fedora 20 for example (which went EOL in June 2015) There's a surprising/disappointing number of people who carry on running EOL Fedora releases, so I think we should definitely be conservative in this respect. 1 year grace after Fedora EOL seems sufficient though, so dropping less than F20 would be OK. > Configuration variables: some of these could still feasibly be 'useful' even > if they aren't programmatically set by any distro conditional, but I think > unless someone actively uses them we should seek to have as few conditionals > as possible, rather than conditionalize everything by default which seems more > common. Some ideas: Every conditional we have increases potential for bugs in the spec file and I'm pretty sure we have some in there, because no one ever tests most of these conditionals when disabled. > * Drop with_* conditionals that are only used by old Fedora or RHEL5. I didn't > audit all of them but the obvious ones are straight forward like: > with_systemd_macros, with_polkit, with_capng, with_netcf, with_yajl, > with_capng, with_avahi, with_hal/all HAL support For that matter I think we can drop HAL *code* entirely since it was EOL after RHEL5, replaced by the udev driver. > * Drop client_only/server_drivers/with_libvirtd ... do these serve any good > purpose? (once the rhel5 bits are gone) The client stuff I'm a bit less sure about. The original rational for this was that RHEL was only shipping libvirt with hypervisor drivers enabled on certain architectures, and we wanted to enable other architectures todo a client only build. eg to allow an ppc64 client to talk to a x86_64 host. Now that we have LXC enabled unconditionally on all archs though, there's probably not a hugely compelling reason for a client only build anymore. > * Drop with_driver_modules: it's unconditionally enabled and has been since it > was introduced Yep, that was there in case we needed to turn off drive modules for compatilbity reasons on existing RHEL platforms. In the end we just switched RHEL over to use them anyway, so they don't serve much use. > Old spec bits cleanup: > > * drop defattr(), it's long since not needed > * drop manual removal of the buildroot, it's long since not needed > * use %global consistently instead of %define > * consider using %{with/without X} pattern instead of defining variables as with_X Assuming %global is supported by RHEL6, its fine. > * The top of the spec has this block: > > # If neither fedora nor rhel was defined, try to guess them from dist > %if !0%{?rhel} && !0%{?fedora} > %{expand:%(echo "%{?dist}" | \ > sed -ne 's/^\.el\([0-9]\+\).*/%%define rhel \1/p')} > %{expand:%(echo "%{?dist}" | \ > sed -ne 's/^\.fc\?\([0-9]\+\).*/%%define fedora \1/p')} > %endif > > Is it actually still relevant? It was added 5 years ago... I don't know in > what case rhel or fedora macros won't be defined In RHEL5 vintage build roots, %rhel/%fedora were not defined IIRC. No longer an issue. 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list