Bug 996311
Summary: | Review Request: perl-CAD-Format-STL - Read and Write STL (STereo Lithography) format files | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | John C Peterson <jcp> |
Component: | Package Review | Assignee: | Petr Pisar <ppisar> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | i, notting, package-review, ppisar |
Target Milestone: | --- | Flags: | ppisar:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | perl-CAD-Format-STL-0.2.1-5.fc20 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2013-10-30 01:57:45 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
John C Peterson
2013-08-12 22:52:52 UTC
1. Change 0%{?rhel} to 0%{?rhel} < 6 as many old stuffs are EL5 only, even EL6 you can safely drop them. Remove this: find %{buildroot} -depth -type d -exec rmdir {} 2>/dev/null \; As ExtUtils has solved this "bug" years ago. Move this: %if 0%{?rhel} in %clean: %clean %if 0%{?rhel} rm -rf %{buildroot} %endif to %if 0%{?rhel} %clean rm -rf %{buildroot} %endif %defattr(-,root,root,-) is also EL5 only, EL6 can drop it. Spec URL: http://www.eskimo.com/~jcp/perl-CAD-Format-STL.spec SRPM URL: http://www.eskimo.com/~jcp/perl-CAD-Format-STL-0.2.1-2.fc19.src.rpm Hi Christopher, Thanks for taking time out to review my package. I believe I have addressed all the issues you raised in this new version. Upon closer examination, I think the conditionals need to look like this; %if 0%{?rhel} %if 0%{?rhel} < 6 Do something special for RHEL5 and older... %endif %endif The outer test is needed, because on non-RHEL systems like Fedora, the %{?rhel} macro is not defined at all, so 0 < 6 evaluates to true on any non-RHEL system which is not what was intended. (I verified this with a quick test case). Regards, John True. That's pointed out at the bottom of the "Conditionals" section in the Wiki: https://fedoraproject.org/wiki/Packaging:DistTag#Conditionals You can use logical AND, however: %if 0%{?rhel} && 0%{?rhel} < 6 ... %endif Spec URL: http://www.eskimo.com/~jcp/perl-CAD-Format-STL.spec SRPM URL: http://www.eskimo.com/~jcp/perl-CAD-Format-STL-0.2.1-3.fc19.src.rpm This version incorporates Michael's suggestion to use the logical AND construct. URL is usable. Ok. Source0 is usable. Ok. Source tar ball is original (SHA-256: a8a1e743b51e7e0b82f4575f4474b2d1e608a85286232f45c42bf4661298dc5b). Ok. Summary verified from lib/CAD/Format/STL.pm. Ok. Description verified from lib/CAD/Format/STL.pm. Ok. License verified from lib/CAD/Format/STL/part.pm and lib/CAD/Format/STL.pm. Ok. No XS code, noarch BuildArch is Ok. TODO: You can replace the `%{__perl}' macro with plain `perl'. TODO: You can remove the deffattr macro completely, because even EPEL-5 has rpm-4.4 where the macro is redundant. TODO: META.yml declares minimal Module::Build version 0.35. Make sure your package builds in EPEL-5 or add this constrain to the corresponding BuildRequire tag. FIX: Remove `v' from `perl(Class::Accessor::Classy) >= v0.1.3' dependency declarations. See what perl-Class-Accessor-Classy package provides and how RPM compares the versions (rpmdev-vercmp v0.1.3 0). TODO: Build-require `perl(bytes)' (lib/CAD/Format/STL.pm:421). version module is optional for tests. Ok. All tests pass. Ok. $ rpmlint perl-CAD-Format-STL.spec ../SRPMS/perl-CAD-Format-STL-0.2.1-3.fc21.src.rpm ../RPMS/noarch/perl-CAD-Format-STL-0.2.1-3.fc21.noarch.rpm 2 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint is Ok. $ rpm -q -lv -p ../RPMS/noarch/perl-CAD-Format-STL-0.2.1-3.fc21.noarch.rpm drwxr-xr-x 2 root root 0 Oct 16 10:35 /usr/share/doc/perl-CAD-Format-STL -rw-r--r-- 1 root root 267 Apr 5 2010 /usr/share/doc/perl-CAD-Format-STL/Changes -rw-r--r-- 1 root root 286 Apr 5 2010 /usr/share/doc/perl-CAD-Format-STL/README drwxr-xr-x 2 root root 0 Apr 5 2010 /usr/share/doc/perl-CAD-Format-STL/files -rw-r--r-- 1 root root 1417 Apr 5 2010 /usr/share/doc/perl-CAD-Format-STL/files/cube.stl -rw-r--r-- 1 root root 684 Apr 5 2010 /usr/share/doc/perl-CAD-Format-STL/files/cube_binary.stl -rw-r--r-- 1 root root 3342 Oct 16 10:35 /usr/share/man/man3/CAD::Format::STL.3pm.gz -rw-r--r-- 1 root root 2544 Oct 16 10:35 /usr/share/man/man3/CAD::Format::STL::part.3pm.gz drwxr-xr-x 2 root root 0 Oct 16 10:35 /usr/share/perl5/vendor_perl/CAD drwxr-xr-x 2 root root 0 Oct 16 10:35 /usr/share/perl5/vendor_perl/CAD/Format drwxr-xr-x 2 root root 0 Oct 16 10:35 /usr/share/perl5/vendor_perl/CAD/Format/STL -rw-r--r-- 1 root root 11740 Oct 16 10:35 /usr/share/perl5/vendor_perl/CAD/Format/STL.pm -rw-r--r-- 1 root root 2297 Oct 16 10:35 /usr/share/perl5/vendor_perl/CAD/Format/STL/part.pm File location and permissions are Ok. $ rpm -q --requires -p ../RPMS/noarch/perl-CAD-Format-STL-0.2.1-3.fc21.noarch.rpm | sort -i | uniq -c 1 perl(:MODULE_COMPAT_5.18.1) 1 perl(CAD::Format::STL::part) 1 perl(Carp) 1 perl(Class::Accessor::Classy) 1 perl(Class::Accessor::Classy) >= v0.1.3 1 perl(strict) 1 perl(warnings) 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 FIX: Remove `v' from `perl(Class::Accessor::Classy) >= v0.1.3' dependency declarations. TODO: Filter the under-specified dependency `perl(Class::Accessor::Classy)'. See <https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering>. TODO: Run-require `perl(bytes)' (lib/CAD/Format/STL.pm:421). $ rpm -q --provides -p ../RPMS/noarch/perl-CAD-Format-STL-0.2.1-3.fc21.noarch.rpm | sort -i | uniq -c 1 perl(CAD::Format::STL) = 0.2.1 1 perl(CAD::Format::STL::part) = 0.2.1 1 perl-CAD-Format-STL = 0.2.1-3.fc21 Binary provides are Ok. $ resolvedeps rawhide ../RPMS/noarch/perl-CAD-Format-STL-0.2.1-3.fc21.noarch.rpm Binary dependencies resolvable. Ok. Package builds in F21 (http://koji.fedoraproject.org/koji/taskinfo?taskID=6064424). Ok. Otherwise the package is in line with Fedora and Perl packaging guidelines. Please correct all `FIX' items, consider fixing `TODO' items, and provide new SPEC file. Resolution: Package NOT approved. Spec URL: http://www.eskimo.com/~jcp/perl-CAD-Format-STL.spec SRPM URL: http://www.eskimo.com/~jcp/perl-CAD-Format-STL-0.2.1-4.fc19.src.rpm Hi Petr, Thank you for taking time out to review my package. This version should correct all of the issues you raised (all of the FIX and TODO items as well). I modified the BuildRequires for perl(Module::Build) to require >= 0.35. I did not check to see if it builds under EPEL-5 as I was planning to support EPEL-6 only for now (and EPEL-7, after RHEL-7 is released). Good eye on the "use bytes" which I missed! Regards, John Spec file changes: --- perl-CAD-Format-STL.spec.old 2013-08-13 20:53:41.000000000 +0200 +++ perl-CAD-Format-STL.spec 2013-10-17 02:08:27.000000000 +0200 @@ -1,29 +1,32 @@ # Declare the CPAN name of the module %define mod_basename CAD-Format-STL +# RPM's auto require for perl(Class::Accessor::Classy) fails, handle manually +%define __requires_exclude ^perl\\(Class::Accessor::Classy\\)$ Name: perl-%{mod_basename} Version: 0.2.1 -Release: 3%{?dist} -Summary: Read and Write STL (STereoLithography) format files +Release: 4%{?dist} +Summary: Read and Write STL (STereo Lithography) format files License: GPL+ or Artistic Group: Development/Libraries URL: http://search.cpan.org/dist/%{mod_basename}/ Source: http://www.cpan.org/modules/by-module/CAD/%{mod_basename}-v%{version}.tar.gz BuildArch: noarch -BuildRequires: perl BuildRequires: perl(strict) BuildRequires: perl(warnings) BuildRequires: perl(Carp) -BuildRequires: perl(Class::Accessor::Classy) >= v0.1.3 -BuildRequires: perl(Module::Build) +BuildRequires: perl(Class::Accessor::Classy) >= 0.1.3 +BuildRequires: perl(Module::Build) >= 0.35 # These are needed for "Build test" +BuildRequires: perl(bytes) BuildRequires: perl(Test::More) +Requires: perl(bytes) Requires: perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version)) -Requires: perl(Class::Accessor::Classy) >= v0.1.3 +Requires: perl(Class::Accessor::Classy) >= 0.1.3 %description The CAD::Format::STL perl module provides object-oriented methods to read -and write files in STL (STereoLithography) format. Support is provided +and write files in STL (STereo Lithography) format. Support is provided for both the ASCII and binary versions of the STL format. %prep @@ -31,7 +34,7 @@ %build # Using Module::Build since a Build.PL is present -%{__perl} Build.PL installdirs=vendor +perl Build.PL installdirs=vendor ./Build %install @@ -50,14 +53,19 @@ %endif %files -%if 0%{?rhel} && 0%{?rhel} < 6 -%defattr(-,root,root,-) -%endif %doc files Changes README %{perl_vendorlib}/* %{_mandir}/man3/* %changelog +* Wed Oct 16 2013 John C. Peterson <jcp> 0.2.1-4 +- Various fixes that were required or suggested by the package reviewer: + Added the package's version number build requirement for perl(Module::Build) + Added filter rule for rpm's auto require of perl(Class::Accessor::Classy) + Added perl(bytes) to both the build and runtime requirements + Removed the redundant defattr macro from the files section + Replaced the __perl macro with just perl in the build section + * Tue Aug 13 2013 John C. Peterson <jcp> 0.2.1-3 - Some cosmetic tweaks to the macros that check for the specific case of RHEL. > TODO: You can replace the `%{__perl}' macro with plain `perl'. TODO: You can do the same at perl(:MODULE_COMPAT_*) definition. > TODO: You can remove the deffattr macro completely, because even EPEL-5 has > rpm-4.4 where the macro is redundant. -%if 0%{?rhel} && 0%{?rhel} < 6 -%defattr(-,root,root,-) -%endif Ok. > TODO: META.yml declares minimal Module::Build version 0.35. Make sure your > package builds in EPEL-5 or add this constrain to the corresponding > BuildRequire tag. -BuildRequires: perl(Module::Build) +BuildRequires: perl(Module::Build) >= 0.35 Ok. > FIX: Remove `v' from `perl(Class::Accessor::Classy) >= v0.1.3' dependency > declarations. See what perl-Class-Accessor-Classy package provides and how > RPM compares the versions (rpmdev-vercmp v0.1.3 0). -BuildRequires: perl(Class::Accessor::Classy) >= v0.1.3 +BuildRequires: perl(Class::Accessor::Classy) >= 0.1.3 Ok. > TODO: Build-require `perl(bytes)' (lib/CAD/Format/STL.pm:421). +BuildRequires: perl(bytes) Ok. > FIX: Remove `v' from `perl(Class::Accessor::Classy) >= v0.1.3' dependency > declarations. -Requires: perl(Class::Accessor::Classy) >= v0.1.3 +Requires: perl(Class::Accessor::Classy) >= 0.1.3 Ok. > TODO: Filter the under-specified dependency `perl(Class::Accessor::Classy)'. +# RPM's auto require for perl(Class::Accessor::Classy) fails, handle manually +%define __requires_exclude ^perl\\(Class::Accessor::Classy\\)$ Ok. TODO: There is a convention to place the macro just before %description section. > TODO: Run-require `perl(bytes)' (lib/CAD/Format/STL.pm:421). +Requires: perl(bytes) Ok. $ rpmlint perl-CAD-Format-STL.spec ../SRPMS/perl-CAD-Format-STL-0.2.1-4.fc21.src.rpm ../RPMS/noarch/perl-CAD-Format-STL-0.2.1-4.fc21.noarch.rpm 2 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint is OPk. $ rpm -q --requires -p ../RPMS/noarch/perl-CAD-Format-STL-0.2.1-4.fc21.noarch.rpm | sort -i | uniq -c 1 perl(:MODULE_COMPAT_5.18.1) 1 perl(CAD::Format::STL::part) 1 perl(Carp) 1 perl(Class::Accessor::Classy) >= 0.1.3 1 perl(bytes) 1 perl(strict) 1 perl(warnings) 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 Binary requires are Ok. $ resolvedeps rawhide ../RPMS/noarch/perl-CAD-Format-STL-0.2.1-4.fc21.noarch.rpm Binary dependencies resolvable. Ok. Package builds in F21 (http://koji.fedoraproject.org/koji/taskinfo?taskID=6069570). Ok. Package is good. Please consider fixing the `TODO' item before building this package. Resolution: Package APPROVED. New Package SCM Request ======================= Package Name: perl-CAD-Format-STL Short Description: Read and Write STL (STereo Lithography) format files Owners: jcp Branches: f18 f19 f20 el6 InitialCC: perl-sig Git done (by process-git-requests). perl-CAD-Format-STL-0.2.1-5.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/perl-CAD-Format-STL-0.2.1-5.fc18 perl-CAD-Format-STL-0.2.1-5.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/perl-CAD-Format-STL-0.2.1-5.fc20 perl-CAD-Format-STL-0.2.1-5.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/perl-CAD-Format-STL-0.2.1-5.el6 perl-CAD-Format-STL-0.2.1-5.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/perl-CAD-Format-STL-0.2.1-5.fc19 perl-CAD-Format-STL-0.2.1-5.el6 has been pushed to the Fedora EPEL 6 testing repository. perl-CAD-Format-STL-0.2.1-5.fc18 has been pushed to the Fedora 18 stable repository. perl-CAD-Format-STL-0.2.1-5.fc19 has been pushed to the Fedora 19 stable repository. perl-CAD-Format-STL-0.2.1-5.el6 has been pushed to the Fedora EPEL 6 stable repository. perl-CAD-Format-STL-0.2.1-5.fc20 has been pushed to the Fedora 20 stable repository. |