[Bug 219025] Review Request: ntop - A network traffic probe similar to the UNIX top command

[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: ntop - A network traffic probe similar to the UNIX top command


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





------- Additional Comments From mtasaka at ioa.s.u-tokyo.ac.jp  2006-12-11 11:42 EST -------
Well, before checking 3.2-3 :

>> * %config(noreplace) %{_sysconfdir}/logrotate.d/ntop
>>   I think Requires: %{_sysconfdir}/logrotate.d or 
>>   Requires: logrotate should be added.

> Please read this thread:
> http://article.gmane.org/gmane.linux.redhat.fedora.devel/46042

Actually I have already known these long discussion because
I always receive mails from fedora-extras-list.

Anyway leaving /etc/logrotate.d unowned by any pacakge is
wrong. You should either
- require logrotate as Requires
- require /etc/logrotate.d
- have this package (ntop) own /etc/logrotate.d (easiest)
*for now* and my thought is this package (ntop)
should require logroate.

>> * Usually calling userdel or groupdel is not recommended.
>>   Usually it is left as it is and deleting user or group should
>>  be manually done by administrator.

> I've been looking for any packaging guidelines regarding this.  It seems sloppy
> to me to leave (program) users hanging around.

I checked http://fedoraproject.org/wiki/PackageUserCreation and
your usermgmt usage seems okay.

>> * For Requires:
>>   Please don't write the explicit dependency which is required
>>   automatically by dependencies of libraries.
>>   For example, libgd.so.2 dependency pulls gd, so adding "gd"
>>   explicitly to Requires is not needed.

> I will work on this. How good is RPM?  How far can it be trusted to find the
> right dependencies?

rpm uses ldd and objdump to check libraries' dependency and this
_should_ work ( _should_ means that if this does not work,
it means that some rpms installed together should be fixed because
they conflict functionally anyway).
Details are on: /usr/lib/rpm/redhat/find-requires

And, writing like "Requires: openssl" does not make sense
because requirement of openssl or so is "version specific".
This is correctly treated by libraries' dependencies, where 
"explicit" requirement of rpm name does not care about these.

Note: If this package needs another "version specific" issue,
this should be written to Requires. A example is xscreensaver-base
(which I maintain), which has a explicit requirement

"pam > 0.80-7" 

because /etc/pam.d/xscreensaver says:
-----------------------------------------------------
auth       include      system-auth
-----------------------------------------------------
this content can be interpreted only by pam >= 0.80


>> * # strip off version number from plugin .so files
>>   Why is this needed?

> This was suggested by Patrice here:
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=197198#c37.  This makes
> sense to me as dl'opened modules aren't versioned.

Leaving symlinks as they are is sufficient. Please check:
-------------------------------------------------------------------------
[tasaka1 at localhost ~]$ for f in /usr/lib/*/*.so ; do  if [ -L $f ] ; then echo
$f ; fi ; done
-------------------------------------------------------------------------
I don't think removing version info is a good idea.

* Another issue:
-----------------------------------
%post
/sbin/chkconfig --add %{name} 2>&1 > /dev/null
-----------------------------------
  scriptlets like these need corresponding requirement as
  Requires(post) or so. Please check the section "Services" of
  http://fedoraproject.org/wiki/Packaging/ScriptletSnippets

By the way, is this okay?
---------------------------------------------------------
 gcc -DHAVE_CONFIG_H -I. -I.. -I.. -I../myrrd -DLINUX -I/usr/include/libgdome
-I/usr/local/include -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables -I/usr/local/include -Wshadow -Wpointer-arith
-Wmissing-prototypes -Wmissing-declarations -Wnested-externs -fPIC -DPIC -MT
libxmldumpPlugin_la-xmldumpPlugin.lo -MD -MP -MF
.deps/libxmldumpPlugin_la-xmldumpPlugin.Tpo -c xmldumpPlugin.c  -fPIC -DPIC -o
.libs/libxmldumpPlugin_la-xmldumpPlugin.o
xmldumpPlugin.c:50:2: warning: #warning 
xmldumpPlugin.c:51:2: warning: #warning
===========================================================
xmldumpPlugin.c:52:2: warning: #warning 
xmldumpPlugin.c:53:2: warning: #warning Missing header files, disabling xmldump
plugin
xmldumpPlugin.c:54:2: warning: #warning 
xmldumpPlugin.c:55:2: warning: #warning FOR MOST USERS THIS IS NOT A PROBLEM
xmldumpPlugin.c:56:2: warning: #warning ntop will build and run just fine...
xmldumpPlugin.c:57:2: warning: #warning 
xmldumpPlugin.c:58:2: warning: #warning Why?
xmldumpPlugin.c:59:2: warning: #warning 
xmldumpPlugin.c:61:2: warning: #warning glibconfig.h unavailable
xmldumpPlugin.c:64:2: warning: #warning glib.h unavailable
xmldumpPlugin.c:67:2: warning: #warning gdome.h unavailable
xmldumpPlugin.c:72:2: warning: #warning 
xmldumpPlugin.c:73:2: warning: #warning
===========================================================
xmldumpPlugin.c:74:2: warning: #warning 
xmldumpPlugin.c:1076:1: warning: multi-line comment
xmldumpPlugin.c:28: warning: 'dumpXML' used but never defined
xmldumpPlugin.c:107: warning: 'traceEvent_forked' declared 'static' but never
defined
xmldumpPlugin.c:242: warning: 'xmlDebug' defined but not used
xmldumpPlugin.c:470: warning: 'handleXmldumpHTTPrequest' defined but not used
---------------------------------------------------------

I have not checked this package 'functionally' yet.

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.