Bug 459908 (freedink)

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.

> 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 :)

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

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.

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,

(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.

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.

(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)

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?

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).

Hans, I am already watching or review 3/3 of freedink related packages submitted by
Sylvain.

(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.

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.

> http://www.freedink.org/snapshots/fedora/dfarc-3.1.20080914-1.fc8.src.rpm

Hmm, that's a mistake in my build. The right one is:
http://www.freedink.org/snapshots/fedora/freedink-dfarc-3.1.20080914-1.fc8.src.rpm

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.

(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.

Once setting FE-Legal until the license issue of src/freedink_xpm.c is clarified.

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

Removing FE-Legal

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.

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.

(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.

(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

Please change the release number every time you modify your spec file
to avoid confusion.

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

Also please write in %changelog what you modified even during review request.

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

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).

(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.

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> - 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

(Removing NEEDSPONSOR: bug 459916)

(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
-----------------------------------------------------------------

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

cvs done.

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" ? :)

(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

By the way please rebuild your srpm also on F-9/8 and submit push requests on bodhi:
https://admin.fedoraproject.org/updates/

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

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

Thanks. Now closing.

When you think the submitted packages can be moved to stable repository,
please modify (edit) the request on bodhi.

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.

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.