Boszormenyi Zoltan schreef op vr 02-12-2011 om 15:13 [+0100]: Thank you for the .spec file! Normally these kind of new packages have to go through the Fedora package review process before they can be allowed in Fedora. However, as the mingw-w64 toolchain currently isn't available yet in Fedora (pending legal approval) I'm willing to do a review of the package through this mailing list. Once the package is approved I'll add it to the testing repo. Would you also be interested in becoming package maintainer for this package in Fedora once the mingw-w64 toolchain is available in Fedora? On with the review. - The patch contains fixes which will also benefit non-fedora users who want to cross-compile GD from a linux host for the mingw32 target. Did you already consider sending the patch upstream? - For mingw-w64 packages, the first 6 lines of the .spec file (%global/% define) can be replaced by a single '%?mingw_package_header' - The mingw_package_header macro got introduced in mingw-filesystem 82, so please bump the BuildRequires: mingw32-filesystem and mingw64-filesystem to >= 82 - The BuildRoot tag, the 'rm -rf $RPM_BUILD_ROOT' from the %install section, the entire %clean section and the various %defattr lines can all be removed as they're unneeded with modern RPM - In the %build section there's a comment which says 'GD can't build..so we build GLib twice'. That looks like a copy/paste typo to me :) - Is building static and shared libraries at the same time really not possible with GD? Most packages support running the configure script with both '--enable-shared' and '--enable-static' set - In the %install section there are some places where just used 'i686-w64-mingw32' and 'x86_64-w64-mingw32' literally. Please use the macros %{mingw32_target} and %{mingw64_target} there - The %post and %postun sections can be dropped as running ldconfig is only needed for native linux binaries - Please don't use wildcards in the %files section. Right now unexpected changes in file names won't be caught when using wildcards, so it's better to use a list containing all individual file names - The -static packages only needs to contain the .a files. The .dll.a and .la files need to be part of the main package as they're required when other packages want to link against GD Could you update the .spec file? Kind regards, Erik van Pienbroek

Boszormenyi Zoltan schreef op vr 02-12-2011 om 21:15 [+0100]: Basically it means that you'll have to update the package once upstream releases new versions and that you'll take care of any bugreports which might get filed against this package. Of course we (the Fedora MinGW SIG) will help you with this, but being package owner will mean that you'll become the primary point of contact. For a package like GD (which doesn't have that much upstream releases these days) I think it doesn't require that much maintenance. Ah I see. The .src.rpm you mentioned is a bit outdated right now. The most recent version of the .spec files in the mingw-w64 testing repo can be found at http://svn.openftd.org/svn/fedora_cross/ (which will also be the location where I'll add your package for the time being). The reason why I didn't upload all the recent .src.rpm's is a lack of disk space on the webhost. All .src.rpm's can easily be regenerated by checking out the svn repo I just mentioned. You can see all the available RPM macros in the files /etc/rpm/macros.mingw, /etc/rpm/macros.mingw32 and /etc/rpm/macros.mingw64. Additionally almost all these RPM macros are also documented in the new Fedora MinGW Packaging Guidelines: https://fedoraproject.org/wiki/Packaging:MinGW_Future Very good! I just tried to build your package in mock, but it failed to build: + libtoolize -c -f -i /var/tmp/rpm-tmp.LdzcCW: line 72: libtoolize: command not found fout: Bad exit status from /var/tmp/rpm-tmp.LdzcCW (%prep) Bad exit status from /var/tmp/rpm-tmp.LdzcCW (%prep) RPM build errors: Child returncode was: 1 Looks like you need to add a 'BuildRequires: libtool' to the .spec file. In your .spec file there's also an 'autoreconf' call, so that means that an additional 'BuildRequires: autoconf automake' also needs to be added. I just tried building with these additional changes, but that still gives me the following error: + autoreconf -f -i configure.ac:69: warning: macro `AM_ICONV' not found in library libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, `config'. libtoolize: copying file `config/ltmain.sh' libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `config'. libtoolize: copying file `config/libtool.m4' libtoolize: copying file `config/ltoptions.m4' libtoolize: copying file `config/ltsugar.m4' libtoolize: copying file `config/ltversion.m4' libtoolize: copying file `config/lt~obsolete.m4' configure.ac:69: warning: macro `AM_ICONV' not found in library configure.ac:69: error: possibly undefined macro: AM_ICONV If this token and others are legitimate, please use m4_pattern_allow. See the Autoconf documentation. autoreconf: /usr/bin/autoconf failed with exit status: 1 fout: Bad exit status from /var/tmp/rpm-tmp.fwW3FS (%prep) RPM build errors: Bad exit status from /var/tmp/rpm-tmp.fwW3FS (%prep) Child returncode was: 1 To fix this, a 'BuildRequires: gettext-devel' also needs to be added. With all these additional BuildRequires the package builds fine in mock. After you've made these changes I'll add the package to the mingw-w64 testing repo. Kind regards, Erik van Pienbroek

Boszormenyi Zoltan schreef op za 03-12-2011 om 20:18 [+0100]: Thanks for this package and the mingw-crypt package! I just published both packages to the mingw-w64 testing repo. Kind regards, Erik van Pienbroek