Bug 427480 - Review Request: perl-XML-TreeBuilder - Perl parser that builds XML trees
Review Request: perl-XML-TreeBuilder - Perl parser that builds XML trees
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
:
Depends On:
Blocks: publican
  Show dependency treegraph
 
Reported: 2008-01-03 22:38 EST by Jeff Fearn
Modified: 2013-06-07 08:42 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-24 18:24:01 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)
patch to upstream tarball (3.32 KB, text/x-patch)
2008-01-17 22:14 EST, Parag AN(पराग)
no flags Details

  None (edit)
Description Jeff Fearn 2008-01-03 22:38:48 EST
Spec URL: http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-XML-TreeBuilder.spec

SRPM URL: http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/perl-XML-TreeBuilder-3.10-2.fc9.src.rpm

Description: Perl arser that builds XML trees. Used for ne documentation-devel packages.
Comment 1 Huzaifa S. Sidhpurwala 2008-01-04 01:42:12 EST
[root@dhcp1-17 ~]# rpmlint perl-XML-TreeBuilder-3.10-2.fc9.src.rpm 
perl-XML-TreeBuilder.src:30: W: setup-not-quiet
perl-XML-TreeBuilder.src:33: W: rpm-buildroot-usage %build %{__perl} Makefile.PL
INSTALLDIRS="vendor" PREFIX="%{buildroot}%{_prefix}"
perl-XML-TreeBuilder.src: W: non-standard-group Applications/CPAN
perl-XML-TreeBuilder.src: W: invalid-license Artistic/GPL
Comment 2 Huzaifa S. Sidhpurwala 2008-01-04 01:51:25 EST
Here is what would suit this better:

1. setup -q, but i guess this should be ok.

2. %{__perl} Makefile.PL INSTALLDIRS=vendor OPTIMIZE="$RPM_OPT_FLAGS"

3. Group: I would rather choose: Development/Libraries

4. License: GPL+ or Artistic would be more appropriate.  
http://fedoraproject.org/wiki/Licensing

Comment 3 Jeff Fearn 2008-01-06 18:44:21 EST
(In reply to comment #2)

All these changes have been made to the spec file.

> Here is what would suit this better:
> 
> 1. setup -q, but i guess this should be ok.
> 
> 2. %{__perl} Makefile.PL INSTALLDIRS=vendor OPTIMIZE="$RPM_OPT_FLAGS"
> 
> 3. Group: I would rather choose: Development/Libraries
> 
> 4. License: GPL+ or Artistic would be more appropriate.  
> http://fedoraproject.org/wiki/Licensing
> 
> 

Comment 4 Huzaifa S. Sidhpurwala 2008-01-06 23:36:02 EST
4. What i meant is GPL+ "OR" Artistic not "GPL+ or Artistic"

Also have you rebuild the srpm with the new spec file.
Comment 5 Parag AN(पराग) 2008-01-07 05:27:43 EST
Review for initial SRPM posted in comment 
1)Not needed following in SPEC
%define perl_vendorlib %(eval "`%{__perl} -V:installvendorlib`"; echo
$installvendorlib)
%define perl_vendorarch %(eval "`%{__perl} -V:installvendorarch`"; echo
$installvendorarch)

2) use simple way instead to use like this
%dir %{perl_vendorlib}/XML/
%{perl_vendorlib}/XML/Element.pm
%{perl_vendorlib}/XML/TreeBuilder.pm
use following
%{perl_vendorlib}/XML

3) License should be GPL+ or Artistic

4) use setup -q
 
5) remove META.yml, MANIFEST and MANIFEST.SKIP from %doc

6) make Group to Development/Libraries

7) missing BR and requires. Add following to SPEC
BuildRequires: perl(HTML::Element)
BuildRequires: perl(HTML::Tagset)
BuildRequires: perl(XML::Parser)
Requires: perl(HTML::Element) perl(HTML::Tagset) perl(XML::Parser)

8)%install should use following
%{__make} pure_install PERL_INSTALL_ROOT=$RPM_BUILD_ROOT


Always keep habit of updating SPEC and SRPM links with updateing release tag and
adding what changes you did in SPEC.

Provide updated SPEC and SRPM links.
Comment 8 Parag AN(पराग) 2008-01-07 22:17:55 EST
1)Any reason for not following point 7 from comment #5?
build.log from mock build showed me
Warning: prerequisite HTML::Element 3.08 not found.
Warning: prerequisite HTML::Tagset 3.02 not found.
Warning: prerequisite XML::Parser 0 not found.

2)Wrong group Development/Languages

Comment 9 Parag AN(पराग) 2008-01-07 22:18:29 EST
Again,
    Please provide updated SPEC and SRPM links fixing above issues.
Comment 11 Parag AN(पराग) 2008-01-09 23:04:29 EST
This package is ready for official review. But as you need sponsor, I can
sponsor and review this and other packages from you provided you also review few
other new packages waiting for review.
Comment 12 Parag AN(पराग) 2008-01-09 23:07:14 EST
also, Change Source URL as provided URL in SPEC is not working. correct one is
http://search.cpan.org/CPAN/authors/id/S/SB/SBURKE/XML-TreeBuilder-%{version}.tar.gz
Comment 13 Parag AN(पराग) 2008-01-09 23:12:02 EST
I see that you have not added 
%check
make test

Add above section. But when added "make test" failed. It should not fail and
should execute all tests successfully.
build.log gave me
+ make test
PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0,
'blib/lib', 'blib/arch')" t/*.t
t/00about....ok
t/10main.....# Failed test 2 in t/10main.t at line 33
#  t/10main.t line 33 is: ok $x->same_as($y);
FAILED test 2
        Failed 1/3 tests, 66.67% okay
Failed Test Stat Wstat Total Fail  Failed  List of Failed
-------------------------------------------------------------------------------
t/10main.t                 3    1  33.33%  2
Failed 1/2 test scripts, 50.00% okay. 1/6 subtests failed, 83.33% okay.
make: *** [test_dynamic] Error 255
Comment 15 Jens Petersen 2008-01-16 00:22:11 EST
It would be nice to shorten the summary to fit on a 80 char line.
Comment 17 Parag AN(पराग) 2008-01-16 23:08:22 EST
why source XML-TreeBuilder-3.10.tar.gz is not on CPAN?
Comment 18 Jeff Fearn 2008-01-16 23:33:38 EST
(In reply to comment #17)
> why source XML-TreeBuilder-3.10.tar.gz is not on CPAN?

Simply a time issue, too many things to do so joining CPAN hasn't made the
action list yet. I hope to get to it after the production system has been upgraded.
Comment 19 Parag AN(पराग) 2008-01-17 00:04:13 EST
Review:
+ package builds in mock (rawhide i386).
+ rpmlint is silent for SRPM and for RPM.
+ source files match upstream url
943d0175044a1a4b1b419daa8dc8ddca  XML-TreeBuilder-3.10.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc is present.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no headers or static libraries.
+ no .pc file present.
+ no -devel subpackage
+ no .la files.
+ no translations are available
+ Does owns the directories it creates.
+ no scriptlets present.
+ no duplicates in %files.
+ file permissions are appropriate.
+ make test gave
t/00about....ok
t/10main.....ok
All tests successful.
Files=2, Tests=6,  1 wallclock secs ( 0.14 cusr +  0.03 csys =  0.17 CPU)
+ Package perl-XML-TreeBuilder-3.10-6.fc9 ->
  Provides: perl(XML::Element) = 3.09 perl(XML::TreeBuilder) = 3.10
  Requires: perl >= 1:5 perl(Carp) perl(HTML::Element) perl(HTML::Element) >=
3.08 perl(HTML::Tagset) perl(HTML::Tagset) perl(XML::Element) perl(XML::Parser)
perl(XML::Parser) perl(strict) perl(vars)

SHOULD:
     try to push new release on CPAN.

APPROVED.
Comment 20 Parag AN(पराग) 2008-01-17 22:12:24 EST
I am really sorry here. I actually asked source tarball issue on #fedora-devel,
But I failed to put it in correct way. Now as Jason got the question what I want
to ask there and he already commented on this issues
https://bugzilla.redhat.com/show_bug.cgi?id=427479#c11, I prefer to take back
approved review.

Sorry Jfearn. If its not allowed in fedora then I was wrong in approving this
review.
Comment 21 Parag AN(पराग) 2008-01-17 22:14:53 EST
Created attachment 292102 [details]
patch to upstream tarball

Please package perl-XML-TreeBuilder-3.09 with patch attached here and submit
new package for review.
Comment 22 Parag AN(पराग) 2008-01-17 23:47:51 EST
oops I just took diff of upstream tarball and your tarball. please create new
patch and apply it in SPEC and submit new SRPM.
Comment 24 Parag AN(पराग) 2008-01-18 00:51:27 EST
This built successfully in koji.
But I don't think providing perl(XML::TreeBuilder) = 3.10 is valid as rpm
version is 3.09 it should provide => perl(XML::TreeBuilder) = 3.09

Comment 26 Parag AN(पराग) 2008-01-20 22:21:51 EST
Review:
+ package builds in mock (rawhide i386).
+ rpmlint is silent for SRPM and for RPM.
+ source files match upstream url
49e04fb6ba12b1414cb490e7f0c712a9  XML-TreeBuilder-3.09.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc is present.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no headers or static libraries.
+ no .pc file present.
+ no -devel subpackage
+ no .la files.
+ no translations are available
+ Does owns the directories it creates.
+ no scriptlets present.
+ no duplicates in %files.
+ file permissions are appropriate.
+ make test gave
t/00about....ok
t/10main.....ok
All tests successful.
Files=2, Tests=6,  0 wallclock secs ( 0.12 cusr +  0.01 csys =  0.13 CPU)
+ Package perl-XML-TreeBuilder-3.09-9.fc9 ->
  Provides: perl(XML::Element) = 3.09 perl(XML::TreeBuilder) = 3.09
  Requires: perl >= 1:5 perl(Carp) perl(HTML::Element) perl(HTML::Element) >=
3.08 perl(HTML::Tagset) perl(HTML::Tagset) perl(XML::Element) perl(XML::Parser)
perl(XML::Parser) perl(strict) perl(vars)
APPROVED.
Comment 27 Jeff Fearn 2008-01-24 00:12:46 EST
New Package CVS Request
=======================
Package Name: perl-XML-TreeBuilder
Short Description: Parser that builds XML trees
Owners: jfearn
Branches: F-8 EL-4 EL-5
InitialCC: mdious
Cvsextras Commits: yes
Comment 28 Jens Petersen 2008-01-24 01:10:12 EST
Thanks, cvs admin done.
Comment 29 Jeff Fearn 2008-01-24 18:24:01 EST
Built OK.

http://koji.fedoraproject.org/koji/taskinfo?taskID=371252
Comment 30 Ruediger Landmann 2013-06-07 03:12:39 EDT
Package Change Request
======================
Package Name: perl-XML-TreeBuilder
New Branches: el6
Owners: rlandmann
Comment 31 Jon Ciesla 2013-06-07 08:42:48 EDT
Git done (by process-git-requests).

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