Note that this is my first package submission. Spec URL: http://wfp.fedorapeople.org/perl-IO-InSitu.spec SRPM URL:http://wfp.fedorapeople.org/perl-IO-InSitu-0.0.2-1.fc13.src.rpm Description: This module provides a function called open_rw(), that is passed two file names and returns two handles, one open for reading and the other for writing. It's like doing to separate open() calls, except that it detects cases where the input and output file are the same, and avoids clobbering the input file when reopening it for output.
I'll look at this for you. Initial quick comments: * Spelling error in %description: "to separate open() calls" should be "two separate open() calls" * Manual runtime dependency on perl(Test::More) is not needed; there's no reference to Test::More in the resulting package and I guess the reference to it in META.yml is probably an error * Manual runtime dependency on perl(version) is redundant as rpm's automatic dependency generator adds it anyway * A manual runtime dependency of perl(IO::File) could be added as the module requires this via a "use base qw( IO::File )" construct * As a matter of style, I personally prefer: %{perl_vendorlib}/IO/ %{_mandir}/man3/IO::InSitu.3pm* to: %{perl_vendorlib}/* %{_mandir}/man3/* as this makes it easier to tell what's included in the package from reading the spec file, and will help you spot if other modules are added to the package in a later upstream release, which may be worthy of a changelog mention.. However, this is not a blocker if you prefer the wildcard style. * The package provides perl(IO::File::SE) as there is a package of this name included in the InSitu.pm file; I think this provide needs filtering as the package is not in a file where perl would look for it if some other package needed it (there should be a separate perl-IO-File-SE package if that became necessary). You can find information on filtering provides at: https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering Regarding sponsorship, you need to demonstrate your understanding and application of the Fedora Packaging Guidelines. The usual way to do this is to point potential sponsors to mock reviews of other packages in the review queue that you've done, and/or other packages you've submitted for review. It's a little hard to tell from this submission as cpanspec produces pretty good specs automatically. Do you have any plans to submit further packages for review, and if so, would they be just perl modules or other things as well?
I've updated my rpm to match your suggestions. The updated files are http://wfp.fedorapeople.org/perl-IO-InSitu.spec and http://wfp.fedorapeople.org/perl-IO-InSitu-0.0.2-2.fc14.src.rpm As for my future plans, it's a matter of finding packages that need packaging. I've been maintaining local rpms for years, most are not perl. They're typically things Fedora has orphaned or things not suitable for Fedora. Neither are good candidates for getting approved as a packager.
Whilst the runtime dependency on perl(version) was redundant, the removed buildrequire of perl(version) wasn't, and the package now fails to build in mock. Executing(%check): /bin/sh -e /var/tmp/rpm-tmp.StvRIX + umask 022 + cd /builddir/build/BUILD + cd IO-InSitu-0.0.2 + unset DISPLAY + ./Build test # Failed test 'use IO::InSitu;' # at t/00.load.t line 4. # Tried to use 'IO::InSitu'. # Error: Can't locate version.pm in @INC (@INC contains: /builddir/build/BUILD/IO-InSitu-0.0.2/blib/lib /builddir/build/BUILD/IO-InSitu-0.0.2/blib/arch /usr/local/lib64/perl5 /usr/local/share/perl5 /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5 .) at /builddir/build/BUILD/IO-InSitu-0.0.2/blib/lib/IO/InSitu.pm line 3. # BEGIN failed--compilation aborted at /builddir/build/BUILD/IO-InSitu-0.0.2/blib/lib/IO/InSitu.pm line 3. # Compilation failed in require at (eval 4) line 2. # BEGIN failed--compilation aborted at (eval 4) line 2. Use of uninitialized value $IO::InSitu::VERSION in concatenation (.) or string at t/00.load.t line 7. # Testing IO::InSitu # Looks like you failed 1 test of 1. t/00.load.t ....... Dubious, test returned 1 (wstat 256, 0x100) Failed 1/1 subtests If you've no further packages to submit, your best route to sponsorship would be to do a few pre-reviews of other packages in the review queue: http://fedoraproject.org/PackageReviewStatus/NEW.html If you do any, make a note of them here for a potential sponsor (such as me) to see.
Serves me right for rushing and not actually testing it on mock myself. Sorry to waste your time. http://wfp.fedorapeople.org/perl-IO-InSitu.spec and http://wfp.fedorapeople.org/perl-IO-InSitu-0.0.2-3.fc14.src.rpm are updated to fix the build issue.
That's better. Just one minor quibble now, in the rpmlint output for the src.rpm: perl-IO-InSitu.src:17: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 17) Just expand the tab on line 17 and all will be well. I'll try to do a full review tomorrow.
-------------------------------------------------------------------------------------- Review checklist: -------------------------------------------------------------------------------------- - rpmlint output mostly OK (see below) - package and spec file naming OK - package meets guidelines - license as same as Perl, spec matches actual license - no bundled license file to include - spec file written in English and is legible - sources match upstream (content and timestamp identical) - package builds OK in mock for Rawhide - build requirements OK - no locale data to worry about - no shared libraries to worry about - no bundled libraries to worry about - package makes no attempt to be relocatable (which is good) - directory ownership is fine - no duplicate files - macro usage is consistent - package is code, not content - no large docs to concern ourselves with - docs don't affect runtime - no header files to worry about - no static or other libraries to worry about - no subpackages created or needed - no libtool archives present - package is not a GUI app so does not need a desktop file - no non-ascii filenames - supplied test suite run in %check and passes in Rawhide - no scriptlets present or needed - no pkgconfig files to worry about -------------------------------------------------------------------------------------- rpmlint output: -------------------------------------------------------------------------------------- perl-IO-InSitu.noarch: W: spelling-error %description -l en_US rw -> re, r, w perl-IO-InSitu.src: W: spelling-error %description -l en_US rw -> re, r, w perl-IO-InSitu.src:17: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 17) 2 packages and 0 specfiles checked; 0 errors, 3 warnings. Spelling "errors" are to be expected. Mixed use of spaces and tabs can be trivially fixed by expanding the one tab in the spec. -------------------------------------------------------------------------------------- requires: -------------------------------------------------------------------------------------- perl(:MODULE_COMPAT_5.12.2) perl(IO::File) rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(CompressedFileNames) <= 3.0.4-1 perl(base) perl(Carp) perl(File::Copy) perl(File::Temp) perl(strict) perl(version) perl(warnings) rpmlib(PayloadIsXz) <= 5.2-1 These are sane. The perl(base) requirement comes from "use base qw( IO::File )"; the manual perl(IO::File) dependency covers this. -------------------------------------------------------------------------------------- provides: -------------------------------------------------------------------------------------- perl(IO::InSitu) perl-IO-InSitu = 0.0.2-3.fc15 These are also reasonable but it would be better if the perl(IO::InSitu) provide was versioned. The reason that it isn't is that the current auto-provides script can't extract the version from this code: package IO::InSitu; use version; $VERSION = qv('0.0.2'); You'd have to write a custom provides script to fix that (I did one for perl-Mail-Mbox-MessageParser), but I wouldn't say that was a blocker. RPM itself may grow a better perl provides/requires checker once rpm 4.9 lands. -------------------------------------------------------------------------------------- Summary -------------------------------------------------------------------------------------- This package is good to go (assuming the tab in the spec is expanded). Now I just need convincing that you understand the Fedora Packaging Guidelines, and a small number of package pre-reviews should do the trick.
http://wfp.fedorapeople.org/perl-IO-InSitu-0.0.2-4.fc14.src.rpm is updated to fix rpmlint warning about spaces and tabs. Thank you so much for your tutelage.
OK, time to take another look at this. With rpm 4.9, the automatic dependency finder detects the "use base" construct and so you no longer need the explicit perl(IO::File) dependency for current Fedora versions. For all EPEL releases, which all have older versions of rpm, the IO::File module is bundled with the main perl package so it's not really needed there either. So I'd now suggest getting rid of that requirement. Looking at build requirements, the module uses the following dual-lived modules that are not explicitly build-required: * perl(base) * perl(Carp) * perl(File::Temp) * perl(IO::File) I'd add build-requires for those given that they may be dual-lived in Fedora either now or in the future. It's not actually necessary to remove empty directories from the buildroot as rpm-build will not include them in the package unless they're included in the %files list, and it won't complain about them either. rpm 4.9 now has a native method for filtering unwanted requires and provides. The method used in this spec file will work for current Fedora releases and EPEL 6 but not EPEL 5. If you want filters that work everywhere, you'll currently need to use two sets of filters - for example as I've done in the perl-SUPER package: http://pkgs.fedoraproject.org/gitweb/?p=perl-SUPER.git;a=blob;f=perl-SUPER.spec That's using rpm 4.9's native filtering and the old %__perl_provides macro as a fallback for older rpm versions. Using macros for commands (e.g. %{__perl}, %{__id_u} -n) is now discouraged: http://fedoraproject.org/wiki/Packaging:Guidelines#Macros I'd just use "perl" and "id -nu" in these cases now.
Here's an update addressing the most recent comments. Spec URL: http://wfp.fedorapeople.org/perl-IO-InSitu.spec SRPM URL:http://wfp.fedorapeople.org/perl-IO-InSitu-0.0.2-5.fc16.src.rpm
OK, last few nits: %defattr is redundant in all currently supported releases, including EPEL-5. You missed a %{__perl} in the :MODULE_COMPAT dependency. Given that there's only one "provide" needing to be filtered, the "traditional" filter is over-escaped and you could simplify the grep to: grep -Fvx 'perl(IO::File::SE)' None of those are blockers though. APPROVED.
New Package SCM Request ======================= Package Name: perl-IO-InSitu Short Description: Avoid clobbering files opened for both input and output Owners: wfp Branches: f16 f17 InitialCC: perl-sig
Git done (by process-git-requests).
perl-IO-InSitu-0.0.2-6.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/perl-IO-InSitu-0.0.2-6.fc16
perl-IO-InSitu-0.0.2-6.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/perl-IO-InSitu-0.0.2-6.fc17
perl-IO-InSitu-0.0.2-6.fc16 has been pushed to the Fedora 16 testing repository.
perl-IO-InSitu-0.0.2-6.fc16 has been pushed to the Fedora 16 stable repository.
perl-IO-InSitu-0.0.2-6.fc17 has been pushed to the Fedora 17 stable repository.