Bug 875406

In cpanspec's auto-generated %description:

"As a jumping off point, refer to:"

... and it ends there. You should probably that last line from the %description.

Will you be building this for EPEL 5?

I mean to say probably *delete* that last line from the %description :)

Well spotted on the description. Ther's an updated spec/srpm at:

http://packages.zonalivre.org/SRPMS/perl-Math-Combinatorics-0.09-1.fc16.src.rpm
http://packages.zonalivre.org/SPECS/perl-Math-Combinatorics.spec

I'm quite keen on building this for EPEL too, but need to figure out how to go about it.  This looks like it:
https://fedoraproject.org/wiki/EPEL_Package_Maintainers

Good, thanks. Here are a couple more issues:

Please increment the release value for each new version that you post to this bug. This will help to clarify to potential reviewers that you are changing the package.

The Math::Combinatorics software is licensed under "the same terms as Perl itself", so the License field in the spec should be "GPL+ or Artistic".

There are a couple older bits that you can remove if you will not be building for EPEL 5, and you'll only be building for EPEL 6. Can you clarify your intention?

Lastly I see the software loads Data::Dumper, but the upstream author has commented out any code that uses Data::Dumper. This looks like a bug in the software that will lead to dependency bloat, so you should file a bug upstream and ask them to comment out the "use Data::Dumper" line. This will prevent RPM from automatically picking it up as a dependency for your package.

Hi Ken, thanks for the thorough review.


I'd like to build for EPEL but for the time being don't really understand what's involved and have some reading up to do first which is mostly limited to weekends.  For the purpose of this discussion, assume I won't be building for EPEL.

If you can give me some pointers about which bits are EPEL specific that would be great information.

I take your point about a module being loaded but not used causing dependency creep.  In this particular case, Data::Dumper is part of the core perl package, so there's no additional dependency that gets added in this instance.  It does show that you are thorough in your review, and that I appreciate, as it leads to better quality.  Eg,
------------------------------------------------------------------------
yum provides "perl(Data::Dumper)"
Loaded plugins: langpacks, presto, refresh-packagekit
4:perl-5.14.1-188.fc16.x86_64 : Practical Extraction and Report Language
Repo        : fedora
Matched from:
Provides    : perl(Data::Dumper) = 2.130
------------------------------------------------------------------------


Finally, your point about updating the release number is valid, if that's ok with you i'll do it going forward when I need to change the .spec file ?

You're right about Data::Dumper being in core. I was mistakenly confusing it with other Perl modules that have been split out. However I can't picture the Perl core package maintainers splitting Data::Dumper out any time soon, so you're fine there and it won't block my review.

If you can fix the License tag issue, then your package is basically set for review. Just post a new version of your spec and I'll do the formal review.

Before I sponsor you, I'd like to see that you understand the packaging guidelines enough to review other people's packages. Have you done any other package reviews? You can just comment on any of the other package review bugs.

https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

Regarding EPEL: EPEL is a cool sub-project, and I encourage you to look into it. It is really no more work to maintain an EL branch than to maintain Fedora branches.

That said, EPEL 5 and EPEL 6 are a bit different. RHEL 5 contains a very old version of RPM, so you have to include some extra bits of code in your spec files if you want them to build on RHEL 5. The cpanspec tool already includes these bits of code:

  In %install, the "rm -rf $RPM_BUILD_ROOT" line.

  In %clean, the "rm -rf $RPM_BUILD_ROOT" line.

  In %files, the "%defattr(-,root,root,-)" line.

For EPEL 6 and all Fedora releases, you do not need to include these bits of code in your spec file. So, if you don't care about EPEL 5, you can remove them. That's why I bring it up.

Please find the updated package here, this addresses the license issue you identified:

- http://packages.zonalivre.org/SPECS/perl-Math-Combinatorics.spec
- http://packages.zonalivre.org/RPMS/noarch/perl-Math-Combinatorics-0.09-2.fc16.noarch.rpm


I am still to review someone else's package, so please hold off the sponsorship request.

(In reply to comment #6)
> You're right about Data::Dumper being in core. I was mistakenly confusing it
> with other Perl modules that have been split out. However I can't picture
> the Perl core package maintainers splitting Data::Dumper out any time soon,
> so you're fine there and it won't block my review.

Actually, Data::Dumper dual-lives since April 2012, see bug #811239.

You can never know what module is going to be split out.
Just list every dependency your code actually has instead of blind assumptions.  Not listing modules only present in the perl package is acceptable (check rawhide for that, not stable; they often differ a lot) but you should watch perl package development closely then.  Or be ready to deal with possible later, undesired FTBFS bugs.

Anyhow, any updates here?

Hi João,

Are you interested in continuing this package review? To summarize the remaining issues:

1. Remove old EL5 compatibility bits if you don't want to ship your package for EL5.

2. Remove "use Data::Dumper;" from Combinatorics.pm (you can just use "sed -i" during %prep to remove that line).

3. Do a couple informal package reviews to show that you're familiar with the packaging guidelines (and post the links here).

Closing for inactivity.