Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 985065
Summary: | Review Request: peg-solitaire - Board game played with pegs | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Mario Blättermann <mario.blaettermann> |
Component: | Package Review | Assignee: | Peter Lemenkov <lemenkov> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | i, kevin, lemenkov, notting, package-review |
Target Milestone: | --- | Flags: | lemenkov:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | peg-solitaire-2.0-4.fc20 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2013-11-09 03:33:44 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | 989437 | ||
Bug Blocks: | 928937 |
Description
Mario Blättermann
2013-07-16 17:18:06 UTC
Hi, 1. I think your desktop file is incorrect. [Desktop Entry] Exec='/usr/games/peg-solitaire' The path is wrong. Icon=/usr/share/pixmaps/peg-solitaire.xpm This kind of way to define a icon is not recommended now, is it possible to change to "Icon=peg-solitaire"?(I'm sorry I haven't tried it) MimeType= Well this is just a game and no new MIME types are being introduced, so you don't need to add %post{un} scriptlet to update the database. 2. In your spec I can find find_lang script, but you've commented out them, and there are language files in the tarball in fact. Please fix. 3. I can see "${RPM_BUILD_ROOT}" in your spec. However as we can see from the guideline http://fedoraproject.org/wiki/Packaging:Guidelines#UsingBuildRootOptFlags You can use $RPM_BUILD_ROOT or %{buildroot} to define the buildroot, but now you use a special style "${RPM_BUILD_ROOT}"... So, if you want to use macro style, change it to %{buildroot}; Otherwise please change it to variable style. Mix using 2 style is WRONG, and you mix them in one new style is WRONG again. (In reply to Christopher Meng from comment #1) > Hi, > > 1. I think your desktop file is incorrect. > > [Desktop Entry] > Exec='/usr/games/peg-solitaire' > > The path is wrong. > > Icon=/usr/share/pixmaps/peg-solitaire.xpm > > This kind of way to define a icon is not recommended now, is it possible to > change to "Icon=peg-solitaire"?(I'm sorry I haven't tried it) > > MimeType= > > Well this is just a game and no new MIME types are being introduced, so you > don't need to add %post{un} scriptlet to update the database. > First you should have a look at the patch. It makes sure that the desktop file becomes valid and the executable binary goes into /usr/bin instead of /usr/games. You refer to that file in the tarball. This is not usable, indeed. Moreover, the scriptlet doesn't contain anything about mime types. It's only for updating the desktop database. > 2. In your spec I can find find_lang script, but you've commented out them, > and there are language files in the tarball in fact. Please fix. > Please read the comment. The find_lang macro is useful and applicable when gettext (or even the qt translation chain) places the language files in /usr/share/locale/language_code/LC_MESSAGES/*. This is not the case, peg-solitaire behaves completely different from other packages. I was wondering if it is possible to get find_lang working here, or if it even makes sense to use it when all the language files are in the same folder anyway. Well, if I'm forced to use it, any ideas would be welcome. > 3. I can see "${RPM_BUILD_ROOT}" in your spec. > > However as we can see from the guideline > > http://fedoraproject.org/wiki/Packaging:Guidelines#UsingBuildRootOptFlags > > You can use $RPM_BUILD_ROOT or %{buildroot} to define the buildroot, but now > you use a special style "${RPM_BUILD_ROOT}"... > > So, if you want to use macro style, change it to %{buildroot}; Otherwise > please change it to variable style. Mix using 2 style is WRONG, and you mix > them in one new style is WRONG again. Fixed. Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/peg-solitaire.spec SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/peg-solitaire-2.0-2.fc19.src.rpm For what it's worth, "${RPM_BUILD_ROOT}" is valid shell syntax, just unusual in this context (because $RPM_BUILD_ROOT is normally followed by a /, making the braces useless, and quoting it is also not supposed to be necessary). As for the other 2 points, unfortunately, there are some differences between what is written in the wiki and what is actually true in practice (and all experienced packagers know): > Moreover, the scriptlet doesn't contain anything about mime types. It's only > for updating the desktop database. Actually, update-desktop-database is only needed if the .desktop file claims any MIME types, otherwise it does nothing (and just wastes the user's time). > Please read the comment. The find_lang macro is useful and applicable when > gettext (or even the qt translation chain) places the language files in > /usr/share/locale/language_code/LC_MESSAGES/*. Actually, it looks for the files with a "find" in the whole RPM_BUILD_ROOT, so it doesn't matter where they are. Please try it (but don't forget the --with-qt --without-mo switches if this is using Qt rather than gettext translations), it should work. (In reply to Kevin Kofler from comment #4) > As for the other 2 points, unfortunately, there are some differences between > what is written in the wiki and what is actually true in practice (and all > experienced packagers know): > > > Moreover, the scriptlet doesn't contain anything about mime types. It's only > > for updating the desktop database. > > Actually, update-desktop-database is only needed if the .desktop file claims > any MIME types, otherwise it does nothing (and just wastes the user's time). > OK, then it has to be changed in the wiki. There must not be a difference between the "law" and the "practice". And in no case we may assume that an "experienced packager" knows about that. Then we could also assume a lot of more things, and we could shrink the guidelines to a minimum. > > Please read the comment. The find_lang macro is useful and applicable when > > gettext (or even the qt translation chain) places the language files in > > /usr/share/locale/language_code/LC_MESSAGES/*. > > Actually, it looks for the files with a "find" in the whole RPM_BUILD_ROOT, > so it doesn't matter where they are. Please try it (but don't forget the > --with-qt --without-mo switches if this is using Qt rather than gettext > translations), it should work. OK, I didn't know about the --without-mo switch. It's the first time I work on non-gettext stuff. I've added the macro with the following line: %find_lang -f solitari.lang --with-qt --without-mo But I get this error: + /usr/lib/rpm/find-lang.sh /home/mariobl/rpmbuild/BUILDROOT/peg-solitaire-2.0-3.fc19.x86_64 -f solitari.lang --with-qt --without-mo No translations found for -f in /home/mariobl/rpmbuild/BUILDROOT/peg-solitaire-2.0-3.fc19.x86_64 The problem is, we don't have files named solitari.qm in separate folders for each supported language. All the qm files are in a single folder with the following naming scheme: $ rpm -ql peg-solitaire ... /usr/share/games/peg-solitaire/locales /usr/share/games/peg-solitaire/locales/solitari.qm /usr/share/games/peg-solitaire/locales/solitari_ca_ES.qm /usr/share/games/peg-solitaire/locales/solitari_de_DE.qm /usr/share/games/peg-solitaire/locales/solitari_en_EN.qm /usr/share/games/peg-solitaire/locales/solitari_en_US.qm /usr/share/games/peg-solitaire/locales/solitari_es_ES.qm /usr/share/games/peg-solitaire/locales/solitari_eu_ES.qm /usr/share/games/peg-solitaire/locales/solitari_fr_FR.qm /usr/share/games/peg-solitaire/locales/solitari_gl_ES.qm /usr/share/games/peg-solitaire/locales/solitari_it_IT.qm /usr/share/games/peg-solitaire/locales/solitari_pl_PL.qm /usr/share/games/peg-solitaire/locales/solitari_pt_BR.qm /usr/share/games/peg-solitaire/locales/solitari_pt_PT.qm As another attempt I tried to use a wildcard (solitari*.lang), but it failed again. The guideline says: "Keep in mind that usage of %find_lang in packages containing locales is a MUST." But is it really worth the effort to patch the package to get this script working? Well, we use find_lang to avoid mistakes such as wrong folder ownerships and writing the translation files in a long list [1]. But peg-solitaire behaves different from the usual way, we should keep this in mind. And if I'm really forced to use it, my knowledge is probably too poor to patch the sources in a way that it works. BTW, some of the translations are made by an automatized tool like Google Translator and are of bad quality, at least the German one. This is another reason to avoid the effort. [1] http://fedoraproject.org/wiki/Packaging:Guidelines#Why_do_we_need_to_use_.25find_lang.3F (In reply to Mario Blättermann from comment #5) > OK, then it has to be changed in the wiki. There must not be a difference > between the "law" and the "practice". And in no case we may assume that an > "experienced packager" knows about that. Then we could also assume a lot of > more things, and we could shrink the guidelines to a minimum. Sorry for the blurb... Just found the correct usage in the scriptlets collection [1]: "Use this when a desktop entry has a 'MimeType key." But it is somewhat strage, though. Such a hint should be in the packaging guidelines, not in the scriptlets collection only. [1] http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database > %find_lang -f solitari.lang --with-qt --without-mo
That's not valid %find_lang syntax. Try:
%find_lang solitari --with-qt --without-mo
In the %files list, you'll want to list:
%dir %{_datadir}/games/peg-solitaire/locales/
%{_datadir}/games/peg-solitaire/locales/solitari.qm
directly and add -f solitary.lang for the rest.
(In reply to Mario Blättermann from comment #6) > (In reply to Mario Blättermann from comment #5) > > OK, then it has to be changed in the wiki. There must not be a difference > > between the "law" and the "practice". And in no case we may assume that an > > "experienced packager" knows about that. Then we could also assume a lot of > > more things, and we could shrink the guidelines to a minimum. > > Sorry for the blurb... Just found the correct usage in the scriptlets > collection [1]: > > "Use this when a desktop entry has a 'MimeType key." > > But it is somewhat strage, though. Such a hint should be in the packaging > guidelines, not in the scriptlets collection only. > > [1] > http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database Yep, you can raise a ticket to FPC later. (In reply to Kevin Kofler from comment #7) > > %find_lang -f solitari.lang --with-qt --without-mo > > That's not valid %find_lang syntax. Try: > %find_lang solitari --with-qt --without-mo > > In the %files list, you'll want to list: > %dir %{_datadir}/games/peg-solitaire/locales/ > %{_datadir}/games/peg-solitaire/locales/solitari.qm > directly and add -f solitary.lang for the rest. Still not working. Error message was: + /usr/lib/rpm/find-lang.sh /home/mariobl/rpmbuild/BUILDROOT/peg-solitaire-2.0-3.fc19.x86_64 solitari --with-qt --without-mo No translations found for solitari in /home/mariobl/rpmbuild/BUILDROOT/peg-solitaire-2.0-3.fc19.x86_64 Here's my current spec file: http://mariobl.fedorapeople.org/Review/SPECS/peg-solitaire.spec Well, you can try to debug /usr/lib/rpm/find-lang.sh to find out whether it can handle those paths. Also note there are bugs in it, such as: bug 729336 (find-lang.sh doesn't find all locales when --with-qt and --all-name are combined) I've filed a bug against rpm. (In reply to Mario Blättermann from comment #11) > I've filed a bug against rpm. The find_lang macro was actually working, but there was a problem in my spec file which has been solved. Here are the new files: Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/peg-solitaire.spec SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/peg-solitaire-2.0-4.fc19.src.rpm I'll review this. Koji scratchbuild for F-19 (just because it faster tenfold than F20/F21 with arm enabled) http://koji.fedoraproject.org/koji/taskinfo?taskID=6113561 REVIEW: Legend: + = PASSED, - = FAILED, 0 = Not Applicable + rpmlint is not silent, but the only message may be ignored now: Auriga ~/Desktop: rpmlint peg-solitaire-* peg-solitaire.src:26: W: rpm-buildroot-usage %build qmake-qt4 PREFIX=%{buildroot}/%{_prefix} %{name}.pro ^^^ This should actually go into %install stage but I don't insist on fixing this right now. 3 packages and 0 specfiles checked; 0 errors, 1 warnings. Auriga ~/Desktop: + The package is named according to the Package Naming Guidelines. + The spec file name matches the base package %{name}, in the format %{name}.spec. + The package meets the Packaging Guidelines. + The package is licensed with a Fedora approved license and meets the Licensing Guidelines. + The License field in the package spec file matches the actual license (GPLv3 or later). + The file, containing the text of the license(s) for the package, is included in %doc. + The spec file is written in American English. + The spec file for the package is legible. + The sources used to build the package, match the upstream source, as provided in the spec URL. sulaco ~/rpmbuild/SOURCES: sha256sum peg-solitaire-2.0.tar.gz* 71ac0a149a10c034051a7ac464fdff64205b45d770b29f76e9d251e6993cead3 peg-solitaire-2.0.tar.gz 71ac0a149a10c034051a7ac464fdff64205b45d770b29f76e9d251e6993cead3 peg-solitaire-2.0.tar.gz.1 sulaco ~/rpmbuild/SOURCES: + The package successfully compiles and builds into binary rpms on at least one primary architecture. See Koji link above. + All build dependencies are listed in BuildRequires. + The spec file handles locales properly (by using the %find_lang macro). 0 No shared library files in some of the dynamic linker's default paths. + The package does NOT bundle copies of system libraries. 0 The package is not designed to be relocatable. + The package owns all directories that it creates. + The package does not list a file more than once in the spec file's %files listings. + Permissions on files are set properly. + The package consistently uses macros. + The package contains code, or permissible content. 0 No extremely large documentation files. + Anything, the package includes as %doc, does not affect the runtime of the application. 0 No C/C++ header files. 0 No static libraries. 0 No pkgconfig(.pc) files. 0 The package doesn't contain library files without a suffix (e.g. libfoo.so) in some of the dynamic linker's default paths. 0 No devel sub-package. + The package does NOT contain any .la libtool archives. + The package includes a %{name}.desktop file, and this file is validated with desktop-file-validate in the %check section. + The package does not own files or directories already owned by other packages. + All filenames in rpm packages are valid UTF-8. I don't see any issues, so this package is APPROVED. Many thanks for the review! New Package SCM Request ======================= Package Name: peg-solitaire Short Description: Board game played with pegs Owners: mariobl Branches: f19 f20 Git done (by process-git-requests). peg-solitaire-2.0-4.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/peg-solitaire-2.0-4.fc20 peg-solitaire-2.0-4.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/peg-solitaire-2.0-4.fc19 peg-solitaire-2.0-4.fc20 has been pushed to the Fedora 20 testing repository. peg-solitaire-2.0-4.fc19 has been pushed to the Fedora 19 stable repository. peg-solitaire-2.0-4.fc20 has been pushed to the Fedora 20 stable repository. |