[Bug 1234791] Review Request: perl-Inline-Python - Write Perl subs and classes in Python

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1234791



--- Comment #5 from Petr Šabata <psabata@xxxxxxxxxx> ---
First of all, familiarise yourself with Fedora Packaging guidelines:
https://fedoraproject.org/wiki/Packaging:Guidelines
There's quite a lot to learn, I know.  Feel free to ask if you need anything.


Okay, so, the review...


#1  Since this is only going to el6+, you can drop the BuildRoot tag, buildroot
cleaning in %install and the whole %clean section.  These aren't really needed
nowadays.
https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag


#2  There's no need to explitly declare %defattr anymore.  You may drop that
too.
https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions


#3  There used to be a list of packages guaranteed to be present in every
buildroot.  This is no longer the case and every package must declare all its
direct build dependencies explicitly.  To ensure your package builds in the
future too, you need to add quite a few to your list of build requirements
(BuildRequires):
 - coreutils -- `rm' is called in the spec
 - findutils -- `find' is called in the spec
 - make -- called in the spec
 - perl -- ditto
 - perl(base) -- t/25py_sub.t:70
 - perl(Carp) -- Python.pm:3
 - perl(Config) -- Makefile.PL:2, t/11factor.t:3
 - perl(Cwd) -- Makefile.PL:3, Makefile.PL:307
 - perl(DynaLoader) -- Python.pm:5
 - perl(Exporter) -- Python.pm:6
 - perl(File::Path) -- t/00init.t:5
 - perl(Getopt::Long) -- Makefile.PL:5
 - perl(Inline::denter) -- Python.pm:184, Python.pm:215
 - perl(Inline::Filters) -- Python.pm:69
 - perl(overload) -- Python.pm:313, Python.pm:370, Python.pm:378
 - perl(Parse::RecDescent) -- t/03parse.t:3
 - perl(POSIX) -- t/30floats.t:6
 - perl(strict) -- all over the place
 - perl(Test) -- most tests
 - perl(Test::Simple) -- t/19testref.t:1, t/26undef.t:1
 - perl(utf8) -- t/20unicode.t:4
 - perl(warnings) -- many tests

Inline::Filters appears to be optional.
Parse::RecDescent is optional.
(However, it's good practice to run as many tests as possible so unless it's
causing some problems, for example dependency cycles, it's a good idea to
require optional dependencies as well.)

One has to go through the sources to figure these out.  There are tools that
can help you with it, e.g. `tangerine' for perl code.

Next, since you want to build against python 2.5+, you need to add a version
constraint to the python-devel build dependency:
BuildRequires:  python-devel >= 2.5


#4  You don't actually need to buildrequire perl(Digest::MD5).  It's not used
anywhere in the code.  Upstream metadata is incorrect.


#5  Next, runtime dependencies.  After the package is built, rpmbuild scans the
resulting files and attempts to generate various `requires' and `provides' for
you.  This works for perl too, to some extent.  The majority of your
dependencies will be found automatically and you don't have to worry about
declaring them explicitly with `Requires'.  It doesn't find everything, though.
 You'll have to add the following to your list of runtime dependencies
(`Requires'):
 - perl(Inline::denter)
 - perl(Inline::Filters) -- possibly; this one is optional, up to you

You may drop the following from your list:
 - perl(Data::Dumper) -- not used at runtime
 - perl(Digest::MD5) -- not needed at all
 - perl(Test::More) -- not used at runtime
 - python -- rpmbuild generates a library dependency from ldd for you, so the
package will always require the library against which it was built

The generators also add an unversioned dependency on perl(Inline).  You
explicitly require `perl(Inline) >= 0.46', which is correct.  To remove the
underspecified generated dependency, you need to add a filter like this:

%global __requires_exclude
%{?__requires_exclude:%__requires_exclude|}^perl\\(Inline\\)$

The most common place to put this is between the dep list and the %description.


#6  The following line isn't needed, feel free to drop it:
find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null \;


#7  Consider using DESTDIR instead of PERL_INSTALL_ROOT.


#8  Your changelog format isn't valid (missing e-mail address).
https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs


#9  Drop META.json and TESTED from %doc.  These files aren't really useful to
the end users.


I hope it's all clear.  If not, let me know :)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]