Bug 730426 - Review Request: mindless - Find the secret code
Summary: Review Request: mindless - Find the secret code
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mario Blättermann
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-08-12 21:18 UTC by Göran Uddeborg
Modified: 2011-08-30 20:45 UTC (History)
3 users (show)

Fixed In Version: mindless-1.0.0-4.fc16
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-08-30 20:45:14 UTC
mario.blaettermann: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Göran Uddeborg 2011-08-12 21:18:08 UTC
Spec URL: ftp://ftp.uddeborg.se/pub/mindless/mindless.spec
SRPM URL: ftp://ftp.uddeborg.se/pub/mindless/mindless-1.0.0-1.fc17.src.rpm
Description:
This is a simple game for two players using the rules of Mastermind.
You can play in human vs. human, human vs. computer or computer
vs. computer mode.  The goal of the game is to crack a secret code
consisting of four balls which can each be one of eight colors.

Rpmlint warnings:
mindless.x86_64: E: incorrect-fsf-address /usr/share/doc/mindless-1.0.0/COPYING
mindless.x86_64: W: no-manual-page-for-binary mindless

I've reported both to the developer, as expected.  (It remains to be seen if he cares enough about it.  This is a rather old program.)

Comment 1 Mario Blättermann 2011-08-13 19:16:24 UTC
(In reply to comment #0)

> mindless.x86_64: E: incorrect-fsf-address /usr/share/doc/mindless-1.0.0/COPYING
> mindless.x86_64: W: no-manual-page-for-binary mindless
> 
> I've reported both to the developer, as expected.  (It remains to be seen if he
> cares enough about it.  This is a rather old program.)

For the COPYING file, you should fetch a new one from fsf.org [1] and replace the old one. I assume the upstream developer is gone for a long time, given the age of more than eight years. All the other occurences of the old address in the source files you may leave untouched.

[1] http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt


If there's no man page available, no problem, it is no blocker for a package.

Some initial issues:

To make your spec better readable, you should put all macros into brackets {}.
The %defattr macro in the file list is obsolete, only for packages for older EPEL versions needed. I assume you don't plan to provide it for EPEL, because you didn't define a BuildRoot and there's no %clean section.

Your package ships the FreeSans font, which is already available for Fedora (from the package gnu-free-sans-fonts). No idea what to do here, but in my mind, although we don't have a file collision, a system doesn't need a font twice.


Koji scratch build:

http://koji.fedoraproject.org/koji/taskinfo?taskID=3270948

Comment 2 Göran Uddeborg 2011-08-15 16:49:48 UTC
Thanks very much for your comments!

> For the COPYING file, you should fetch a new one from fsf.org

Seems I read http://lists.fedoraproject.org/pipermail/legal/2011-August/001701.html a bit too quickly.  I thought I should not do that, but on a second reading, I see I'm allowed, (though not required).  I'll replace it as you suggest.

> I assume the upstream developer is gone for a long time

I actually could get in touch with the upstreams developer.  The e-mail address in the package wasn't valid, but with a bit of googling I found him.  He would consider doing a little update of the document, so maybe there will be an updated version.  I've made the README file into a man page and sent him.  In the mean time, I add it to the package as an additional source.

> The %defattr macro in the file list is obsolete

Right, I forgot.  And no, I have no plans to put it in EPEL.  I just forgot this wasn't needed any more.

> Your package ships the FreeSans font, which is already available for Fedora

A good point.  After thinking a little, I made a patch that uses fontconfig to find the file for the font FreeSans:Bold, and uses that instead of the bundled version.  In addition to the obvious BuildRequires (fontconfig itself and automake16), I also added gnu-free-sans-fonts as a regular Requires.

> To make your spec better readable, you should put all macros into brackets {}

On this point I would like to disagree.  I find those additional brackets reducing readability.

Of course, it's not up to my taste, but what the guidelines says.  I looked for some note on this point in the guidelines, but couldn't find anything.  But I seem to remember this point was raised with some other package I was involved in too.  I will ask on the packaging list for a "ruling".

Updated version:
SPEC: ftp://ftp.uddeborg.se/pub/mindless/mindless.spec
SRPM: ftp://ftp.uddeborg.se/pub/mindless/mindless-1.0.0-2.fc17.src.rpm

Comment 3 Göran Uddeborg 2011-08-20 17:44:45 UTC
The opinion on the packaging list is to require brackets around all (simple) macros.  Here is an updated version which follows the upcoming rule.

SPEC: ftp://ftp.uddeborg.se/pub/mindless/mindless.spec
SRPM: ftp://ftp.uddeborg.se/pub/mindless/mindless-1.0.0-3.fc15.src.rpm

(My mock has problems building for rawhide at the moment.  Not just this package; it fails in yum for any rawhide build.  I built for F15 instead for now.)

Comment 4 Mario Blättermann 2011-08-21 15:05:22 UTC
Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3290034

$ rpmlint -i -v *
mindless.i686: I: checking
mindless.i686: I: enchant-dictionary-not-found sv
A dictionary for the Enchant spell checking library is not available for the
language given in the info message.  Spell checking will proceed with
rpmlint's built-in implementation for localized tags in this language. For
better spell checking results in this language, install the appropriate
dictionary that Enchant will use for this language, often for example
hunspell-* or aspell-*.

mindless.i686: I: checking-url http://www.lysator.liu.se/~mbrx/mindless/ (timeout 10 seconds)
mindless.src: I: checking
mindless.src: I: checking-url http://www.lysator.liu.se/~mbrx/mindless/ (timeout 10 seconds)
mindless.src: I: checking-url http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt (timeout 10 seconds)
mindless.src: I: checking-url http://www.lysator.liu.se/~mbrx/mindless/mindless-1.0.0.tar.gz (timeout 10 seconds)
mindless.x86_64: I: checking
mindless.x86_64: I: checking-url http://www.lysator.liu.se/~mbrx/mindless/ (timeout 10 seconds)
mindless-debuginfo.i686: I: checking
mindless-debuginfo.i686: I: checking-url http://www.lysator.liu.se/~mbrx/mindless/ (timeout 10 seconds)
mindless-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/setupMode.cc
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/mindless.cc
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/setupMode.h
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/general.h
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/general.cc
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/game.cc
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/game.h
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/mainMode.cc
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/mainMode.h
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.i686: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/gameMode.cc
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.x86_64: I: checking
mindless-debuginfo.x86_64: I: checking-url http://www.lysator.liu.se/~mbrx/mindless/ (timeout 10 seconds)
mindless-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/setupMode.cc
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/mindless.cc
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/general.h
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/general.cc
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/game.cc
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/setupMode.h
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/mainMode.cc
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/mainMode.h
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/game.h
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/mindless-1.0.0/src/gameMode.cc
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

mindless.spec: I: checking-url http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt (timeout 10 seconds)
mindless.spec: I: checking-url http://www.lysator.liu.se/~mbrx/mindless/mindless-1.0.0.tar.gz (timeout 10 seconds)
5 packages and 1 specfiles checked; 20 errors, 0 warnings.

Besides the old FSF address in the source files and the missing Swedish dictionary on my system, no issues.

Comment 5 Mario Blättermann 2011-08-21 15:16:08 UTC
---------------------------------
key:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
    GPLv2+
[+] MUST: The License field in the package spec file must match the actual
license.
[+] MUST: The file containing the text of the license(s) for the package must
be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ md5sum *
    e1fa6ea24978fa2a09302e3ef88fe24c  mindless-1.0.0.tar.gz
    e1fa6ea24978fa2a09302e3ef88fe24c  mindless-1.0.0.tar.gz.packaged

[+] MUST: The package MUST successfully compile and build into binary rpms on
at least one primary architecture.
    - Succesful Koji build available.
[.] MUST: If the package does not successfully compile, build or work on an
architecture, ...
[+] MUST: All build dependencies must be listed in BuildRequires.
[.] MUST: The spec file MUST handle locales properly.
[.] MUST: If a package installs files below %{_datadir}/icons, the icon cache
must be updated.
[.] MUST: Packages storing shared library files (not just symlinks) must call
ldconfig in %post and %postun.
[.] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates.
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Packages must not provide RPM dependency information when that
information is not global in nature, or are otherwise handled.
[.] MUST: When filtering automatically generated RPM dependency information,
the filtering system implemented by Fedora must be used.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[.] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[.] MUST: If a package contains library files with a suffix (e.g.
libfoo.so.1.1), ...
[.] MUST: devel packages must require the base package using a fully versioned
dependency.
[.] MUST: Packages must NOT contain any .la libtool archives.
[+] MUST: Packages containing GUI applications must include a %{name}.desktop
file
[.] MUST: .desktop files must be properly installed with desktop-file-install
in the %install section.
[+] MUST: Packages must not own files or directories already owned by other
packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

[.] SHOULD: If the source package does not include license text(s) as a
    separate file from upstream, the packager SHOULD query upstream...
[+] SHOULD: Timestamps of files should be preserved.
[+] SHOULD: The reviewer should test that the package builds in mock.
    See Koji build above (which uses mock anyway)
[+] SHOULD: The reviewer should test that the package functions as described.
    I assume the packager has tested it.
[.] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base
package using a fully versioned dependency.
[.] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg.
[.] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin,
/usr/bin, or /usr/sbin ...
[+] SHOULD: Your package should contain man pages for binaries/scripts.

Comment 6 Mario Blättermann 2011-08-21 15:22:30 UTC
Sorry, forgotten to mention:

PACKAGE APPROVED.

BTW, please add the following German summary and description to your package:

Summary(de): Finden Sie den Geheimcode

%description -l de
Dies ist ein kleines Spiel für zwei Teilnehmer, das nach den Regeln von
Mastermind gespielt wird. Die Kombinationen Mensch gegen Mensch, Mensch
gegen Rechner oder Rechner gegen Rechner sind möglich. 
Ziel des Spiels ist es, einen durch vier verschieden farbige Kugeln
dargestellten Geheimcode zu knacken.

Comment 7 Göran Uddeborg 2011-08-21 21:32:25 UTC
Thank you for the review.  And thanks for the German translation of the summary and description! :-)

When doing koji builds for f16 and rawhide now, I discovered that automake16 has been removed from F16.  So I switched to automake17 instead, which seems to work just fine.  I had to add some auto* commands before running configure, to remove saved automake version from the distributed files.

I'm not sure why my previous rawhide builds using mock didn't catch this.

Koji scratch builds:
F16: http://koji.fedoraproject.org/koji/taskinfo?taskID=3290431
Rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=3290434

Comment 8 Göran Uddeborg 2011-08-21 21:38:13 UTC
New Package SCM Request
=======================
Package Name: mindless
Short Description: Find the secret code
Owners: goeran
Branches: f16
InitialCC:

Comment 9 Gwyn Ciesla 2011-08-22 10:02:37 UTC
Git done (by process-git-requests).

Comment 10 Fedora Update System 2011-08-22 16:12:41 UTC
mindless-1.0.0-4.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/mindless-1.0.0-4.fc16

Comment 11 Fedora Update System 2011-08-22 18:33:39 UTC
mindless-1.0.0-4.fc16 has been pushed to the Fedora 16 testing repository.

Comment 12 Fedora Update System 2011-08-30 20:45:08 UTC
mindless-1.0.0-4.fc16 has been pushed to the Fedora 16 stable repository.


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