Bug 725128 - Review Request: phat - GTK library for audio software
Summary: Review Request: phat - GTK library for audio software
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Martin Gieseking
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-07-22 23:44 UTC by Brendan Jones
Modified: 2011-11-13 05:31 UTC (History)
4 users (show)

Fixed In Version: phat-0.4.1-4.fc16
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-11-13 05:31:11 UTC
Type: ---
Embargoed:
martin.gieseking: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Brendan Jones 2011-07-22 23:44:29 UTC
This package has existed in Planet CCRMA for some time. I will be requiring it for another package.

I have updated to meet Fedora's guidelines:

http://bsjones.fedorapeople.org/phat-0.4.1-1.fc16.src.rpm
http://bsjones.fedorapeople.org/phat.spec

Comment 1 Martin Gieseking 2011-07-23 07:47:53 UTC
Here are some initial comments:

- The license seems to be GPLv2+ according to the copyright info in 
  phat/phatknob.c. 

- Drop BR: gtk-doc, and add Requires: gtk-doc to the docs package.

- I recommend to replace the explicit name "phat" with %{name} in the Requires
  fields. It ensures that the subpackages always require the base package 
  even if the name changes for some reason.

- As the devel package contains more than just the header files, I suggest to
  mention the other files as well.

- Please be a bit more specific in %files:
  %{_bindir}/phat*

- To simplify future updates, avoid mentioning the soversion and replace
  %{_libdir}/libphat.so.0* with %{_libdir}/libphat.so.*

- Fedora usually doesn't ship static libraries except if necessary for some
  purpose. Thus, add --disable-static to %configure and drop the .a file
  from %files devel.

Comment 2 Brendan Jones 2011-07-23 13:30:26 UTC
Thanks Martin, I have made the required changes.

bsjones@f15laptop ~$rpmlint /home/rawhide/rpmbuild/RPMS/x86_64/phat*0.4.1-2*.rpm /home/rawhide/rpmbuild/SRPMS/phat-0.4.1-2.fc16.src.rpm
phat.x86_64: W: no-manual-page-for-binary phatfanslider
phat.x86_64: W: no-manual-page-for-binary phatsliderbutton
phat.x86_64: W: no-manual-page-for-binary phatpad
phat.x86_64: W: no-manual-page-for-binary phatknob
phat.x86_64: W: no-manual-page-for-binary phatkeyboard
phat-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/phat-0.4.1/phat/phatknob.c
phat-devel.x86_64: W: spelling-error %description -l en_US developming -> developing, development
phat-devel.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 1 errors, 7 warnings.

Ignore the address warning, it is split over two lines and the '*' in the comment causes the false positive.

http://bsjones.fedorapeople.org/phat-0.4.1-2.fc16.src.rpm
http://bsjones.fedorapeople.org/phat.spec

Cheers

Comment 3 Brendan Jones 2011-07-23 13:35:06 UTC
And corrected spelling.

bsjones@f15laptop ~$rpmlint /home/rawhide/rpmbuild/RPMS/x86_64/phat*0.4.1-3*.rpm /home/rawhide/rpmbuild/SRPMS/phat-0.4.1-3.fc16.src.rpm
phat.x86_64: W: no-manual-page-for-binary phatfanslider
phat.x86_64: W: no-manual-page-for-binary phatsliderbutton
phat.x86_64: W: no-manual-page-for-binary phatpad
phat.x86_64: W: no-manual-page-for-binary phatknob
phat.x86_64: W: no-manual-page-for-binary phatkeyboard
phat-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/phat-0.4.1/phat/phatknob.c
phat-devel.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 1 errors, 6 warnings.

http://bsjones.fedorapeople.org/phat-0.4.1-3.fc16.src.rpm
http://bsjones.fedorapeople.org/phat.spec

Comment 4 Veeti Paananen 2011-07-23 15:50:48 UTC
The base package requirement in the subpackages needs to be in the format "%{name}%{?_isa} = %{version}-%{release}" (http://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package).

Comment 6 Martin Gieseking 2011-07-25 14:06:03 UTC
The package looks fine now. If you want to build it for EPEL < 6 too, you have to add Requires: pkgconfig to the devel package. Otherwise, you can drop all the buildroot stuff, but that's not a blocker.


$ rpmlint  *.rpm
phat.i686: W: no-manual-page-for-binary phatsliderbutton
phat.i686: W: no-manual-page-for-binary phatkeyboard
phat.i686: W: no-manual-page-for-binary phatknob
phat.i686: W: no-manual-page-for-binary phatfanslider
phat.i686: W: no-manual-page-for-binary phatpad
phat-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/phat-0.4.1/phat/phatknob.c
phat-devel.i686: W: no-documentation
5 packages and 0 specfiles checked; 1 errors, 6 warnings.


---------------------------------
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.
    - GPLv2+

[+] 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 phat-0.4.1.tar.gz*
    b8d1d3ae0d7094d705a33753fe821ebc  phat-0.4.1.tar.gz
    b8d1d3ae0d7094d705a33753fe821ebc  phat-0.4.1.tar.gz.1

[+] 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.
[+] 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. 
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] 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 .so files with a suffix, then .so (without suffix) must go in a -devel package.
[+] 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,...
    - the sample (GUI) applications don't need .desktop files as they are
      plain demos without any useful functionality to the user.

[+] 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:
[+] MUST: The spec file must contain a valid BuildRoot field.
[+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}.
[+] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot}.
[X] 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 package should compile and build into binary rpms on all supported architectures.
[+] 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.

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

Comment 7 Brendan Jones 2011-07-25 21:09:50 UTC
Thanks Martin.

Comment 8 Brendan Jones 2011-07-25 21:11:19 UTC
New Package SCM Request
=======================
Package Name: phat
Short Description: GTK library for audio applications
Owners: bsjones
Branches: f14 f15
InitialCC:

Comment 9 Brendan Jones 2011-07-27 20:59:15 UTC
New Package SCM Request
=======================
Package Name: phat
Short Description: GTK library for audio applications
Owners: bsjones
Branches: f16
InitialCC:

Comment 10 Veeti Paananen 2011-07-27 21:11:58 UTC
Looks like you forgot to set the fedora-cvs flag to "?".

Comment 11 Brendan Jones 2011-07-27 21:14:44 UTC
Indeed!

Comment 12 Gwyn Ciesla 2011-07-28 00:34:11 UTC
Git done (by process-git-requests).

Comment 13 Fedora Update System 2011-10-28 15:16:36 UTC
phat-0.4.1-4.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/phat-0.4.1-4.fc16

Comment 14 Fedora Update System 2011-10-28 21:33:14 UTC
phat-0.4.1-4.fc16 has been pushed to the Fedora 16 testing repository.

Comment 15 Fedora Update System 2011-11-13 05:31:11 UTC
phat-0.4.1-4.fc16 has been pushed to the Fedora 16 stable repository.


Note You need to log in before you can comment on or make changes to this bug.