Fedora Merge Review: ethtool http://cvs.fedora.redhat.com/viewcvs/devel/ethtool/ Initial Owner: jgarzik
* RPM name is OK * Source ethtool-5.tar.gz is the same as upstream * This is the latest version * Builds fine in mock * File list looks OK Needs work: * BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) (wiki: PackagingGuidelines#BuildRoot) * Missing SMP flags. If it doesn't build with it, please add a comment (wiki: PackagingGuidelines#parallelmake) * Spec file: some paths are not replaced with RPM macros (wiki: QAChecklist item 7) * The %makeinstall macro should not be used (wiki: PackagingGuidelines#MakeInstall) Rpmlint is not clean: [ruben@odin ethtool]$ rpmlint -i ethtool-5-1.fc6.i386.rpm W: ethtool symlink-should-be-relative /usr/sbin/ethtool /sbin/ethtool Absolute symlinks are problematic eg. when working with chroot environments.
I [cw]ould maintain ethtool for Fedora - but I think, I can't for RHEL ;-) If somebody from Red Hat comes up for RHEL, please let me know, whether he or she wants or not wants to maintain ethtool in Fedora as well...
Jeff, ping? Can you please follow the review and perform the suggested changes?
I signed up for co-maintainer as Jeff seems to be AWOL. Looks like he's maybe a better upstream maintainer rather a downstream one - sorry Jeff. When looking around in Red Hat Bugzilla for open bug reports, I also had a look to upstream version control system in order to gather maybe some patches solving mentioned issues or even referenced fixes being not in Fedora: git clone git://git.kernel.org/pub/scm/network/ethtool/ethtool.git Unluckily, I've now to add a FE_LEGAL blocker as Jeff was somehow so nice to remove *any* hint how ethtool is now licensed after version 6. And the only license hint in this clone seems to be just the man page - which is referring in a comment, that the man page itself (and just the man page) is licensed as "GNU Public License." - could be a mislabeled GPL. So: Tom, your turn first before continuing with something else in this review.
It's not missing licensing at all. The bog-standard COPYING file is present in every release tarball, including the one in each Fedora and RHEL source rpm. And autogen.sh installs COPYING locally via automake if you are a developer building the git repo. It is 100% standard GNU autotools setup including license.
So depending on the Version of automake I'm using to build (the package from vcs) I get a different license (GPLv2+ vs. GPLV3+)? Can I change the license at random by building it using evilmake, my very own automake-fork?
Following from Freenode #fedora-devel (time is UTC+01:00): [23:19:32] < spot> rsc: jgarzik is going to add a LICENSE file to git that states the licensing
Tom has clarified, that the legal points are solved for Fedora, removing the blocker. --- snipp --- Date: Fri, 16 Jan 2009 09:53:41 -0500 From: Tom spot Callaway To: Robert Scheck Cc: Sven Lankes, Jeff Garzik Subject: Re: COPYING for ethtool On Fri, 2009-01-16 at 15:50 +0100, Robert Scheck wrote: > On Fri, 16 Jan 2009, Tom spot Callaway wrote: > > As Jeff has pointed out, he's not inserting COPYING, the autotool stack > > is. This is one main reason why we cannot go off the version it uses. > > GPLv3 COPYING gets stuck in all sorts of things that are not GPLv3 as a > > result. > > That's wrong. It's caused, because he's putting no COPYING there in GIT > already and thus the --add-missing at autotools are putting that into. As > far as I know, an existing COPYING gets not overwritten except with use > of maybe --force (which can be overwritten by IIRC --foreign). Well, feel free to try to convince him. We don't need this additional step for Fedora licensing. ~spot --- snapp --- To continue the review as co-maintainer, I've made multiple changes to the package, I'm suggesting the following for formal review. Update to git seems to be necessary for me as it solves some bug reports and feature requests. In order to make me and Sven happy, I've solved the COPYING thing downstream. Spec URL: http://labs.linuxnetz.de/bugzilla/ethtool.spec SRPM URL: http://labs.linuxnetz.de/bugzilla/ethtool-6-2.20090115git.src.rpm Sven, will you go for the formal review?
Package Review ============== Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines. [x] Spec file name must match the base package %{name}, in the format %{name}.spec. [x] Package meets the Packaging Guidelines. [x] Package successfully compiles and builds into binary rpms on at least one supported architecture. Tested on: devel/x86_64 [x] Rpmlint output: source RPM: empty binary RPM:empty [x] Package is not relocatable. [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)) [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x] License field in the package spec file matches the actual license. License type: GPLv2 [x] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x] Spec file is legible and written in American English. [x] Sources used to build the package match the upstream source, as provided in the spec URL. SHA1SUM of package: e2cc807b553da0f7df337638aeb4319afd6f48db ethtool-6.tar.gz Note: this is a snapshot version [x] Package is not known to require ExcludeArch [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [-] The spec file handles locales properly. [-] ldconfig called in %post and %postun if required. [x] Package must own all directories that it creates. [x] Package requires other packages for directories it uses. [x] Package does not contain duplicates in %files. [x] Permissions on files are set properly. [x] Package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT. [x] Package consistently uses macros. [x] Package contains code, or permissable content. [-] Large documentation files are in a -doc subpackage, if required. [x] Package uses nothing in %doc for runtime. [-] Header files in -devel subpackage, if present. [-] Static libraries in -devel subpackage, if present. [-] Package requires pkgconfig, if .pc files are present. [-] Development .so files in -devel subpackage, if present. [-] Fully versioned dependency in subpackages, if present. [x] Package does not contain any libtool archives (.la). [-] Package contains a properly installed %{name}.desktop file if it is a GUI application. [x] Package does not own files or directories owned by other packages. [x] Final provides and requires are sane. === SUGGESTED ITEMS === [x] Latest version is packaged. [x] Package does not include license text files separate from upstream. [ ] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x] Reviewer should test that the package builds in mock. Tested on: devel/x86_64 [x] Package should compile and build into binary rpms on all supported architectures. Tested on: koji [x] Package functions as described. [-] Scriptlets must be sane, if used. [-] The placement of pkgconfig(.pc) files is correct. [-] File based requires are sane. [x] %check is present and the test passes. === Issues === 1. Timestamps are not preserved 2. autoconf is already brought in by automake so it's not really needed to BR it ================ *** APPROVED *** ================
Comment #10 was meant for the src.rpm from #9...
Manuel, thanks for doing the review. As we've spoken in IRC, preserving timestamps is not supported in GIT. Seems that old-style CVS and SVN have a useful feature more at that point rather the cool GIT... ;-) I've commited my updates into CVS, tagged and built packages from it.
ethtool-6-2.20090115git.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/ethtool-6-2.20090115git.fc9
ethtool-6-2.20090115git.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/ethtool-6-2.20090115git.fc10