[Bug 408941] Review Request: Unicornscan - Scalable, Accurate, Flexible, and Efficient Network Probing

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: Unicornscan - Scalable, Accurate, Flexible, and Efficient Network Probing


https://bugzilla.redhat.com/show_bug.cgi?id=408941





------- Additional Comments From wolfy at nobugconsulting.ro  2007-12-21 12:18 EST -------
A couple of things that I have noticed:

1. could you please rename the spec to "unicornscan.spec" ? the naming scheme
you are using contradicts
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-625ebbc8678382beca8d0b02504d30e7b6f23791

2. are the %{_libdir}/unicornscan/modules/*.la files needed for runtime usage ?
I could not test because of issue number 3 below, but if they are needed, please
make a note about that in the spec.

3. the package still does not build in mock, because of PostgreSQL. I have
attached the build log.

4. please use %configure, not ./configure and use macros, not absolute paths. I
have attached a modified spec which fixes this.

5. Please do NOT link against a private copy of a library (ltdl in this case),
you should use the ones provided by the system. See also
http://fedoraproject.org/wiki/Packaging/Guidelines#head-17396a3b06ec849a7c0c6fc3243673b17e5fed90
In order to avoid using your own copy of ltdl, you have to BR libtool-ltld-devel
(see my spec)

6. The configure script seems happier after adding libcap-devel and
libdnet-devel. Any particular reason to avoid using them ?

7.rpmlint complains about the source rpm:
unicornscan.src: E: invalid-spec-name unicornscan-0.4.7.spec
unicornscan.src:25: E: configure-without-libdir-spec
unicornscan.src:30: E: hardcoded-library-path in
$RPM_BUILD_ROOT/usr/lib/unicornscan/modules/http.a
unicornscan.src:31: E: hardcoded-library-path in
$RPM_BUILD_ROOT/usr/lib/unicornscan/modules/httpexp.a
unicornscan.src:32: E: hardcoded-library-path in
$RPM_BUILD_ROOT/usr/lib/unicornscan/modules/ntalk.a
unicornscan.src:33: E: hardcoded-library-path in
$RPM_BUILD_ROOT/usr/lib/unicornscan/modules/osdetect.a
unicornscan.src:34: E: hardcoded-library-path in
$RPM_BUILD_ROOT/usr/lib/unicornscan/modules/pgsqldb.a
unicornscan.src:35: E: hardcoded-library-path in
$RPM_BUILD_ROOT/usr/lib/unicornscan/modules/rdns.a
unicornscan.src:36: E: hardcoded-library-path in
$RPM_BUILD_ROOT/usr/lib/unicornscan/modules/sip.a
unicornscan.src:37: E: hardcoded-library-path in
$RPM_BUILD_ROOT/usr/lib/unicornscan/modules/upnp.a
unicornscan.src: W: summary-ended-with-dot Scalable, Accurate, Flexible, and
Efficient Network Probing.
The hardcoded-library stuff are non issues because you delete the files before
inclusion, but the rest could (and should) be easily fixed.

8. I did not verify ALL the files, but at least the dozen of .c and .h I have
examined specify
 * This program is free software; you can redistribute it and/or      *
 * modify it under the terms of the GNU General Public License        *
 * as published by the Free Software Foundation; either               *
 * version 2 of the License, or (at your option) any later            *
 * version. 
Therefore please
- make sure all the files specify the license they are released under (for
instance the src/output_modules/database/attic/mysqldb.c does not have a license
specified but mentions GPLv2... unlike the other sources I have examined)
- update the license tag of the rpm to GPLv2+ instead of GPLv2.

9.Since you have decided to include a LICENSE file in the tarball, you
absolutely MUST include it. See also
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#head-90d644ce2c5db60bad3ba8773fe11653c7629dc3

10. I fail to see the usefulness of including the INSTALL file. On the other
hand, please make sure that none of the other INSTALL.xxx files is useful. I for
one would like to see a unicorn-frontend.rpm including the www-front-end
directory (and maybe the related INSTALL file). That is, assuming the php stuff
over there really works. I have never tested that.

11. And last but not least, please add a corresponding changelog entry for each
modification of the spec file.



-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.