Bug 664817
Summary: | Review Request: perl-HTML-Selector-XPath - CSS Selector to XPath compiler | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Ralf Corsepius <rc040203> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, notting, ppisar |
Target Milestone: | --- | Flags: | j:
fedora-review+
notting: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | perl-HTML-Selector-XPath-0.04-1.fc13 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2011-02-13 08:56:53 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 665096 |
Description
Ralf Corsepius
2010-12-21 18:39:48 UTC
Source tar ball is original. Ok. Summary verified from lib/HTML/Selector/XPath.pm. Ok. License verified from lib/HTML/Selector/XPath.pm. Ok. Description verified from lib/HTML/Selector/XPath.pm. Ok. FIX: BuildRequire perl(Exporter) (lib/HTML/Selector/XPath.pm:7) as it can dual-live in the future (http://search.cpan.org/~ferreira/Exporter/). All test passes. Ok. $ rpmlint perl-HTML-Selector-XPath.spec ../SRPMS/perl-HTML-Selector-XPath-0.04-1.fc14.src.rpm ../RPMS/noarch/perl-HTML-Selector-XPath-0.04-1.fc14.noarch.rpm 2 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint Ok. $ rpm -q -lv -p ../RPMS/noarch/perl-HTML-Selector-XPath-0.04-1.fc14.noarch.rpm drwxr-xr-x 2 root root 0 led 11 15:33 /usr/share/doc/perl-HTML-Selector-XPath-0.04 -rw-r--r-- 1 root root 447 úno 27 2010 /usr/share/doc/perl-HTML-Selector-XPath-0.04/Changes -rw-r--r-- 1 root root 558 úno 27 2010 /usr/share/doc/perl-HTML-Selector-XPath-0.04/README -rw-r--r-- 1 root root 2517 led 11 15:33 /usr/share/man/man3/HTML::Selector::XPath.3pm.gz drwxr-xr-x 2 root root 0 led 11 15:33 /usr/share/perl5/HTML drwxr-xr-x 2 root root 0 led 11 15:33 /usr/share/perl5/HTML/Selector -rw-r--r-- 1 root root 6059 úno 27 2010 /usr/share/perl5/HTML/Selector/XPath.pm File permissions and layout Ok. $ rpm -q --requires -p ../RPMS/noarch/perl-HTML-Selector-XPath-0.04-1.fc14.noarch.rpm | sort | uniq -c 1 perl(Carp) 1 perl(Exporter) 1 perl(:MODULE_COMPAT_5.12.2) 1 perl(strict) 1 perl >= 0:5.008_001 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 Binary requires Ok. $ rpm -q --provides -p ../RPMS/noarch/perl-HTML-Selector-XPath-0.04-1.fc14.noarch.rpm | sort | uniq -c 1 perl(HTML::Selector::XPath) = 0.04 1 perl-HTML-Selector-XPath = 0.04-1.fc14 Binary provides Ok. $ resolvedeps-f15 ../RPMS/noarch/perl-HTML-Selector-XPath-0.04-1.fc14.noarch.rpm Binary dependencies resolvable. Ok. Package builds in F15 (http://koji.fedoraproject.org/koji/taskinfo?taskID=2714859). 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: BuildRequire perl(Exporter) (lib/HTML/Selector/XPath.pm:7) as it can > dual-live in the future (http://search.cpan.org/~ferreira/Exporter/). I do not agree with this. As I repeatedly said before: Packagers can not take into account any arbitrary package which might possibly be split out from Fedora's core perl package at some more or less distant point in future. Enforcing such a strategy is non-helpful. Perl packaging guidelines <https://fedoraproject.org/wiki/Packaging:Perl#Perl_Requires_and_Provides> says: > It is recommended to buildrequire core modules explicitly, because they can > move between sub-packages or disappear from core perl. I agree with guidelines that relying on fact the module is currently provided by perl-core is silly. (In reply to comment #3) > Perl packaging guidelines > <https://fedoraproject.org/wiki/Packaging:Perl#Perl_Requires_and_Provides> > says: > > > It is recommended to buildrequire core modules explicitly, because they can > > move between sub-packages or disappear from core perl. > > I agree with guidelines that relying on fact the module is currently provided > by perl-core is silly. You violently don't want to understand? When a core module moves, building a package will break - Packagers will have to change their packages - mission accomplished. As it's impossible to know which modules will move, it's silly anticipatory obedience to demand such modules to be added now. Worse, such demands are harmful, because a formerly needed dependency may vanish with upstreams updates, a situation which is easy to miss for maintainers. Finally, how do you want to continue with this review? You to continue blocking this review? I understand perfectly. The guidelines says you should explicitly depend because it can move to different package. At fist you can prevent problem now, you do not need to wait for breaking things later. At second the package builds even after `breakage'. It will just use old possibly buggy module. Adding explicit dependency you assure using latest dual-lived package. This question is not about willing or not willing. This is about compliance to guidelines. As the spec file does not comply to guidelines statement I quoted above, I cannot approve this package. You can try your luck with another reviewer, although I think the guidelines are the same for all reviewers and packagers. Thank you very much for having abandoned this review. Please understand that I consider your behavior in this review to not have been helpful. I think that guideline bears further scrutiny. If the perl maintainers do intend to unbundle all modules from the base perl package then it makes sense as eventually most perl packages will have to change. If not, then requiring packagers to anticipate which modules may or may not be unbundled isn't really productive. Not to mention finding these things isn't necessarily easy (because the package builds just fine currently) so checking them in the review process isn't really possible. Personally, I would change that guideline from "should" to "are encouraged to". Anyway, it's just a suggestion, not a requirement. Perfectly reasonable for packagers and reviewers to disagree about those, but no reason to assume that a different reviewer will not have a different opinion. And my opinion is that this package is just fine. Ralf, would you please open a ticket so that FPC can discuss amending or removing that bit from the guidelines? Of course I'm sure you know that you can remove BuildRoot, %clean and the cleaning of the build root in %install if you don't intend to target el5. * source files match upstream. sha256sum: fd0735f32a49a357025b0cb16669be34906ab8d8e514cff94075d71daa3fa99d HTML-Selector-XPath-0.04.tar.gz * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * summary is OK. * description is OK. * dist tag is present. * license field matches the actual license. * license is open source-compatible. * license text not included upstream. * latest version is being packaged. * BuildRequires are proper. * package builds in mock (rawhide, x86_64). * package installs properly. * rpmlint is silent. * final provides and requires are sane: perl(HTML::Selector::XPath) = 0.04 perl-HTML-Selector-XPath = 0.04-1.fc15 = perl >= 0:5.008_001 perl(:MODULE_COMPAT_5.12.3) perl(Carp) perl(Exporter) perl(strict) * %check is present and all tests pass: All tests successful. Files=4, Tests=36, 0 wallclock secs ( 0.02 usr 0.02 sys + 0.22 cusr 0.03 csys = 0.29 CPU) Result: PASS * no bundled libraries. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no generically named files. * code, not content. * documentation is small, so no -doc subpackage is necessary. * %docs are not necessary for the proper functioning of the package. APPROVED Thanks, Jason. New Package SCM Request ======================= Package Name: perl-HTML-Selector-XPath Short Description: CSS Selector to XPath compiler Owners: corsepiu Branches: f13 f14 InitialCC: perl-sig (In reply to comment #7) > requiring packagers to anticipate which modules may or may not be unbundled > isn't really productive. You do not need an oraculum to know which module can get unbundled. You can use CPAN for that to see which modules are, in fact, bundled to core. (I always provided link to relevant separate CPAN module.) BTW, I actually looked up the guideline and it doesn't say "should". It says "it is recommended" which is even weaker. If people are going to block reviews on this then you have my vote on FPC to weaken this further to "it is suggested" or "packagers may wish to consider" which are weaker still. " It is recommended to buildrequire core modules explicitly, because they can move between sub-packages or disappear from core perl. " Git done (by process-git-requests). perl-HTML-Selector-XPath-0.04-1.fc13 has been submitted as an update for Fedora 13. https://admin.fedoraproject.org/updates/perl-HTML-Selector-XPath-0.04-1.fc13 perl-HTML-Selector-XPath-0.04-1.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/perl-HTML-Selector-XPath-0.04-1.fc14 Dear Ralf, I reevaluated my opinion on the not yet dual-lived dependencies. As reviewed package is perfect at time of review, it's valid to accept the package. I will mark such speculative requires as TODO item in my reviews in the future if there will be no dual-lived package at that time. I apology for any inconveniences. perl-HTML-Selector-XPath-0.04-1.fc14 has been pushed to the Fedora 14 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update perl-HTML-Selector-XPath'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/perl-HTML-Selector-XPath-0.04-1.fc14 perl-HTML-Selector-XPath-0.04-1.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report. perl-HTML-Selector-XPath-0.04-1.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report. |