Bug 459908 - (freedink) Review Request: freedink - Adventure and role-playing game (engine)
Review Request: freedink - Adventure and role-playing game (engine)
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On: freedink-data freedink-dfarc
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-24 05:43 EDT by Sylvain Beucler
Modified: 2008-10-20 18:17 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-10-06 09:55:48 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Sylvain Beucler 2008-08-24 05:43:18 EDT
(This is my first package and I need a sponsor.)

GNU FreeDink is composed of 3 packages:
- game engine
- game data
- front-end

Spec URL: http://www.freedink.org/snapshots/fedora/freedink.spec
SRPM URL: http://www.freedink.org/snapshots/fedora/freedink-1.08.20080823-1.fc8.src.rpm
Description: Adventure and role-playing game (engine)
Dink Smallwood is an adventure/role-playing game, similar to Zelda,
made by RTsoft. Besides twisted humour, it includes the actual game
editor, allowing players to create hundreds of new adventures called
Dink Modules or D-Mods for short.

GNU FreeDink is a new and portable version of the game engine, which
runs the original game as well as its D-Mods, with close
compatibility, under multiple platforms.



Spec URL: http://www.freedink.org/snapshots/fedora/dink-data.spec
SRPM URL: http://www.freedink.org/snapshots/fedora/dink-data-1.08-1.fc8.src.rpm
Description: Adventure and role-playing game (game data)
Dink Smallwood is an adventure/role-playing game, similar to Zelda,
made by RTsoft. Besides twisted humour, it includes the actual game
editor, allowing players to create hundreds of new adventures called
Dink Modules or D-Mods for short.

This package contains architecture-independent data for the original
game (except for non-free sounds).


Spec URL: http://www.freedink.org/snapshots/fedora/dfarc.spec
SRPM URL: http://www.freedink.org/snapshots/fedora/dfarc-2.99.20080823-1.fc8.src.rpm
Description: Frontend and .dmod installer for GNU FreeDink
DFArc2 makes it easy to play and manage the Dink Smallwood game and
it's numerous Dink Modules (or D-Mods).


I'd like to get feedback about the packaging and eventally upload them!


Legal: the Contribution is the following files:
- freedink.spec
- dfarc.spec
- dink-data.spec
- dink-data_README.Fedora

Any other file is not part of the Contribution.
Comment 1 Mamoru TASAKA 2008-08-24 06:43:45 EDT
Hi:

Please create new review requests for each srpm and add proper "Depends on/Blocks"
dependency checker on the bug.

Also, as you need sponsors, please make the submitted review requests block
FE-NEEDSPONSOR blockers.
Comment 2 Sylvain Beucler 2008-08-24 09:53:34 EDT
> Please create new review requests for each srpm and add proper
> "Depends on/Blocks" dependency checker on the bug.
> 
> Also, as you need sponsors, please make the submitted review
> requests block FE-NEEDSPONSOR blockers.

All done :)
Comment 3 Sylvain Beucler 2008-08-26 18:58:09 EDT
Hello,

Do you need anything else?


I also uploaded an updated version (remove a dependency and amd64 fixes)
Spec URL: http://www.freedink.org/snapshots/fedora/freedink.spec
SRPM URL:
http://www.freedink.org/snapshots/fedora/freedink-1.08.20080826-1.fc8.src.rpm
Comment 4 Mamoru TASAKA 2008-09-05 13:51:47 EDT
Well,
* Same with bug 459916, the license of src/freedink_xpm.c is unclear.
* This package installs LiberationSans-Regular.ttf. However this font is already
  provided by other Fedora package and shipping duplicate fonts like this way
  is not allowed.
  Please modify the source so that this package uses system widely provided
  fonts.
Comment 5 Sylvain Beucler 2008-09-06 05:59:06 EDT
Hi,

> * This package installs LiberationSans-Regular.ttf. However this font is
> already provided by other Fedora package and shipping duplicate fonts like
> this way is not allowed.  Please modify the source so that this package uses
> system widely provided fonts.

FreeDink is using a precise version of LiberationSans-Regular.ttf (ttf-3) and the currently released version has a bug https://bugzilla.redhat.com/show_bug.cgi?id=458592

The latest version of the font also introduces some non-bug graphical differences (especially in the lowercase 'w' letter) and it's not decided whether we'll upgrade yet.

So currently it is important that FreeDink uses the font it ships with, as it isn't available from the system.


For the 32x32 icon, I'll check with Seth next time I discuss game data with him. 

I'm currently searching or creating replacements for the game data sounds and music that weren't freed, and preparing a new release that can use them. This will take a few days.

Cheers,
Comment 6 Mamoru TASAKA 2008-09-06 06:14:14 EDT
(In reply to comment #5)
> Hi,
> 
> > * This package installs LiberationSans-Regular.ttf. However this font is
> > already provided by other Fedora package and shipping duplicate fonts like
> > this way is not allowed.  Please modify the source so that this package uses
> > system widely provided fonts.
> 
> FreeDink is using a precise version of LiberationSans-Regular.ttf (ttf-3) and
> the currently released version has a bug
> https://bugzilla.redhat.com/show_bug.cgi?id=458592
> 
> The latest version of the font also introduces some non-bug graphical
> differences (especially in the lowercase 'w' letter) and it's not decided
> whether we'll upgrade yet.

In such case Fedora Liberation fonts must be fixed and we still don't approve
this case.
Comment 7 Sylvain Beucler 2008-09-06 13:26:15 EDT
Hi,

> In such case Fedora Liberation fonts must be fixed and we still don't approve
> this case.

I think there is a misunderstanding: I'm saying 2 things:

- there was a bug which I reported and that is _fixed_ in a pre-10 version; this means that the included Font is necessary for any Fedora < 10 package, otherwise there's a nasty display bug. But this is not so much relevant in this particular sponsor request - just an example of when this can be necessary.

- I'm also saying that the new version of the font is essentially a different font, with a different look, and that Fedora does not provide the old version. Are you saying that you want to change the game font just because Fedora does not package the upstream font?


Nonetheless, FreeDink will probably upgrade. Which raises a more technical question:

> Please modify the source so that this package uses
> system widely provided fonts.

Is there a _standard_ path to look for fonts? Fedora installs liberation-fonts in /usr/share/fonts/liberation/ and Debian installs ttf-liberation in /usr/share/fonts/truetype/ttf-liberation/, namely. Is there a portable way to look for a particular font location?

Cheers.
Comment 8 Mamoru TASAKA 2008-09-07 12:13:20 EDT
(In reply to comment #7)
> - there was a bug which I reported and that is _fixed_ in a pre-10 version;
> this means that the included Font is necessary for any Fedora < 10 package,

 You must ask the font maintainer to push the fixed font to F-9/8 repository
 in that case...

> - I'm also saying that the new version of the font is essentially a different
> font, with a different look, and that Fedora does not provide the old version.

 If the old font package is unavoidable for this package, you must submit a review
 request for the old font package with enough rationale (like compat-foo library
 package) and make the font imported into Fedora in a proper way:
 https://fedoraproject.org/wiki/Font_package_lifecycle
  
> Is there a _standard_ path to look for fonts? Fedora installs liberation-fonts
> in /usr/share/fonts/liberation/ and Debian installs ttf-liberation in
> /usr/share/fonts/truetype/ttf-liberation/, namely. Is there a portable way to
> look for a particular font location?

  I don't know well. I guess with some proper way generally knowing the location
  of fonts should not be needed (because one of the packages I maintain exactly
  do it), however I don't know the way (I guess functions in cairo or pango will
  do this)
Comment 9 Sylvain Beucler 2008-09-14 17:02:37 EDT
I implemented the new dependency scheme:
http://www.freedink.org/snapshots/fedora/freedink.spec

This spec files contains 2 packages (freedink and freedink-engine, mainly because I couldn't make a source-less 'freedink' meta-package). I have a problem though: the debug files are in freedink-debuginfo-1.08.20080914-1.fc8.i386.rpm rather than freedink-engine-debuginfo-1.08.20080914-1.fc8.i386.rpm :/
Do you know how to fix this?
Comment 10 Hans de Goede 2008-09-15 17:55:53 EDT
Hi Sylvain,

I just stumbled over freedink and I think you are doing a good job with packaging and a great job with replacing all the non free resources! So I would like to review this package and sponsor you.

But not right now I'm afraid as I'm leaving to go to the Linux Plumbers Conference. I'll try to think of doing this when I'm back (in the weekend) but I'll probably forget. So if you haven't heared anything from me in 1 1/2 weeks, please ping me (private mail or add a note to this bugzilla ticket).
Comment 11 Mamoru TASAKA 2008-09-15 22:59:45 EDT
Hans, I am already watching or review 3/3 of freedink related packages submitted by
Sylvain.
Comment 12 Mamoru TASAKA 2008-09-16 12:03:55 EDT
(In reply to comment #9)
> the debug files are in
> freedink-debuginfo-1.08.20080914-1.fc8.i386.rpm rather than
> freedink-engine-debuginfo-1.08.20080914-1.fc8.i386.rpm :/
> Do you know how to fix this?
It is normal.

By the way would you also provide your srpm? Also, please remove
suse related parts.
Comment 13 Sylvain Beucler 2008-09-16 14:04:54 EDT
Hi,

Too bad the debug info can't be added to the -engine package.


The SRPMS are in the same directory:
http://www.freedink.org/snapshots/fedora/
More precisely:
http://www.freedink.org/snapshots/fedora/freedink-1.08.20080914-1.fc8.src.rpm
http://www.freedink.org/snapshots/fedora/dfarc-3.1.20080914-1.fc8.src.rpm
http://www.freedink.org/snapshots/fedora/freedink-data-1.08.20080914-1.fc8.src.rpm


For the suse parts, I maintain the spec file for both distros, so it would be slower and error-prone to split and duplicate them, unless there's a problem that I didn't see.
Comment 15 Mamoru TASAKA 2008-09-17 03:30:02 EDT
Would you write the URLs of srpm on the each corresponding review request?
Otherwise it is hard to find out if each srpm is updated or not on each review request.
Comment 16 Mamoru TASAKA 2008-09-17 03:41:47 EDT
(In reply to comment #13)
> For the suse parts, I maintain the spec file for both distros, so it would be
> slower and error-prone to split and duplicate them, unless there's a problem
> that I didn't see.

Please remove suse part. I know some other persons who maintain there packages
on other distributions. There are (and will be) many distro-specific rpm
writting issues on Fedora and leaving that information which are specific to
SUSE or so which is never used on Fedora just leaves the spec file less
readable.
Comment 17 Mamoru TASAKA 2008-09-17 03:54:11 EDT
Once setting FE-Legal until the license issue of src/freedink_xpm.c is clarified.
Comment 18 Sylvain Beucler 2008-09-21 06:06:17 EDT
Hi,

Here's a new release:
http://www.freedink.org/snapshots/fedora/freedink.spec
http://www.freedink.org/snapshots/fedora/freedink-1.08.20080920-1.src.rpm

- opensuse parts removed
(for reference: sed '/%if 0%{?suse_version}/,/%endif/d' < freedink.spec)

- icon replaced
Comment 19 Mamoru TASAKA 2008-09-22 11:54:31 EDT
Removing FE-Legal
Comment 20 Mamoru TASAKA 2008-09-22 13:34:03 EDT
For 1.08.20080920-1:

* Dependency between subpackages
  - Generally speaking, dependencies between packages generated from
    the same srpm must be EVR (Epoch-Version-Release) specific.
    (i.e. freedink must have "freedink-engine = %{version}-%{release})

* %fedora_version
  - is not defined. Perhaps you want to use %{?fedora}.

* BuildRequires
  - build.log shows:
----------------------------------------------------
   124  checking for help2man... 
   125  no
   126  configure: WARNING: You need to install help2man
----------------------------------------------------
     Perhaps "BuildRequires: help2man" is needed.

* Timestamps
  - Please consider to use
----------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
----------------------------------------------------
    to keep timestamps as much as possible. This method
    usually works for Makefiles generated from recent
    autotools

* Desktop files
  - must be treated by desktop-file-{install,validate}:
    https://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usage


By the way I tried to play freedink but my mouse pointer does not
seem to be recognized.
Comment 21 Sylvain Beucler 2008-09-22 14:27:42 EDT
Hi,

I've got a statement of intent from Seth which clarifies the icon license:
http://lists.gnu.org/archive/html/bug-freedink/2008-09/msg00008.html

Is it good enough?


I'll post a new .spec soon, I already fixed a few things common with dfarc.
Comment 22 Mamoru TASAKA 2008-09-22 14:59:24 EDT
(In reply to comment #21)
> Hi,
> 
> I've got a statement of intent from Seth which clarifies the icon license:
> http://lists.gnu.org/archive/html/bug-freedink/2008-09/msg00008.html
> 
> Is it good enough?

Yes, thanks for asking to upstream.
Comment 23 Sylvain Beucler 2008-09-23 17:12:28 EDT
(In reply to comment #20)
> For 1.08.20080920-1:
> 
> * Dependency between subpackages
>   - Generally speaking, dependencies between packages generated from
>     the same srpm must be EVR (Epoch-Version-Release) specific.
>     (i.e. freedink must have "freedink-engine = %{version}-%{release})

OK, fixed.


I have a question: the 'freedink' package is built as 'i386' instead of 'noarch' (E: no-binary). I couldn't find a way to use a different BuildArch for the 2 packages though.


> * %fedora_version
>   - is not defined. Perhaps you want to use %{?fedora}.

Yes indeed. (I had copied this one from a .spec from another project, but it was actually meant for the opensuse build service, with other variables)

> * BuildRequires
>   - build.log shows:
> ----------------------------------------------------
>    124  checking for help2man... 
>    125  no
>    126  configure: WARNING: You need to install help2man
> ----------------------------------------------------
>      Perhaps "BuildRequires: help2man" is needed.

I clarified this in Git, it's a developer tool. The build system takes care of pregenerating man pages to avoid the 'help2man' dependency.

> * Timestamps
>   - Please consider to use
> ----------------------------------------------------
> make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
> ----------------------------------------------------
>     to keep timestamps as much as possible. This method
>     usually works for Makefiles generated from recent
>     autotools

Did so.

> * Desktop files
>   - must be treated by desktop-file-{install,validate}:
>    
> https://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usage

Done too.


> By the way I tried to play freedink but my mouse pointer does not
> seem to be recognized.

Hmmm, is there anything special about your mouse? Is it under X11?
You mean you can't move the mouse in the intro screen, is that right?

I didn't have this problem yet, either it's a SDL bug, either it's the way I continuously recenter the mouse to get relative motions without letting the mouse get out of the window.



Here are the new files:
http://www.freedink.org/snapshots/fedora-review/freedink.spec
http://www.freedink.org/snapshots/fedora-review/freedink-1.08.20080920-1.fc8.src.rpm
Comment 24 Mamoru TASAKA 2008-09-24 02:52:51 EDT
Please change the release number every time you modify your spec file
to avoid confusion.

https://fedoraproject.org/wiki/Packaging/FrequentlyMadeMistakes
Comment 25 Mamoru TASAKA 2008-09-24 03:02:38 EDT
Also please write in %changelog what you modified even during review request.
Comment 27 Mamoru TASAKA 2008-09-24 04:17:59 EDT
Rebuild failed:
http://koji.fedoraproject.org/koji/taskinfo?taskID=841506

! build.log shows some stange error comment:
  http://koji.fedoraproject.org/koji/getfile?taskID=841508&name=build.log
--------------------------------------
ENTER do(['bash', '--login', '-c', 'rpmbuild -bs --target x86_64 --nodeps builddir/build/SPECS/freedink.spec'], False, '/var/lib/mock/dist-f10-build-261193-47237/root/', None, 86400, True, 0, 101, 102, None, logger=<mock.trace_decorator.getLog object at 0x2b90b825bf90>)
Executing command: ['bash', '--login', '-c', 'rpmbuild -bs --target x86_64 --nodeps builddir/build/SPECS/freedink.spec']
/etc/profile: line 42: /bin/hostname: No such file or directory
error: line 67: second %install
--------------------------------------
  Perhaps this is because %install in comment is not protected by %% properly
  (see my comment: bug 459916 comment 19)

Some quick note (I have not checked yet because the srpm does not build)

* EVR dependency
--------------------------------------
Requires:	freedink-engine=%{version}-%{release} freedink-dfarc
--------------------------------------
  - This will make freedink require the rpm named "freedink-engine=1.08.20080920-2.fc10"
    (then cannot be installed), i.e. spaces needed between equality.

* update-desktop-database
  - is needed when desktop files contains MimeType information (i.e. MimeType=application/x-dmod;
    like freedink-dfarc.desktop).
Comment 28 Mamoru TASAKA 2008-09-24 04:37:27 EDT
(In reply to comment #23)
> I have a question: the 'freedink' package is built as 'i386' instead of
> 'noarch' (E: no-binary). I couldn't find a way to use a different BuildArch for
> the 2 packages though.

Please ignore this rpmlint for this case.
Comment 29 Sylvain Beucler 2008-09-24 16:23:58 EDT
Here's the updated package:

http://www.freedink.org/snapshots/fedora-review/freedink.spec
http://www.freedink.org/snapshots/fedora-review/freedink-1.08.20080920-3.fc8.src.rpm

* Wed Sep 24 2008 Sylvain Beucler <beuc@beuc.net> - 1.08.20080920-3
- Don't use 'update-desktop-database' for simple desktop files
- Fix unescaped macros in comments
- Use spaces around '=' in version-specific dependency
Comment 30 Mamoru TASAKA 2008-09-24 23:06:50 EDT
(Removing NEEDSPONSOR: bug 459916)
Comment 31 Mamoru TASAKA 2008-09-25 14:07:59 EDT
(In reply to comment #23)
> (In reply to comment #20)
 I have a question: the 'freedink' package is built as 'i386' instead of
> 'noarch' (E: no-binary). I couldn't find a way to use a different BuildArch for
> the 2 packages though.

  - Currently there is no way to avoid this (so just ignore).

> > By the way I tried to play freedink but my mouse pointer does not
> > seem to be recognized.
> 
> Hmmm, is there anything special about your mouse? Is it under X11?
> You mean you can't move the mouse in the intro screen, is that right?

  - Well, after some try I found that this was not a problem (in the intro
    screen the mouse is working)

Now this package can be approved.
-----------------------------------------------------------------
    This package (freedink) is APPROVED by mtasaka
-----------------------------------------------------------------
Comment 32 Sylvain Beucler 2008-09-25 18:24:00 EDT
Thanks!

On with the cvs request. CVS maintainers, I would be glad if you could create this module:

New Package CVS Request
=======================
Package Name: freedink
Short Description: Adventure and role-playing game
Owners: beuc
Branches: F-8 F-9
InitialCC: beuc
Comment 33 Kevin Fenzi 2008-09-28 15:21:24 EDT
cvs done.
Comment 34 Sylvain Beucler 2008-10-01 14:49:40 EDT
I imported the package in CVS:
https://admin.fedoraproject.org/pkgdb/packages/name/freedink

koji reported no errrors.

I'm a bit puzzled about the inclusion status now, especially about Fedora 10 (which is in freeze).
When will people be able to write "yum install freedink" ? :)
Comment 35 Mamoru TASAKA 2008-10-01 14:54:49 EDT
(In reply to comment #34)
> I'm a bit puzzled about the inclusion status now, especially about Fedora 10
> (which is in freeze).
Freeze is now over.

> When will people be able to write "yum install freedink" ? :)
Already in rawhide tree:
https://www.redhat.com/archives/fedora-devel-list/2008-October/msg00019.html
Comment 36 Mamoru TASAKA 2008-10-01 14:55:57 EDT
By the way please rebuild your srpm also on F-9/8 and submit push requests on bodhi:
https://admin.fedoraproject.org/updates/
Comment 37 Fedora Update System 2008-10-05 12:39:52 EDT
freedink-1.08.20080920-4.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/freedink-1.08.20080920-4.fc9
Comment 38 Fedora Update System 2008-10-05 12:55:12 EDT
freedink-1.08.20080920-4.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/freedink-1.08.20080920-4.fc8
Comment 39 Mamoru TASAKA 2008-10-06 09:55:48 EDT
Thanks. Now closing.

When you think the submitted packages can be moved to stable repository,
please modify (edit) the request on bodhi.
Comment 40 Fedora Update System 2008-10-20 16:27:44 EDT
freedink-1.08.20080920-4.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 41 Fedora Update System 2008-10-20 18:17:28 EDT
freedink-1.08.20080920-4.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

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