On Tue, Aug 09, 2022 at 02:05:10PM +0200, Pavel Hrdina wrote: > On Tue, Aug 09, 2022 at 12:59:36PM +0100, Daniel P. Berrangé wrote: > > On Tue, Aug 09, 2022 at 01:56:43PM +0200, Pavel Hrdina wrote: > > > On Tue, Aug 09, 2022 at 12:54:09PM +0100, Daniel P. Berrangé wrote: > > > > On Tue, Aug 09, 2022 at 01:52:33PM +0200, Pavel Hrdina wrote: > > > > > On Mon, Aug 08, 2022 at 01:22:17PM -0400, Daniel P. Berrangé wrote: > > > > > > One specfile containing both native and mingw builds is the > > > > > > new best practice for Fedora. This reduces the maint burden > > > > > > and ensures the mingw packages don't fall behind. > > > > > > > > > > > > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> > > > > > > --- > > > > > > .gitlab-ci.yml | 2 +- > > > > > > libvirt.spec.in | 287 ++++++++++++++++++++++++++++++++++++ > > > > > > meson.build | 17 +-- > > > > > > mingw-libvirt.spec.in | 327 ------------------------------------------ > > > > > > 4 files changed, 293 insertions(+), 340 deletions(-) > > > > > > delete mode 100644 mingw-libvirt.spec.in > > > > > > > > > > The patch looks good but there are some changes not mentioned directly. > > > > > > > > > > With this patch we will build MinGW packages by default on Fedora. Not > > > > > sure if that is desirable. I would rather have it the other way around > > > > > if it works for Fedora best practice. > > > > > > > > Fedora has shipped the native & mingw builds for years now. This just > > > > merges them into one spec. There's no change in what we actually build > > > > from Fedora POV. Or am I misunderstanding what you mean ? > > > > > > From Fedora POV everything is probably the same but for everybody else > > > this might be regression that would require using the --define as we > > > need to do for gitlab-ci. > > > > IMHO if a contributor using the upstream spec to build RPMs, they need > > to just deal with whatever the current packaging has defined. Ultimately > > you can still do 'dnf builddep' to get the list of deps installeds, > > including the mingw ones now. > > I just wanted to point it out as it was not mentioned in the commit > message and might not be obvious from the code itself. Since I don't > have a strong opinion about this change > > Reviewed-by: Pavel Hrdina <phrdina@xxxxxxxxxx> I'll add a line to the commit message mentioning that RPM builds now need extra BRs to be installed. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|