Bug 239087 - (perl-Nmap-Parser) Review Request: perl-Nmap-Parser - Parse nmap scan data with perl
Review Request: perl-Nmap-Parser - Parse nmap scan data with perl
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Chris Weyl
Fedora Package Reviews List
: Reopened
Depends On:
Blocks: 239088
  Show dependency treegraph
 
Reported: 2007-05-04 16:19 EDT by Sindre Pedersen Bjørdal
Modified: 2012-01-15 17:05 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-05-27 00:35:31 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
cweyl: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Sindre Pedersen Bjørdal 2007-05-04 16:19:22 EDT
Spec URL: http://folk.ntnu.no/sindrb/packages/green_nyc/perl-Nmap-Parser.spec
SRPM URL: http://folk.ntnu.no/sindrb/packages/green_nyc/perl-Nmap-Parser-1.05-1.fc7.src.rpm

Description:
This module implements a interface to the information contained in an nmap 
scan. It is implemented by parsing the xml scan data that is generated by 
nmap. This will enable anyone who utilizes nmap to quickly create fast and 
robust security scripts that utilize the powerful port scanning abilities of 
nmap.
Comment 1 Jason Tibbitts 2007-05-08 16:16:35 EDT
I was typing this in before cweyl took this for review; hopefully the comments
will still be useful.

Some rpmlint complaints:
  E: perl-Nmap-Parser no-changelogname-tag
The changelog is empty

  W: perl-Nmap-Parser spurious-executable-perm 
  /usr/share/doc/perl-Nmap-Parser-1.05/README
  W: perl-Nmap-Parser spurious-executable-perm 
  /usr/share/doc/perl-Nmap-Parser-1.05/LICENSE
Executable README and LICENSE files?

  E: perl-Nmap-Parser wrong-script-end-of-line-encoding 
  /usr/share/doc/perl-Nmap-Parser-1.05/README

Perhaps this package came from DOS or something.  All of the line endings need
to be fixed up with a quick trip through tr or sed.

   W: perl-Nmap-Parser manifest-in-perl-module 
   /usr/share/doc/perl-Nmap-Parser-1.05/MANIFEST
The manifest file isn't to be packaged.

   W: perl-Nmap-Parser spurious-executable-perm 
   /usr/share/doc/perl-Nmap-Parser-1.05/MANIFEST
   E: perl-Nmap-Parser wrong-script-end-of-line-encoding 
   /usr/share/doc/perl-Nmap-Parser-1.05/MANIFEST
Hmmm....

   E: perl-Nmap-Parser script-without-shebang 
   /usr/lib/perl5/vendor_perl/5.8.8/Nmap/Parser.pm
This shouldn't be executable.  Or, if it needs to be executable, it needs to
have the usual #!/usr/bin/perl as its first line.

   E: perl-Nmap-Parser wrong-script-end-of-line-encoding 
   /usr/lib/perl5/vendor_perl/5.8.8/Nmap/Parser.pm
tr or sed again.

Next, you'll need BuildRequires: perl(ExtUtils::MakeMaker,) perl(Test::More)
so that your package will still build and run its tests once the perl-devel
split from the main perl package is complete.
Comment 2 Sindre Pedersen Bjørdal 2007-05-08 18:29:08 EDT
Updated:

- Don't package Manifest
- Fix permissions
- Fix end-of-line encoding
- Add missing BRs

Spec URL: http://folk.ntnu.no/sindrb/packages/green_nyc/perl-Nmap-Parser.spec
SRPM URL:
http://folk.ntnu.no/sindrb/packages/green_nyc/perl-Nmap-Parser-1.05-2.fc7.src.rpm

rpmlint is silent here now.
Comment 3 Ralf Corsepius 2007-05-09 03:25:19 EDT
Some remarks:

* You are using "dos2unix" to convert files.
Please use /usr/bin/dos2unix instead
and add
BuildRequires: /usr/bin/dos2unix

* Please move file conversion and permission fixing to %prep instead of %install
and fix the source files instead of the installed files.
Comment 4 Sindre Pedersen Bjørdal 2007-05-09 08:24:47 EDT
Updated: 
- Move end-of-line fix to %prep
- Update dos2unix BR

Spec URL: http://folk.ntnu.no/sindrb/packages/green_nyc/perl-Nmap-Parser.spec
SRPM URL:
http://folk.ntnu.no/sindrb/packages/green_nyc/perl-Nmap-Parser-1.05-3.fc7.src.rpm
Comment 5 Chris Weyl 2007-05-09 10:16:10 EDT
Now that tibbs and Ralf have done the hard work, I'll post my review :)

Note that you can also use sed itself to fix end-of-line-encoding warnings;
"sed -i '/\r//' filename" will do it.  I'm just pointing this out, it's
certainly not a blocker to use dos2unix.

+ source files match upstream:
 5d5f113a9d166b07e041a5dc52f9c3ee  Nmap-Parser-1.05.tar.gz
 5d5f113a9d166b07e041a5dc52f9c3ee  ../Nmap-Parser-1.05.tar.gz
+ package meets naming and versioning guidelines.
+ specfile is properly named, is cleanly written and uses macros consistently.
+ dist tag is present.
+ build root is correct.
+ license field matches the actual license.
+ license is open source-compatible.  License text included in package.
+ latest version is being packaged.
+ BuildRequires are proper.
+ compiler flags are appropriate. (noarch)
+ %clean is present.
+ package installs properly
+ rpmlint is silent.
+ final provides and requires are sane:
** perl-Nmap-Parser-1.05-3.fc6.noarch.rpm
== rpmlint
== provides
perl(Nmap::Parser) = 1.05
perl(Nmap::Parser::Host)  
perl(Nmap::Parser::Host::OS)  
perl(Nmap::Parser::Host::Service)  
perl(Nmap::Parser::Session)  
perl-Nmap-Parser = 1.05-3.fc6
== requires
perl(:MODULE_COMPAT_5.8.8)  
perl(Storable)  
perl(XML::Twig)  
perl(strict)  
perl(vars)  
+ %check is present and all tests pass:
 All tests successful.
 Files=4, Tests=171,  1 wallclock secs ( 1.07 cusr +  0.28 csys =  1.35 CPU)
+ no shared libraries are added to the regular linker search paths.
+ owns the directories it creates.
+ doesn't own any directories it shouldn't.
+ no duplicates in %files.
+ file permissions are appropriate.
+ no scriptlets present.
+ code, not content.
+ documentation is small, so no -docs subpackage is necessary.
+ %docs are not necessary for the proper functioning of the package.
+ no headers.
+ no pkgconfig files.
+ no libtool .la droppings.
+ not a GUI app.

APPROVED
Comment 6 Sindre Pedersen Bjørdal 2007-05-09 11:26:19 EDT
New Package CVS Request
=======================
Package Name: perl-Nmap-Parser
Short Description: Parse nmap scan data with perl
Owners: foolish@guezz.net
Branches: FC-5 FC-6 EL-4 EL-5
InitialCC: jaa@redhat.com
Comment 7 Sindre Pedersen Bjørdal 2007-05-23 11:07:10 EDT
Seems the F7 branch wasn't created during merge, please add it.

New Package CVS Request
=======================
Package Name: perl-Nmap-Parser
Short Description: Parse nmap scan data with perl
Owners: foolish@guezz.net
Branches: FC-5 FC-6 EL-4 EL-5 F-7
Comment 8 Jens Petersen 2007-05-27 00:21:32 EDT
F-7 branch added
Comment 9 Jens Petersen 2007-05-27 00:35:31 EDT
Changing state back to how it was before cvs request.
Comment 10 Iain Arnell 2012-01-15 04:28:12 EST
Package Change Request
======================
Package Name: perl-Nmap-Parser
InitialCC: perl-sig

Please add perl-sig to this package with watchbugzilla and watchcommits bits on all fedora branches.
Comment 11 Jon Ciesla 2012-01-15 17:04:52 EST
Done.

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