Bug 664817

Summary: Review Request: perl-HTML-Selector-XPath - CSS Selector to XPath compiler
Product: [Fedora] Fedora Reporter: Ralf Corsepius <rc040203>
Component: Package ReviewAssignee: Jason Tibbitts <j>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: 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
Spec URL: http://corsepiu.fedorapeople.org/packages/perl-HTML-Selector-XPath.spec
SRPM URL: http://corsepiu.fedorapeople.org/packages/perl-HTML-Selector-XPath-0.04-1.fc15.src.rpm

Description: 
HTML::Selector::XPath is a utility function to compile CSS2 selector to the
equivalent XPath expression.

Comment 1 Petr Pisar 2011-01-11 14:44:15 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.

Comment 2 Ralf Corsepius 2011-01-27 06:27:58 UTC
(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.

Comment 3 Petr Pisar 2011-01-27 10:17:07 UTC
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.

Comment 4 Ralf Corsepius 2011-01-28 15:17:51 UTC
(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?

Comment 5 Petr Pisar 2011-01-31 09:31:50 UTC
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.

Comment 6 Ralf Corsepius 2011-02-01 07:27:00 UTC
Thank you very much for having abandoned this review.

Please understand that I consider your behavior in this review to not have been helpful.

Comment 7 Jason Tibbitts 2011-02-01 15:02:10 UTC
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

Comment 8 Ralf Corsepius 2011-02-02 01:15:06 UTC
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

Comment 9 Petr Pisar 2011-02-02 09:11:14 UTC
(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.)

Comment 10 Jason Tibbitts 2011-02-02 17:00:15 UTC
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.
"

Comment 11 Bill Nottingham 2011-02-02 22:31:47 UTC
Git done (by process-git-requests).

Comment 12 Fedora Update System 2011-02-03 06:57:30 UTC
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

Comment 13 Fedora Update System 2011-02-03 06:57:38 UTC
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

Comment 14 Petr Pisar 2011-02-03 11:12:30 UTC
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.

Comment 15 Fedora Update System 2011-02-03 20:22:04 UTC
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

Comment 16 Fedora Update System 2011-02-13 08:56:47 UTC
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.

Comment 17 Fedora Update System 2011-02-13 08:59:30 UTC
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.