Bug 885038 - Review Request: pentobi - Program that plays the board game Blokus
Review Request: pentobi - Program that plays the board game Blokus
Status: NEW
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-Legal
  Show dependency treegraph
 
Reported: 2012-12-07 05:47 EST by Christophe Burgun
Modified: 2013-11-21 10:19 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Christophe Burgun 2012-12-07 05:47:00 EST
Spec URL: http://jouty.fedorapeople.org/pentobi.spec
SRPM URL: http://jouty.fedorapeople.org/pentobi-4.3-1.fc17.src.rpm
Description: Pentobi is a computer program that plays the board game Blokus
Fedora Account System Username:jouty
Comment 1 Christophe Burgun 2012-12-11 04:25:50 EST
Updating spec file with new pentobi version

Spec URL: http://jouty.fedorapeople.org/pentobi.spec
SRPM URL: http://jouty.fedorapeople.org/pentobi-5.0-1.fc17.src.rpm
Comment 2 Christophe Burgun 2012-12-11 09:14:15 EST
Add requires and some other changes (changelog)

Spec URL: http://jouty.fedorapeople.org/pentobi.spec
SRPM URL: http://jouty.fedorapeople.org/pentobi-5.0-2.fc17.src.rpm
Comment 3 Antonio Trande 2012-12-24 13:37:41 EST
Hi Christophe.

I'm not an official packager. Following is an 'unofficial package review' that is not valid and could contain some (my) errors.

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

Key:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
[!]: 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 is included in %doc.
     Note: Cannot find license.html in rpm(s)
See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text

See below.

===== MUST items =====

C/C++:
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[-]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package contains no bundled libraries.
[x]: Changelog in prescribed format.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Sources contain only permissible code or content.
[x]: Each %files section contains %defattr if rpm < 4.4
[!]: Macros in Summary, %description expandable at SRPM build time.

You can use: %{name} is a computer program to play the board game Blokus

[x]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install if there is
     such a file.
[-]: Development files must be in a -devel package
[-]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[x]: Package is not known to require ExcludeArch.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package complies to the Packaging Guidelines
[x]: Spec file lacks Packager, Vendor, PreReq tags.
[x]: 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 is included in %doc.
     Note: Cannot find license.html in rpm(s)
[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "Unknown or generated". 1 files have unknown license. Detailed output of
     licensecheck in /home/sagitter/885038-pentobi/licensecheck.txt

The license should be GPLv3+ (GPLv3 or later)
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#.22or_later_version.22_licenses

[!]: Package consistently uses macro is (instead of hard-coded directory
     names).

You can use %{name} in place of 'pentobi' in spec file. For example:

%{_datadir}/icons/hicolor/16x16/apps/%{name}.png instead of 
%{_datadir}/icons/hicolor/16x16/apps/pentobi.png

and so on.

[x]: Package is named using only allowed ASCII characters.
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[x]: Package do not use a name that already exist
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package installs properly.
[-]: Package is not relocatable.
[x]: Requires correct, justified where necessary.
[x]: CheckResultdir
[x]: Rpmlint is run on all rpms the build produces.
     Note: No rpmlint messages.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file is legible and written in American English.

BuildRequires and Requires entries should be listed one-by-one.

BuildRequires:	boost-devel
BuildRequires:  qt-devel
BuildRequires:  cmake
BuildRequires:  desktop-file-utils


[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: Package contains systemd file(s) if in need.
[x]: File names are valid UTF-8.
[x]: Useful -debuginfo package or justification otherwise.
[-]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 51200 bytes in 3 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[-]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: SourceX tarball generation or download is documented.
[x]: SourceX / PatchY prefixed with %{name}.
[x]: SourceX is a working URL.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Spec use %global instead of %define.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: No rpmlint messages.
[x]: Spec file according to URL is the same as in SRPM.
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.


Rpmlint
-------
Checking: pentobi-debuginfo-5.0-2.fc16.x86_64.rpm
          pentobi-5.0-2.fc16.src.rpm
          pentobi-5.0-2.fc16.x86_64.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint pentobi-debuginfo pentobi
pentobi-debuginfo.x86_64: I: enchant-dictionary-not-found en_US
2 packages and 0 specfiles checked; 0 errors, 0 warnings.
# echo 'rpmlint-done:'



Requires
--------
pentobi-debuginfo-5.0-2.fc16.x86_64.rpm (rpmlib, GLIBC filtered):
    

pentobi-5.0-2.fc16.x86_64.rpm (rpmlib, GLIBC filtered):
    
    boost
    libQtCore.so.4()(64bit)
    libQtGui.so.4()(64bit)
    libboost_filesystem-mt.so.1.47.0()(64bit)
    libboost_program_options-mt.so.1.47.0()(64bit)
    libboost_system-mt.so.1.47.0()(64bit)
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libm.so.6()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    qt
    rtld(GNU_HASH)



Provides
--------
pentobi-debuginfo-5.0-2.fc16.x86_64.rpm:
    
    pentobi-debuginfo = 5.0-2.fc16
    pentobi-debuginfo(x86-64) = 5.0-2.fc16

pentobi-5.0-2.fc16.x86_64.rpm:
    
    mimehandler(application/x-blokus-sgf)
    pentobi = 5.0-2.fc16
    pentobi(x86-64) = 5.0-2.fc16



MD5-sum check
-------------
http://downloads.sourceforge.net/pentobi/pentobi-5.0.tar.gz :
  CHECKSUM(SHA256) this package     : 0eaa142ca79ec2d24f86066d5a45e55e81b13875a689f8956b2fed4c78c55c33
  CHECKSUM(SHA256) upstream package : 0eaa142ca79ec2d24f86066d5a45e55e81b13875a689f8956b2fed4c78c55c33


Generated by fedora-review 0.3.1 (b71abc1) last change: 2012-10-16
Buildroot used: fedora-16-x86_64
Command line :/usr/bin/fedora-review -b 885038
Comment 4 Michael Schwendt 2012-12-27 05:53:02 EST
> Note: Cannot find license.html in rpm(s)

That could be a bug in fedora-review, because the file _is_ included, and the package also includes a COPYING as %doc.

fedora-review is not 100% safe. It certainly doesn't know all of the packaging guidelines to tell whether a package meets them or not. I wouldn't trust it too much, but suggest using it only to see where it complains and then double-check those items.


> [!]: Macros in Summary, %description expandable at SRPM build time.
> 
> You can use: %{name} is a computer program to play the board game Blokus

If fedora-review flagged that as '[!]', that's strange. The guidelines say:
https://fedoraproject.org/wiki/Packaging:Guidelines#Source_RPM_Buildtime_Macros

But this package doesn't use any macros in %summary or %description, so I don't understand what should be wrong here.

Btw, on the web page the game is named "Pentobi" with an upper-case first character. The package is named "pentobi", because more often than not we write everything in lower-case. If %name were used here, the %description would start the sentence with a lower-case character, which would look unusual.

The Naming Guidelines _try_ to explain when it may make sense to use a specific case in the package name,

  https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Case_Sensitivity

but I think there are only very few examples where developers have tried to influence the naming of RPM packages actually.

Conclusively, "Name: pentobi" is fine, and not using %{name} in the summary or description is fine, too.


> [x]: Package uses nothing in %doc for runtime.

This isn't trivial to check. And it's hard to tell how many packagers/reviewers examine it at all. For this package, it would be sufficient to check whether it wants to display the manual (not in a docdir, however) or the three %doc files via its "Help" menu. => It doesn't seem to do that.


> BuildRequires and Requires entries should be listed one-by-one.

Packager is free to disagree, however. ;-)


> [-]: %check is present and all tests pass.

Remains to be examined. Are the unit tests suitable for %check section?


[...]


A few findings:

> Requires:	boost,qt

https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires


> %{_prefix}/games/pentobi
> %dir %{_datadir}/games/%{name}

http://fedoraproject.org/wiki/SIGs/Games
http://fedoraproject.org/wiki/SIGs/Games/Packaging

| Data files (maps, pixmaps, sounds) go in  %{_datadir}/%{name} ,
| not %{_datadir}/games/%{name} . Binaries go in  %{_bindir} and
| not /usr/games. According to the FHS, the use of /usr/share/games
| and /usr/games is optional, and we recommend not using either for
| consistency, so that games are packaged like all other applications. 


> %{_datadir}/mime/packages/pentobi-mime.xml

https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#mimeinfo

> %{_datadir}/icons/hicolor/16x16/apps/pentobi.png

https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

> %{_datadir}/thumbnailers/pentobi.thumbnailer

$ rpm -qf /usr/share/thumbnailers
file /usr/share/thumbnailers is not owned by any package

https://fedoraproject.org/wiki/Packaging:UnownedDirectories

$ repoquery --whatprovides /usr/share/thumbnailers
thunar-vfs-0:1.2.0-7.fc18.x86_64
thunar-vfs-0:1.2.0-7.fc18.i686
ffmpegthumbnailer-0:2.0.8-2.fc18.x86_64
whaawmp-0:0.2.14-4.fc18.noarch
Comment 5 Antonio Trande 2012-12-27 06:23:20 EST
(In reply to comment #4)
> > Note: Cannot find license.html in rpm(s)
> 
> That could be a bug in fedora-review, because the file _is_ included, and
> the package also includes a COPYING as %doc.
> 
> fedora-review is not 100% safe. It certainly doesn't know all of the
> packaging guidelines to tell whether a package meets them or not. I wouldn't
> trust it too much, but suggest using it only to see where it complains and
> then double-check those items.
> 
> 
> > [!]: Macros in Summary, %description expandable at SRPM build time.
> > 
> > You can use: %{name} is a computer program to play the board game Blokus
> 
> If fedora-review flagged that as '[!]', that's strange. The guidelines say:
> https://fedoraproject.org/wiki/Packaging:
> Guidelines#Source_RPM_Buildtime_Macros
> 
> But this package doesn't use any macros in %summary or %description, so I
> don't understand what should be wrong here.
> 
> Btw, on the web page the game is named "Pentobi" with an upper-case first
> character. The package is named "pentobi", because more often than not we
> write everything in lower-case. If %name were used here, the %description
> would start the sentence with a lower-case character, which would look
> unusual.
> 
> The Naming Guidelines _try_ to explain when it may make sense to use a
> specific case in the package name,
> 
>   https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Case_Sensitivity
> 
> but I think there are only very few examples where developers have tried to
> influence the naming of RPM packages actually.
> 
> Conclusively, "Name: pentobi" is fine, and not using %{name} in the summary
> or description is fine, too.
> 
> 
> > [x]: Package uses nothing in %doc for runtime.
> 
> This isn't trivial to check. And it's hard to tell how many
> packagers/reviewers examine it at all. For this package, it would be
> sufficient to check whether it wants to display the manual (not in a docdir,
> however) or the three %doc files via its "Help" menu. => It doesn't seem to
> do that.
> 
> 
> > BuildRequires and Requires entries should be listed one-by-one.
> 
> Packager is free to disagree, however. ;-)
> 
> 
> > [-]: %check is present and all tests pass.
> 
> Remains to be examined. Are the unit tests suitable for %check section?
> 
> 
> [...]
> 
> 
> A few findings:
> 
> > Requires:	boost,qt
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires
> 
> 
> > %{_prefix}/games/pentobi
> > %dir %{_datadir}/games/%{name}
> 
> http://fedoraproject.org/wiki/SIGs/Games
> http://fedoraproject.org/wiki/SIGs/Games/Packaging
> 
> | Data files (maps, pixmaps, sounds) go in  %{_datadir}/%{name} ,
> | not %{_datadir}/games/%{name} . Binaries go in  %{_bindir} and
> | not /usr/games. According to the FHS, the use of /usr/share/games
> | and /usr/games is optional, and we recommend not using either for
> | consistency, so that games are packaged like all other applications. 
> 
> 

Hi Michael. Thank you for your helping me.

Really I noted these points but I'm confuse, because by compiling this software manually, its binaries are located precisely in those paths. 

> > %{_datadir}/mime/packages/pentobi-mime.xml
> 
> https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#mimeinfo
> 
> > %{_datadir}/icons/hicolor/16x16/apps/pentobi.png
> 
> https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
> 
> > %{_datadir}/thumbnailers/pentobi.thumbnailer
> 
> $ rpm -qf /usr/share/thumbnailers
> file /usr/share/thumbnailers is not owned by any package
> 
> https://fedoraproject.org/wiki/Packaging:UnownedDirectories
> 
> $ repoquery --whatprovides /usr/share/thumbnailers
> thunar-vfs-0:1.2.0-7.fc18.x86_64
> thunar-vfs-0:1.2.0-7.fc18.i686
> ffmpegthumbnailer-0:2.0.8-2.fc18.x86_64
> whaawmp-0:0.2.14-4.fc18.noarch

mmh... I have not seen a lot things.
Comment 6 Michael Schwendt 2012-12-28 06:46:57 EST
> by compiling this software manually, its binaries are located precisely
> in those paths. 

That would be an interesting exercise to examine further. How did your compilation commands differ from the spec file's %build and %install sections?

pentobi-5.0/CMakeLists.txt  contains several hardcoded install paths, which are the default.

The file also does the things RPM packages need to do in their scriptlets instead (GConf2 stuff, update-mime-database, update-desktop-database).
Comment 7 Antonio Trande 2012-12-28 07:32:45 EST
(In reply to comment #6)
> > by compiling this software manually, its binaries are located precisely
> > in those paths. 
> 
> That would be an interesting exercise to examine further. How did your
> compilation commands differ from the spec file's %build and %install
> sections?
> 
> pentobi-5.0/CMakeLists.txt  contains several hardcoded install paths, which
> are the default.

All data files hardcoded-install-paths point to {_datadir}/games/%{name} which are not correct according to packaging guidelines; I'm compiling manually so they become /usr/local/share/games/{name}.
Therefore let's change paths in CMakeLists.txt or we edit .spec file properly in order to change them.
Comment 8 Michael Schwendt 2012-12-28 13:35:17 EST
/usr/local/share/games is not different, because /usr/local/share is just the local (site-specific) datadir, which you get when installing software below /usr/local due to defaults.

$ rpm --eval %_datadir
/usr/share
$ rpm --eval %_prefix
/usr
Comment 9 Antonio Trande 2012-12-28 13:51:26 EST
(In reply to comment #8)
> /usr/local/share/games is not different, because /usr/local/share is just
> the local (site-specific) datadir, which you get when installing software
> below /usr/local due to defaults.
> 
> $ rpm --eval %_datadir
> /usr/share
> $ rpm --eval %_prefix
> /usr

Yes, this is what I mean. :)

Now, I don't know if another easiest way exists, I would change .spec file so:

...

%install
make install DESTDIR=%{buildroot}

# Make the correct binaries and data file paths according to the packaging guidelines for games
mkdir -p %{buildroot}%{_bindir}
cp -p %{buildroot}%{_prefix}/games/%{name} %{buildroot}%{_bindir}/%{name}
cp -p %{buildroot}%{_prefix}/games/%{name}-thumbnailer %{buildroot}%{_bindir}/%{name}-thumbnailer
rm    %{buildroot}%{_prefix}/games/%{name}*

mkdir -p %{buildroot}%{_datadir}/%{name}
cp -r -p %{buildroot}%{_datadir}/games/%{name}/* %{buildroot}%{_datadir}/%{name}
rm -rf   %{buildroot}%{_datadir}/games/%{name}

#check desktop file
desktop-file-validate %{buildroot}/%{_datadir}/applications/%{name}.desktop

%post
/usr/bin/update-mime-database %{_datadir}/mime &> /dev/null || :
/bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || :

%postun
/usr/bin/update-mime-database %{_datadir}/mime &> /dev/null || :
if [ $1 -eq 0 ] ; then
    /bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null
    /usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
fi

%posttrans
/usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :

> $ rpm -qf /usr/share/thumbnailers
> file /usr/share/thumbnailers is not owned by any package
> 
> https://fedoraproject.org/wiki/Packaging:UnownedDirectories
>

%files
%doc COPYING NEWS README
%{_bindir}/%{name}
%{_bindir}/%{name}-thumbnailer
%{_datadir}/applications/%{name}.desktop
%{_datadir}/icons/hicolor/16x16/apps/%{name}.png
%{_datadir}/icons/hicolor/16x16/mimetypes/application-x-blokus-sgf.png
%{_datadir}/icons/hicolor/32x32/apps/%{name}.png
%{_datadir}/icons/hicolor/48x48/apps/%{name}.png
%{_datadir}/icons/hicolor/48x48/mimetypes/application-x-blokus-sgf.png
%{_datadir}/icons/hicolor/scalable/apps/%{name}.svg
%{_datadir}/icons/hicolor/scalable/mimetypes/application-x-blokus-sgf.svg
%{_datadir}/mime/packages/%{name}-mime.xml

%dir %{_datadir}/thumbnailers
%{_datadir}/thumbnailers/%{name}.thumbnailer

%dir %{_datadir}/%{name}
%{_datadir}/%{name}/books/
%{_datadir}/%{name}/translations/

%lang(de) %{_datadir}/%{name}/manual/de/
%lang(en) %{_datadir}/%{name}/manual/en/
%lang(en_CA) %{_datadir}/%{name}/manual/en_CA/
%lang(en_GB) %{_datadir}/%{name}/manual/en_GB/

%{_mandir}/man6/%{name}-thumbnailer.6.*
%{_mandir}/man6/%{name}.6.*


$ rpm -qf /usr/share/thumbnailers
thunar-vfs-1.2.0-7.fc18.x86_64
pentobi-5.0-2.fc18.x86_64
Comment 10 Michael Schwendt 2012-12-28 14:05:20 EST
Simply moving the files in %install will break, because some of the paths get built/compiled into the program. For example:

  $ grep PENTOBI_ pentobi-5.0/src/* -R|grep DIR
  pentobi-5.0/src/pentobi/Main.cpp:#ifdef PENTOBI_MANUAL_DIR
  pentobi-5.0/src/pentobi/Main.cpp:        manualDir = PENTOBI_MANUAL_DIR;
  pentobi-5.0/src/pentobi/Main.cpp:#ifdef PENTOBI_BOOKS_DIR
  pentobi-5.0/src/pentobi/Main.cpp:        booksDir = PENTOBI_BOOKS_DIR;

For some of those paths, it should be enough to define several of the CMake variables to use non-default dirs at build-time already. From CMakeLists.txt:

if(NOT DEFINED PENTOBI_BOOKS_DIR)
  if(UNIX AND NOT APPLE)
    set(PENTOBI_BOOKS_DIR
      "${CMAKE_INSTALL_PREFIX}/share/games/pentobi/books")
  endif()
endif()
if(NOT DEFINED PENTOBI_MANUAL_DIR)
  if(UNIX AND NOT APPLE)
    set(PENTOBI_MANUAL_DIR
      "${CMAKE_INSTALL_PREFIX}/share/games/pentobi/manual")
  endif()
endif()
if(NOT DEFINED PENTOBI_TRANSLATIONS)
  if(UNIX AND NOT APPLE)
    set(PENTOBI_TRANSLATIONS
      "${CMAKE_INSTALL_PREFIX}/share/games/pentobi/translations")
  endif()
endif()

For installation of the executables (also the thumbnailer), it looks different.
Comment 11 Antonio Trande 2012-12-28 15:23:04 EST
To put an end to the matter, it seems me that it needs a patch at the root cause (CMakeLists.txt).
Comment 12 Michael Schwendt 2012-12-28 17:01:26 EST
Which is what I've just tried, because I was curious to figure out how much effort would be needed to adhere to the Games SIG's packaging guidelines.

I had started with adding

    -DPENTOBI_BOOKS_DIR=%{_datadir}/%{name}/books \
    -DPENTOBI_MANUAL_DIR=%{_datadir}/%{name}/manual \
    -DPENTOBI_TRANSLATIONS=%{_datadir}/%{name}/translations \

to the cmake invocation in the spec file. Then I built locally with "rpmbuild -bi pentobi.spec" to check what of the %files section would need to be adjusted.
For subsequent tests I've modified the files directly in the builddir and ran "rpm --short-circuit -bi pentobi.spec" to reinstall without recompiling. I've had to modify these

  ./src/books/CMakeLists.txt
  ./src/pentobi/CMakeLists.txt
  ./src/pentobi_thumbnailer/CMakeLists.txt
  ./src/libpentobi_gui/CMakeLists.txt

in trivial ways in their "install …" lines, since they contain hardcoded paths and don't use the PENTOBI_* variables defined above. One also cannot install into /usr/bin instead of /usr/games without patching out a hardcoded "games" destination value. Finally, I had to modify the spec to use

    -DPENTOBI_MANUAL_DIR=%{_datadir}/%{name}/ \

instead, because the cmake files install a full directory "manual" into that directory and ended up with a double "manual/manual/".

Shipping a patch upstream for the four CMakeLists.txt files is a package maintenance task that will need to be handled.
Comment 13 Antonio Trande 2012-12-29 11:53:23 EST
(In reply to comment #12)
> Which is what I've just tried, because I was curious to figure out how much
> effort would be needed to adhere to the Games SIG's packaging guidelines.
> 

If you have not already seen, this is reported in NEWS file in the original source:


Version 1.0 (1 Jan 2012)
========================
...

* Changed installation directories according to Filesystem
  Hierarchy Standard (/usr/bin to /usr/games, /usr/share to
  /usr/share/games)

...

Maintainer could already have a reverse patch.
Comment 14 Christophe Burgun 2013-01-24 10:45:34 EST
Hi Antonio, Hi Michael,

Thanks for the feedback and sorry for the long delay i takke for answer

> Note: Cannot find license.html in rpm(s)

This is a fedora-review bug 
%doc COPYING contain license

>For package name i have take default to lowercase naming.

> BuildRequires and Requires entries should be listed one-by-one.
I have changed each buildrequires and requires entries one-by-one

> [-]: %check is present and all tests pass.
I have add 
%check
ctest cmake_install.cmake

but :

Exécution_de(%check) : /bin/sh -e /var/tmp/rpm-tmp.PryLyS
+ umask 022
+ cd /builddir/build/BUILD
+ cd pentobi-5.0
+ ctest cmake_install.cmake
Test project /builddir/build/BUILD/pentobi-5.0
No tests were found!!!

so in check section i have only the desktop-file-validate

> Requires:	boost,qt
in Install notes it is written for this requirements
Are you sure that i need put away requires ?

> %{_prefix}/games/pentobi
> %dir %{_datadir}/games/%{name}
> | Data files (maps, pixmaps, sounds) go in  %{_datadir}/%{name} ,
> | not %{_datadir}/games/%{name} . Binaries go in  %{_bindir} and
> | not /usr/games. According to the FHS, the use of /usr/share/games
> | and /usr/games is optional, and we recommend not using either for
> | consistency, so that games are packaged like all other applications. 

https://sourceforge.net/p/pentobi/bugs/7/

for moment i apply the patch and this will be better in the next pentobi version

> %{_datadir}/mime/packages/pentobi-mime.xml
%post and %postun have been added

> %{_datadir}/icons/hicolor/16x16/apps/pentobi.png
%post and %postun have been added

> %{_datadir}/thumbnailers/pentobi.thumbnailer
%dir %{_datadir}/thumbnailers has been added
and bugreport has been open https://bugzilla.redhat.com/show_bug.cgi?id=893988
http://lists.fedoraproject.org/pipermail/packaging/2013-January/008850.html

Changelog Updated

Spec URL: http://jouty.fedorapeople.org/pentobi.spec
SRPM URL: http://jouty.fedorapeople.org/pentobi-5.0-3.fc17.src.rpm
Comment 15 Michael Schwendt 2013-01-24 16:44:04 EST
> Requires:	boost,qt

https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires


> No tests were found!!!

What about building with -DPENTOBI_BUILD_TESTS=ON and running "make test" in %check? Would that make sense for this program?

(Generally, there are different opinions about running test-suites at RPM package build-time. Some think only the software developers should run those test-suites. But often enough the target environment -- libs, compiler, options, bugs in libs -- differs and results in issues, so running test-suites more often, e.g. during automatic rebuilds, can be helpful.)
Comment 16 Christophe Burgun 2013-01-25 08:19:06 EST
Hi michael,

Changelog Updated

Spec URL: http://jouty.fedorapeople.org/pentobi.spec
SRPM URL: http://jouty.fedorapeople.org/pentobi-5.0-4.fc17.src.rpm
Comment 17 Christophe Burgun 2013-04-16 05:30:07 EDT
Updating spec file with new pentobi version

See changelog

Spec URL: http://jouty.fedorapeople.org/pentobi.spec
SRPM URL: http://jouty.fedorapeople.org/pentobi-6.0-1.fc17.src.rpm
Comment 18 Jason Tibbitts 2013-06-20 18:48:00 EDT
I'm pretty sure that Blokus is a registered trademark, and it looks to me that it's not being used correctly in this package.  http://fedoraproject.org/wiki/Packaging:Guidelines#Trademarks_in_Summary_or_Description

Blocking FE-Legal for an opinion.
Comment 19 Tom "spot" Callaway 2013-06-26 10:46:23 EDT
I concur. You will need to replace the "Blokus" trademark with "a popular block-based abstract strategy board game" in all the places it is visible to the end-user, including in the .spec description.
Comment 20 Christophe Burgun 2013-06-28 04:57:11 EDT
Hi, thanks for the help

I have write on mailing list of pentobi and the answer :

http://jouty.fedorapeople.org/trademark

So need i do a patch to replace this occurrences or like Markus says in spec description is it enough ?

In my opinion like Tom says "all the places it is visible to the end-user" but just want a confirmation to start the patch
Comment 21 Tom "spot" Callaway 2013-07-01 10:32:16 EDT
We want to err on the side of caution here, especially since the Blokus trademark is registered for use in computer games. Hopefully, this patch will not be too intrusive (but we need to carry it even if upstream is not interested).

To be pedantic, this is a grey area. The use might be considered fair use, or a judge might find that the use is infringing because people would confuse Pentobi for the "Blokus" computer game. We just want to eliminate the risk.

Note You need to log in before you can comment on or make changes to this bug.