Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: mingw32-hunspell - MinGW Windows spell checker and morphological analyzer library
https://bugzilla.redhat.com/show_bug.cgi?id=502387
Summary: Review Request: mingw32-hunspell - MinGW Windows spell checker and morphological analyzer library Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: low Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: erik-fedora@vanpienbroek.nl QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com, fedora-mingw@lists.fedoraproject.org Classification: Fedora
Spec URL: http://www.ftd4linux.nl/contrib/mingw32-hunspell.spec SRPM URL: http://www.ftd4linux.nl/contrib/mingw32-hunspell-1.2.8-6.fc11.src.rpm Description: Hunspell is a spell checker and morphological analyzer library and program designed for languages with rich morphology and complex word compounding or character encoding. Hunspell interfaces: Ispell-like terminal interface using Curses library, Ispell pipe interface, OpenOffice.org UNO module.
This is the MinGW build of hunspell
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1374924
Approved MinGW packaging guidelines are here: http://fedoraproject.org/wiki/Packaging/MinGW
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=502387
Erik van Pienbroek erik-fedora@vanpienbroek.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |502388
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=502387
Thomas Sailer t.sailer@alumni.ethz.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |t.sailer@alumni.ethz.ch AssignedTo|nobody@fedoraproject.org |t.sailer@alumni.ethz.ch Flag| |fedora-review?
--- Comment #1 from Thomas Sailer t.sailer@alumni.ethz.ch 2009-05-26 06:13:51 EDT --- Fedora review http://www.ftd4linux.nl/contrib/mingw32-libgnurx-2.5.1-1.fc11.src.rpm 2009-05-26
rpmlint output: $ rpmlint * mingw32-hunspell.noarch: E: arch-independent-package-contains-binary-or-object /usr/i686-pc-mingw32/sys-root/mingw/lib/libparsers.a mingw32-hunspell-static.noarch: E: arch-independent-package-contains-binary-or-object /usr/i686-pc-mingw32/sys-root/mingw/lib/libhunspell-1.2.a mingw32-hunspell-static.noarch: W: no-documentation 3 packages and 1 specfiles checked; 2 errors, 1 warnings.
As per Packaging/MinGW, these errors can be ignored.
+ OK ! needs attention
+ rpmlint output + Package is named according to Fedora MinGW packaging guidelines + Specfile name matches the package base name + Package follows the Fedora MinGW packaging guidelines version seems to be slightly ahead of native (1.1 vs. 1.0) please try to stick to the native version + License meets guidelines and is acceptable to Fedora LGPLv2+ or GPLv2+ or MPLv1.1 + License matches the actual package license + The package contains the license file (COPYING,COPYING.LGPL,COPYING.MPL) + Spec file is written in American English + Spec file is legible - IMO, you don't need to include the whole history of the native package in %changelog, just a reference that it was derived from the native package would be sufficient
+ Upstream sources match sources in the srpm 1177af54a09e320d2c24015f29c3a93e hunspell-1.2.8.tar.gz 1177af54a09e320d2c24015f29c3a93e x/hunspell-1.2.8.tar.gz
n/a Package builds in mock n/a ExcludeArch bugs filed + BuildRequires list all build dependencies n/a %find_lang instead of %{_datadir}/locale/* n/a binary RPM with shared library files must call ldconfig in %post and %postun + Does not use Prefix: /usr + Package owns all directories it creates + No duplicate files in %files + %files has %defattr + %clean contains rm -rf $RPM_BUILD_ROOT + Consistent use of macros + Package must contain code or permissible content n/a Large documentation files should go in -doc subpackage + Files marked %doc should not affect package n/a Header files should be in -devel Fedora MinGW guidelines allow headers in main package + Static libraries should be in -static + Packages containing pkgconfig (.pc) files need 'Requires: pkgconfig' n/a libfoo.so must go in -devel n/a -devel must require the fully versioned base n/a Packages should not contain libtool .la files Fedora MinGW guidelines allow .la files n/a Packages containing GUI apps must include %{name}.desktop file + Packages must not own files or directories owned by other packages + %install begins with rm -rf $RPM_BUILD_ROOT + Filenames must be valid UTF-8
Have you tried to upstream your patches? Especially hunspell-build-dll.patch seems like very upstream-worthy. Also, what is the failure mode of AC_FUNC_MALLOC? I guess a bug report with autoconf would be in order.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=502387
--- Comment #2 from Erik van Pienbroek erik-fedora@vanpienbroek.nl 2009-05-26 06:36:02 EDT --- (In reply to comment #1)
Have you tried to upstream your patches? Especially hunspell-build-dll.patch seems like very upstream-worthy.
Filed at https://sourceforge.net/tracker/?func=detail&aid=2796772&group_id=14...
Also, what is the failure mode of AC_FUNC_MALLOC? I guess a bug report with autoconf would be in order.
The AC_FUNC_MALLOC is some kind of fallback for environments which don't have a malloc() function in their libc. If this is the case (apparently it is for MinGW), then a '#define malloc rpl_malloc' is added to config.h.
According to rjones, the rpl_malloc() function is part of gnulib. We don't have gnulib in our MinGW toolchain, so this ends up in a undefined reference (to rpl_malloc). After removing the AC_FUNC_MALLOC from the configure.ac file, the compile completes without problems (and the malloc from the MinGW libraries is used).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=502387
Thomas Sailer t.sailer@alumni.ethz.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #3 from Thomas Sailer t.sailer@alumni.ethz.ch 2009-05-26 06:56:13 EDT --- (In reply to comment #2)
The AC_FUNC_MALLOC is some kind of fallback for environments which don't have a malloc() function in their libc. If this is the case (apparently it is for MinGW), then a '#define malloc rpl_malloc' is added to config.h.
I agree we don't want rpl_malloc.
The question is why does it fail to find the malloc in libmsvcrt.a (or newer friends). libmsvcrt.a provides _malloc:
i686-pc-mingw32-nm /usr/i686-pc-mingw32/sys-root/mingw/lib/libmsvcrt.a dxnmbs00676.o: 00000000 b .bss 00000000 d .data 00000000 i .idata$4 00000000 i .idata$5 00000000 i .idata$6 00000000 i .idata$7 00000000 t .text U __head_libmsvcrt_a 00000000 I __imp__malloc 00000000 T _malloc
But that shouldn't hold up packaging.
APPROVED by sailer.
Easy!
The AC_FUNC_MALLOC and AC_FUNC_REALLOC try to execute a program to see whether malloc(0) will return a NULL pointer. If that is not the case, it will use the rpl_malloc. Since cross-compiling, binaries cannot be executed and the test fails even though the malloc and realloc are in MS C runtime. Those malloc and realloc functions are compliant in the sense of AC_FUNC_MALLOC and AC_FUNC_REALLOC test, so patching them out is safe when cross-compiling for win32.
Cheers
Fridrich
On Tue, 2009-05-26 at 06:56 -0400, bugzilla-H+wXaHxf7aLQT0dZR--- Comment #3 from Thomas Sailer t.sailer@alumni.ethz.ch 2009-05-26 06:56:13 EDT ---
(In reply to comment #2)
The AC_FUNC_MALLOC is some kind of fallback for environments which don't have a malloc() function in their libc. If this is the case (apparently it is for MinGW), then a '#define malloc rpl_malloc' is added to config.h.
I agree we don't want rpl_malloc.
The question is why does it fail to find the malloc in libmsvcrt.a (or newer friends). libmsvcrt.a provides _malloc:
i686-pc-mingw32-nm /usr/i686-pc-mingw32/sys-root/mingw/lib/libmsvcrt.a dxnmbs00676.o: 00000000 b .bss 00000000 d .data 00000000 i .idata$4 00000000 i .idata$5 00000000 i .idata$6 00000000 i .idata$7 00000000 t .text U __head_libmsvcrt_a 00000000 I __imp__malloc 00000000 T _malloc
But that shouldn't hold up packaging.
APPROVED by sailer.
Op dinsdag 26-05-2009 om 13:14 uur [tijdzone +0200], schreef Fridrich Strba:
Easy!
The AC_FUNC_MALLOC and AC_FUNC_REALLOC try to execute a program to see whether malloc(0) will return a NULL pointer. If that is not the case, it will use the rpl_malloc. Since cross-compiling, binaries cannot be executed and the test fails even though the malloc and realloc are in MS C runtime. Those malloc and realloc functions are compliant in the sense of AC_FUNC_MALLOC and AC_FUNC_REALLOC test, so patching them out is safe when cross-compiling for win32.
Cheers
Fridrich
Thanks for the clarification!
In the future, could you please leave comments on bugzilla tickets in bugzilla itself instead of the mailing list? That way it'll be easier to keep all information together and prevent it from getting scattered.
Regards,
Erik van Pienbroek
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=502387
--- Comment #4 from Erik van Pienbroek erik-fedora@vanpienbroek.nl 2009-05-26 07:24:45 EDT --- On the fedora-mingw mailing list Fridrich Strba clarified the AC_FUNC_MALLOC behaviour:
Easy!
The AC_FUNC_MALLOC and AC_FUNC_REALLOC try to execute a program to see whether malloc(0) will return a NULL pointer. If that is not the case, it will use the rpl_malloc. Since cross-compiling, binaries cannot be executed and the test fails even though the malloc and realloc are in MS C runtime. Those malloc and realloc functions are compliant in the sense of AC_FUNC_MALLOC and AC_FUNC_REALLOC test, so patching them out is safe when cross-compiling for win32.
Cheers
Fridrich
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=502387
Erik van Pienbroek erik-fedora@vanpienbroek.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #5 from Erik van Pienbroek erik-fedora@vanpienbroek.nl 2009-05-26 07:26:16 EDT --- Thomas, thanks for the review!
New Package CVS Request ======================= Package Name: mingw32-hunspell Short Description: MinGW Windows spell checker and morphological analyzer library Owners: epienbro rjones Branches: F-10 F-11 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=502387
--- Comment #6 from Thomas Sailer t.sailer@alumni.ethz.ch 2009-05-26 08:09:33 EDT --- (In reply to comment #4)
The AC_FUNC_MALLOC and AC_FUNC_REALLOC try to execute a program to see whether malloc(0) will return a NULL pointer. If that is not the case,
So that means these autoconf tests can't be used when crosscompiling to any target, not just mingw. Bad. That should IMO at least be documented (it isn't in autoconf 2.63).
C runtime. Those malloc and realloc functions are compliant in the sense of AC_FUNC_MALLOC and AC_FUNC_REALLOC test, so patching them out is safe when cross-compiling for win32.
That's not what I'm worried about. I'm worried about the tediousness to patch these tests out in all packages that use them, vs. fixing the macro in autoconf.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=502387
Fridrich Strba fridrich.strba@bluewin.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fridrich.strba@bluewin.ch
--- Comment #7 from Fridrich Strba fridrich.strba@bluewin.ch 2009-05-26 10:47:01 EDT --- OK, I don't see how you want to change that. You need to run a programme to be sure that malloc(0) returns NULL or non-null. You have some other situations where you cannot determine something without actually running a programme, like allignments of types. The standard way of proceding is either to patch the system for known platform with values that we know to be true (I did this here http://svn.gnome.org/viewvc/evolution-data-server/trunk/configure.in?r1=9959... ) or to pre-populate the compile cache with values for tests that cannot be run. In this case, it is just to pre-fill the cache with: ac_cv_func_malloc_0_nonnull=yes
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=502387
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #8 from Jason Tibbitts tibbs@math.uh.edu 2009-05-26 18:48:58 EDT --- CVS done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=502387
--- Comment #9 from Fedora Update System updates@fedoraproject.org 2009-06-02 11:16:20 EDT --- mingw32-hunspell-1.2.8-6.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/mingw32-hunspell-1.2.8-6.fc11
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=502387
--- Comment #10 from Fedora Update System updates@fedoraproject.org 2009-06-04 17:18:58 EDT --- mingw32-hunspell-1.2.8-6.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=502387
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Fixed In Version| |1.2.8-6.fc11 Resolution| |NEXTRELEASE