Spec URL: http://people.redhat.com/green/FE/FC5/vkeybd.spec SRPM URL: http://people.redhat.com/green/FE/FC5/vkeybd-0.1.17-3.src.rpm Description: This is a virtual keyboard for AWE, MIDI and ALSA drivers. It's a simple fake of a MIDI keyboard on X-windows system. Enjoy a music with your mouse and "computer" keyboard :-) Adapted from old ccmra spec file.
Added nando's vkeybd icon. Minor cleanups. Spec URL: http://people.redhat.com/green/FE/FC5/vkeybd.spec SRPM URL: http://people.redhat.com/green/FE/FC5/vkeybd-0.1.17-4.src.rpm
Not a Review but some Chnages you need in SPEC :- 1) Use of dist tag in Release tag 2) Consider using make %{?_smp_mflags} in %build 3) Include License file
(In reply to comment #2) > Not a Review but some Chnages you need in SPEC :- Thanks for the comments. New bits here.. Spec URL: http://people.redhat.com/green/FE/FC5/vkeybd.spec SRPM URL: http://people.redhat.com/green/FE/FC5/vkeybd-0.1.17-5.src.rpm
Some source files do not have an Copyright / GPL header: fskip.c malloc.c itypes.h util.h About the license file: MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc. Please ask upstream to add the license and add copyright headers to the mentioned files.
I noticed something else: A BR is: tcl-devel >= 8.4 and in %build you use TCL_VERSION=8.4 It seems to me that because of this the spec file will not work for tcl-devel version 8.6 so you may better use tcl-devel = 8.4 imho.
Sorry for writing so many comments, I forgot to mention that rpmlint shows an error: E: vkeybd non-executable-script /usr/share/vkeybd/vkeybd.tcl 0444
(In reply to comment #4) > Please ask upstream to add the license and add copyright headers to the > mentioned files. I've asked upstream.
(In reply to comment #4) > Please ask upstream to add the license and add copyright headers to the > mentioned files. This was the response: "I don't mind to add but am too lazy to release a newer version for such small pieces. The license issue is written in README." So, should I just remove the COPYING file I added and proceed?
just remove your COPYING file and proceed. Hopefully there will be a new release sometime with the file added. Here are some other items that need to be taken care of: - move the .desktop file to a separate file - I think the invocation of "update-desktop-database %{_datadir}/applications" is not needed. According to http://fedoraproject.org/wiki/Packaging/ScriptletSnippets?highlight=%28update-desktop-database%29#head-de6770dd9867fcd085a73a4700f6bcd0d10294ef it is only need when the .desktop file has an MimeType entry - then you do not need "Requires(post): desktop-file-utils" and "Requires(postun): desktop-file-utils" - I am not sure, but $RPM_BUILD_ROOT/%{_datadir}/icons/vkeybd.png seems not the be the correct location for the icon since on my system all other icons are in subdirectories of icons, e.g. hicolor/<size> - alsa-lib-devel, jack-audio-connection-kit-devel and e2fsprogs-devel can be removed from BuildRequires since they are already required by lash-devel - tcl-devel can be removed from BuildRequires since it is already required by tk-devel if the versioned dependency can be done with tk-devel instead of tcl-devel and please do not forget the issues in comment 5 and 6
I've fixed everything mentioned so far and more.... New bits here: Spec URL: http://people.redhat.com/green/FE/FC5/vkeybd.spec SRPM URL: http://people.redhat.com/green/FE/FC5/vkeybd-0.1.17-6.src.rpm Thanks.
The specfile is not up to date. The requires for gtk should be removed, see: http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-7103f6c38d1b5735e8477bdd569ad73ea2c49bda
(In reply to comment #11) > The requires for gtk should be removed, see: D'oh! Thanks for pointing that out. New bits here: Spec URL: http://people.redhat.com/green/FE/FC5/vkeybd.spec SRPM URL: http://people.redhat.com/green/FE/FC5/vkeybd-0.1.17-7.src.rpm
It is too late for me to do a complete review now but there are still some minor issues: - please do not use a full path in the .desktop file in the Exec line because this make the path to the binary hardcoded. - there are some warnings in build.log in mock: sffile.c:122: warning: ignoring return value of 'fread', declared with attribute warn_unused_result - see them all in the attachment. - ChangeLog is not packaged - did you ask upstream to include your icon / desktop files? There is already an desktop file in the upstream tarball, so maybe upstream will include your improved desktop file. - the lash patch does not patch the README properly(LADCCA is still mentioned): - --ladcca bool + --lash bool Specify the support of LADCCA. Give yes or no as the - the manpage does not mention the --lash option (the upstream version not the --ladcca option) - have you submitted lash patch to upstream? (Just out of curiosity, what are the advantages of lash against ladcca? - some files have strange permissions, but I don't know whether or not this needs to be fixed: $ rpm -vql vkeybd | grep -- -r--r--r -r--r--r-- 1 root root 2278 Sep 19 23:41 /usr/share/man/man1/vkeybd.1.gz -r--r--r-- 1 root root 5765 Sep 19 23:41 /usr/share/vkeybd/vkeybd.list -r--r--r-- 1 root root 282 Sep 19 23:41 /usr/share/vkeybd/vkeybdmap -r--r--r-- 1 root root 590 Sep 19 23:41 /usr/share/vkeybd/vkeybdmap-german - changing %{_datadir}/vkeybd to %{_datadir}/vkeybd/ in %files makes it more obvious that an directory is meant
Just noticed something else: You can remove these lines from your spec --add-category Application --add-category AudioVideo and add to the .desktop file: Categories=Application;AudioVideo; or Categories=Application;AudioVideo;Audio;Midi;Music; This makes the .desktop file even more useful for inclusion in upstream.
Created attachment 136691 [details] mock build.log from revision 6 for development
The 0444 permission on some files do not need to be changed.
(In reply to comment #13) > - please do not use a full path in the .desktop file in the Exec line because > this make the path to the binary hardcoded. Fixed. > - there are some warnings in build.log in mock: sffile.c:122: warning: ignoring > return value of 'fread', declared with attribute warn_unused_result - see them > all in the attachment. I'll let upstream know. > - ChangeLog is not packaged Fixed. > - did you ask upstream to include your icon / desktop files? There is already an > desktop file in the upstream tarball, so maybe upstream will include your > improved desktop file. I'll do that. > - the lash patch does not patch the README properly(LADCCA is still mentioned): > > - --ladcca bool > + --lash bool > Specify the support of LADCCA. Give yes or no as the Fixed. > - the manpage does not mention the --lash option (the upstream version not the > --ladcca option) I'll report upstream. > - have you submitted lash patch to upstream? (Just out of curiosity, what are > the advantages of lash against ladcca? LADCCA is dead. LASH is the new LADCCA. > - some files have strange permissions, but I don't know whether or not this > needs to be fixed: > $ rpm -vql vkeybd | grep -- -r--r--r > -r--r--r-- 1 root root 2278 Sep 19 23:41 > /usr/share/man/man1/vkeybd.1.gz Fixed. > -r--r--r-- 1 root root 5765 Sep 19 23:41 > /usr/share/vkeybd/vkeybd.list > -r--r--r-- 1 root root 282 Sep 19 23:41 > /usr/share/vkeybd/vkeybdmap > -r--r--r-- 1 root root 590 Sep 19 23:41 > /usr/share/vkeybd/vkeybdmap-german I didn't change these. > - changing %{_datadir}/vkeybd to %{_datadir}/vkeybd/ in %files makes it more > obvious that an directory is meant Done. I also updated the .desktop file as per comment #14. Updated bits here: Spec URL: http://people.redhat.com/green/FE/FC5/vkeybd.spec SRPM URL: http://people.redhat.com/green/FE/FC5/vkeybd-0.1.17-8.src.rpm
- I just noticed that it is not the latest version, 0.1.17a is available at http://www.alsa-project.org/~iwai/vkeybd-0.1.17a.tar.bz2 with french keyboard layout in vkeybdmap-french - Requires: jack-audio-connection-kit is not needed because lash already depends on it. Builds fine in mock and rpmlint does not complain, naming, licenses, files and scriptlets are ok. md5sum: 3be348329a5aac3cdc21f89458d9369a matches. So remove Requires: jack-audio-connection-kit and update to the latest version and the package is approved.
(In reply to comment #18) > So remove Requires: jack-audio-connection-kit and update to the latest version > and the package is approved. Done and done. Thanks!