[Bug 996311] New: Review Request: perl-CAD-Format-STL - Read and Write STL (STereoLithography) format files
https://bugzilla.redhat.com/show_bug.cgi?id=996311
Bug ID: 996311 Summary: Review Request: perl-CAD-Format-STL - Read and Write STL (STereoLithography) format files Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: jcp@eskimo.com QA Contact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org
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-1.fc19.src.rpm
Description: The CAD::Format::STL perl module provides object-oriented methods to read and write files in STL (STereoLithography) format. Support is provided for both the ASCII and binary versions of the STL format.
Notes: This software is licensed under the same terms as Perl itself (GPL+ or Artistic), so there should not be any issues there. One of the build pre-requisites is perl-Class-Accessor-Classy, which should now be available from F18/19 updates. This package is a candidate for the 3D Printing Feature.
Fedora Account System Username: jcp
https://bugzilla.redhat.com/show_bug.cgi?id=996311
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cickumqt@gmail.com
--- Comment #1 from Christopher Meng cickumqt@gmail.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=996311
--- Comment #2 from John C Peterson jcp@eskimo.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=996311
--- Comment #3 from Michael Schwendt bugs.michael@gmx.net --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=996311
--- Comment #4 from John C Peterson jcp@eskimo.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=996311
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |ppisar@redhat.com Assignee|nobody@fedoraproject.org |ppisar@redhat.com Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=996311
--- Comment #5 from Petr Pisar ppisar@redhat.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=996311
--- Comment #6 from John C Peterson jcp@eskimo.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=996311
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #7 from Petr Pisar ppisar@redhat.com --- 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/%%7Bmod_basename%7D/ Source: http://www.cpan.org/modules/by-module/CAD/%%7Bmod_basename%7D-v%%7Bversion%7... 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@eskimo.com 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@eskimo.com 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.
https://bugzilla.redhat.com/show_bug.cgi?id=996311
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: |Review Request: |perl-CAD-Format-STL - Read |perl-CAD-Format-STL - Read |and Write STL |and Write STL (STereo |(STereoLithography) format |Lithography) format files |files |
https://bugzilla.redhat.com/show_bug.cgi?id=996311
John C Peterson jcp@eskimo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #8 from John C Peterson jcp@eskimo.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=996311
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? |fedora-cvs+
https://bugzilla.redhat.com/show_bug.cgi?id=996311
--- Comment #9 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=996311
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=996311
--- Comment #10 from Fedora Update System updates@fedoraproject.org --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=996311
--- Comment #11 from Fedora Update System updates@fedoraproject.org --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=996311
--- Comment #12 from Fedora Update System updates@fedoraproject.org --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=996311
--- Comment #13 from Fedora Update System updates@fedoraproject.org --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=996311
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #14 from Fedora Update System updates@fedoraproject.org --- perl-CAD-Format-STL-0.2.1-5.el6 has been pushed to the Fedora EPEL 6 testing repository.
https://bugzilla.redhat.com/show_bug.cgi?id=996311
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |perl-CAD-Format-STL-0.2.1-5 | |.fc18 Resolution|--- |ERRATA Last Closed| |2013-10-29 21:57:45
--- Comment #15 from Fedora Update System updates@fedoraproject.org --- perl-CAD-Format-STL-0.2.1-5.fc18 has been pushed to the Fedora 18 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=996311
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|perl-CAD-Format-STL-0.2.1-5 |perl-CAD-Format-STL-0.2.1-5 |.fc18 |.fc19
--- Comment #16 from Fedora Update System updates@fedoraproject.org --- perl-CAD-Format-STL-0.2.1-5.fc19 has been pushed to the Fedora 19 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=996311
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|perl-CAD-Format-STL-0.2.1-5 |perl-CAD-Format-STL-0.2.1-5 |.fc19 |.el6
--- Comment #17 from Fedora Update System updates@fedoraproject.org --- perl-CAD-Format-STL-0.2.1-5.el6 has been pushed to the Fedora EPEL 6 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=996311
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|perl-CAD-Format-STL-0.2.1-5 |perl-CAD-Format-STL-0.2.1-5 |.el6 |.fc20
--- Comment #18 from Fedora Update System updates@fedoraproject.org --- perl-CAD-Format-STL-0.2.1-5.fc20 has been pushed to the Fedora 20 stable repository.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla