Spec URL: http://people.redhat.com/nhorman/rpms/netdump-server.spec SRPM URL: http://people.redhat.com/nhorman/rpms/netdump-server-0.7.16-15.fc8.src.rpm Description: netdump-server is teh capture server for netdump core recovery clients. It is teh primary memory image capture tool used in a crash post-mortem analysis for pre-RHEL-5 /FC-6 based systems I'd like to get this reviewed please, for inclusion in Extras and EPEL.
Some comments: This fails to build for me due to a missing dependency on popt-devel. Adding that gets it building. Your BuildRoot: is incorrect; see http://fedoraproject.org/wiki/Packaging/Guidelines If you call useradd in %pre, you need to have Requires(pre): shadow-utils. You can't just Require: it. Probably best to follow the established guidelines for adding users/groups: http://fedoraproject.org/wiki/Packaging/UsersAndGroups License tag need to specify the version of the GPL which applies; see http://fedoraproject.org/wiki/Licensing Is there really no upstream source for this package? Let's go over the rpmlint output: netdump-server.x86_64: W: spurious-executable-perm /usr/share/doc/netdump-server-0.7.16/example_scripts/netdump-reboot netdump-server.x86_64: W: spurious-executable-perm /usr/share/doc/netdump-server-0.7.16/example_scripts/netdump-nospace netdump-server.x86_64: W: spurious-executable-perm /usr/share/doc/netdump-server-0.7.16/example_scripts/netdump-crash netdump-server.x86_64: W: spurious-executable-perm /usr/share/doc/netdump-server-0.7.16/example_scripts/netdump-start Generally documentation shouldn't be executable. netdump-server.x86_64: E: zero-length /var/crash/.ssh/authorized_keys2 Since comments are valid in that file, it might be nice to at least include one indicating what's supposed to go there. netdump-server.x86_64: E: non-readable /var/crash/.ssh/authorized_keys2 0600 This is OK; the file isn't supposed to be public. netdump-server.x86_64: W: hidden-file-or-dir /var/crash/.ssh This is OK; it's not as if you have a choice of what to name the .ssh directory. netdump-server.x86_64: W: incoherent-version-in-changelog -0.7.16-15 0.7.16-15.fc9 A space between the dash and the version should quiet this. See the Changelogs section of http://fedoraproject.org/wiki/Packaging/Guidelines netdump-server.x86_64: W: invalid-license GPL Need to specify GPL version. netdump-server.x86_64: W: no-url-tag Depends on whether there's really an upstream for this package. The rest are all non-standard-{uid,gid} complaints, which are OK in this case.
Spec URL: http://people.redhat.com/nhorman/rpms/netdump-server.spec SRPM URL: http://people.redhat.com/nhorman/rpms/netdump-server-0.7.16-16.fc8.src.rpm new package available "Your BuildRoot: is incorrect" fixed "call useradd in %pre..." fixed "License tag need to specify the version of the GPL" fixed "Is there really no upstream source for this package?" Nope. Netdump never made it upstream, nor did the server component "spurious-executable-perm " files removed. They arent needed/helpful "zero-length /var/crash/.ssh/authorized_keys2" commdent added to file "incoherent-version-in-changelog" should be fixed
> Nope. Netdump never made it upstream, nor did the server component Then please see the "We are Upstream" section of http://fedoraproject.org/wiki/Packaging/SourceURL Unfortunately the -16 release still fails to build for me. Here's a scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=253474 Scratch builds are really easy to make: koji build --scratch dist-f9 netdump-server-0.7.16-16.fc8.src.rpm If adding a build dependency on popt-devel isn't the way you want to solve this, then what should be done?
thanks, I built locally in amock bukildroot that already had popt-devel. sorry I missed the dep. new pkg available with needed fixes: Spec URL: http://people.redhat.com/nhorman/rpms/netdump-server.spec SRPM URL: http://people.redhat.com/nhorman/rpms/netdump-server-0.7.16-17.fc8.src.rpm
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.
ok, new version available: Spec URL: http://people.redhat.com/nhorman/rpms/netdump-server.spec SRPM URL: http://people.redhat.com/nhorman/rpms/netdump-server-0.7.16-18.fc8.src.rpm >netdump-server.x86_64: W: incoherent-version-in-changelog -0.7.16-17 Fixed >Looking more closely at your scriptlets, I see that you're restarting the service in %post while the recommended scriptlets at... I think the %post condrestart is vestigual. I've added an identical %postun scriptles to be compliant. I'll remove the %post script at a later date, when I can confirm that its no longer needed. >The Summary: is a little confusing Fixed >I don't see anything in the package which indicates that indicates the GPL version See COPYING in the docs. Its included in the srpm >Looking at the build process, I see that the compiler isn't being called with the proper set of flags Fixed >I note that /var/crash is owned in rawhide by kexec-tools I've moved this package to write to /var/netdump/. We need to allow this package and kexec-tools to co-exist.
I'll look more closely after work, but I can note now that the COPYING file is simply the usual GPL notice and does not indicate which version of the GPL the sources are under. See http://fedoraproject.org/wiki/Licensing and specifically the statement: " A GPL or LGPL licensed package that lacks any statement of what version that it's licensed under in the source code/program output/accompanying docs is technically licensed under *any* version of the GPL or LGPL, not just the version in whatever COPYING file they include. " So what I'm not finding is any statement in the source or documentation indicating just what version of the GPL this program is under. Or at least I hadn't, but now I see that netdump-server.8 includes comments (not visible in the formatted output) with a proper GPL notice saying "version 2 or later". So if that's to believed, then the proper License: tag should contain "GPLv2+". But honestly that's not the best place for the only license notice to go.
OK, your move to /var/netdump has elicited the following from rpmlint: netdump-server.x86_64: W: non-standard-dir-in-var netdump which I don't suppose is problematic. Personally I'd think /var/lib/netdump would be better, but if /var/crash is there and this stores the same data then I don't see why it shouldn't be in the same place. The %postun scriptlet looks proper but there's certainly no reason to restart the service twice on upgrade. Currently you're doing a condrestart of the service in %post at install time ($1 == 1) and at upgrade time ($1 == 2), and then at upgrade time in %postun. Honestly, I can't see what the point of that is; just restart it in %postun like any other service and be done with it. At this point I think the package is fine save for two issues: License: tag. Please try to determine whether this is GPLv2 or GPLv2+ (or something else) and then set the License: tag appropriately. Judging from the only statement anywhere in the package (in comments at the start of netdump-server.8) I'd say GPLv2+ is proper. Scriptlets. Please just remove the condrestart call from %post. I really can't envision a reason for the way it is now. Or, if you'd rather not, we can get a third party to comment.
Spec URL: http://people.redhat.com/nhorman/rpms/netdump-server.spec SRPM URL: http://people.redhat.com/nhorman/rpms/netdump-server-0.7.16-19.fc8.src.rpm I fixed up the %post script appropriately. As for the license, the COPYING file which is included with both the SRPM and the RPM clearly indicates that the distribution is under the terms of strict GPLv2
Neil, The GPL is unique, in that it explicitly says: "If the Program does not specify a version number of this License, you may choose any version ever published by the Free Software Foundation." Thus, we cannot go off of the versioning in COPYING, we need to look at what the code itself says. (This is why it is important to include license/copyright information in code headers). Most of the code in this package has no license or copyright attribution, but one of the headers (netconsole.h) specifies GPLv2+, so go with GPLv2+ for the license tag.
No. The one header that includes copyright information specifies that we can use a later version than GPLv2 at my option. I'm opting not to. The COPYING file indicates GPLv2, thats what I'm going with.
That is simply wrong. Blocking this package until the License tag is corrected.
So I'm told that in bug 248013, which I cannot access but which I see has now been closed, Neil indicated that he would not make the change and that he would rather abandon this package than make the change. For my part, I simply cannot approve the package with an incorrect License: tag. So I guess this ticket should be closed. If someone comes along that doesn't object to the single '+' and wishes to take over this submission, I will be happy to approve it with that change.
you were told wrong. I closed the bug you reference as wonfix, because I won't change the license without more authority than that which tom represents. I'll happily reopen it in the event that Tom comes around on this issue, or a legal authority indicates that its ok to change (which I've sent a private email off to request). I specifically left this bug open to handle that resolution. Your assumption that the License tag is currently incorrect is in question, and waiting for a legal opinion to resolve.
You're welcome to wait for a lawyer to talk with you on this issue, but the Fedora licensing policies are very clear on this issue. COPYING is trumped by what the code says. Alternately, you might consider asking Ingo Molnar what the license should be, since he wrote that code.
Mingo isn't the only one (although I'm getting hold of him as well). Of the three places you have noted the possibility of a GPLv2+ option: 2 are not code at all, but rather man pages, documentation no different from a COPYING file. These are not written by Ingo, but rather the project author, Alex Larson, who is no longer here. Of these two man pages, the netdump.8 man page is no longer relevant to the package and should be removed (given that the functionality it documents is no longer present) 1 header file , whos contents are borrowed from the kernel netdump.h header file, which is covered strictly by GPLv2. Clearly this issue is not as cut and dry as you would like to make it out to be.
The header file in question is CLEARLY marked as GPLv2 or later. The GPLv2 or later license is the ONLY license that appears in the source. The text of the GPL says that if a version is specified, then that is the version of the license to be used. The version specified is GPLv2 or later. COPYING does NOT trump this. I honestly still cannot believe we're having this conversation.
"The version specified is GPLv2 or later. COPYING does NOT trump this." So I've copied you on the email I sent to red hat legal regarding this, as well as on the note to Ingo requesting clarification on his license (given that its included in a project listed as GPLv2 only). I'll wait to see what they say. If you can point me to a documentation from some legal authority that clearly states the COPYING file is irrelevant when a conflicting statement is contained in a piece of code, I'll consider changing it. You had mentioned that you had discussed this at length with Mark W., I imagine thats written down somewhere.
Spec URL: http://people.redhat.com/nhorman/rpms/netdump-server.spec SRPM URL: http://people.redhat.com/nhorman/rpms/netdump-server-0.7.16-20.fc8.src.rpm As per the legal response to this issue, I've posted an update to this rpm. Same as previous, with additions to all the files to make them unambiguously GPLv2.
License matches the code now. Setting this review to +.
I concurr. APPROVED
New Package CVS Request ======================= Package Name: netdump-server Short Description: Server for network kernel message logging and crash dumps Owners: nhorman Branches: EL-5
cvs done.
I just happened to catch something odd which I didn't notice before: a bunch of cc: unrecognized option '-j8' messages, arising from this: %build export CFLAGS="%{optflags} %{?_smp_mflags} `glib-config --cflags`"; make %{?_smp_mflags} should be passed to make, not inserted into CFLAGS, like so: export CFLAGS="%{optflags} `glib-config --cflags`"; make %{?smp_mflags}
yeah, I just saw that too, EL-5 also seems to have some issues with glib-config too that should be fixed in a separate bug. I'll take care of them shortly.
Can we close this now?
yep, all done. Thanks
Closing.