Bug 697247

Summary: Review Request: gsmartcontrol - Graphical user interface for smartctl
Product: [Fedora] Fedora Reporter: Eric Smith <spacewar>
Component: Package ReviewAssignee: Martin Gieseking <martin.gieseking>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, martin.gieseking, notting, richmattes
Target Milestone: ---Flags: martin.gieseking: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: gsmartcontrol-0.8.6-3.el6 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-12-23 03:29:55 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 Eric Smith 2011-04-17 08:08:30 UTC
Spec URL: http://fedorapeople.org/~brouhaha/gsmartcontrol/gsmartcontrol.spec
SRPM URL: http://fedorapeople.org/~brouhaha/gsmartcontrol/gsmartcontrol-0.8.5-1.fc14.src.rpm
Koji scratch build for rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=3006321
Description:
GSmartControl is a graphical user interface for smartctl (from
smartmontools package), which is a tool for querying and controlling
SMART (Self-Monitoring, Analysis, and Reporting Technology) data on
modern hard disk drives. It allows you to inspect the drive's SMART
data to determine its health, as well as run various tests on it.

Comment 1 Eric Smith 2011-04-17 08:13:04 UTC
Various parts of this package are covered by various licenses, all of which are already OK for Fedora except the "Whatever" license:

     Whatever License

     THIS SOFTWARE IS PROVIDED 'AS-IS', WITHOUT ANY EXPRESS OR IMPLIED
     WARRANTY.  IN NO EVENT WILL THE AUTHORS BE HELD LIABLE FOR ANY DAMAGES
     ARISING FROM THE USE OF THIS SOFTWARE.

     Everyone is permitted to copy and distribute verbatim or modified
     copies of this license document.

     Permission is granted to anyone to use this software for any purpose,
     including commercial applications, and to alter it and redistribute it
     freely.

I have posted a request to the Fedora legal mailing list about it.

Comment 2 Eric Smith 2011-04-19 00:27:10 UTC
Tom Callaway posted to the Fedora legal mailing list that the "Whatever" license is acceptable as an extremely permissive variant of the MIT license, and should be marked as such.  I've updated the License tag in the spec accordingly:

Spec URL: http://fedorapeople.org/~brouhaha/gsmartcontrol/gsmartcontrol.spec
SRPM URL:
http://fedorapeople.org/~brouhaha/gsmartcontrol/gsmartcontrol-0.8.5-2.fc14.src.rpm

Comment 3 Rich Mattes 2011-07-17 00:51:38 UTC
A few comments:

1. You should add a runtime requirement for the hicolor-icon-theme package, which provides the /usr/share/icons/hicolor/* folders.

2. You should also probably add a runtime requirement on smartmontools for the smartctl binary.  I was able to install and run gsmartctl without smartctl installed.

Comment 4 Eric Smith 2011-10-08 07:42:36 UTC
Thanks for the suggestions!

Spec URL: http://fedorapeople.org/~brouhaha/gsmartcontrol/gsmartcontrol.spec
SRPM URL:
http://fedorapeople.org/~brouhaha/gsmartcontrol/gsmartcontrol-0.8.6-1.fc14.src.rpm
Koji scratch build for F15:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3413520

- Updated to latest upstream release.
- Removed obsolte BuildRoot tag, clean section, defattr, etc.
- Added runtime requirements for smartmontools and hicolor-icon-theme

Comment 5 Martin Gieseking 2011-11-22 21:55:30 UTC
Eric, here's the formal review of your package. There are only a few things that need to be addressed (some are optional):

- please add parentheses around "GPLv2 or GPLv3" in the License field
  http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Combined_Dual_and_Multiple_Licensing_Scenario

- add the scriptlets required to update the icon cache:
  http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

- You can drop --prefix=%{_prefix} from %configure. It's already part of the 
  %configure macro.

- Add a second percent sign to #%PAM-1.0 to prevent a potential macro 
  expansion in the heredoc and to make rpmlint happy.

- The image files in %{_datadir}/%{name}/ are added twice in %files. Either 
  prefix %{_datadir}/%{name} with "%dir" or drop the two lines 
  %{_datadir}/%{name}/icon_cddvd.png and %{_datadir}/%{name}/icon_hdd.png.

- Maybe you want to replace the icon/hicolor lines by a single one:
  %{_datadir}/icons/hicolor/*x*/apps/%{name}.png

- I suggest to move all the docs to %{_defaultdocdir}/%{name}-%{version}/ 
  instead of providing two doc dirs (versioned and unversioned).


$ rpmlint ./gsmartcontrol-*.rpm
gsmartcontrol.src: W: spelling-error Summary(en_US) smartctl -> smarts
gsmartcontrol.src: W: spelling-error %description -l en_US smartctl -> smarts
gsmartcontrol.src: W: spelling-error %description -l en_US smartmontools -> smartypants
gsmartcontrol.src:54: W: macro-in-comment %PAM
gsmartcontrol.x86_64: W: spelling-error Summary(en_US) smartctl -> smarts
gsmartcontrol.x86_64: W: spelling-error %description -l en_US smartctl -> smarts
3 packages and 0 specfiles checked; 0 errors, 6 warnings.

The spelling errors are false positive.


---------------------------------
key:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: The file containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ md5sum gsmartcontrol-0.8.6.tar.bz2*
    d2ac685e2e73b05f7ba74c93623af739  gsmartcontrol-0.8.6.tar.bz2
    d2ac685e2e73b05f7ba74c93623af739  gsmartcontrol-0.8.6.tar.bz2.upstream

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
[.] MUST: If the package does not successfully compile, build or work on an architecture, ...
[+] MUST: All build dependencies must be listed in BuildRequires.
[+] MUST: When compiling C, C++, or Fortran files, %{optflags} must be applied.
[.] MUST: The spec file MUST handle locales properly.
[X] MUST: If a package installs files below %{_datadir}/icons, the icon cache must be updated.
[.] MUST: Packages storing shared library files (not just symlinks) must call ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates. 
[X] MUST: A Fedora package must not list a file more than once in %files.
    - the image files in %{_datadir}/%{name}/ are added twice

[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[.] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[.] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), ...
[.] MUST: devel packages must require the base package using a fully versioned dependency.
[+] MUST: Packages must NOT contain any .la libtool archives.
[+] MUST: Packages containing GUI applications must include a %{name}.desktop file.
[+] MUST: .desktop files must be properly installed with desktop-file-install in the %install section.
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

EPEL <= 5 only:
[X] MUST: The spec file must contain a valid BuildRoot field.
[X] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}.
[X] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot}.
[.] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'

[.] SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The reviewer should test that the package functions as described.
[.] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[.] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg.
[+] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.
[+] SHOULD: Your package should contain man pages for binaries/scripts.

Comment 7 Martin Gieseking 2011-12-06 20:25:28 UTC
The package looks good now and is ready for check-in. 

----------------
Package APPROVED
----------------

Comment 8 Eric Smith 2011-12-08 04:39:58 UTC
New Package SCM Request
=======================
Package Name: gsmartcontrol
Short Description: Graphical user interface for smartctl
Owners: brouhaha
Branches: f16 el6
InitialCC:

Comment 9 Gwyn Ciesla 2011-12-08 12:58:44 UTC
Git done (by process-git-requests).

Comment 10 Eric Smith 2011-12-11 01:33:55 UTC
Ugh!  gsmartcontrol builds fine for F16 but won't build for rawhide due to changes to the Glib thread API in 2.31.  Aside from deprecation warnings, I get some compile errors that I don't know how to deal with.  I've asked the author if he has any advice.

Comment 11 Fedora Update System 2011-12-13 07:29:42 UTC
gsmartcontrol-0.8.6-3.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/gsmartcontrol-0.8.6-3.fc16

Comment 12 Eric Smith 2011-12-13 07:33:44 UTC
Applied simple patch from upstream that fixes build with Glib 2.31.  Only a few lines of code changed.

Comment 13 Fedora Update System 2011-12-13 07:39:47 UTC
gsmartcontrol-0.8.6-3.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/gsmartcontrol-0.8.6-3.el6

Comment 14 Fedora Update System 2011-12-13 19:58:18 UTC
gsmartcontrol-0.8.6-3.el6 has been pushed to the Fedora EPEL 6 testing repository.

Comment 15 Fedora Update System 2011-12-23 03:29:55 UTC
gsmartcontrol-0.8.6-3.fc16 has been pushed to the Fedora 16 stable repository.

Comment 16 Fedora Update System 2011-12-28 19:54:09 UTC
gsmartcontrol-0.8.6-3.el6 has been pushed to the Fedora EPEL 6 stable repository.