Bug 641093

Hi Hannes,

here are some initial comments:

- According to the source file headers, the license of the library and the
  Lua bindings is MIT. Only the Python bindings are GPLv2+.
  Thus, you should adapt the License fields accordingly. Also, add file
  COPYING to the python subpackage.

- The %description lines should not exceed 80 characters.

- The summary of the python package should start with a capital letter.

- Add a filter to prevent providing the python library:
  %filter_provides_in %{python_sitearch}.*\.so$
  %filter_setup
  For further details see
  https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering

- add %postun -p /sbin/ldconfig

- drop empty file ChangeLog

- in %files, replace %{_libdir}/libkeybinder.so* with 
  %{_libdir}/libkeybinder.so.* to prevent packaging the .so file which belongs
  in the devel package

- add %{_libdir}/libkeybinder.so to the devel package

- remove _keybinder.la from the python subpackage

- I think the Lua binding library should go in a lua subpackage.



$ rpmlint /var/lib/mock/fedora-13-x86_64/result/*.rpm
keybinder.src: W: spelling-error %description -l en_US lua -> la, luau, lea
keybinder.src: E: description-line-too-long C keybinder is a library for registering global keyboard shortcuts. Keybinder works
keybinder.x86_64: W: spelling-error %description -l en_US lua -> la, luau, lea
keybinder.x86_64: E: description-line-too-long C keybinder is a library for registering global keyboard shortcuts. Keybinder works
keybinder.x86_64: E: zero-length /usr/share/doc/keybinder-0.2.2/ChangeLog
keybinder.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libkeybinder.so
keybinder.x86_64: E: library-without-ldconfig-postun /usr/lib64/libkeybinder.so.0.0.1
keybinder-devel.x86_64: W: no-documentation
python-keybinder.x86_64: W: summary-not-capitalized C keybinder python bindings
python-keybinder.x86_64: W: private-shared-object-provides /usr/lib64/python2.6/site-packages/keybinder/_keybinder.so _keybinder.so()(64bit)
python-keybinder.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 4 errors, 7 warnings.

Hey Martin,

I tried to implement all your suggestions. Glad that someone noticed it ;)
It is a dependency of xfce4-volumed and so should be in fedora...

Spec URL: http://hannes.fedorapeople.org/keybinder/keybinder.spec
SRPM URL:
http://hannes.fedorapeople.org/keybinder/keybinder-0.2.2-2.fc14.src.rpm

rpmlint keybinder.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

rpmlint ../RPMS/x86_64/*.rpm
keybinder.x86_64: W: spelling-error %description -l en_US lua -> la, luau, lea
keybinder-devel.x86_64: W: no-documentation
lua-keybinder.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 0 errors, 3 warnings.

Hi Hannes,

here are some more notes. Sorry if my previous comments were not clear enough.

- The license tag of the base package should be MIT only. Add a License field
  to the python subpackage with tag GPLv2+. The "+" is required because of 
  the addition "or at your option, any later version" in the copyright notice
  (see file __init__.py).

- The subpackage lua-keybinder must not own the directories %{_libdir}/lua/ 
  and %{_libdir}/lua/5.1/ since they belong to the package providing the Lua
  interpreter. Thus, replace %{_libdir}/lua with %{_libdir}/lua/5.1/keybinder.so

- There's sill a .la file in the python package:

$ rpmls python-keybinder-0.2.2-2.fc13.x86_64.rpm 
drwxr-xr-x  /usr/lib64/python2.6/site-packages/keybinder
-rw-r--r--  /usr/lib64/python2.6/site-packages/keybinder/__init__.py
-rw-r--r--  /usr/lib64/python2.6/site-packages/keybinder/__init__.pyc
-rw-r--r--  /usr/lib64/python2.6/site-packages/keybinder/__init__.pyo
-rwxr-xr-x  /usr/lib64/python2.6/site-packages/keybinder/_keybinder.la
-rwxr-xr-x  /usr/lib64/python2.6/site-packages/keybinder/_keybinder.so
drwxr-xr-x  /usr/share/doc/python-keybinder-0.2.2
-rw-r--r--  /usr/share/doc/python-keybinder-0.2.2/COPYING

Ok, fixed those issues. 
Spec URL: http://hannes.fedorapeople.org/keybinder/keybinder.spec
SRPM URL:
http://hannes.fedorapeople.org/keybinder/keybinder-0.2.2-3.fc14.src.rpm

I wish you a nice sunday...

I'd like to do the formal review then. :)

$ rpmlint keybinder.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

$ rpmlint keybinder-0.2.2-3.fc14.src.rpm
keybinder.src: W: spelling-error %description -l en_US lua -> la, luau, lea
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

This spelling error can safely be ignored.

$ rpmlint *keybinder*
keybinder.x86_64: W: spelling-error %description -l en_US lua -> la, luau, lea
keybinder-devel.x86_64: W: no-documentation
lua-keybinder.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 0 errors, 3 warnings.


Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines
 [x] Specfile name matches %{name}.spec
 [x] Package seems to meet Packaging Guidelines
 [x] Package successfully compiles and builds into binary RPMs on at least one
     supported architecture.
     Tested on: Fedora 14/x86_64
 [x] Rpmlint output:
     source RPM: see above
     binary RPM: see above
 [x] Package is not relocatable.
 [!] License in specfile matches actual License and meets Licensing Guidelines
     The tarball provides a file COPYING containing the GPLv2+, the README file
     also points to GPLv2+.

     The file ax_lua.m4 in the m4/ directory says GPLv3+.

	 This results in GPLv3+ for any file marked with a GPLv2+ or GPLv3+ hint
	 in it.

	 The file lua-keybinder/lkeybinder.c is MIT, thus the lua subpackage
	 should be tagged with "License: MIT".

	 Any file in libkeybinder/ is also MIT. Since there is no subpackage for
	 the library, this results in tagging the main package with MIT, yes.

	 But: as there are GPLv3+ files in the main package also (I'm referring
	 to m4/ax_lua.m4) - even if they are just part of the source RPM - which
	 were not moved to any subpackage, you will have to tag the main package
	 with "License: MIT and GPLv3+".

	 Of course, alternatively you can ask upstream if this is intended and if
	 they may fix the license of m4/ax_lua.m4.

     Feel free to correct me if it seems I may understood something wrong.

 [x] License file is included in %doc.
 [x] Specfile is legible and written in AE
 [x] Sourcefile in the Package is the same as provided in the mentioned Source
     SHA1SUM of Source: 9e7e62727fd236f2d3c49cc9ff1eaadc78841987
 [x] Package compiles successfully
 [x] All build dependencies are listed in BuildRequires
 [-] Specfile handles locales properly
 [x] ldconfig called in %post and %postun if required
 [x] Package owns directorys it creates
 [-] Package requires other packages for directories it uses.
 [x] Package does not list a file more than once in the %files listing
 [x] %files section includes %defattr and permissions are set properly
 [x] %clean section is there and contains rm -rf %{buildroot}
 [x] Macros are consistently used
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage
 [x] Program runs properly without files listed in %doc
 [x] Header files are in a -devel package
 [-] Static libraries are in a -static package
 [!] Package requires pkgconfig if .pc files are present
	 The "Requires: pkgconfig" should be added, at least for the -devel subpackage

 [x] .so-files are put into a -devel subpackage
 [!] Subpackages include fully versioned dependency for the base package
	 The python and lua subpackage lacks the %{release} dependency

 [x] Any libtool archives (*.la) are removed
 [-] contains desktop file (%{name}.desktop) if it is a GUI application
 [x] Package does not own files or directories owned by other packages.
 [x] %{buildroot} is removed at beginning of %install
 [-] Filenames are encoded in UTF-8

=== SUGGESTED ITEMS ===
 [x] Package contains latest upstream version
 [x] Package does not include license text files separate from upstream.
 [-] non-English translations for description and summary
 [x] Package builds in mock
     Tested on: F14/x86_64
 [x] Package should compile and build into binary RPMs on all supported
     architectures.
     tested build with koji
 [?] Program runs
	 Keybinder is basically a library and can not be run directly.

 [-] Scriptlets must be sane, if used.
 [x] pkgconfig (*.pc) files are placed in a -devel package
 [-] require package providing a file instead of the file itself
     no files outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin are required

(In reply to comment #6)
>   But: as there are GPLv3+ files in the main package also (I'm referring
>   to m4/ax_lua.m4) - even if they are just part of the source RPM - which
>   were not moved to any subpackage, you will have to tag the main package
>   with "License: MIT and GPLv3+".

Sorry, but that's incorrect. The License fields must reflect the license of the *binary* rpm(s). Files that are not packaged (or compiled and linked into a packaged file) don't affect the License field. See
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License:_field

The m4 files are only used in conjunction with the autotools during configuration. Thus, they can be ignored when determining the license.

Ok, so I leave the license as it is, or?
Added the %{release} tag to the subpackages:
Spec URL: http://hannes.fedorapeople.org/keybinder/keybinder.spec
SRPM URL:
http://hannes.fedorapeople.org/keybinder/keybinder-0.2.2-4.fc14.src.rpm

Yes, you can leave the License:-Tag as is then. Thanks a lot for pointing us to that, Martin. :)

The other issues which were claimed in my preview seem to be fixed I see, thus the package is APPROVED. :)

New Package SCM Request
=======================
Package Name: keybinder
Short Description: A library for registering global keyboard shortcuts
Owners: hannes
Branches: f14 f13
InitialCC:

Git done (by process-git-requests).

build in rawhide:
http://koji.fedoraproject.org/koji/packageinfo?packageID=11135

http://koji.fedoraproject.org/koji/taskinfo?taskID=2573267

Why does the keybinder package have "Requires: pkgconfig"? it is keybinder-devel that requires that requires pkgconfig and you don't need to list this since rpm will figure that out automatically if a package includes a *.pc file.

Hmm, ok. Should I push an update or is it sufficient to wait with that until a new version comes out. 
Is only a minor thing, or?
It was initially because keybinder-devel needs it.

I think you should make an update. pkgconfig is a development tool and normal users should not need to install any development stuff just to run the app.

The best time to do an update is now as the other package has not been pushed out to the repos yet.

keybinder-0.2.2-5.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/keybinder-0.2.2-5.fc14

keybinder-0.2.2-5.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/keybinder-0.2.2-5.fc13

keybinder-0.2.2-5.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

keybinder-0.2.2-5.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

We need this in EL6 for xfce4-volumed.  Who is on board to be a maintainer there?

Looks like keybinder requires gtk 2.20.  EL6 has gtk 2.18.9.  We could package up version 0.0.9, but that doesn't seem like a good idea.  So I'm dropping this at the moment.

Created attachment 913193 [details]
spec file patch

Keybinder is missing it's typelib (due to missing build requires)


$ foobnix 
/usr/bin/xclip
start program
/usr/lib64/python2.7/site-packages/gobject/constants.py:24: Warning: g_boxed_type_register_static: assertion 'g_type_from_name (name) == 0' failed
  import gobject._gobject
[ERROR   ] [17:36:14] [importer.py:51] Could not find any typelib for Keybinder

It also includes a .la file in the python subpackage.

I have attached a patch to correct this issues please apply to all current branches.

This package is in Fedora so closing this bug to clean up the queue. 

If there are any issues or requests please file a bug against the package and not comment on this review ticket.