Bug 737263

Summary: Review Request: phoronix-test-suite - An Automated, Open-Source Testing Framework
Product: [Fedora] Fedora Reporter: Markus Mayer <LotharLutz>
Component: Package ReviewAssignee: Jerry James <loganjerry>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: loganjerry, marbolangos, notting, package-review, pahan, valent.turkovic
Target Milestone: ---Flags: loganjerry: fedora‑review+
limburgher: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
See Also: https://bugzilla.redhat.com/show_bug.cgi?id=485596
Whiteboard:
Fixed In Version: phoronix-test-suite-3.4.0-3.el6 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-12-10 14:40:46 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:

Description Markus Mayer 2011-09-10 08:02:11 EDT
Spec URL: http://lotharlutz.fedorapeople.org/phoronix-test-suite.spec
SRPM URL: http://lotharlutz.fedorapeople.org/phoronix-test-suite-3.4.0-1.fc15.src.rpm
Description: The Phoronix Test Suite is the most comprehensive testing and benchmarking 
platform available for the Linux operating system. This software is designed to 
effectively carry out both qualitative and quantitative benchmarks in a clean, 
reproducible, and easy-to-use manner. The Phoronix Test Suite consists of a 
lightweight processing core (pts-core) with each benchmark consisting of an 
XML-based profile with related resource scripts. The process from the benchmark 
installation, to the actual benchmarking, to the parsing of important hardware 
and software components is heavily automated and completely repeatable, asking 
users only for confirmation of actions.


This packages must be review because it was deprecated in F16 and devel
Comment 1 Valent Turkovic 2011-10-24 05:42:19 EDT
Any news of this package update? I can't find any mention of phoronix test suite in rawhide repos...
Comment 2 Markus Mayer 2011-10-25 07:29:59 EDT
I am still interested bringing this package to fedora. But as long as nobody is willing to do a review I am not able to finish this.
Comment 3 Valent Turkovic 2011-10-25 09:06:32 EDT
I believe that standard procedure is to mail to development mailing list if you get stuck in review process, as it is the case with this package. Have you mailed on development mailing list?
Comment 4 Valent Turkovic 2011-10-25 09:08:21 EDT
You can look at mail I sent few days ago and continue discussion on devel mailing list.
Comment 5 Jerry James 2011-10-29 18:15:57 EDT
I will take this review.  Thanks for swapping reviews, Markus.
Comment 6 Jerry James 2011-10-29 18:38:22 EDT
Some elements of the spec file have no effect in any released version of Fedora.  Unless you plan to use this spec file with EPEL also, you can remove the BuildRoot tag, the "rm -rf %{buildroot}" line at the top of %install, the entire %clean script, and the %defattr at the top of %files.

+: OK
-: must be fixed
=: should be fixed (at your discretion)
N: not applicable

MUST:
[+] rpmlint output:
phoronix-test-suite.noarch: W: spelling-error %description -l en_US benchmarking -> bench marking, bench-marking, benchmark
phoronix-test-suite.noarch: W: spelling-error %description -l en_US pts -> pt, ts, pets
phoronix-test-suite.noarch: W: non-conffile-in-etc /etc/bash_completion.d/phoronix-test-suite
phoronix-test-suite.src: W: spelling-error %description -l en_US benchmarking -> bench marking, bench-marking, benchmark
phoronix-test-suite.src: W: spelling-error %description -l en_US pts -> pt, ts, pets
2 packages and 1 specfiles checked; 0 errors, 5 warnings.

The spelling warnings are bogus.  How about the bash completion file, though?  Should it be marked %config?

[+] follows package naming guidelines
[+] spec file base name matches package name
[+] package meets the packaging guidelines: with the caveat that I don't really understand the PHP guidelines, but this package doesn't appear to meet the conditions established in those guidelines.
[+] package uses a Fedora approved license
[+] license field matches the actual license
[+] license file is included in %doc
[+] spec file is in American English
[+] spec file is legible
[+] sources match upstream: md5sum is 2f075538fbe45bb69b3e9d7bfef63948 for both
[+] package builds on at least one primary arch (tried x86_64)
[N] appropriate use of ExcludeArch
[+] all build requirements in BuildRequires
[N] spec file handles locales properly
[N] ldconfig in %post and %postun
[+] no bundled copies of system libraries
[+] no relocatable packages
[+] package owns all directories that it creates
[+] no files listed twice in %files
[+] proper permissions on files
[+] consistent use of macros
[+] code or permissible content
[N] large documentation in -doc
[+] no runtime dependencies in %doc
[N] header files in -devel
[N] static libraries in -static
[N] .so in -devel
[N] -devel requires main package
[+] package contains no libtool archives
[+] package contains a desktop file, uses desktop-file-install: actually desktop-file-validate, which is also okay
[-] package does not own files/dirs owned by other packages: owns the following dirs already owned by hicolor-icon-theme:
/usr/share/icons/hicolor
/usr/share/icons/hicolor/48x48
/usr/share/icons/hicolor/48x48/apps
/usr/share/icons/hicolor/64x64
/usr/share/icons/hicolor/64x64/mimetypes

[+] all filenames in UTF-8

SHOULD:
[N] query upstream for license text
[N] description and summary contain available translations
[+] package builds in mock: tried fedora-rawhide-i386
[+] package builds on all supported arches: tried i386 and x86_64
[+] package functions as described: minimal testing only
[+] sane scriptlets
[N] subpackages require the main package
[N] placement of pkgconfig files
[N] file dependencies versus package dependencies
[+] package contains man pages for binaries/scripts
Comment 7 Markus Mayer 2011-10-31 08:19:56 EDT
Thank you for reviewing this package.

I (In reply to comment #6)
> Some elements of the spec file have no effect in any released version of
> Fedora.  Unless you plan to use this spec file with EPEL also, you can remove
> the BuildRoot tag, the "rm -rf %{buildroot}" line at the top of %install, the
> entire %clean script, and the %defattr at the top of %files.
> 

Yep. I plan to use this spec file in EPEL so I will not remove them


> 
> MUST:
> [+] rpmlint output:
> phoronix-test-suite.noarch: W: spelling-error %description -l en_US
> benchmarking -> bench marking, bench-marking, benchmark
> phoronix-test-suite.noarch: W: spelling-error %description -l en_US pts -> pt,
> ts, pets
> phoronix-test-suite.noarch: W: non-conffile-in-etc
> /etc/bash_completion.d/phoronix-test-suite
> phoronix-test-suite.src: W: spelling-error %description -l en_US benchmarking
> -> bench marking, bench-marking, benchmark
> phoronix-test-suite.src: W: spelling-error %description -l en_US pts -> pt, ts,
> pets
> 2 packages and 1 specfiles checked; 0 errors, 5 warnings.
> 
> The spelling warnings are bogus.  How about the bash completion file, though? 
> Should it be marked %config?

I have changed it %config(nonreplace).


> 
> [+] follows package naming guidelines
> [+] spec file base name matches package name
> [+] package meets the packaging guidelines: with the caveat that I don't really
> understand the PHP guidelines, but this package doesn't appear to meet the
> conditions established in those guidelines.
> [+] package uses a Fedora approved license
> [+] license field matches the actual license
> [+] license file is included in %doc
> [+] spec file is in American English
> [+] spec file is legible
> [+] sources match upstream: md5sum is 2f075538fbe45bb69b3e9d7bfef63948 for both
> [+] package builds on at least one primary arch (tried x86_64)
> [N] appropriate use of ExcludeArch
> [+] all build requirements in BuildRequires
> [N] spec file handles locales properly
> [N] ldconfig in %post and %postun
> [+] no bundled copies of system libraries
> [+] no relocatable packages
> [+] package owns all directories that it creates
> [+] no files listed twice in %files
> [+] proper permissions on files
> [+] consistent use of macros
> [+] code or permissible content
> [N] large documentation in -doc
> [+] no runtime dependencies in %doc
> [N] header files in -devel
> [N] static libraries in -static
> [N] .so in -devel
> [N] -devel requires main package
> [+] package contains no libtool archives
> [+] package contains a desktop file, uses desktop-file-install: actually
> desktop-file-validate, which is also okay
> [-] package does not own files/dirs owned by other packages: owns the following
> dirs already owned by hicolor-icon-theme:
> /usr/share/icons/hicolor
> /usr/share/icons/hicolor/48x48
> /usr/share/icons/hicolor/48x48/apps
> /usr/share/icons/hicolor/64x64
> /usr/share/icons/hicolor/64x64/mimetypes
> 

This package does not require hicolor-icon-theme neither implicit nor explecit. Acourding to the Package guidlines (The directory is owned by a package which is not required for your package to function) this package must own these directories.

> [+] all filenames in UTF-8
> 
> SHOULD:
> [N] query upstream for license text
> [N] description and summary contain available translations
> [+] package builds in mock: tried fedora-rawhide-i386
> [+] package builds on all supported arches: tried i386 and x86_64
> [+] package functions as described: minimal testing only
> [+] sane scriptlets
> [N] subpackages require the main package
> [N] placement of pkgconfig files
> [N] file dependencies versus package dependencies
> [+] package contains man pages for binaries/scripts


New Version:
Spec URL: http://lotharlutz.fedorapeople.org/phoronix-test-suite.spec
SRPM URL:
http://lotharlutz.fedorapeople.org/phoronix-test-suite-3.4.0-2.fc15.src.rpm
Comment 8 Jerry James 2011-10-31 18:27:55 EDT
I think we're nearly there....

(In reply to comment #7)
> This package does not require hicolor-icon-theme neither implicit nor explecit.
> Acourding to the Package guidlines (The directory is owned by a package which
> is not required for your package to function) this package must own these
> directories.

I wasn't sure how this was supposed to be handled, so I asked on fedora-devel.  Would you, please, take a look at the thread starting here?

http://lists.fedoraproject.org/pipermail/devel/2011-October/158861.html
Comment 9 Markus Mayer 2011-11-01 06:19:37 EDT
The packaging guidelines are quite clear about "artificial filesystem" packages. I have just not recognized hicolor-icon-theme as a filesystem package. I should stop judging packages by their names ;-)

I have included hicolor-icon-theme as an requirenment and changed the package to just own the icon files instead of the whole directory tree.

New Version:
Spec URL: http://lotharlutz.fedorapeople.org/phoronix-test-suite.spec
SRPM URL:
http://lotharlutz.fedorapeople.org/phoronix-test-suite-3.4.0-3.fc15.src.rpm

Thank you very much your afford so far. This was probably the most qualitative review I ever had. I have learned very much and I realy appreciated it.
Comment 10 Jerry James 2011-11-02 18:04:39 EDT
Sorry for the delay.  $DAYJOB has kept me extra busy for a couple of days.

And thanks for the compliment!

Everything looks good now.  This package is APPROVED.
Comment 11 Markus Mayer 2011-11-03 03:45:47 EDT
Package Change Request
======================
Package Name: phoronix-test-suite
New Branches: f16 devel
Owners: lotharlutz
InitialCC: 

This package is deprecated in f16 and devel. I have already taken ownership of the remaining braches.
Comment 12 Markus Mayer 2011-11-03 03:48:24 EDT
*** Bug 526852 has been marked as a duplicate of this bug. ***
Comment 13 Jon Ciesla 2011-11-03 08:27:46 EDT
Git done (by process-git-requests).
Comment 14 Fedora Update System 2011-11-29 08:43:36 EST
phoronix-test-suite-3.4.0-3.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/phoronix-test-suite-3.4.0-3.fc16
Comment 15 Fedora Update System 2011-11-29 08:43:45 EST
phoronix-test-suite-3.4.0-3.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/phoronix-test-suite-3.4.0-3.el6
Comment 16 Fedora Update System 2011-11-29 08:43:54 EST
phoronix-test-suite-3.4.0-3.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/phoronix-test-suite-3.4.0-3.fc15
Comment 17 Fedora Update System 2011-11-29 14:02:52 EST
Package phoronix-test-suite-3.4.0-3.el6:
* should fix your issue,
* was pushed to the Fedora EPEL 6 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=epel-testing phoronix-test-suite-3.4.0-3.el6'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-EPEL-2011-5109/phoronix-test-suite-3.4.0-3.el6
then log in and leave karma (feedback).
Comment 18 Fedora Update System 2011-12-10 14:40:46 EST
phoronix-test-suite-3.4.0-3.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 19 Fedora Update System 2011-12-10 15:07:09 EST
phoronix-test-suite-3.4.0-3.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 20 Fedora Update System 2011-12-14 15:27:57 EST
phoronix-test-suite-3.4.0-3.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.