Spec URL: http://corsepiu.fedorapeople.org/packages/perl-HTML-TreeBuilder-LibXML.spec SRPM URL: http://corsepiu.fedorapeople.org/packages/perl-HTML-TreeBuilder-LibXML-0.12-1.fc15.src.rpm Description: HTML::TreeBuilder::XPath is a libxml based compatible interface to HTML::TreeBuilder, which could be slow for a large document.
Source file is original. Ok. Summary verified from lib/HTML/TreeBuilder/LibXML.pm. Ok. License verified from lib/HTML/TreeBuilder/LibXML.pm. Ok. Description verified from lib/HTML/TreeBuilder/LibXML.pm. Ok. All tests pass. Ok. $ rpmlint perl-HTML-TreeBuilder-LibXML.spec ../SRPMS/perl-HTML-TreeBuilder-LibXML-0.12-1.fc14.src.rpm ../RPMS/noarch/perl-HTML-TreeBuilder-LibXML-0.12-1.fc14.noarch.rpm 2 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint Ok. $ rpm -q -lv -p ../RPMS/noarch/perl-HTML-TreeBuilder-LibXML-0.12-1.fc14.noarch.rpm drwxr-xr-x 2 root root 0 led 17 17:31 /usr/share/doc/perl-HTML-TreeBuilder-LibXML-0.12 -rw-r--r-- 1 root root 1685 zář 21 01:42 /usr/share/doc/perl-HTML-TreeBuilder-LibXML-0.12/Changes -rw-r--r-- 1 root root 576 bře 24 2009 /usr/share/doc/perl-HTML-TreeBuilder-LibXML-0.12/README -rw-r--r-- 1 root root 2470 led 17 17:31 /usr/share/man/man3/HTML::TreeBuilder::LibXML.3pm.gz -rw-r--r-- 1 root root 1993 led 17 17:31 /usr/share/man/man3/HTML::TreeBuilder::LibXML::Node.3pm.gz drwxr-xr-x 2 root root 0 led 17 17:31 /usr/share/perl5/HTML drwxr-xr-x 2 root root 0 led 17 17:31 /usr/share/perl5/HTML/TreeBuilder drwxr-xr-x 2 root root 0 led 17 17:31 /usr/share/perl5/HTML/TreeBuilder/LibXML -rw-r--r-- 1 root root 3778 zář 21 01:41 /usr/share/perl5/HTML/TreeBuilder/LibXML.pm -rw-r--r-- 1 root root 4788 pro 25 2009 /usr/share/perl5/HTML/TreeBuilder/LibXML/Node.pm File permissions and layout Ok. $ rpm -q --requires -p ../RPMS/noarch/perl-HTML-TreeBuilder-LibXML-0.12-1.fc14.noarch.rpm | sort | uniq -c 1 perl(base) 1 perl(Carp) 1 perl(:MODULE_COMPAT_5.12.2) 1 perl(strict) 1 perl(warnings) 1 perl(XML::LibXML) 1 rpmlib(CompressedFileNames) <= 3.0.4-1 1 rpmlib(FileDigests) <= 4.6.0-1 1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 1 rpmlib(PayloadIsXz) <= 5.2-1 1 rpmlib(VersionedDependencies) <= 3.0.3-1 FIX: Require perl(HTML::TreeBuilder::XPath) >= 0.11 (lib/HTML/TreeBuilder/LibXML.pm:73:, META.yml) FIX: Version perl(XML::LibXML) to >= 1.7 (META.yml) $ rpm -q --provides -p ../RPMS/noarch/perl-HTML-TreeBuilder-LibXML-0.12-1.fc14.noarch.rpm | sort | uniq -c 1 perl(HTML::TreeBuilder::LibXML::Node) 1 perl(HTML::TreeBuilder::LibXML) = 0.12 1 perl-HTML-TreeBuilder-LibXML = 0.12-1.fc14 Binary provides Ok. $ resolvedeps-f15 ../RPMS/noarch/perl-HTML-TreeBuilder-LibXML-0.12-1.fc14.noarch.rpm Binary dependencies resolvable. Ok. Package builds in F15 (http://koji.fedoraproject.org/koji/taskinfo?taskID=2726221). Ok. Otherwise package is in line with Fedora and perl packaging guidelines. Please correct all `FIX' prefixed issues and provide new spec file. Resolution: Package NOT approved.
(In reply to comment #1) > FIX: Require perl(HTML::TreeBuilder::XPath) >= 0.11 > (lib/HTML/TreeBuilder/LibXML.pm:73:, META.yml) > FIX: Version perl(XML::LibXML) to >= 1.7 (META.yml) Please explain in detail why you want these versions to be added and which problem these would fix. I consider these versions to be superflous dependency bloat, because at the very moment, this package is provided in Fedora, these versions are implicitly guaranteed through the BuildRequires (otherwise this package would not build).
See the references I provided in parenthesis. perl(HTML::TreeBuilder::XPath) >= 0.11 is (not only) run time dependency: lib/HTML/TreeBuilder/LibXML.pm: sub replace_original { require HTML::TreeBuilder::XPath; Thus binary package needs to Require it. rpm-build is not perfect. It cannot find all dependencies. Like this one. In addition the module stated as run time dependency in META.yml. Provided developers are not silly, the have a reason why to require it. Even the replace_original() method is used in module synopsis. The perl(XML::LibXML) is not explicitly versioned in installed code. Thus rpm-build could not discover the version and export the dependency unversioned. However required version is defined in META.yml. This is current (unfortunate) practice of perl developers. The track versions in META.yml only and they assume user installs modules from CPAN directly. You cannot assume a user has the same package versions as were present at build time in Koji. The binary package must be self-describing. You can get the package into system in many ways. E.g. by downloading the package by hand and installing the package through rpm. You are right all F13--15 bring perl(XML::LibXML) >= 1.70 (http://koji.fedoraproject.org/koji/packageinfo?packageID=962). Then you needn't BuildRequire the version explicitly too. It's inconsistent. So, I take back the perl(XML::LibXML) version FIX. However I insist on the perl(HTML::TreeBuilder::XPath) requirement.
Anything happening here? It appears that this is holding up the last dependency for rt4.
(In reply to comment #3) > See the references I provided in parenthesis. > > > perl(HTML::TreeBuilder::XPath) >= 0.11 is (not only) run time dependency: > > lib/HTML/TreeBuilder/LibXML.pm: > sub replace_original { > require HTML::TreeBuilder::XPath; > > Thus binary package needs to Require it. rpm-build is not perfect. It cannot > find all dependencies. Like this one. In addition the module stated as run time > dependency in META.yml. Provided developers are not silly, the have a reason > why to require it. > > Even the replace_original() method is used in module synopsis. > > > The perl(XML::LibXML) is not explicitly versioned in installed code. Thus > rpm-build could not discover the version and export the dependency unversioned. > However required version is defined in META.yml. This is current (unfortunate) > practice of perl developers. The track versions in META.yml only and they > assume user installs modules from CPAN directly. > > You cannot assume a user has the same package versions as were present at build > time in Koji. The binary package must be self-describing. You can get the > package into system in many ways. E.g. by downloading the package by hand and > installing the package through rpm. > > You are right all F13--15 bring perl(XML::LibXML) >= 1.70 > (http://koji.fedoraproject.org/koji/packageinfo?packageID=962). Then you > needn't BuildRequire the version explicitly too. It's inconsistent. > > > So, I take back the perl(XML::LibXML) version FIX. However I insist on the > perl(HTML::TreeBuilder::XPath) requirement. Can the same logic used for perl(XML::LibXML) be used for perl(HTML::TreeBuilder::XPath)? According to http://koji.fedoraproject.org/koji/packageinfo?packageID=8377, all maintained version of fedora have perl-HTML-TreeBuilder-XPath >= 0.11 -josh
Yeap. It's possible not to specify version for perl(HTML::TreeBuilder::XPath).
(In reply to comment #6) > Yeap. It's possible not to specify version for perl(HTML::TreeBuilder::XPath). Does that mean the package is now approved since both FIX items have been resolved? Looking forward to getting rt4 packaged! Thanks, -josh
(In reply to comment #7) > (In reply to comment #6) > > Yeap. It's possible not to specify version for > > perl(HTML::TreeBuilder::XPath). > > Does that mean the package is now approved since both FIX items have been > resolved? > No, the perl(HTML::TreeBuilder::XPath) is still missing from requires of binary package (compiled in F14). I will re-check it in F16. Maybe the rpmbuild requires generator has been fixed in the meantime.
(In reply to comment #8) > (In reply to comment #7) > > Does that mean the package is now approved since both FIX items have been > > resolved? > > > No, the perl(HTML::TreeBuilder::XPath) is still missing from requires of binary > package (compiled in F14). I will re-check it in F16. Maybe the rpmbuild > requires generator has been fixed in the meantime. So the same problem presents in my local F16 and in Koji (http://koji.fedoraproject.org/koji/taskinfo?taskID=3099960). The Requires: perl(HTML::TreeBuilder::XPath) must be added into the spec file.
Still the same with F17 (http://koji.fedoraproject.org/koji/taskinfo?taskID=3635186). Ralf, do you ever plan to add this dependency or is this review stuck until rpmbuild picks it up someday?
Created attachment 555367 [details] The spec file containing "Requires: perl(HTML::TreeBuilder::XPath)" Added the reviewers reguested one liner to the spec file... Are you guys telling me that we have been holding back introducing RT-4 for ca 8 - 9 months due to a one liner? Ralf just remove it once requires generator has been fixed. Petr I assume that the attached spec file passes your requirement?
> Are you guys telling me that we have been holding back introducing RT-4 for ca > 8 - 9 months due to a one liner? No. The rt4 delay is caused by a _long_ series of incidents and bugs interacting. Updated package: Spec URL: http://corsepiu.fedorapeople.org/packages/perl-HTML-TreeBuilder-LibXML.spec SRPM URL: http://corsepiu.fedorapeople.org/packages/perl-HTML-TreeBuilder-LibXML-0.15-1.fc17.src.rpm
This is rebase to 0.15. Performing complete review. Source tar ball is original. Ok. Summary verified from lib/HTML/TreeBuilder/LibXML.pm. Ok. License verified from lib/HTML/TreeBuilder/LibXML.pm. Ok. URL and Source0 verified. Ok. FIX: Augment description text to cover this package. Current sentence talks about different Perl package only. No XS code, noarch BuildArch is Ok. FIX: Build-require perl(Carp) (lib/HTML/TreeBuilder/LibXML/Node.pm:4). TODO: Build-require perl(base) (lib/HTML/TreeBuilder/LibXML.pm:6) TODO: Remove useless %defattr from %files section. All tests pass. Ok. $ rpmlint perl-HTML-TreeBuilder-LibXML.spec ../SRPMS/perl-HTML-TreeBuilder-LibXML-0.15-1.fc17.src.rpm ../RPMS/noarch/perl-HTML-TreeBuilder-LibXML-0.15-1.fc17.noarch.rpm 2 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint Ok. $ rpm -q -lv -p ../RPMS/noarch/perl-HTML-TreeBuilder-LibXML-0.15-1.fc17.noarch.rpm drwxr-xr-x 2 root root 0 Jan 16 13:03 /usr/share/doc/perl-HTML-TreeBuilder-LibXML-0.15 -rw-r--r-- 1 root root 2461 Nov 29 19:21 /usr/share/doc/perl-HTML-TreeBuilder-LibXML-0.15/Changes -rw-r--r-- 1 root root 576 Sep 21 00:50 /usr/share/doc/perl-HTML-TreeBuilder-LibXML-0.15/README -rw-r--r-- 1 root root 2469 Jan 16 13:03 /usr/share/man/man3/HTML::TreeBuilder::LibXML.3pm.gz -rw-r--r-- 1 root root 2045 Jan 16 13:03 /usr/share/man/man3/HTML::TreeBuilder::LibXML::Node.3pm.gz drwxr-xr-x 2 root root 0 Jan 16 13:03 /usr/share/perl5/vendor_perl/HTML drwxr-xr-x 2 root root 0 Jan 16 13:03 /usr/share/perl5/vendor_perl/HTML/TreeBuilder drwxr-xr-x 2 root root 0 Jan 16 13:03 /usr/share/perl5/vendor_perl/HTML/TreeBuilder/LibXML -rw-r--r-- 1 root root 3925 Nov 29 19:21 /usr/share/perl5/vendor_perl/HTML/TreeBuilder/LibXML.pm -rw-r--r-- 1 root root 5905 Nov 29 19:20 /usr/share/perl5/vendor_perl/HTML/TreeBuilder/LibXML/Node.pm File permissions and layout are Ok. $ rpm -q --requires -p ../RPMS/noarch/perl-HTML-TreeBuilder-LibXML-0.15-1.fc17.noarch.rpm | sort | uniq -c 1 perl >= 0:5.008001 1 perl(base) 1 perl(Carp) 1 perl(HTML::TreeBuilder::LibXML::Node) 1 perl(HTML::TreeBuilder::XPath) >= 0.14 1 perl(:MODULE_COMPAT_5.14.2) 1 perl(strict) 1 perl(warnings) 1 perl(XML::LibXML) >= 1.7 1 rpmlib(CompressedFileNames) <= 3.0.4-1 1 rpmlib(FileDigests) <= 4.6.0-1 1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 1 rpmlib(PayloadIsXz) <= 5.2-1 Binary requires Ok. $ rpm -q --provides -p ../RPMS/noarch/perl-HTML-TreeBuilder-LibXML-0.15-1.fc17.noarch.rpm | sort | uniq -c 1 perl(HTML::TreeBuilder::LibXML) = 0.15 1 perl-HTML-TreeBuilder-LibXML = 0.15-1.fc17 1 perl(HTML::TreeBuilder::LibXML::Node) Binary provides Ok. $ resolvedeps rawhide ../RPMS/noarch/perl-HTML-TreeBuilder-LibXML-0.15-1.fc17.noarch.rpm Binary dependencies resolvable. Ok. Package builds in F17 (http://koji.fedoraproject.org/koji/taskinfo?taskID=3705598). ??? Otherwise package is in line with Fedora and Perl packaging guidelines. Please correct all `FIX' prefixed issues, consider fixing `TODO' items, and provide new spec file. Resolution: Package NOT approved.
(In reply to comment #13) > This is rebase to 0.15. Performing complete review. > > Source tar ball is original. Ok. > Summary verified from lib/HTML/TreeBuilder/LibXML.pm. Ok. > License verified from lib/HTML/TreeBuilder/LibXML.pm. Ok. > URL and Source0 verified. Ok. > > FIX: Augment description text to cover this package. Current sentence talks > about different Perl package only. This is upstream's description, as cpanspec had extracted it. I can add the next line from upstream's desciption, if you like this better. > FIX: Build-require perl(Carp) (lib/HTML/TreeBuilder/LibXML/Node.pm:4). > TODO: Build-require perl(base) (lib/HTML/TreeBuilder/LibXML.pm:6) perl(base) is never going to move outside of the perl package. And even if, this will be causing an FTBS, which can be fixed then. Enforcing BR: perl(base) is bureacratic nit-pickery. > TODO: Remove useless %defattr from %files section. Will not do so - %defattr is still allowed, not using it is not mandated. You are enforcing a non existing rule. > Resolution: Package NOT approved. Petr, some open and direct words: In case you're not aware about it, the style of your reviews is hardly bearable and childishly pedantic. It is driving people away from Fedora.
(In reply to comment #14) > (In reply to comment #13) > > FIX: Augment description text to cover this package. Current sentence talks > > about different Perl package only. > This is upstream's description, as cpanspec had extracted it. > I can add the next line from upstream's desciption, if you like this better. > I know, cpanspec takes first paragraph only which is not good enough in this case. Adding the second paragraph from upstream POD would be great to make the description useful for RPM users. > > FIX: Build-require perl(Carp) (lib/HTML/TreeBuilder/LibXML/Node.pm:4). > > TODO: Build-require perl(base) (lib/HTML/TreeBuilder/LibXML.pm:6) > perl(base) is never going to move outside of the perl package. Until it get superseded by `parent' module. Or become dual-lived in Fedora because `base' lives on CPAN already. > And even if, this will be causing an FTBS, which can be fixed then. > Which is good reason to add add it now to prevent FTBS in the future. > Enforcing BR: perl(base) is bureacratic nit-pickery. > I know. Thus I marked it as `TODO'. In contrast to perl(Carp) I evaluated as `FIX' because it has already lived dual in Fedora. > > TODO: Remove useless %defattr from %files section. > Will not do so - %defattr is still allowed, not using it is not mandated. > You are enforcing a non existing rule. > Your choice which I respect (see, this is `TODO', not a `FIX'). The non-existing rule is to keep spec file as minimal as possible. There is no reason to keep a line which is equivalent to empty line. Or would you append an poetry to the spec file? This is also not forbidden. > > Resolution: Package NOT approved. > > Petr, some open and direct words: In case you're not aware about it, the style > of your reviews is hardly bearable and childishly pedantic. That's purpose of a review. To criticize anything that diverts a package from the ideal and to assure Fedora will deliver high-quality packages. > It is driving people away from Fedora. Better smaller good distribution than bigger crappy one.
(In reply to comment #15) > > Petr, some open and direct words: In case you're not aware about it, the style > > of your reviews is hardly bearable and childishly pedantic. > > That's purpose of a review. No, the purpose of a review is to assure a package integrates properly into a system. This often means to find pragmatical compromises and not to behave infantile, silly and bureaucratic. > To criticize anything that diverts a package from > the ideal and to assure Fedora will deliver high-quality packages. That's what bureaucrats believe - I guess, you're too young to comprehend you're in error. > > It is driving people away from Fedora. > Better smaller good distribution than bigger crappy one. Sure, but that's not what you are doing - With all due respect, you have turned contributing perl modules into tedious burdon, nobody can be interested in. Finally, the reason for the crappy shape Fedora is in is not perl - they are elsewhere.
Back to topic: FIX: Augment description text to cover this package. Current sentence talks about different Perl package only. FIX: Build-require perl(Carp) (lib/HTML/TreeBuilder/LibXML/Node.pm:4). Do these and the package can be approved. If you don't agree lets dispute in front of wider audience, e.g. FPC, why off-topic package description and missing perl(Carp) are (not) acceptable.
I think it's best Ralf that you address those issue Petr points out then bring any disputable items to the attention of the Fedora Perl SIG or FPC so things dont get stuck for another year.
I've exchanged emails with Ralf and he given me his OK for me to package this module so that this one can be unblocked. I've thus submitted bug #809633 and would very much appreciate a review. *** This bug has been marked as a duplicate of bug 809633 ***