[Bug 759757] New: Review Request: perl-Encode-EUCJPASCII - EucJP-ascii - An eucJP-open mapping
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: perl-Encode-EUCJPASCII - EucJP-ascii - An eucJP-open mapping
https://bugzilla.redhat.com/show_bug.cgi?id=759757
Summary: Review Request: perl-Encode-EUCJPASCII - EucJP-ascii - An eucJP-open mapping Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: xavier@bachelot.org QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: ---
Spec URL: http://www.bachelot.org/fedora/SPECS/perl-Encode-EUCJPASCII.spec SRPM URL: http://www.bachelot.org/fedora/SRPMS/perl-Encode-EUCJPASCII-0.03-1.fc15.src.... Description: This module provides eucJP-ascii, one of eucJP-open mappings, and its derivative.
This module is needed for better test coverage in perl-MIME-Charset.
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=759757
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |ppisar@redhat.com AssignedTo|nobody@fedoraproject.org |ppisar@redhat.com Flag| |fedora-review?
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=759757
--- Comment #1 from Petr Pisar ppisar@redhat.com 2012-01-05 07:58:41 EST --- Source tar ball is original. Ok. Summary verified from EUCJPASCII.pm. Ok. Description verified from EUCJPASCII.pm. Ok.
Note: I'm not sure `eucJP-ascii' and `eucJP-open' are properly capitalized. Different texts use different capitalization and hyphenation. Let's hope イケダソジ, author of this module, knows better.
License verified from EUCJPASCII.pm. Ok. URL and Source0 values are useful. Ok.
TODO: Remove useless BuildRoot definition, it's cleaning in %install section, and whole %clean section. They are not needed anymore.
TODO: Build-require perl(Encode) for tests (EUCJPASCII.pm:6) TODO: Build-require perl(XSLoader) for tests (EUCJPASCII.pm:7) TODO: Build-require perl(base) for tests (EUCJPASCII.pm:18) TODO: Build-require perl(Encode::CJKConstants) for tests (EUCJPASCII.pm:23) TODO: Build-require perl(Encode::JP::JIS7) for tests (EUCJPASCII.pm:23)
FIX: Build-require perl(File::Spec) (Makefile.PL:18)
All tests pass. Ok.
TODO: Remove useless %defattr from %files section.
$ rpmlint perl-Encode-EUCJPASCII.spec ../SRPMS/perl-Encode-EUCJPASCII-0.03-1.fc17.src.rpm ../RPMS/x86_64/perl-Encode-EUCJPASCII-* perl-Encode-EUCJPASCII.src: W: spelling-error Summary(en_US) eucJP -> eggcup, equip, uncap perl-Encode-EUCJPASCII.src: W: spelling-error %description -l en_US eucJP -> eggcup, equip, uncap perl-Encode-EUCJPASCII.src: W: spelling-error %description -l en_US ascii -> ASCII, ASCIIs, As perl-Encode-EUCJPASCII.x86_64: W: spelling-error Summary(en_US) eucJP -> eggcup, equip, uncap perl-Encode-EUCJPASCII.x86_64: W: spelling-error %description -l en_US eucJP -> eggcup, equip, uncap perl-Encode-EUCJPASCII.x86_64: W: spelling-error %description -l en_US ascii -> ASCII, ASCIIs, As perl-Encode-EUCJPASCII.x86_64: E: zero-length /usr/lib64/perl5/vendor_perl/auto/Encode/EUCJPASCII/EUCJPASCII.bs 3 packages and 1 specfiles checked; 1 errors, 6 warnings.
FIX: Remove empty *.bs files. (These empty bootstrap files for DynaLoader are useless.)
$ rpm -q -lv -p ../RPMS/x86_64/perl-Encode-EUCJPASCII-0.03-1.fc17.x86_64.rpm drwxr-xr-x 2 root root 0 Jan 5 13:40 /usr/lib64/perl5/vendor_perl/Encode -rw-r--r-- 1 root root 5662 Oct 19 2009 /usr/lib64/perl5/vendor_perl/Encode/EUCJPASCII.pm drwxr-xr-x 2 root root 0 Jan 5 13:40 /usr/lib64/perl5/vendor_perl/auto drwxr-xr-x 2 root root 0 Jan 5 13:40 /usr/lib64/perl5/vendor_perl/auto/Encode drwxr-xr-x 2 root root 0 Jan 5 13:40 /usr/lib64/perl5/vendor_perl/auto/Encode/EUCJPASCII -rw-r--r-- 1 root root 0 Jan 5 13:40 /usr/lib64/perl5/vendor_perl/auto/Encode/EUCJPASCII/EUCJPASCII.bs -rwxr-xr-x 1 root root 759816 Jan 5 13:40 /usr/lib64/perl5/vendor_perl/auto/Encode/EUCJPASCII/EUCJPASCII.so drwxr-xr-x 2 root root 0 Jan 5 13:40 /usr/share/doc/perl-Encode-EUCJPASCII-0.03 -rw-r--r-- 1 root root 569 Oct 19 2009 /usr/share/doc/perl-Encode-EUCJPASCII-0.03/Changes -rw-r--r-- 1 root root 496 Oct 19 2009 /usr/share/doc/perl-Encode-EUCJPASCII-0.03/README Files permissions and layout are Ok.
$ rpm -q --requires -p ../RPMS/x86_64/perl-Encode-EUCJPASCII-0.03-1.fc17.x86_64.rpm |sort |uniq -c 1 libc.so.6()(64bit) 1 libc.so.6(GLIBC_2.2.5)(64bit) 1 perl(base) 1 perl(bytes) 1 perl(Encode) 1 perl(Encode::CJKConstants) 1 perl(Encode::Encoding) 1 perl(Encode::JP::JIS7) 1 perl(:MODULE_COMPAT_5.14.2) 1 perl(strict) 1 perl(warnings) 1 perl(XSLoader) 1 rpmlib(CompressedFileNames) <= 3.0.4-1 1 rpmlib(FileDigests) <= 4.6.0-1 1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 1 rpmlib(PayloadIsXz) <= 5.2-1 1 rtld(GNU_HASH) Binary requires are Ok.
$ rpm -q --provides -p ../RPMS/x86_64/perl-Encode-EUCJPASCII-0.03-1.fc17.x86_64.rpm |sort |uniq -c 1 perl(Encode::EUCJPASCII) = 0.03 1 perl-Encode-EUCJPASCII = 0.03-1.fc17 1 perl-Encode-EUCJPASCII(x86-64) = 0.03-1.fc17 Binary provides are Ok.
$ resolvedeps rawhide ../RPMS/x86_64/perl-Encode-EUCJPASCII-0.03-1.fc17.x86_64.rpm Binary dependencies resolvable. Ok.
Package builds in F17 (http://koji.fedoraproject.org/koji/taskinfo?taskID=3621985). Ok.
Otherwise package is in line with Fedora and Perl packaging guidelines.
Please correct all `FIX' prefixed issues, consider fixing `TODO' items, and provide new spec file.
Resolution: Package NOT approved.
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=759757
--- Comment #2 from Xavier Bachelot xavier@bachelot.org 2012-01-05 11:28:28 EST --- Thanks for the review, Petr. Comments for FIX and TODO inline.
Spec URL: http://www.bachelot.org/fedora/SPECS/perl-Encode-EUCJPASCII.spec SRPM URL: http://www.bachelot.org/fedora/SRPMS/perl-Encode-EUCJPASCII-0.03-2.fc15.src....
(In reply to comment #1)
TODO: Remove useless BuildRoot definition, it's cleaning in %install section, and whole %clean section. They are not needed anymore.
Needed for EPEL5, so I'd rather keep them.
TODO: Build-require perl(Encode) for tests (EUCJPASCII.pm:6) TODO: Build-require perl(XSLoader) for tests (EUCJPASCII.pm:7) TODO: Build-require perl(base) for tests (EUCJPASCII.pm:18) TODO: Build-require perl(Encode::CJKConstants) for tests (EUCJPASCII.pm:23) TODO: Build-require perl(Encode::JP::JIS7) for tests (EUCJPASCII.pm:23)
Added.
FIX: Build-require perl(File::Spec) (Makefile.PL:18)
Added.
TODO: Remove useless %defattr from %files section.
Needed for EPEL5.
FIX: Remove empty *.bs files. (These empty bootstrap files for DynaLoader are useless.)
Done. The spec is now following more closely the perl spec template, while the previous version was generated with cpanspec.
I've compared the build log from both the older spec and the newer with the BuildRequires added, but I don't see a difference, so I'm a bit puzzled... Are the BRs really needed ?
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=759757
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #3 from Petr Pisar ppisar@redhat.com 2012-01-06 04:19:39 EST --- SPEC file changes:
--- perl-Encode-EUCJPASCII.spec 2011-11-29 11:16:42.000000000 +0100 +++ perl-Encode-EUCJPASCII.spec.1 2012-01-05 17:19:24.000000000 +0100 @@ -1,6 +1,6 @@ Name: perl-Encode-EUCJPASCII Version: 0.03 -Release: 1%{?dist} +Release: 2%{?dist} Summary: EucJP-ascii - An eucJP-open mapping License: GPL+ or Artistic Group: Development/Libraries @@ -10,6 +10,12 @@ BuildRequires: perl(ExtUtils::MakeMaker) BuildRequires: perl(Test::More) BuildRequires: perl(Test::Pod) +BuildRequires: perl(File::Spec) +BuildRequires: perl(Encode) +BuildRequires: perl(XSLoader) +BuildRequires: perl(base) +BuildRequires: perl(Encode::CJKConstants) +BuildRequires: perl(Encode::JP::JIS7) Requires: perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
%{?perl_default_filter} @@ -22,17 +28,16 @@ %setup -q -n Encode-EUCJPASCII-%{version}
%build -%{__perl} Makefile.PL INSTALLDIRS=vendor +%{__perl} Makefile.PL INSTALLDIRS=vendor OPTIMIZE="$RPM_OPT_FLAGS" make %{?_smp_mflags}
%install rm -rf $RPM_BUILD_ROOT - -make pure_install PERL_INSTALL_ROOT=$RPM_BUILD_ROOT - -find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} ; -find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null ; - +make pure_install DESTDIR=$RPM_BUILD_ROOT +find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} ';' +# Remove the next line from noarch packages (unneeded) +find $RPM_BUILD_ROOT -type f -name '*.bs' -a -size 0 -exec rm -f {} ';' +find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null ';' %{_fixperms} $RPM_BUILD_ROOT/*
%check @@ -45,7 +50,12 @@ %defattr(-,root,root,-) %doc Changes README %{perl_vendorarch}/* +%exclude %dir %{perl_vendorarch}/auto/
%changelog +* Thu Jan 05 2012 Xavier Bachelot xavier@bachelot.org 0.03-2 +- Follow the rpmdevtools perl spec template to fix packaging bugs. +- Add missing BuildRequires. + * Tue Nov 29 2011 Xavier Bachelot xavier@bachelot.org 0.03-1 - Initial Fedora release.
TODO: Remove useless BuildRoot definition, it's cleaning in %install section, and whole %clean section. They are not needed anymore. TODO: Remove useless %defattr from %files section.
Needed for EPEL5, so I'd rather keep them.
Ok.
TODO: Build-require perl(Encode) for tests (EUCJPASCII.pm:6) TODO: Build-require perl(XSLoader) for tests (EUCJPASCII.pm:7) TODO: Build-require perl(base) for tests (EUCJPASCII.pm:18) TODO: Build-require perl(Encode::CJKConstants) for tests (EUCJPASCII.pm:23) TODO: Build-require perl(Encode::JP::JIS7) for tests (EUCJPASCII.pm:23)
Added.
FIX: Build-require perl(File::Spec) (Makefile.PL:18)
Added.
+BuildRequires: perl(File::Spec) +BuildRequires: perl(Encode) +BuildRequires: perl(XSLoader) +BuildRequires: perl(base) +BuildRequires: perl(Encode::CJKConstants) +BuildRequires: perl(Encode::JP::JIS7) Ok.
FIX: Remove empty *.bs files. (These empty bootstrap files for DynaLoader are useless.)
Done. The spec is now following more closely the perl spec template, while the previous version was generated with cpanspec.
+find $RPM_BUILD_ROOT -type f -name '*.bs' -a -size 0 -exec rm -f {} ';' Ok.
I've compared the build log from both the older spec and the newer with the BuildRequires added, but I don't see a difference, so I'm a bit puzzled... Are the BRs really needed ?
The Perl modules are used directly by this code, so you need to build-require them directly too. You cannot assume a module keeps in the the same RPM package forever (e.g. the Encode is currently part of perl, but If someone decides to sub-package it into perl-Encode or even dual-live it as packe sourced from CPAN, this will not be true anymore) or a module keeps available through indirect dependency (like File::Spec packaged as perl-PathTools).
$ rpmlint perl-Encode-EUCJPASCII.spec ../SRPMS/perl-Encode-EUCJPASCII-0.03-2.fc17.src.rpm ../RPMS/x86_64/perl-Encode-EUCJPASCII-* perl-Encode-EUCJPASCII.src: W: spelling-error Summary(en_US) eucJP -> eggcup, equip, uncap perl-Encode-EUCJPASCII.src: W: spelling-error %description -l en_US eucJP -> eggcup, equip, uncap perl-Encode-EUCJPASCII.src: W: spelling-error %description -l en_US ascii -> ASCII, ASCIIs, As perl-Encode-EUCJPASCII.x86_64: W: spelling-error Summary(en_US) eucJP -> eggcup, equip, uncap perl-Encode-EUCJPASCII.x86_64: W: spelling-error %description -l en_US eucJP -> eggcup, equip, uncap perl-Encode-EUCJPASCII.x86_64: W: spelling-error %description -l en_US ascii -> ASCII, ASCIIs, As 3 packages and 1 specfiles checked; 0 errors, 6 warnings. rpmlint is Ok.
$ rpm -q -lv -p ../RPMS/x86_64/perl-Encode-EUCJPASCII-0.03-2.fc17.x86_64.rpm drwxr-xr-x 2 root root 0 Jan 6 09:57 /usr/lib64/perl5/vendor_perl/Encode -rw-r--r-- 1 root root 5662 Oct 19 2009 /usr/lib64/perl5/vendor_perl/Encode/EUCJPASCII.pm drwxr-xr-x 2 root root 0 Jan 6 09:57 /usr/lib64/perl5/vendor_perl/auto/Encode drwxr-xr-x 2 root root 0 Jan 6 09:57 /usr/lib64/perl5/vendor_perl/auto/Encode/EUCJPASCII -rwxr-xr-x 1 root root 759824 Jan 6 09:57 /usr/lib64/perl5/vendor_perl/auto/Encode/EUCJPASCII/EUCJPASCII.so drwxr-xr-x 2 root root 0 Jan 6 09:57 /usr/share/doc/perl-Encode-EUCJPASCII-0.03 -rw-r--r-- 1 root root 569 Oct 19 2009 /usr/share/doc/perl-Encode-EUCJPASCII-0.03/Changes -rw-r--r-- 1 root root 496 Oct 19 2009 /usr/share/doc/perl-Encode-EUCJPASCII-0.03/README File permissions and layout is Ok.
Package builds in F17 (http://koji.fedoraproject.org/koji/taskinfo?taskID=3624174). Ok.
Resolution: Package APPROVED.
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=759757
Xavier Bachelot xavier@bachelot.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #4 from Xavier Bachelot xavier@bachelot.org 2012-01-06 04:55:21 EST --- Thanks for the review and explanations.
New Package SCM Request ======================= Package Name: perl-Encode-EUCJPASCII Short Description: EucJP-ascii - An eucJP-open mapping Owners: xavierb Branches: f15 f16 el5 el6 InitialCC: perl-sig
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=759757
--- Comment #5 from Jon Ciesla limburgher@gmail.com 2012-01-06 09:22:55 EST --- Git done (by process-git-requests).
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=759757
Xavier Bachelot xavier@bachelot.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE Last Closed| |2012-01-06 12:21:48
--- Comment #6 from Xavier Bachelot xavier@bachelot.org 2012-01-06 12:21:48 EST --- Imported and built for rawhide, F16, F15, EL6 and EL5.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla