Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: keybinder - A library for registering global keyboard shortcuts
https://bugzilla.redhat.com/show_bug.cgi?id=641093
Summary: Review Request: keybinder - A library for registering global keyboard shortcuts Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: johannes.lips@googlemail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
hannes johannes.lips@googlemail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |631199
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
Martin Gieseking martin.gieseking@uos.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.gieseking@uos.de
--- Comment #1 from Martin Gieseking martin.gieseking@uos.de 2010-10-15 15:05:47 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #2 from hannes johannes.lips@googlemail.com 2010-10-16 15:22:11 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #3 from Martin Gieseking martin.gieseking@uos.de 2010-10-16 15:59:15 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #4 from hannes johannes.lips@googlemail.com 2010-10-17 08:18:01 EDT --- 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...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
Dominic Hopf dmaphy@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |dmaphy@fedoraproject.org Flag| |fedora-review?
--- Comment #5 from Dominic Hopf dmaphy@fedoraproject.org 2010-10-17 08:36:23 EDT --- I'd like to do the formal review then. :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #6 from Dominic Hopf dmaphy@fedoraproject.org 2010-10-31 06:34:27 EDT --- $ 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #7 from Martin Gieseking martin.gieseking@uos.de 2010-10-31 07:08:17 EDT --- (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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #8 from hannes johannes.lips@googlemail.com 2010-11-01 17:17:57 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #9 from Dominic Hopf dmaphy@fedoraproject.org 2010-11-01 17:33:34 EDT --- 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. :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
Dominic Hopf dmaphy@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
hannes johannes.lips@googlemail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #10 from hannes johannes.lips@googlemail.com 2010-11-02 13:15:25 EDT --- New Package SCM Request ======================= Package Name: keybinder Short Description: A library for registering global keyboard shortcuts Owners: hannes Branches: f14 f13 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #11 from Kevin Fenzi kevin@tummy.com 2010-11-03 00:07:31 EDT --- Git done (by process-git-requests).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
hannes johannes.lips@googlemail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE Last Closed| |2010-11-03 02:12:20
--- Comment #12 from hannes johannes.lips@googlemail.com 2010-11-03 02:12:20 EDT --- build in rawhide: http://koji.fedoraproject.org/koji/packageinfo?packageID=11135
http://koji.fedoraproject.org/koji/taskinfo?taskID=2573267
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
Christoph Wickert cwickert@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cwickert@fedoraproject.org
--- Comment #13 from Christoph Wickert cwickert@fedoraproject.org 2010-11-03 04:43:23 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #14 from hannes johannes.lips@googlemail.com 2010-11-03 06:25:23 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #15 from Christoph Wickert cwickert@fedoraproject.org 2010-11-03 06:57:52 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #16 from Fedora Update System updates@fedoraproject.org 2010-11-03 12:52:06 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #17 from Fedora Update System updates@fedoraproject.org 2010-11-03 12:54:02 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #18 from Fedora Update System updates@fedoraproject.org 2010-11-19 17:36:03 EST --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #19 from Fedora Update System updates@fedoraproject.org 2010-11-19 17:38:01 EST --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
Orion Poplawski orion@cora.nwra.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |kevin@scrye.com, | |orion@cora.nwra.com
--- Comment #20 from Orion Poplawski orion@cora.nwra.com 2011-05-27 12:57:31 EDT --- We need this in EL6 for xfce4-volumed. Who is on board to be a maintainer there?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=641093
--- Comment #21 from Orion Poplawski orion@cora.nwra.com 2011-05-27 15:28:03 EDT --- 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.
package-review@lists.fedoraproject.org