On Tue, Mar 2, 2021 at 6:46 AM Richard W.M. Jones <rjones@redhat.com> wrote:
[Adding the devel list, since this change would obviously affect
several "base" packages.]

On Mon, Mar 01, 2021 at 01:31:13PM +0000, Daniel P. Berrangé wrote:
> Way back when we first started the mingw project in Fedora we took the
> decision to maintain the mingw builds as separate source RPMs in Fedora.
> The rationale was that mingw support was a new concept to Fedora with
> many unknowns. We often had custom mingw only patches, and there was a
> decent chance there would be breakage upon rebases. We didn't want to
> burden the native package maintainers with mingw support as that would
> have increased pushback against the effort.
>
> This decision was always going to cause us problems with keeping the
> mingw dist-git packages in sync with native dist-git packages. This
> is especially important when it comes to security fixes, and there
> have been reasonably well justified complaints about mingw lagging
> native wrt to CVE fixing in recent times.
>
>
> Since the early days we merged the separate mingw32-$PKG / mingw64-$PKG
> source RPMS into a single mingw-$PKG in dist-git. The need for patching
> mingw RPMs to fix problems in Fedora doesn't seem any higher than the
> need for patching to fix native build problem. Generally things just
> work when rebasing, because most upstreams recognize the importance
> of keeping their projects working on mingw.
>
> IOW, we were afraid of a maintainer burden that has turned out to
> largely not exist. To address this non-existant burden, we intentionally
> made the maint of mingw packages harder than it needs to be in Fedora.
>
>
> This is a long winded way of saying I think it is time to update
> the mingw packaging guidelines to allow for, even recommend, mingw
> packaging to be a standard part of the native RPM spec. In doing
> so we eliminate the main burden of mingw packaging in Feora and
> guarantee that we'll always be up to date wrt bug/security fixes
> and rebases.
>
> I've done some basic tests with libvirt-glib upstream which recently
> changed to using Meson. Supporting mingw in the main libvirt-glib.spec
> was largely trivial. I simply needed to copy the various blocks of
> the mingw-libvirt-glib.spec file into the libvirt-glib.spec, ending
> up with a diff:
>
> diff --git a/libvirt-glib.spec.in b/libvirt-glib.spec.in
> index 58e6242..81f972b 100644
> --- a/libvirt-glib.spec.in
> +++ b/libvirt-glib.spec.in
> @@ -1,5 +1,15 @@
>  # -*- rpm-spec -*-

> +%bcond_without mingw
> +
> +%define _with_mingw 0
> +%if 0%{?fedora}
> +%define _with_mingw 0%{with mingw}
> +%endif
> +
> +%define mingw32_pkg_name mingw32-%{name}
> +%define mingw64_pkg_name mingw64-%{name}
> +
>  Name: libvirt-glib
>  Version: @VERSION@
>  Release: 1%{?dist}
> @@ -16,6 +26,16 @@ BuildRequires: libxml2-devel
>  BuildRequires: vala-tools
>  BuildRequires: gettext

> +%if %{_with_mingw}
> +BuildRequires:  mingw32-filesystem
> +BuildRequires:  mingw64-filesystem
> +BuildRequires:  mingw32-glib2
> +BuildRequires:  mingw64-glib2
> +BuildRequires:  mingw32-libvirt
> +BuildRequires:  mingw64-libvirt
> +%endif
> +
> +
>  %package devel
>  Summary: libvirt glib integration for events development files
>  Requires: %{name} = %{version}-%{release}
> @@ -37,6 +57,42 @@ Requires: libvirt-gconfig-devel = %{version}-%{release}
>  Requires: libvirt-gobject = %{version}-%{release}
>  Requires: libvirt-devel

> +
> +%if %{_with_mingw}
> +
> +%package -n mingw32-libvirt-glib
> +Summary: MingwGW Windows libvirt-gconfig virtualization library
> +BuildArch: noarch
> +Requires: pkgconfig
> +
> +%package -n mingw32-libvirt-gconfig
> +Summary: MingwGW Windows libvirt-gconfig virtualization library
> +BuildArch: noarch
> +Requires: pkgconfig
> +
> +%package -n mingw32-libvirt-gobject
> +Summary: MingwGW Windows libvirt-gobject virtualization library
> +BuildArch: noarch
> +Requires: pkgconfig
> +
> +%package -n mingw64-libvirt-glib
> +Summary: MingwGW Windows libvirt-gconfig virtualization library
> +BuildArch: noarch
> +Requires: pkgconfig
> +
> +%package -n mingw64-libvirt-gconfig
> +Summary: MingwGW Windows libvirt-gconfig virtualization library
> +BuildArch: noarch
> +Requires: pkgconfig
> +
> +%package -n mingw64-libvirt-gobject
> +Summary: MingwGW Windows libvirt-gobject virtualization library
> +BuildArch: noarch
> +Requires: pkgconfig
> +
> +%endif
> +
> +
>  %description
>  This package provides integration between libvirt and the glib
>  event loop.
> @@ -61,6 +117,31 @@ objects
>  This package provides development header files and libraries for
>  managing virtualization host objects

> +%if %{_with_mingw}
> +
> +%description -n mingw32-libvirt-glib
> +MinGW Windows libvirt-glib virtualization library.
> +
> +%description -n mingw32-libvirt-gconfig
> +MinGW Windows libvirt-gconfig virtualization library.
> +
> +%description -n mingw32-libvirt-gobject
> +MinGW Windows libvirt-gobject virtualization library.
> +
> +
> +%description -n mingw64-libvirt-glib
> +MinGW Windows libvirt-glib virtualization library.
> +
> +%description -n mingw64-libvirt-gconfig
> +MinGW Windows libvirt-gconfig virtualization library.
> +
> +%description -n mingw64-libvirt-gobject
> +MinGW Windows libvirt-gobject virtualization library.
> +
> +%{?mingw_debug_package}
> +%endif
> +
> +
>  %prep
>  %setup -q

> @@ -68,11 +149,23 @@ managing virtualization host objects
>  %meson -Drpath=disabled
>  %meson_build

> +%if %{_with_mingw}
> +%mingw_meson -Drpath=disabled -Ddocs=disabled
> +%mingw_ninja
> +%endif
> +
>  %install
>  %meson_install

>  %find_lang %{name}

> +%if %{_with_mingw}
> +%mingw_ninja_install
> +
> +%mingw_find_lang libvirt-glib
> +%mingw_debug_install_post
> +%endif
> +
>  %check
>  %meson_test

> @@ -140,4 +233,87 @@ managing virtualization host objects
>  %{_datadir}/vala/vapi/libvirt-gobject-1.0.deps
>  %{_datadir}/vala/vapi/libvirt-gobject-1.0.vapi

> +
> +%if %{_with_mingw}
> +
> +%files -n mingw32-libvirt-glib -f mingw32-libvirt-glib.lang
> +%doc README COPYING AUTHORS NEWS
> +%{mingw32_bindir}/libvirt-glib-1.0-0.dll
> +
> +%{mingw32_libdir}/libvirt-glib-1.0.dll.a
> +
> +%{mingw32_libdir}/pkgconfig/libvirt-glib-1.0.pc
> +
> +%dir %{mingw32_includedir}/libvirt-glib-1.0
> +%dir %{mingw32_includedir}/libvirt-glib-1.0/libvirt-glib
> +%{mingw32_includedir}/libvirt-glib-1.0/libvirt-glib/libvirt-glib.h
> +%{mingw32_includedir}/libvirt-glib-1.0/libvirt-glib/libvirt-glib-*.h
> +
> +%files -n mingw64-libvirt-glib -f mingw64-libvirt-glib.lang
> +%doc README COPYING AUTHORS NEWS
> +%{mingw64_bindir}/libvirt-glib-1.0-0.dll
> +
> +%{mingw64_libdir}/libvirt-glib-1.0.dll.a
> +
> +%{mingw64_libdir}/pkgconfig/libvirt-glib-1.0.pc
> +
> +%dir %{mingw64_includedir}/libvirt-glib-1.0
> +%dir %{mingw64_includedir}/libvirt-glib-1.0/libvirt-glib
> +%{mingw64_includedir}/libvirt-glib-1.0/libvirt-glib/libvirt-glib.h
> +%{mingw64_includedir}/libvirt-glib-1.0/libvirt-glib/libvirt-glib-*.h
> +
> +
> +
> +%files -n mingw32-libvirt-gconfig
> +%{mingw32_bindir}/libvirt-gconfig-1.0-0.dll
> +
> +%{mingw32_libdir}/libvirt-gconfig-1.0.dll.a
> +
> +%{mingw32_libdir}/pkgconfig/libvirt-gconfig-1.0.pc
> +
> +%dir %{mingw32_includedir}/libvirt-gconfig-1.0
> +%dir %{mingw32_includedir}/libvirt-gconfig-1.0/libvirt-gconfig
> +%{mingw32_includedir}/libvirt-gconfig-1.0/libvirt-gconfig/libvirt-gconfig.h
> +%{mingw32_includedir}/libvirt-gconfig-1.0/libvirt-gconfig/libvirt-gconfig-*.h
> +
> +%files -n mingw64-libvirt-gconfig
> +%{mingw64_bindir}/libvirt-gconfig-1.0-0.dll
> +
> +%{mingw64_libdir}/libvirt-gconfig-1.0.dll.a
> +
> +%{mingw64_libdir}/pkgconfig/libvirt-gconfig-1.0.pc
> +
> +%dir %{mingw64_includedir}/libvirt-gconfig-1.0
> +%dir %{mingw64_includedir}/libvirt-gconfig-1.0/libvirt-gconfig
> +%{mingw64_includedir}/libvirt-gconfig-1.0/libvirt-gconfig/libvirt-gconfig.h
> +%{mingw64_includedir}/libvirt-gconfig-1.0/libvirt-gconfig/libvirt-gconfig-*.h
> +
> +
> +
> +%files -n mingw32-libvirt-gobject
> +%{mingw32_bindir}/libvirt-gobject-1.0-0.dll
> +
> +%{mingw32_libdir}/libvirt-gobject-1.0.dll.a
> +
> +%{mingw32_libdir}/pkgconfig/libvirt-gobject-1.0.pc
> +
> +%dir %{mingw32_includedir}/libvirt-gobject-1.0
> +%dir %{mingw32_includedir}/libvirt-gobject-1.0/libvirt-gobject
> +%{mingw32_includedir}/libvirt-gobject-1.0/libvirt-gobject/libvirt-gobject.h
> +%{mingw32_includedir}/libvirt-gobject-1.0/libvirt-gobject/libvirt-gobject-*.h
> +
> +%files -n mingw64-libvirt-gobject
> +%{mingw64_bindir}/libvirt-gobject-1.0-0.dll
> +
> +%{mingw64_libdir}/libvirt-gobject-1.0.dll.a
> +
> +%{mingw64_libdir}/pkgconfig/libvirt-gobject-1.0.pc
> +
> +%dir %{mingw64_includedir}/libvirt-gobject-1.0
> +%dir %{mingw64_includedir}/libvirt-gobject-1.0/libvirt-gobject
> +%{mingw64_includedir}/libvirt-gobject-1.0/libvirt-gobject/libvirt-gobject.h
> +%{mingw64_includedir}/libvirt-gobject-1.0/libvirt-gobject/libvirt-gobject-*.h
> +
> +%endif
> +
>  %changelog
>
>
> Some notes:
>
>   * We need to set  mingw32_pkg_name/mingw64_pkg_name explicitly
>     because the current logic that sets these doesn't work:
>
>      %mingw32_pkg_name        %(echo %{name} | sed 's/^mingw-/mingw32-/')
>
>     Presumably we can fix that macro so that it does the right thing
>     when no mingw- prefix exists in the first place
>
>  * We can't use %mingw_package_header because that splatters the
>    native debuginfo generation. So we must explicitly add mingw
>    debuginfo packages by referencing
>
>       %{?mingw_debug_package}
>
>    and at end of %install add
>
>       %mingw_debug_install_post
>
>
>  * %mingw_package_header has reference to overriding strip/objdump
>    to prevent corruption of binaries. We can't do that override
>    because we need native strip/objdump for the native builds.
>   
>    AFAICT though, no corruption happened to my DLLs even without
>    this strip/objdump override. Looks like this caveat might be
>    obsolete, unless the scenarios it hits are more niche than I
>    tested.
>
>  * %mingw_package_header tries to disable the internal dependancy
>    generator on RHEL 6. For reasons I don't understand, it ends
>    up disabling it on Fedora too.
>
>      %mingw_package_header                                         \
>      %global __strip %{mingw_strip}                                \
>      %global __objdump %{mingw_objdump}                            \
>      %if 0%{?rhel} == 6                                            \
>      %global _use_internal_dependency_generator 0                  \
>      %global __find_requires %{mingw_findrequires}                 \
>      %global __find_provides %{mingw_findprovides}                 \
>      %endif                                                         \
>      %global __debug_install_post %%{mingw_debug_install_post}     \
>      %{nil}

This looks just like a bug.

>    Those three %global inside the rhel conditional *do* get
>    evaluated on Fedora. We can see this in the old build logs
>    in fact
>
>      https://kojipkgs.fedoraproject.org/packages/mingw-libvirt-glib/4.0.0/1.fc35/data/logs/noarch/build.log
>   
>     warning: Deprecated external dependency generator is used!
>     Finding  Provides: /usr/lib/rpm/mingw-find-provides.sh mingw32 mingw64
>     ....
>
>    When I hack macros.mingw to remove the dep generator override
>    code entirely, almost everything still seems to work. rpm's
>    built-in dep generator still adds the mingw32(libssp-0.dll)
>    virtual deps. The only thing that gets lost is the dep on
>    mingw-pkg-config
>
>    IOW it looks like RPMs built-in dep generator pretty much does
>    the right thing. Just need to add a manual mingw-pkg-config
>    dep
>
>  * Converting meson packages was  easy because meson always uses
>    a separate build dir.  Many autoconf based packages default
>    to building in the source dir. The first step would thus have
>    to be to make the native build use a separate build dir. In
>    theory this should be easy as mingw autoconf macros are
>    already doing this, but I always expect dragons with autoconf.
>
>
> In summary based on my tests I think killing off the separate dist-git
> / RPM spec for mingw looks feasible unless someone knows of hidden
> show stoppers I haven't hit yet.
>
> I think we should go ahead and do this for some packages to demonstrate
> the concept in the real world, and I'm volunteering to coordinate it for
> all the virtualization packages I'm involved in maint of because I can't
> even reliably keep them in sync myself. libvirt, libvirt-glib, libosinfo,
> osinfo-db-tools, gtk-vnc, spice-gtk all use meson, so should mirror the
> approach above and be quite easy.
>
> Once we can demonstrate the real world impact, we can socialize the idea
> on Fedora devel list more widely and then also approach maintainers
> of other native packages to attempt to convince them to accept mingw
> sub-RPMs into their specs. Every mingw package we can get merged into
> native package frees up a little more time for to spend on the remaining
> mingw packages that are still separate.  Ideally we'd get 100% merged
> long term, but even if we get refusals from native maintainers, we'll
> still be better off with those we do succeed in merging.
>
> Regards,
> Daniel

Sounds in general like a good idea, but I think we should make it
opt-in only for the foreseeable future.  Some packagers won't
appreciate the extra overhead of all the mingw stuff.

I, for one, welcome this. For several of the packages I maintain, I'm the only person to support them in both native and mingw biulds.

Do you have a suggested path I would take to deprecate the mingw-foo packages once I roll over building them into the native package?

--Greg