This package built on koji:  https://koji.fedoraproject.org/koji/taskinfo?taskID=44492730

> #{?mingw_debug_package}

Is there a reason for disabling the debuginfo packages?


> %mingw_make -j12

Probably don't want to hardcode j12?


> # CMake option for install location seems to be ignored on MingGW, force it.
> # We'll worry if it breaks something later.
> #mkdir -p %{buildroot}%{mingw32_datadir}/cmake
> #mkdir -p %{buildroot}%{mingw64_datadir}/cmake
> #mv %{buildroot}%{mingw32_prefix}/CMake %{buildroot}%{mingw32_datadir}/cmake/fltk
> #mv %{buildroot}%{mingw64_prefix}/CMake %{buildroot}%{mingw64_datadir}/cmake/fltk

These lines don't seem to be neccessary anymore?

(In reply to Sandro Mani from comment #2)
> > #{?mingw_debug_package}
> 
> Is there a reason for disabling the debuginfo packages?

I created this spec a long time ago so I've slept too many times since then to remember. For the last part I think I created a cmake patch to fix it the "right" way.

 
> > %mingw_make -j12
> 
> Probably don't want to hardcode j12?

For some reason I'm getting only single threaded builds in my local mock and it was taking forever...

 
> > # CMake option for install location seems to be ignored on MingGW, force it.
> > # We'll worry if it breaks something later.
> > #mkdir -p %{buildroot}%{mingw32_datadir}/cmake
> > #mkdir -p %{buildroot}%{mingw64_datadir}/cmake
> > #mv %{buildroot}%{mingw32_prefix}/CMake %{buildroot}%{mingw32_datadir}/cmake/fltk
> > #mv %{buildroot}%{mingw64_prefix}/CMake %{buildroot}%{mingw64_datadir}/cmake/fltk
> 
> These lines don't seem to be necessary anymore?

Removed.

Spec URL: https://hobbes1069.fedorapeople.org//mingw-fltk.spec
SRPM URL: https://hobbes1069.fedorapeople.org//mingw-fltk-1.3.5-2.fc32.src.rpm

* Fri May 22 2020 Richard Shaw <hobbes1069> - 1.3.5-2
- Revise per reviewer feedback.

I added %{?_smp_mflags} I just thought the mingw macro should do that for me :)

I'm actually testing

%mingw_make_build           %mingw_make -O %{?_smp_mflags} V=1 VERBOSE=1
%mingw_make_install         %mingw_make install DESTDIR=%{buildroot} INSTALL="/usr/bin/install -p"

right now ;)

> License:        LGPLv2+ with exceptions

Consider adding a comment (like the following from the native package)
# see COPYING (or http://www.fltk.org/COPYING.php ) for exceptions details


> Patch0:         mingw-fltk-cmake.patch

It's good practice to add a minimal comment as to the purpose of the patch.


> {?mingw_debug_package}

Typo, should be %{?mingw_debug_package}


> %exclude %{mingw32_bindir}/lib*.dll.debug
> %exclude %{mingw32_datadir}/man/*

The debug exclude shouldn't isn't necessary. The man pages you'll probably want to explicitly remove them in %install.

Rest is good. Please fix at least the debug typo when importing. Approved.

(General mingw remark: I typically try to patch the buildsystem to output versioned dlls, i.e. in this case something like

diff -rupN fltk-1.3.5/CMake/macros.cmake fltk-1.3.5-new/CMake/macros.cmake
--- fltk-1.3.5/CMake/macros.cmake	2019-03-03 09:40:23.000000000 +0100
+++ fltk-1.3.5-new/CMake/macros.cmake	2020-05-22 20:31:04.755781310 +0200
@@ -51,6 +51,9 @@ macro(FL_ADD_LIBRARY LIBNAME LIBTYPE LIB
 	    SOVERSION ${FLTK_VERSION_MAJOR}.${FLTK_VERSION_MINOR}
 	    PREFIX "lib"    # for MSVC static/shared coexistence
 	    )
+	if(MINGW)
+	    set_target_properties(${LIBRARY_NAME} PROPERTIES SUFFIX "-${FLTK_VERSION_MAJOR}${CMAKE_SHARED_LIBRARY_SUFFIX}")
+	endif(MINGW)
     endif (${LIBTYPE} STREQUAL "SHARED")
 
     if (MSVC)

but up to you)

(In reply to Sandro Mani from comment #6)
> > License:        LGPLv2+ with exceptions
> 
> Consider adding a comment (like the following from the native package)
> # see COPYING (or http://www.fltk.org/COPYING.php ) for exceptions details
> 
> 
> > Patch0:         mingw-fltk-cmake.patch
> 
> It's good practice to add a minimal comment as to the purpose of the patch.
> 
> 
> > {?mingw_debug_package}
> 
> Typo, should be %{?mingw_debug_package}
> 
> 
> > %exclude %{mingw32_bindir}/lib*.dll.debug
> > %exclude %{mingw32_datadir}/man/*
> 
> The debug exclude shouldn't isn't necessary. The man pages you'll probably
> want to explicitly remove them in %install.
> 
> Rest is good. Please fix at least the debug typo when importing. Approved.
> 
> (General mingw remark: I typically try to patch the buildsystem to output
> versioned dlls, i.e. in this case something like
> 
> diff -rupN fltk-1.3.5/CMake/macros.cmake fltk-1.3.5-new/CMake/macros.cmake
> --- fltk-1.3.5/CMake/macros.cmake	2019-03-03 09:40:23.000000000 +0100
> +++ fltk-1.3.5-new/CMake/macros.cmake	2020-05-22 20:31:04.755781310 +0200
> @@ -51,6 +51,9 @@ macro(FL_ADD_LIBRARY LIBNAME LIBTYPE LIB
>  	    SOVERSION ${FLTK_VERSION_MAJOR}.${FLTK_VERSION_MINOR}
>  	    PREFIX "lib"    # for MSVC static/shared coexistence
>  	    )
> +	if(MINGW)
> +	    set_target_properties(${LIBRARY_NAME} PROPERTIES SUFFIX
> "-${FLTK_VERSION_MAJOR}${CMAKE_SHARED_LIBRARY_SUFFIX}")
> +	endif(MINGW)
>      endif (${LIBTYPE} STREQUAL "SHARED")
>  
>      if (MSVC)
> 
> but up to you)

Hmm... Is this useful on mingw? Typically the dlls get bundled up with an installer so we don't have the same issue as a system based install on linux. Also, wouldn't setting the SOVERSION property be the correct way?

It's actually more useful as a broken dependency alert mechanism at Fedora package level than once deployed. I picked up the suffix trick some time back, I believe SOVERSION has no effect, but would need to recheck (possibly VERSION has an effect though?)[1]

[1] https://cmake.org/cmake/help/v3.6/prop_tgt/VERSION.html#prop_tgt:VERSION

So I guess the question is, is anyone going to build a mingw package against FLTK? I'm not sure what the use case would be, binaries are almost always not packaged unless needed as part of a build dependency.

Having given this some thought, we really don't know when upstream breaks ABI, per the SOVERSION they set we would assume <MAJOR>.<MINOR>. 

Does <fed>abipkgdiff even work on MinGW libraries? Can we actually check for ABI breakages? A quick search shows abi-compliance-checker supposedly does. 

Perhaps it would just be better to always rebuild dependencies? Outside of the forthcoming data center move, it shouldn't abuse koji too much as the MinGW ecosystem is pretty small.

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/mingw-fltk

Rebuilding all dependencies is definitely the safe way, but sometimes updates are pushed without the dependencies being taken care of. In these cases it's useful to have the update blocked by borken dependencies that discovering when depolying the application that symbols are missing.

Well, I would love some automation (on Fedora and MinGW) to prevent packagers from "doing it wrong" but unfortunately that's not in place. I still think that catching it by enforcing an arbitrary version bump, whether <MAJOR> or <MAJOR>.<MINOR> as we still don't really know if there was an ABI change.