Hide Forgot
Spec URL: http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec SRPM URL: http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-5.fc9.src.rpm Description: Perl lexical analyzer. Required for new documentation-devel packages.
No vendor or packager fields needed: please remove. Is there a web page for the package that could go in the url field? Maybe something like http://search.cpan.org/dist/ParseLex? It would be nice to use caps for the rpm fields: eg "Name", "Version", etc. %NVR is unused. make test should be move to a %check section and then %maketest would not be needed.
(In reply to comment #1) I've updated all of these in the spec file and checked it in. The SRPM hasn't been updated yet/ > No vendor or packager fields needed: please remove. > > Is there a web page for the package that could go in the url field? > Maybe something like http://search.cpan.org/dist/ParseLex? > > It would be nice to use caps for the rpm fields: eg "Name", "Version", etc. > > %NVR is unused. > > make test should be move to a %check section and then %maketest would not be needed.
Please make yourself familiar with the Fedora Packaging Guidelines. http://fedoraproject.org/wiki/Packaging/Guidelines Needswork: - Package is noarch => remove OPTIMIZE="$RPM_OPT_FLAGS" from main make call - Remove the %doc from the mans in %files mans are automatically %doc'ed by rpm - This construct is unnecessarily complex: %dir %{perl_vendorlib}/Parse %{perl_vendorlib}/Parse/*.pm %{perl_vendorlib}/Parse would suffice - The chmod -R u+w %{_builddir}/%{pkgname}-%{version} in %prep is superfluous, remove it. - The construct as being used in %clean is being frowned upon in fedora. Use %clean rm -rf %{buildroot} instead. - Your %buildroot doesn't comply to the packaging guidelines. - The %defines at the beginning of the spec are unused. Remove them.
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-7.fc9.noarch.rpm
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-7.fc9.src.rpm
Some additional comments: This package doesn't build in mock against rawhide; it is missing a build dependency on perl(ExtUtils::MakeMaker). The tarball includes a test suite, but the spec doesn't call it. And unfortunately, when I add a check section to call it, test4 fails. I think the test is has simply been broken by some perl update over the past nine years; the difference in the tests is: < can't analyze: "this is an invalid string with a "" in it"" at examples/ctokenizer.pl line 17, <DATA> line 4. --- > can't analyze: "this is an invalid string with a "" in it"" at examples/ctokenizer.pl line 17, <DATA> chunk 4. So s/chunk/line/ in the expected data from the test and it should pass. rpmlint says the following: perl-ParseLex.noarch: W: file-not-utf8 /usr/share/doc/perl-ParseLex-2.15/Changes This should be passed through iconv; see the example in http://fedoraproject.org/wiki/PackageMaintainers/CommonRpmlintIssues perl-ParseLex.noarch: E: useless-explicit-provides perl(Parse::Token) This is more interesting; rpm extracts the following two dependencies (in addition to several others, of course): perl(Parse::Token) perl(Parse::Token) = 2.15 This comes from the "package Parse::Token;" lines in Token.pm and Token-t.pm. Unfortunately rpm screws up sometimes and these errant dependencies need to be filtered. If you were using cpanspec to generate your specfile then this comes for free with --filter-provides but since you've rolled your own, take a look at the examples at http://fedoraproject.org/wiki/Packaging/Perl.
* Wed Jan 16 2008 Jeff Fearn <jfearn@redhat.com> 2.15-8 - Changed Development/Languages to Development/Libraries - Fixed test - Removed useless-explicit-provides - Converted Changes to utf-8 http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-8.fc9.src.rpm
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-9.fc9.src.rpm http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec Missed that BuildRequires <blush>
I'll do a full review.... Builds OK for me. rpmlint is down to just perl-ParseLex.noarch: E: useless-explicit-provides perl(Parse::Token) I see where you added the filter, but I think you neglected to change something on the sed line. What you need is: cat << \EOF > ParseLex-prov #!/bin/sh %{__perl_provides} $* |\ sed -e '/perl(Parse::Token)$/d' EOF Note the change from "unwanted_provides" to "Parse::Token", but also note that I've anchored the match to the end of the string so the proper versioned provide doesn't get removed as well. I notice that the tarball included in the srpm is not the same as the tarball on CPAN. Do you know what's going on there? Upstream source hasn't changed in nine years but the tarball in the srpm has a modification time of the 15th. If you are going to use macros like %{__make}, you should consistently use macros. Which means %{__rm} in %clean and %{__cat}, %{__sed}, %{__chmod} in %prep. (There's no macro for find for whatever reason.) Or you can just drop the macros. It's up to your personal preference, but you must be consistent. The Summary: is a bit content-free (it's just the name turned around with module appended). Upstream says "Generator of lexical analyzers" which at least says something about what it does. Checklist: X source files don't match upstream. * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written X specfile does not use macros consistently. X summary would use some improvement. * description is OK. * dist tag is present. * build root is OK. * license field matches the actual license. * license is open source-compatible. * license text not included upstream. * latest version is being packaged. * BuildRequires are proper. * %clean is present. * package builds in mock (rawhide, x86_64). * package installs properly X rpmlint has a valid complaint. X still an errant provide: perl(Parse::ALex) = 2.15 perl(Parse::CLex) perl(Parse::Lex) perl(Parse::LexEvent) = 1.00 perl(Parse::Template) = 0.32 X perl(Parse::Token) perl(Parse::Token) = 2.15 perl(Parse::Token::Action) perl(Parse::Token::Delimited) perl(Parse::Token::Nested) perl(Parse::Token::Quoted) perl(Parse::Token::Segmented) perl(Parse::Token::Simple) perl(Parse::Tokenizer) perl(Parse::Trace) perl(Parse::YYLex) = 0.91 perl-ParseLex = 2.15-9.fc9 = perl >= 0:5.000 perl >= 0:5.003 perl >= 0:5.004 perl >= 0:5.005 perl(Carp) perl(Parse::ALex) perl(Parse::Lex) perl(Parse::Template) perl(Parse::Token) >= 2.15 perl(Parse::Trace) perl(constant) perl(integer) perl(strict) perl(vars) * %check is present and all tests pass: All tests successful. Files=7, Tests=7, 1 wallclock secs ( 0.17 cusr + 0.06 csys = 0.23 CPU) * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no scriptlets present. * code, not content. * documentation is small, so no -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package.
(In reply to comment #9) > I'll do a full review.... > > Builds OK for me. rpmlint is down to just > perl-ParseLex.noarch: E: useless-explicit-provides perl(Parse::Token) > I see where you added the filter, but I think you neglected to change something on the sed line. > > What you need is: > cat << \EOF > ParseLex-prov > #!/bin/sh > %{__perl_provides} $* |\ > sed -e '/perl(Parse::Token)$/d' > EOF > Note the change from "unwanted_provides" to "Parse::Token", but also note that I've anchored the match to the end of the string so the proper versioned provide doesn't get removed as well. Fixed. > I notice that the tarball included in the srpm is not the same as the tarball on CPAN. Do you know what's going on there? Upstream source hasn't changed in nine years but the tarball in the srpm has a modification time of the 15th. As per #6 the Changes file was converted to UTF8 and test 4 was tweaked for newer test modules. These two changes + ParseLex-2.15-syntax.patch need to be pushed upstream, time limitations are the only reason they haven't been. > If you are going to use macros like %{__make}, you should consistently use macros. Which means %{__rm} in %clean and %{__cat}, %{__sed}, %{__chmod} in %prep. (There's no macro for find for whatever reason.) Or you can just drop the macros. It's up to your personal preference, but you must be consistent. Consistorised ... if it's not a word it should be :) > The Summary: is a bit content-free (it's just the name turned around with module appended). Upstream says "Generator of lexical analyzers" which at least says something about what it does. Updated. http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-10.fc9.src.rpm
It's not acceptable to modify the upstream tarball. You must take a pristine tarball from upstream, untar it, and make your changes via patching or sed/perl/iconv/whatever in %prep. But the tarball that's in the SRPM must be identical to the tarball I can download from the Source: URL. (There are certain exceptions to this such as when the upstream tarball contains something we can't legally distribute, but this is not one of those cases.) Also, note that upstream hasn't updated this package in nine years; I think the chances are slim that you'll be able to push patches to them. At this point the package looks OK, but I simply cannot approve it with a modified tarball.
* Fri Jan 18 2008 Jeff Fearn <jfearn@redhat.com> 2.16-0 - Become upstream http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.16-0.fc9.src.rpm New Source file: http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/ParseLex-2.16.tar.gz
Isn't that a bit extreme? Have you communicated with upstream about this? I mean, forking the package to avoid one call to iconv and a single patch seems a bit over the top.
It's an easy solution that allows more pressing issue to be attended to. When there is less time pressure I will get these changes upstream. Until then I am the upstream for this package.
That I can't go along with; it's just not the proper way to do things in Fedora. Sorry. Returning this ticket to the review queue.
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex.spec http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-ParseLex-2.15-11.fc9.src.rpm
Someone pinged me to let me know that you had updated this package, so I went ahead and had a look and I'm pleased to say that this package is fine. Thanks for taking the effort to understand how we do things in Fedora. * source files now match upstream: a6201f0522b36733ec237e37ff8328f306b23081ea8473de7456d18831d3e997 ParseLex-2.15.tar.gz * specfile now uses macros consistently. * summary is OK. * rpmlint is silent. * The errant provide is gone. APPROVED Please let me know if you need assistance with the rest of the process.
New Package CVS Request ======================= Package Name: perl-ParseLex Short Description: Perl lexical analyzer Owners: jfearn Branches: F-8 EL-4 EL-5 InitialCC: mdious Cvsextras Commits: yes
cvs admin done
(In reply to comment #19) > cvs admin done $ cvs co perl-ParseLex cvs server: cannot find module `perl-ParseLex' - ignored cvs [checkout aborted]: cannot expand modules $ cvs -f co rpms/perl-ParseLex cvs checkout: warning: new-born rpms/perl-ParseLex has disappeared $ cvs exteminate --target gremlins --force extreeme cvs exterminate: too many gremlins, getting the hell out of here!
I saw it in pkgdb, but I couldn't find it in CVS at all. I went ahead and did an addpackage and it seems to be there now.
yay, it's all working now, thanks. Built OK. http://koji.fedoraproject.org/koji/taskinfo?taskID=371350