[Bug 642208] Review Request: mingw32-win-iconv - iconv implementation using Win32 API

2011-05-07 Thread bugzilla
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=642208

Kalev Lember  changed:

   What|Removed |Added

 CC||ka...@smartlink.ee

--- Comment #4 from Kalev Lember  2011-05-07 12:20:08 EDT 
---
Very nice and thorough review, Amorilia!

F15 is almost out of the door and it might be a good time to give it a try in
rawhide. I sent a mail about win_iconv to the Fedora MinGW mailing list, asking
for comments:
http://lists.fedoraproject.org/pipermail/mingw/2011-May/003606.html

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Bug 642208] Review Request: mingw32-win-iconv - iconv implementation using Win32 API

2011-05-10 Thread bugzilla
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=642208

Kalev Lember  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
 AssignedTo|nob...@fedoraproject.org|ka...@smartlink.ee
   Flag||fedora-review+

--- Comment #5 from Kalev Lember  2011-05-11 02:52:29 EDT 
---
No feedback to the mail, so I guess lets give it a try.

I'm approving the package based on Amorilia's review. Thanks Amorilia!

Erik, if you want to simplify some things in the spec file, then the BuildRoot
tag, the 'rm -rf $RPM_BUILD_ROOT' at the beginning of %install, the whole
%clean section, and the default %defattrs are no longer needed with current
Fedora releases.

Also, the Obsoletes and Provides currently have %dist macro in them; it's more
common to leave that out, since the 'mingw32-iconv < 1.12-14' comparison would
already match the current 'mingw32-iconv-1.12-13.fc15' package:
Obsoletes: mingw32-iconv < 1.12-14
Provides:  mingw32-iconv = 1.12-14

But these are all things one would do during the normal maintenance of a
package and not blocking the review.

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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Bug 642208] Review Request: mingw32-win-iconv - iconv implementation using Win32 API

2011-05-13 Thread bugzilla
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=642208

--- Comment #6 from Erik van Pienbroek  2011-05-13 
13:04:36 EDT ---
Thanks for approving it Kalev.
I think it would be wise to wait for some more days before requesting a git
repo. The reason behind this is the legal issues surrounding mingw-w64 support.
Once those are cleared (which should be any day now) we can decide to use the
new mingw packaging guidelines and thus the new naming for mingw packages
(mingw-* instead of mingw32-*). This package will only be introduced in rawhide
anyway, so nothing is lost is we wait for some more time

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Bug 642208] Review Request: mingw32-win-iconv - iconv implementation using Win32 API

2010-12-28 Thread bugzilla
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=642208

amori...@users.sourceforge.net changed:

   What|Removed |Added

 CC||amori...@users.sourceforge.
   ||net

--- Comment #1 from amori...@users.sourceforge.net 2010-12-28 09:16:28 EST ---
Beware, this is my very first package review. Feedback on the review by a more
experienced packager is welcome, particularly if I missed any crucial steps.

[+] OK
[!] Needs to be looked into
[/] Not applicable
[*] Overridden by MinGW guidelines


Rpmlint
---

$ rpmlint mingw32-win-iconv.spec 
mingw32-win-iconv.spec: W: invalid-url Source0:
http://win-iconv.googlecode.com/files/win-iconv-0.0.1.tar.bz2 HTTP Error 404:
Not Found
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

$ rpmlint mingw32-win-iconv-0.0.1-1.fc14.src.rpm 
mingw32-win-iconv.src: W: invalid-url Source0:
http://win-iconv.googlecode.com/files/win-iconv-0.0.1.tar.bz2 HTTP Error 404:
Not Found
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Harmless warning, URL works:

$ wget http://win-iconv.googlecode.com/files/win-iconv-0.0.1.tar.bz2
--2010-12-28 11:15:59-- 
http://win-iconv.googlecode.com/files/win-iconv-0.0.1.tar.bz2
Resolving win-iconv.googlecode.com... 209.85.229.82
Connecting to win-iconv.googlecode.com|209.85.229.82|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: 17338 (17K) [application/x-bzip2]
Saving to: “win-iconv-0.0.1.tar.bz2”

100%[==>] 17,338  --.-K/s   in 0.1s

2010-12-28 11:16:00 (165 KB/s) - “win-iconv-0.0.1.tar.bz2” saved [17338/17338]

$ rpm -i mingw32-win-iconv-0.0.1-1.fc14.src.rpm
$ md5sum win-iconv-0.0.1.tar.bz2
22ee1bbaae404fe34dca835f1c669a1e  win-iconv-0.0.1.tar.bz2
$ md5sum ~/rpmbuild/SOURCES/win-iconv-0.0.1.tar.bz2
22ee1bbaae404fe34dca835f1c669a1e 
/home/amorilia/rpmbuild/SOURCES/win-iconv-0.0.1.tar.bz2
$ diff mingw32-win-iconv.spec ~/rpmbuild/SPECS/mingw32-win-iconv.spec  -s
Files mingw32-win-iconv.spec and
/home/amorilia/rpmbuild/SPECS/mingw32-win-iconv.spec are identical

$ rpmbuild -ba mingw32-win-iconv.spec
...

Build succeeds.

$ rpmlint ~/rpmbuild/RPMS/noarch/mingw32-win-iconv-0.0.1-1.fc14.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings

$ rpmlint
~/rpmbuild/RPMS/noarch/mingw32-win-iconv-debuginfo-0.0.1-1.fc14.noarch.rpm
mingw32-win-iconv-debuginfo.noarch: E: debuginfo-without-sources
1 packages and 0 specfiles checked; 1 errors, 0 warnings.

$ rpmlint
~/rpmbuild/RPMS/noarch/mingw32-win-iconv-static-0.0.1-1.fc14.noarch.rpm
mingw32-win-iconv-static.noarch: E:
arch-independent-package-contains-binary-or-object
/usr/i686-pc-mingw32/sys-root/mingw/lib/libiconv.a
mingw32-win-iconv-static.noarch: W: no-documentation
1 packages and 0 specfiles checked; 1 errors, 1 warnings.

Looks fine, follows MinGW guidelines.

Dependencies


$ su -c 'yum install mingw32-iconv*'
$ rpm -qa | grep mingw32-iconv
mingw32-iconv-static-1.12-12.fc12.noarch
mingw32-iconv-debuginfo-1.12-12.fc12.noarch
mingw32-iconv-1.12-12.fc12.noarch
$ su -c 'rpm -Uv /home/amorilia/rpmbuild/RPMS/noarch/mingw32-win-iconv*.rpm'
Preparing packages for installation...
mingw32-win-iconv-0.0.1-1.fc14
mingw32-win-iconv-static-0.0.1-1.fc14
mingw32-win-iconv-debuginfo-0.0.1-1.fc14
$ rpm -qa | grep mingw32-iconv
mingw32-iconv-debuginfo-1.12-12.fc12.noarch

[!] mingw32-iconv-debuginfo-1.12-12.fc12.noarch should have been removed.

$ rpmquery --requires mingw32-win-iconv
rpmlib(VersionedDependencies) <= 3.0.3-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(CompressedFileNames) <= 3.0.4-1
mingw32-filesystem >= 63
mingw32-runtime  
mingw32(kernel32.dll)  
mingw32(msvcrt.dll)  
rpmlib(PayloadIsXz) <= 5.2-1

$ rpmquery --provides mingw32-win-iconv
mingw32-iconv = 1.12-13.fc14
mingw32(libiconv.dll)  
mingw32-win-iconv = 0.0.1-1.fc14

$ rpmquery --requires mingw32-win-iconv-static
mingw32-win-iconv = 0.0.1-1.fc14
rpmlib(VersionedDependencies) <= 3.0.3-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(CompressedFileNames) <= 3.0.4-1
mingw32-filesystem >= 63
mingw32-runtime  
rpmlib(PayloadIsXz) <= 5.2-1

$ rpmquery --provides mingw32-win-iconv-static
mingw32-iconv-static = 1.12-13.fc14
mingw32-win-iconv-static = 0.0.1-1.fc14

$ rpmquery --requires mingw32-win-iconv-debuginfo
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(CompressedFileNames) <= 3.0.4-1
mingw32-filesystem >= 63
mingw32-runtime  
rpmlib(PayloadIsXz) <= 5.2-1

Does not require mingw32-win-iconv, but apparently native debuginfos are
similar, so I guess this is fine.

$ rpmquery --provides mingw32-win-iconv-debuginfo
mingw32-win-iconv-debuginfo = 0.0.1

[Bug 642208] Review Request: mingw32-win-iconv - iconv implementation using Win32 API

2011-02-17 Thread bugzilla
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=642208

--- Comment #2 from Erik van Pienbroek  2011-02-17 
07:39:02 EST ---
Thanks for the initial review. You're doing the review very well for a
first-timer!

Spec URL: http://ftd4linux.nl/contrib/mingw32-win-iconv.spec
SRPM URL: http://ftd4linux.nl/contrib/mingw32-win-iconv-0.0.2-1.fc15.src.rpm
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2846284

* Thu Feb 17 2011 Erik van Pienbroek  - 0.0.2-1
- Update to version 0.0.2
- Dropped upstreamed patch
- Dropped the win_iconv.exe binary
- Bumped the mingw32-iconv obsoletes

The debuginfo stuff is wrapped inside the %{?_mingw32_debug_package} macro.
There's no way to inject custom tags in this macro, so adding an obsoletes:
mingw32-iconv-debuginfo there isn't possible at the moment. AFAIK, native
Fedora packages also don't have an obsoletes on old debuginfo packages when a
new package is introduced which obsoletes an old package

Once this package gets added to Fedora all packages which depend on libiconv
have to be rebuild. I tested this in my mingw-w64 testing repository (which
contains all current fedora mingw32 packages) and it causes no (compilation)
breakage. I've also done a runtime test using my GTK-based project and it
doesn't cause any side effects. This library is also used by the GLib/Gtk
developers for their Win32/Win64 binaries of the Gtk stack:
http://ftp.gnome.org/pub/gnome/binaries/win64/dependencies/

The packages which need to be rebuilt are (according to repoquery on a F15
host):
mingw32-fontconfig-0:2.6.0-11.fc15.noarch
mingw32-gettext-0:0.17-14.fc15.noarch
mingw32-hunspell-0:1.2.12-2.fc15.noarch
mingw32-libidn-0:1.14-7.fc15.noarch
mingw32-libxml2-0:2.7.6-2.fc15.noarch
A simple rebuild is sufficient for these packages

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Bug 642208] Review Request: mingw32-win-iconv - iconv implementation using Win32 API

2011-02-21 Thread bugzilla
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=642208

--- Comment #3 from amori...@users.sourceforge.net 2011-02-21 13:25:02 EST ---
Thanks - I've learnt a lot!

Agreed with all changes.

If I had special packager powers, I would approve your package, but
until that time, it will be up to a real packager.

For the sake of sanity checking, rpmlint output for the update package
below. Everything looks ok, as expected.

Rpmlint
---

$ rpmlint mingw32-win-iconv.spec 
mingw32-win-iconv.spec: W: invalid-url Source0:
http://win-iconv.googlecode.com/files/win-iconv-0.0.2.tar.bz2 HTTP Error 404:
Not Found
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

As before, spurious warning.

$ rpmlint mingw32-win-iconv-0.0.2-1.fc15.src.rpm
mingw32-win-iconv.src: W: invalid-url Source0:
http://win-iconv.googlecode.com/files/win-iconv-0.0.2.tar.bz2 HTTP Error 404:
Not Found
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

As before, spurious warning.

$  rpm -i mingw32-win-iconv-0.0.2-1.fc15.src.rpm

$ md5sum ~/rpmbuild/SOURCES/win-iconv-0.0.2.tar.bz2
4300d7f337a3c13ab255d4d855057c16
$ curl -s http://win-iconv.googlecode.com/files/win-iconv-0.0.2.tar.bz2 |
md5sum
4300d7f337a3c13ab255d4d855057c16

$ diff mingw32-win-iconv.spec ~/rpmbuild/SPECS/mingw32-win-iconv.spec -s
Files mingw32-win-iconv.spec and
/home/amorilia/rpmbuild/SPECS/mingw32-win-iconv.spec are identical

$ rpmbuild -ba mingw32-win-iconv.spec
...

Build succeeds.

$ rpmlint ~/rpmbuild/RPMS/noarch/mingw32-win-iconv-0.0.2-1.fc14.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

$ rpmlint
~/rpmbuild/RPMS/noarch/mingw32-win-iconv-debuginfo-0.0.2-1.fc14.noarch.rpm 
mingw32-win-iconv-debuginfo.noarch: E: debuginfo-without-sources
1 packages and 0 specfiles checked; 1 errors, 0 warnings.

$ rpmlint
~/rpmbuild/RPMS/noarch/mingw32-win-iconv-static-0.0.2-1.fc14.noarch.rpm 
mingw32-win-iconv-static.noarch: E:
arch-independent-package-contains-binary-or-object
/usr/i686-pc-mingw32/sys-root/mingw/lib/libiconv.a
mingw32-win-iconv-static.noarch: W: no-documentation
1 packages and 0 specfiles checked; 1 errors, 1 warnings.

Looks fine, follows MinGW guidelines.

-- 
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review