Bug 589162
| Summary: | Review Request: stress - A tool to put given subsystems under a specified load | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Gwyn Ciesla <gwync> |
| Component: | Package Review | Assignee: | Terje Røsten <terje.rosten> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | low | ||
| Version: | rawhide | CC: | fedora-package-review, notting, pahan, supercyper1, terje.rosten |
| Target Milestone: | --- | Flags: | terje.rosten:
fedora-review+
j: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | stress-1.0.4-4.fc12 | Doc Type: | Bug Fix |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2010-05-10 23:42:53 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
Gwyn Ciesla
2010-05-05 14:49:58 UTC
Some initial comments: - many files in the tarball has execute bit set, giving: stress.x86_64: W: spurious-executable-perm /usr/share/doc/stress-1.0.4/README stress.x86_64: W: spurious-executable-perm /usr/share/doc/stress-1.0.4/TODO stress.x86_64: W: spurious-executable-perm /usr/share/doc/stress-1.0.4/doc/stress.texi stress.x86_64: W: spurious-executable-perm /usr/share/doc/stress-1.0.4/AUTHORS stress.x86_64: W: spurious-executable-perm /usr/share/doc/stress-1.0.4/INSTALL stress.x86_64: W: spurious-executable-perm /usr/share/doc/stress-1.0.4/doc/Makefile.am stress.x86_64: W: spurious-executable-perm /usr/share/doc/stress-1.0.4/doc/mdate-sh stress.x86_64: W: spurious-executable-perm /usr/share/doc/stress-1.0.4/NEWS stress-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/stress-1.0.4/src/stress.c Please fix that by chmod in %prep. - You include the whole doc/ dir in %doc, however the only useful file (already not included) is stress.html. - License: headers in stress.c has GPLv2+, while COPYING has GPLv3. Adding to the mess the web page states GPLv2. Current sitaution means license tag is GPLv2+, however you should contact upstream about this issue. SRPM: http://zanoni.jcomserv.net/fedora/stress/stress-1.0.4-2.fc12.src.rpm SPEC: http://zanoni.jcomserv.net/fedora/stress/stress.spec I've addressed the permissions and doc issues, but I'm not sure I agree on the licensing issue. The license file is GPL3, the code says GPLv2+. I see no reason I can't then use GPLv3+. I agree that upstream should probably change their website. If you like, you can block FE-LEGAL and we can get a formal decision on this, because I very well may be wrong. I probably should have commented in the spec my reasoning on the license tag, and I can certainly do that if you like. Please read the following: https://fedoraproject.org/wiki/Licensing/FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F and see if you agree that license is GPLv2+. Please move the chmod to %prep section. Modification of sources should happen there. Ah, I'd forgotten about that page. GPLv2+ it is. If I move the chmod to prep, it's executed before setup extracts the tarball, and fails. I earliest I can move it is just after setup. SRPM: http://zanoni.jcomserv.net/fedora/stress/stress-1.0.4-3.fc12.src.rpm SPEC: http://zanoni.jcomserv.net/fedora/stress/stress.spec It'll be better to use Requires(post): info instead of /sbin/install-info http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Texinfo INSTALL should not be included in %doc http://fedoraproject.org/wiki/Packaging/Guidelines#Documentation Addressed: SRPM: http://zanoni.jcomserv.net/fedora/stress/stress-1.0.4-4.fc12.src.rpm SPEC: http://zanoni.jcomserv.net/fedora/stress/stress.spec Thanks, review in the works. ok rpmlint, just spelling warnings
ok sources
sha1sum stress-1.0.4.tar.gz*
7ccb6d76d27ddd54461a21411f2bc8491ba65168 stress-1.0.4.tar.gz
7ccb6d76d27ddd54461a21411f2bc8491ba65168 stress-1.0.4.tar.gz.spec
However use wget _n to preserve timestamps on the tarball
ok license GPLv2+, see comment above
ok dirs, perms and owns
ok correct language
! spec file
remove a strange rm -f in %install under
rm -f $RPM_BUILD_ROOT%{_infodir}/dir
ok docs and man page
ok buildopts
ok macros
ok utf-8 file names
ok koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2172740
Fix the rm issue on import.
The package stress is APPROVED.
Thanks for the review! Fix rm in what way, replace with exclude? New Package CVS Request ======================= Package Name: stress Short Description: A tool to put given subsystems under a specified load Owners: limb Branches: F-13 F-12 InitialCC: Sorry, I was a bit short: From: http://zanoni.jcomserv.net/fedora/stress/stress.spec in the %install section: %install rm -rf $RPM_BUILD_ROOT make install DESTDIR=$RPM_BUILD_ROOT rm -f $RPM_BUILD_ROOT%{_infodir}/dir rm -f This last line a 'rm -f' without argument should be removed. Oh, duh. Yes, of course. Thanks! CVS done (by process-cvs-requests.py). stress-1.0.4-4.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/stress-1.0.4-4.fc13 stress-1.0.4-4.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/stress-1.0.4-4.fc12 stress-1.0.4-4.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report. stress-1.0.4-4.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report. Package Change Request ====================== Package Name: stress New Branches: el6 Owners: limb Git done (by process-git-requests). |