Bug 528847
Summary: | Review Request: Netpipe - A protocol independent network performance tool | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Scott Collier <boodle11> |
Component: | Package Review | Assignee: | Matt Domsch <matt_domsch> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | fedora-package-review, i, jeff_nichols, matt_domsch, notting, rc040203 |
Target Milestone: | --- | Flags: | matt_domsch:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | 3.7.1-2.fc11 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2009-10-19 21:04:10 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Scott Collier
2009-10-14 02:30:09 UTC
I can sponsor you. Just a few recommendations. The comments at the top of the file don't make sense in Fedora; just remove them. add %{?dist} tag on the Release field. %setup lacks -q flag make lacks CFLAGS="$RPM_OPT_FLAGS" to pick up standard options. Drop current setting of CFLAGS. fix permission of SPECS/netpipe.spec to be 0644. BuildRoot line could be one of the better ones. For F11 and higher it'll get ignored anyhow, but I like to use the best one available when we can. https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag Drop Obsoletes line. This is the first implementation for Fedora. remove executable permission on docs files. Or, actually install them into /usr/bin/ and include them in %files. If the latter, rpmbuild will automatically find the dependency on /bin/csh for you. ;-) be sure the manpage isn't marked %doc. once you've done the above, you can use %defattr(-, root, root, -) in %files. With these changes, rpmlint is silent. Then I'll do the formal review. Thanks, Matt Tarball is named NetPIPE => Fedora package name should be NetPIPE. good catch Ralf. Matt, Thanks for the review. Here's the modified spec file and source RPM: Spec URL: http://boodle.fedorapeople.org/RPMS/NetPIPE.spec SRPM URL: http://boodle.fedorapeople.org/RPMS/NetPIPE-3.7.1-1.2.fc11.src.rpm The old Spec file is still there. http://boodle.fedorapeople.org/RPMS/ I made the changes you recommended (I hope I got it right), please take another look. rpmlint is silent on the spec file and rpm: [collier_s@ws1 SPECS]$ rpmlint NetPIPE.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. [collier_s@ws1 SPECS]$ rpmlint ../RPMS/i586/NetPIPE-3.7.1-1.2.fc11.i586.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Please let me know if I need to make any other changes to move foward here. Ralf, Thanks for picking up that naming issue. I've corrected that as well. Let me know if you see anything else with this package and I'll work on it. A few more items: By changing the name, you can drop the define real_name and its use in %setup. https://fedoraproject.org/wiki/Licensing says that the License tag should be: License: GPL+ because no version is specified in the source. I think omitting the [fg]eplot and nplaunch files is wrong. You can chmod them 0644 and then include them in %doc. And bump the Release number, from 1.2 to 2, in case anyone happens to have the older release somehow. Thanks, Matt (In reply to comment #5) > A few more items: > > By changing the name, you can drop the define real_name and its use in %setup. done > > https://fedoraproject.org/wiki/Licensing > says that the License tag should be: > License: GPL+ > because no version is specified in the source. done > > I think omitting the [fg]eplot and nplaunch files is wrong. You can chmod them > 0644 and then include them in %doc. I wasn't sure how to handle those. I've added them back. They are bash scripts, so I put them in bindir with 0755. Is that OK? > > And bump the Release number, from 1.2 to 2, in case anyone happens to have the > older release somehow. done rpmlint on rpm: $ rpmlint ../RPMS/i586/NetPIPE-3.7.1-1.2.fc11.i586.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. rpmlint on spec: $ rpmlint ../RPMS/i586/NetPIPE-3.7.1-1.2.fc11.i586.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Spec URL: http://boodle.fedorapeople.org/RPMS/NetPIPE.spec SRPM URL: http://boodle.fedorapeople.org/RPMS/NetPIPE-3.7.1-1.2.fc11.src.rpm Thanks again for the guidance. -Scott > > Thanks, > Matt > rpmlint on rpm: > > $ rpmlint ../RPMS/i586/NetPIPE-3.7.1-1.2.fc11.i586.rpm > 1 packages and 0 specfiles checked; 0 errors, 0 warnings. > > rpmlint on spec: > > $ rpmlint ../RPMS/i586/NetPIPE-3.7.1-1.2.fc11.i586.rpm > 1 packages and 0 specfiles checked; 0 errors, 0 warnings. > > Spec URL: http://boodle.fedorapeople.org/RPMS/NetPIPE.spec > SRPM URL: http://boodle.fedorapeople.org/RPMS/NetPIPE-3.7.1-1.2.fc11.src.rpm Made a mistake when I uploaded my packages: $ rpmlint NetPIPE.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint ../SRPMS/NetPIPE-3.7.1-2.fc11.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Spec URL: http://boodle.fedorapeople.org/RPMS/NetPIPE.spec SRPM URL: http://boodle.fedorapeople.org/RPMS/NetPIPE-3.7.1-2.fc11.src.rpm [fg]eplot in particular should be in %doc I think, and unmarked as executable. They're very poor shell scripts, similar to each other and only really differing in where they're placing their output file, which doesn't take the output file name as an argument (it has a file name hardcoded, one in the current directory and one in ~/ but still poor form). That's crap I wouldn't want in /usr/bin. nplaunch is a useless wrapper around invoking ssh, and including it in /usr/bin/ sucks in tcsh as a dependency. Ugh. I think they're OK for %doc chmod 0644, but not in %bin. (In reply to comment #8) > [fg]eplot in particular should be in %doc I think, and unmarked as executable. > They're very poor shell scripts, similar to each other and only really > differing in where they're placing their output file, which doesn't take the > output file name as an argument (it has a file name hardcoded, one in the > current directory and one in ~/ but still poor form). That's crap I wouldn't > want in /usr/bin. > > nplaunch is a useless wrapper around invoking ssh, and including it in > /usr/bin/ sucks in tcsh as a dependency. Ugh. > > I think they're OK for %doc chmod 0644, but not in %bin. Ok. Done. Spec URL: http://boodle.fedorapeople.org/RPMS/NetPIPE.spec SRPM URL: http://boodle.fedorapeople.org/RPMS/NetPIPE-3.7.1-2.fc11.src.rpm No errors with rpmlint. Scott, looks great now. Thanks for your hard work on this. I would ask you to delete the %define real_name, as it's no longer used. You can do that at CVS checkin. APPROVED. New Package CVS Request ======================= Package Name: NetPIPE Short Description: A protocol independent network performance tool Owners: boodle Branches: F-10 F-11 F-12 EL-5 InitialCC: mdomsch For the record Name: OK spec name: OK packaging guidelines: ok license: ok license field: ok license text file: not present in upstream. ok. english spec: ok legible spec: ok sources match upstream: ok builds on x86_64: ok doesn't build on some arch: unknown, nothing indicated. ok. buildrequires: ok locales: not used, ok. shared libs: none. ok no system libs: ok relocatable: no. ok. own dirs: ok no duplicate files: ok file perms: ok clean section: ok consistent use of macros: ok code not content: code. ok large docs: none. ok. runtime docs: no. ok. header files: none. ok static libs: none. ok pkgconfig: none. ok devel versioned dep: none. ok. no .la: none. ok desktop file: none. ok dir ownership: ok install rm-rf: ok utf8: ok SHOULDs: license: noted above translated description: not available builds in mock: didn't try; builds locally, with no special BRs. builds for target arches: didn't try tested: not done during review scriptlets sane: none, ok. subpackages: none, ok pkgconfig: none, ok file deps: none, ok cvs done. NetPIPE-3.7.1-2.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/NetPIPE-3.7.1-2.fc12 NetPIPE-3.7.1-2.el5 has been submitted as an update for Fedora EPEL 5. http://admin.fedoraproject.org/updates/NetPIPE-3.7.1-2.el5 NetPIPE-3.7.1-2.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/NetPIPE-3.7.1-2.fc10 NetPIPE-3.7.1-2.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/NetPIPE-3.7.1-2.fc11 Packages built. Closing. NetPIPE-3.7.1-2.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report. NetPIPE-3.7.1-2.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. NetPIPE-3.7.1-2.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report. Package Change Request ====================== Package Name: NetPIPE New Branches: el6 EPEL7 Owners: cicku Git done (by process-git-requests). |