Bug 701812

Hi - I'm an unsponsored packager doing an informal package review.

MUST Items
==========

OK - rpmlint must be run on all rpms

$ rpmlint brutalchess-0.5.2-1.fc15.src.rpm
brutalchess.src: W: strange-permission brutalchess.desktop 0775L
brutalchess.src:9: W: macro-in-comment %{name}
brutalchess.src:9: W: macro-in-comment %{name}
brutalchess.src:9: W: macro-in-comment %{version}
brutalchess.src: W: invalid-url Source0: brutalchess-alpha-0.5.2-src.tar.gz
1 packages and 0 specfiles checked; 0 errors, 5 warnings.

$ rpmlint brutalchess-0.5.2-1.fc15.x86_64.rpm brutalchess-debuginfo-0.5.2-1.fc15.x86_64.rpm
brutalchess.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/brutalchess-0.5.2/NEWS
brutalchess.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/brutalchess-0.5.2/README
brutalchess.x86_64: W: no-manual-page-for-binary brutalchess
2 packages and 0 specfiles checked; 0 errors, 3 warnings.

After install:
$ rpmlint brutalchess
brutalchess.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/brutalchess-0.5.2/NEWS
brutalchess.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/brutalchess-0.5.2/README
brutalchess.x86_64: W: no-manual-page-for-binary brutalchess
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

OK - Package must meet naming guidelines
OK - Spec file name must match base package name
! - Package must meet packaging guidelines

There seem to be several bundled fonts in the package.
The patch is missing an upstream bug report link.

OK - Package must meet licensing guidelines
OK - License tag must match actual license
OK - Any license files must be in %doc
OK - Spec file must be in American English
OK - Spec file must be legible
OK - Sources must match upstream

$ sha1sum brutalchess-alpha-0.5.2-src.tar.gz brutalchess-alpha-0.5.2-src.tar.gz.1
f5e9d66eb34406a8627e51a8dabbba9cb4cecf0a  brutalchess-alpha-0.5.2-src.tar.gz
f5e9d66eb34406a8627e51a8dabbba9cb4cecf0a  brutalchess-alpha-0.5.2-src.tar.gz.1

OK - Package must build on at least one primary arch

Builds in mock on x86_64.

N/A - Arches that the package doesn't build on must be excluded with a relevant
bug
OK - All necessary build dependencies must be in BuildRequires
N/A - Locales must be handled properly
N/A - Binary rpms containing libraries must call ldconfig
OK - Package must not bundle system libraries
N/A - Relocatable packages must have rationalization
! - Package must own all directories it creates

Package does not appear to own /usr/share/brutalchess/

OK - Package must not list a file more than once in %files
OK - Files must have correct permissions
OK - Macros must be consistent
OK - Package must contain code or permissible content
N/A - Large documentation files must be in a -doc subpackage
OK - %doc files must not affect program operation
N/A - Header files must be in a -devel subpackage
N/A - Static libraries must be in a -static package
N/A - Library files that end in .so must go in a -devel package
N/A - -devel packages must require the base package using a fully versioned dependency
OK - Package must NOT contain any .la libtool archives
OK - Packages containing GUI applications must include a %{name}.desktop file
OK - Packages must not own files or directories already owned by other packages
OK - All filenames in rpm packages must be valid UTF-8

SHOULD Items
============

N/A - If the package is missing license text in a separate file, the packager should query upstream for it
N/A - Description and summary should contain translations if available
OK - Package should build in mock
OK - Package should build on all supported architectures

Koji scratch build seems okay.
http://koji.fedoraproject.org/koji/taskinfo?taskID=3076615

! - Package should function as described

Segfaults shortly after starting.

OK - Scriptlets should be sane
N/A - Non-devel subpackages should require the base package with a full version
N/A - pkgconfig files should be placed appropriately
N/A - File dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin should require package instead
! - Binaries/scripts should have man pages

Issues
======

Blocking:

1) The package bundles a few fonts (as discussed on IRC, some of these can be fixed with a patch from Debian). VeraMono.ttf is provided by bitstream-vera-sans-mono-fonts, so that should probably be handled with an explicit Requires and a symlink.

2) The package needs to own /usr/share/brutalchess/

Non-blocking:

3) The patch doesn't have a link to an upstream bug report (although upstream does seem inactive at the moment).

4) Brutal Chess segfaults after a few seconds when I run it. I'm running some memory heavy stuff in the background though, so it's probably not the package.

5) brutalchess doesn't have a man page.

6) It might be better just to put the complete source URL:
http://downloads.sourceforge.net/%{name}/%{name}-alpha-%{version}-src.tar.gz

7) The NEWS and README files have DOS line-endings. I'm not sure it's worth correcting them though.

Adding FE-NEEDSPONSOR, because Timur is not sponsored in the package maintainers group yet.

Timur was indeed sponsored at some point.

I am triaging old review tickets.  I can't promise a review if you reply, but by closing out the stale tickets we can devote extra attention to the ones which aren't stale.

This fails to build for me.  A scratch build for rawhide (f18 fails the same way): http://koji.fedoraproject.org/koji/taskinfo?taskID=5318470

And just a note that 6 and 7 above are indeed blockers.

Are you still interested in packaging BrutalChess? If you want, I could take the maintainment. Maybe a new bug is useful for a fresh review request from scratch.

Note: The embedded fonts are licensed as "freeware". So some doubts this package is ready for Fedora, maybe better for RPMFusion.

SPEC URL: https://raphgro.fedorapeople.org/review/brutalchess/brutalchess.spec

As written already in comment #5, I see legal issues with the included fonts. So I don't want to request an official review for now. The referenced sources could be downloaded via spectool.

Hi Raphael,

Unfortunately I no longer have the time to do this. So feel free to take it.
You can use my own packaging, the only issue with the package was the non-free fonts. The solution is to exclude the fonts included in the game and patch it to use something else. If I recall correctly, the Debian guys already had a patch for that, so you can try to use that patch.

Cheers,
Timur

(In reply to Timur Kristóf from comment #7)

> If I recall correctly, the Debian guys
> already had a patch for that, so you can try to use that patch.

Honestly, I don't like the debian patch. It uses an hard coded absolute path and inserts it into the code. My suggestion is to better use a symlink, default to GNU FreeFonts in package brutalchess-fonts. So we could provide the official fonts in RPMFusion as brutalchess-fonts-official with a special nonfree licence note.

Fontforge warns me that Ghostwri.ttf has a "not editable" licence:
marked with an FS Type of 2 (Restricted Licence)

(In reply to Timur Kristóf from comment #7)
> Hi Raphael,
> 
> Unfortunately I no longer have the time to do this. So feel free to take it.
> You can use my own packaging, the only issue with the package was the
> non-free fonts. The solution is to exclude the fonts included in the game
> and patch it to use something else. If I recall correctly, the Debian guys
> already had a patch for that, so you can try to use that patch.
> 
> Cheers,
> Timur

Hi Timur,

could you please close this bug as WONTFIX and with FE-DEADREVIEW as blocker?
https://fedoraproject.org/wiki/Package_Review_Process#Special_blocker_tickets

I would like to clone and start a new review process with me as the new packager. Then maybe feel free to switch into the reviewer role.

Thanks,
R.

Hi Raphael,

Sure, here you go. Good luck with your new package! :)

*** This bug has been marked as a duplicate of bug 1157213 ***

As this package was resubmitted in bug #1157213, lifting FE-DEADREVIEW.

Furthermore, this bug is closed as a duplicate of the new review request. As this bug does not to be "solved" before the review in the other bug can be done, it does not need to block it.