Hide Forgot
Spec URL: http://wfp.fedorapeople.org/perl-XML-DTDParser.spec SRPM URL: http://wfp.fedorapeople.org/perl-XML-DTDParser-2.01-1.fc16.src.rpm Description: A simple DTD parser that may be useful in conjunction with XML::Rules.
First pass: Package fails to build in mock: Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.YlZXgq + umask 022 + cd /builddir/build/BUILD + LANG=C + export LANG + unset DISPLAY + cd /builddir/build/BUILD + rm -rf XML-DTDParser-2.01 + /usr/bin/gzip -dc /builddir/build/SOURCES/XML-DTDParser-2.01.tar.gz + /usr/bin/tar -xf - + STATUS=0 + '[' 0 -ne 0 ']' + cd XML-DTDParser-2.01 + /usr/bin/chmod -Rf a+rX,u+w,g-w,o-w . Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.feEGiz + exit 0 + umask 022 + cd /builddir/build/BUILD + cd XML-DTDParser-2.01 + LANG=C + export LANG + unset DISPLAY + /usr/bin/perl Makefile.PL INSTALLDIRS=vendor Checking if your kit is complete... Looks good Writing Makefile for XML::DTDParser + make -j4 cp DTDParser.pm blib/lib/XML/DTDParser.pm Manifying blib/man3/XML::DTDParser.3pm + find . -type f -exec sed -i 's/\r//' '{}' ';' + exit 0 Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.mHe7cJ + umask 022 + cd /builddir/build/BUILD + '[' /builddir/build/BUILDROOT/perl-XML-DTDParser-2.01-1.fc18.x86_64 '!=' / ']' + rm -rf /builddir/build/BUILDROOT/perl-XML-DTDParser-2.01-1.fc18.x86_64 ++ dirname /builddir/build/BUILDROOT/perl-XML-DTDParser-2.01-1.fc18.x86_64 + mkdir -p /builddir/build/BUILDROOT + mkdir /builddir/build/BUILDROOT/perl-XML-DTDParser-2.01-1.fc18.x86_64 + cd XML-DTDParser-2.01 + LANG=C + export LANG + unset DISPLAY + rm -rf /builddir/build/BUILDROOT/perl-XML-DTDParser-2.01-1.fc18.x86_64 + make pure_install PERL_INSTALL_ROOT=/builddir/build/BUILDROOT/perl-XML-DTDParser-2.01-1.fc18.x86_64 Makefile out-of-date with respect to Makefile.PL Cleaning current config before rebuilding Makefile... make -f Makefile.old clean > /dev/null 2>&1 /usr/bin/perl Makefile.PL "INSTALLDIRS=vendor" Checking if your kit is complete... Looks good Writing Makefile for XML::DTDParser ==> Your Makefile has been rebuilt. <== ==> Please rerun the make command. <== false make: *** [Makefile] Error 1 error: Bad exit status from /var/tmp/rpm-tmp.mHe7cJ (%install) Bad exit status from /var/tmp/rpm-tmp.mHe7cJ (%install) RPM build errors: Child return code was: 1 I suspect that this is because you are fixing the line endings in %build after "make". You should fix line endings and make other source changes in %prep where possible, which would avoid this problem. Also I think only the README file actually needs its line-endings fixed anyway, so there's no need to touch the other files. It's usual practice now to use DESTDIR rather than PERL_INSTALL_ROOT when installing. Consider adding buildreqs for Perl core modules used by the module and its test suite: Exporter, FileHandle, strict, File::Spec, Cwd and Test.
It's odd that it doesn't build in mock for you, as I test build these before I upload them. Anyhow, I've moved the line endings fix into prep and added the BuildRequires, except for strict at that doesn't seem to make sense since there's no way that could be broken out as a separate module/feature. Also, the use of PERL_INSTALL_ROOT instead of destdir is required as this module doesn't appear to honor destdir. Spec URL: http://wfp.fedorapeople.org/perl-XML-DTDParser.spec SRPM URL: http://wfp.fedorapeople.org/perl-XML-DTDParser-2.01-2.fc16.src.rpm
(In reply to comment #2) > It's odd that it doesn't build in mock for you, as I test build these before I > upload them. Odd indeed. > Anyhow, I've moved the line endings fix into prep and added the BuildRequires, > except for strict at that doesn't seem to make sense since there's no way that > could be broken out as a separate module/feature. > > Also, the use of PERL_INSTALL_ROOT instead of destdir is required as this > module doesn't appear to honor destdir. destdir (lower-case) is a parameter that Module::Build uses, but this is an ExtUtils::MakeMaker based distribution and that supports DESTDIR (upper case). perl-XML-DTDParser Review ========================= rpmlint output: perl-XML-DTDParser.noarch: W: spelling-error %description -l en_US optionality -> nationality, proportionality, rationality perl-XML-DTDParser.src: W: spelling-error %description -l en_US optionality -> nationality, proportionality, rationality perl-XML-DTDParser.src:12: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 12) The spelling-error is a false positive and can be ignored. You should fix the mixed-use-of-spaces-and-tabs and use one or the other consistently. - package and spec naming OK - package meets guidelines - license in spec matches upstream and is OK for Fedora - no upstream license file to include in package - spec file written in English and is legible - source tarball matches upstream, including timestamp - package builds fine in mock - build dependencies are comprehensive and correct - no locale data or libraries to worry about - package not designed to be relocatable - directory ownership OK - no duplicate files - permissions sane - macro usage is consistent - code, not content - no large documentation to consider - docs don't affect runtime - no static libraries or devel files to consider - no libtool archives to consider - not a GUI application so no desktop file needed - filenames are all plain ASCII Suggestions (all non-blockers): * Use DESTDIR rather than PERL_INSTALL_ROOT * Sort buildreqs for readability * For larger modules, it may help to split buildreqs into groups based on where they're needed, e.g. Module Build/Module Runtime/Test Suite/Release Tests See for example: http://pkgs.fedoraproject.org/gitweb/?p=perl-Test-Version.git;a=blob;f=perl-Test-Version.spec;hb=master * Consider making the %files list more explicit, so you won't accidentally ship additional files if an upstream update includes some bundled code that shouldn't be in Fedora: %files %defattr(-,root,root,-) %doc Changes README %{perl_vendorlib}/XML/ %{_mandir}/man3/XML::DTDParser.3pm* * Consider dropping the use of macros for commands; there's no real benefit to using "%{__id_u} -n" rather than "id -nu", "%{__perl}" rather than "perl", or "%{__sed}" rather than "sed" * Try to maintain the same format for your changelog entries, as it makes it more readable; one entry in this spec has a dash between your email address and the version-release but the other doesn't. I'd suggest always using a dash as the changelog entries added during mass rebuilds do so. The only thing that definitely needs fixing here is the mixed use of spaces and tabs. Having now seen three of your packages, I'm comfortable enough to sponsor you. They're all small perl modules, which don't really lend themselves to demonstrating the full range of your packaging skills and knowledge of the guidelines, but on the other hand you are receptive to advice without just accepting anything a reviewer tells you, which I think bodes well. APPROVED. I've now sponsored you for membership of the packager group. Feel free to email me if you have any issues with Fedora packaging, processes etc.
Missing the tabs and spaces thing, I see I have to get more in the habit of running rpmlint on everything I do. So addressing that and a few other complaints. Spec URL: http://wfp.fedorapeople.org/perl-XML-DTDParser.spec SRPM URL: http://wfp.fedorapeople.org/perl-XML-DTDParser-2.01-3.fc16.src.rpm
New Package SCM Request ======================= Package Name: perl-XML-DTDParser Short Description: Quick and dirty DTD parser Owners: wfp Branches: f16 f17 InitialCC:
Git done (by process-git-requests).
It's usual for perl module packages to have perl-sig in the InitialCC: so that the perl-devel-list gets copied in on bug reports etc. Not compulsory though.
Bill, you only seem to have done a build for Rawhide; is there some problem with doing the f16/f17 builds?
perl-XML-DTDParser-2.01-3.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/perl-XML-DTDParser-2.01-3.fc16
perl-XML-DTDParser-2.01-3.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/perl-XML-DTDParser-2.01-3.fc17
perl-XML-DTDParser-2.01-3.fc17 has been pushed to the Fedora 17 testing repository.
perl-XML-DTDParser-2.01-3.fc16 has been pushed to the Fedora 16 stable repository.
Something I missed in the review: The directory %{perl_vendorlib}/XML/ should be owned by the package, and it isn't. Please change in the %%files list: %{perl_vendorlib}/XML/DTDParser.pm to: %{perl_vendorlib}/XML/ (this is one of 119 perl module packages I found with this type of problem)
perl-XML-DTDParser-2.01-4.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/perl-XML-DTDParser-2.01-4.fc16
perl-XML-DTDParser-2.01-4.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/perl-XML-DTDParser-2.01-4.fc17
perl-XML-DTDParser-2.01-3.fc17 has been pushed to the Fedora 17 stable repository.
perl-XML-DTDParser-2.01-4.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report.
perl-XML-DTDParser-2.01-4.fc16 has been pushed to the Fedora 16 stable repository. If problems still persist, please make note of it in this bug report.