Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 172332 - Review Request: perl-XML-XQL
Summary: Review Request: perl-XML-XQL
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ralf Corsepius
QA Contact: David Lawrence
URL: http://search.cpan.org/dist/XML-XQL/
Whiteboard:
Depends On: 172331
Blocks: 128879 FE-ACCEPT 169908
TreeView+ depends on / blocked
 
Reported: 2005-11-02 21:41 UTC by Ville Skyttä
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-11-06 16:15:56 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Proposed patch (714 bytes, patch)
2005-11-03 11:50 UTC, Ralf Corsepius
no flags Details | Diff

Description Ville Skyttä 2005-11-02 21:41:14 UTC
http://cachalot.mine.nu/4/SRPMS/perl-XML-XQL-0.68-0.1.src.rpm

This is a Perl extension that allows you to perform XQL queries on XML
object trees. Currently only the XML::DOM module is supported, but
other implementations, like XML::Grove, may soon follow.

For FC5+ only, this is part 3/3 of the replacements of the former (but still needed) perl-libxml-enno package, see bug 128879.

Comment 1 Ville Skyttä 2005-11-03 06:23:58 UTC
http://cachalot.mine.nu/4/SRPMS/perl-XML-XQL-0.68-0.2.src.rpm 
 
* Thu Nov  3 2005 Ville Skyttä <ville.skytta at iki.fi> - 0.68-0.2 
- Fix insecure $PATH error in taint mode (#147465). 
- Avoid warnings with empty (but defined) $TERM (#147465). 
 

Comment 2 Ralf Corsepius 2005-11-03 11:50:03 UTC
Created attachment 120685 [details]
Proposed patch

Two minor issues (cf. the patch):

* rpmlint perl-XML-XQL-0.68-0.2.noarch.rpm
E: perl-XML-XQL useless-explicit-provides perl(XML::XQL)

* Missing: "BR: perl(XML::DOM) >= 1.29"
(This causes the package to abort building early instead of at the end of
building, if an insufficient XML::DOM is installed)

Comment 3 Ville Skyttä 2005-11-03 16:20:20 UTC
Re: XML::DOM BR version: ack.  
  
Regarding the "useless-explicit-provides" one, surely you  
meant /perl(XML::XQL)$/d, right?  Without the trailing "$" (or similar), the 
package wouldn't provide any versioned nor versionless perl(XML::XQL).  
  
And of course, this being a perl module package, we'll use either perl or grep  
for stream editing, not sed, for crying out loud ;) 
 
http://cachalot.mine.nu/4/SRPMS/perl-XML-XQL-0.68-0.3.src.rpm 

Comment 4 Ralf Corsepius 2005-11-03 17:09:00 UTC
(In reply to comment #3)

> Regarding the "useless-explicit-provides" one, surely you  
> meant /perl(XML::XQL)$/d, right?
Of cause, stupid oversight of mine ;)

> And of course, this being a perl module package, we'll use either perl or grep  
> for stream editing, not sed, for crying out loud ;) 
Nah, as a portability adict I prefer using a lean and POSIX-compliant tool, 
like "sed".

Comment 5 Ralf Corsepius 2005-11-06 05:43:56 UTC
APPROVED

2 minor issues, without visible effect:

* I'd add 
BuildRequires: perl(XML::Parser) >= 2.30
BuildRequires: perl(Date::Manip) >= 5.33
to make these deps easier traceable should perl-packaging change (e.g. a module
be dropped) in future.

* XQLParser/Makefile.PL contains a hidden build-time dep on /usr/bin/yapp.
/usr/bin/yapp currently is part of perl-Parse-Yapp.
ATM, you BR perl(Parse::Yapp) [i.e. .../Parse/Yapp.pm]. I.e. /usr/bin/yapp is
only being pulled-in as a side effect of BR-ing perl(Parse::Yapp). Should the
location of /usr/bin/yapp ever change, this will break.



Comment 6 Ville Skyttä 2005-11-06 12:31:16 UTC
Thanks for the review, committed.  Will try to build later when the buildsys 
can see "perl(XML::DOM) >= 1.29".  Dunno why it currently can't. 
     
I didn't add any of the suggestions in comment 5, though:    
- Nothing in XML-XQL directly depends on XML::Parser    
- Date::Manip 5.36 was already in Red Hat 6.2, unversioned BR already in pkg  
- /usr/bin/yapp is only needed if XQLParser/Parser.pm is outdated or missing     

Comment 7 Ville Skyttä 2005-11-06 16:15:56 UTC
Build succeeded. 

Comment 8 Ralf Corsepius 2005-11-06 16:34:46 UTC
(In reply to comment #6)
> Thanks for the review, committed.  Will try to build later when the buildsys 
> can see "perl(XML::DOM) >= 1.29".  Dunno why it currently can't. 
Me thinks something rpm's perl module dependency tracking probably is broken.
(C.f. the perl(DBI::Pg) issue.

> I didn't add any of the suggestions in comment 5, though:    
> - Nothing in XML-XQL directly depends on XML::Parser
Doesn't matter, Makefile.PL explictly checks for it, for whatever reasons.

> - Date::Manip 5.36 was already in Red Hat 6.2, unversioned BR already in pkg
> - /usr/bin/yapp is only needed if XQLParser/Parser.pm is outdated or missing
Or being split out ;)

Comment 9 Ville Skyttä 2005-11-06 17:36:50 UTC
> (In reply to comment #6)
> Me thinks something rpm's perl module dependency tracking probably is broken.
> (C.f. the perl(DBI::Pg) issue.

Not the same thing here.  This was apparently a buildsys local issue, repos not
updating or something.  Works now.

> > - Nothing in XML-XQL directly depends on XML::Parser
> Doesn't matter, Makefile.PL explictly checks for it, for whatever reasons.

That results in a warning at worst, and doesn't affect the build.
 
> > - /usr/bin/yapp is only needed if XQLParser/Parser.pm is outdated or missing
> Or being split out ;)

What being split from where?  /usr/bin/yapp being split from perl-Parse-Yapp? 
XQLParser/Parser.pm being split from perl-XML-XQL?  Anyway, /usr/bin/yapp is not
needed to build whatever package XQLParser/Parser.pm is in as long as it's up to
date wrt XQLParser/Parser.yp, no matter what packages things are in.  Already
tested before committing by manually moving /usr/bin/yapp elsewhere and rebuilding.


Note You need to log in before you can comment on or make changes to this bug.