[Bug 787878] Review Request: mingw32-icu - MinGW compilation of International Components for Unicode Tools

[bugzilla at redhat.com]

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=787878

Kalev Lember <kalevlember at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-review+

--- Comment #7 from Kalev Lember <kalevlember at gmail.com> 2012-02-08 10:28:14 EST ---
Fedora review of mingw-icu-4.8.1.1-2.fc16.src.rpm 2012-02-08

+ OK
! needs attention

rpmlint output:
$ $ rpmlint mingw32-icu \
            mingw32-icu-debuginfo-4.8.1.1-2.fc16.noarch.rpm \
            mingw-icu-4.8.1.1-2.fc16.src.rpm
mingw32-icu.noarch: E: rpath-in-buildconfig
/usr/i686-pc-mingw32/sys-root/mingw/bin/icu-config lines ['100']
mingw32-icu-debuginfo.noarch: E: debuginfo-without-sources
mingw-icu.src:72: W: configure-without-libdir-spec
3 packages and 0 specfiles checked; 2 errors, 1 warnings.

+ rpmlint warnings are harmless and can be ignored
+ The package is named according to Fedora MinGW packaging guidelines
+ The spec file name matches the base package name.
+ The package meets the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the
  Licensing Guidelines.
+ The license field in the spec file matches the actual license
+ The stated license is the same as the one for the corresponding
  native Fedora package
+ The package contains the license files (license.html, unicode-license.txt)
+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match sources in the srpm. md5sum:
  ea93970a0275be6b42f56953cd332c17  icu4c-4_8_1_1-src.tgz
  ea93970a0275be6b42f56953cd332c17  Download/icu4c-4_8_1_1-src.tgz
+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
n/a The spec file MUST handle locales properly
n/a ldconfig in %post and %postun
+ Package does not bundle copies of system libraries
n/a Package isn't relocatable
+ Package owns all directories it creates
+ No duplicate files in %files
+ Permissions are properly set
+ Consistent use of macros
+ The 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
n/a Static libraries should be in -static
n/a Library files that end in .so must go in a -devel package
n/a -devel must require the fully versioned base
+ Packages should not contain libtool .la files
n/a Packages containing GUI apps must include %{name}.desktop file
+ Directory ownership sane
+ Filenames are valid UTF-8

Looks very nice overall, considering how difficult ICU is to cross compile.


A remaining issue appears to be with icu-config, which is looking for
nonexisting .lib files:
$ /usr/i686-pc-mingw32/sys-root/mingw/bin/icu-config --cflags
### icu-config: Can't find /usr/i686-pc-mingw32/sys-root/mingw/bin/icuuc48.lib
- ICU prefix is wrong.
###      Try the --prefix= option
###      or --detect-prefix
###      (If you want to disable this check, use  the --noverify option)
### icu-config: Exitting.

I think this is caused by the sed snippet: 's,^SO=.*$$,SO="dll",'
which appears to have a typo, should be one $ instead of the two $$ it
currently has. Please fix it before importing the package.

Also, some nitpicking about small issues (not review blockers) that you could
clean up before importing, if you want to:
 - "BuildRequires:  gcc-c++" is not needed, because gcc-c++ is in the default
buildroot
 - instead of "%{_mingw32_make} %{?_smp_mflags} || %{_mingw32_make}" I'd
personally just use
   "make %{?_smp_mflags}"


APPROVED

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.