[Bug 378791] Review Request: netdump-server - netdump crash recovery capture server

[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: netdump-server - netdump crash recovery capture server


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


bugzilla at redhat.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|devel                       |rawhide

tibbs at math.uh.edu changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|nobody at fedoraproject.org    |tibbs at math.uh.edu
             Status|NEW                         |ASSIGNED
               Flag|                            |fedora-review?




------- Additional Comments From tibbs at math.uh.edu  2007-12-02 20:14 EST -------
OK, this one builds for me, but some issues remain.  This has turned out to be a
bit more complicated than I thought it would be.

rpmlint still has the same complaint:
  netdump-server.x86_64: W: incoherent-version-in-changelog -0.7.16-17 
   0.7.16-17.fc9
This is due to the lack of a space between the dash and the version.  I know
it's nitpicking, but
http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs is rather specific
and we do have code which parses changelog entries.

Looking more closely at your scriptlets, I see that you're restarting the
service in %post while the recommended scriptlets at
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#services do it in
%postun.  I'll admit that I don't fully understand why, and I'm no fan of
blindly following arcane knowledge, but there must be some reason for the more
complicated version.  I'm trying to figure that out now.

The Summary: is a little confusing; it says "Client setup..." but the package is
netdump-server.

I don't see anything in the package which indicates that indicates the GPL
version.  Your spec says GPLv2 but I don't see where that comes from.

Looking at the build process, I see that the compiler isn't being called with
the proper set of flags.  -g is there so at least the debuginfo looks OK, but
the rest of the flags aren't.  A simple override of CFLAGS on the make line
doesn't work, but overriding DEBUG_FLAGS does:
  make DEBUG_FLAGS="%{optflags}" %{?_smp_mflags}

I note that /var/crash is owned in rawhide by kexec-tools.  Normally this
wouldn't be a significant problem, but kexec-tools has the directory owned by
root while this package has it owned by netdump:netdump.  So at minimum this
package needs to conflict with kexec-tools (and the reverse), but perhaps it
would be prudent to consider having this package use something other than
/var/crash.


* No upstream to compare source files against.
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
? summary is somewhat confusing.
* description is OK.
* dist tag is present.
* build root is OK.
? license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* BuildRequires are proper.
X compiler flags are not correct.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly
* debuginfo package looks complete.
X rpmlint still has a valid complaint.
* final provides and requires are sane:
   config(netdump-server) = 0.7.16-17.fc9
   netdump-server = 0.7.16-17.fc9
     =
   /bin/sh
   /sbin/ifconfig
   /usr/bin/ssh
   /usr/bin/ssh-keygen
   config(netdump-server) = 0.7.16-17.fc9
   fileutils
   gawk
   libglib-1.2.so.0()(64bit)
   libpopt.so.0()(64bit)
   libpopt.so.0(LIBPOPT_0)(64bit)
   shadow-utils
   textutils

* %check is not present; no test suite upstream.  I have no means to test this.
* no shared libraries are added to the regular linker search paths.
? /var/crash ownership causes some issues.
*  no duplicates in %files.
* file permissions are appropriate.
? service restart scriptlet is not the recommended one.
* 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 static libraries.
* no libtool .la files.

-- 
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.