Red Hat Bugzilla – Bug 641093
Review Request: keybinder - A library for registering global keyboard shortcuts
Last modified: 2015-12-03 20:59:00 EST
Spec URL: http://hannes.fedorapeople.org/keybinder/keybinder.spec SRPM URL: http://hannes.fedorapeople.org/keybinder/keybinder-0.2.2-1.fc14.src.rpm Description: keybinder is a library for registering global keyboard shortcuts. Keybinder works with GTK-based applications using the X Window System.
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.