Bug 239087 (perl-Nmap-Parser) - Review Request: perl-Nmap-Parser - Parse nmap scan data with perl
Summary: Review Request: perl-Nmap-Parser - Parse nmap scan data with perl
Alias: perl-Nmap-Parser
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Chris Weyl
QA Contact: Fedora Package Reviews List
Keywords: Reopened
Depends On:
Blocks: 239088
TreeView+ depends on / blocked
Reported: 2007-05-04 20:19 UTC by Sindre Pedersen Bjørdal
Modified: 2012-01-15 22:05 UTC (History)
2 users (show)

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

Attachments (Terms of Use)

Description Sindre Pedersen Bjørdal 2007-05-04 20:19:22 UTC
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

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 

Comment 1 Jason Tibbitts 2007-05-08 20:16:35 UTC
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 
  W: perl-Nmap-Parser spurious-executable-perm 
Executable README and LICENSE files?

  E: perl-Nmap-Parser wrong-script-end-of-line-encoding 

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 
The manifest file isn't to be packaged.

   W: perl-Nmap-Parser spurious-executable-perm 
   E: perl-Nmap-Parser wrong-script-end-of-line-encoding 

   E: perl-Nmap-Parser script-without-shebang 
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 
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 22:29:08 UTC

- 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

rpmlint is silent here now.

Comment 3 Ralf Corsepius 2007-05-09 07:25:19 UTC
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 12:24:47 UTC
- Move end-of-line fix to %prep
- Update dos2unix BR

Spec URL: http://folk.ntnu.no/sindrb/packages/green_nyc/perl-Nmap-Parser.spec

Comment 5 Chris Weyl 2007-05-09 14:16:10 UTC
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 = 1.05-3.fc6
== requires
+ %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.


Comment 6 Sindre Pedersen Bjørdal 2007-05-09 15:26:19 UTC
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 15:07:10 UTC
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 04:21:32 UTC
F-7 branch added

Comment 9 Jens Petersen 2007-05-27 04:35:31 UTC
Changing state back to how it was before cvs request.

Comment 10 Iain Arnell 2012-01-15 09:28:12 UTC
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 Gwyn Ciesla 2012-01-15 22:04:52 UTC

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