[Bug 229476] Review Request: xblast - Lay bombs and Blast the other players of the field (SDL version)

bugzilla at redhat.com bugzilla at redhat.com
Sat Feb 24 19:21:58 UTC 2007


Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: xblast - Lay bombs and Blast the other players of the field (SDL version)


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229476





------- Additional Comments From j.w.r.degoede at hhs.nl  2007-02-24 14:21 EST -------
(In reply to comment #3)
> Well,
> 
> A. First for xblast-2.10.4-1:
> 
> * File dependency
>   - Writing the package which provides the file is recommended
>     expect you have somewhat strong reason to write file dependency
>     (for vera font). Please check:
>     http://fedoraproject.org/wiki/PackagingDrafts/FileDeps
>     (This is a draft)
> 

Fonts have been known to move from one location to another as packaging insights
surrounding fonts change, since xblast opens the font through an absolute patch
I want xblast to break when this happens.

> * Source URL
>   - Please use http://downloads.sourceforge.net/<package_name>/XXXX.tar.gz
>     if it is possible. Please check:
>     http://fedoraproject.org/wiki/PackagingDrafts/SourceUrl
>     (This is a draft).

This Draft is clearly written by someone with not so much sourceforge
experience, downloads.sourceforge.net (or dl.sf.net which is a shorter alias but
otherwise exactly the same) will do a dumb redirect to a mirror, I say a dumb
redirect as it will take the file location after the hostname as is without any
checking and then postfix this to the choosen mirrors hostname to get the URL to
redirect to.

Now most mirrors will work fine with dl.sf.net/%{name}/xxx, but some mirrors
will only work with dl.sf.net/sourceforge/%{name}/xxx, notice that this longer
version will also work on mirrors which accept dl.sf.net/%{name}/xxx, as they
seem to have a symlink to / called sourceforge :)

Thus the draft is wrong. I've added a comment to this extend to the draft.

>   - Please specify the URL of xblast.png if possible.
> 

I took this from a suse srom, so no URL I'm afraid.


> * Timestamps
> ----------------------------------------------------------
> install -m 755 %{SOURCE3} $RPM_BUILD_ROOT%{_bindir}/%{name}
> ----------------------------------------------------------
>   - This is only a wrapper script and keeping timestamp
>    (i.e. install -p) is recommended.
> 

I will fix this as soon as the other points are clear.

> * Documentation
>   - Perhaps the following files can be used.
> ----------------------------------------------------------
> ./xblast.man
> ----------------------------------------------------------
> 

Good catch, it needs some work amongst others the trademark bomberman name must
be stripped from it, but its salvegable :) I will fix this as soon as the other
points are clear.


> * Functionality
>   - xblast-x11 cannot be launched for me.
> ----------------------------------------------------------
> [tasaka1 at localhost xblast]$ xblast-x11 
> could not load font 24
> could not load font 18
> could not load font 14
> X Error of failed request:  BadFont (invalid Font parameter)
>   Major opcode of failed request:  56 (X_ChangeGC)
>   Resource id in failed request:  0x800010
>   Serial number of failed request:  519
>   Current serial number in output stream:  541
> -----------------------------------------------------------
> 

Looks like it will need a dep on some not by default installed X11 fonts which I
have installed and you don't. Can you try installing "xorg-x11-fonts-misc" ?

> * Directory/file ownership
>   - Well as the build log says:
> -----------------------------------------------------------
> -DGAME_DATADIR=\"/usr/share/xblast\"
> -----------------------------------------------------------
>     I think that %{_datadir}/xblast should be owned by
>     xblast-common, not by xblast-data because xblast requires
>     that the files are installed under %{_datadir}/xblast.
> 

Erm, why xblast and xblast-x11 need files from under this dir, but they place no
files there themselves, since they need the files they require xblast-data,
which has the files and does the dir. Whats the use of owning a dir you don't
put files in? Actually by doing things that way, combined with the circular deps
this has chances are that the directory will not be removed, because xblast-data
could be removed after xblast-data at which moment it will not be empty.


>   - And currently the location of gettext mo files are
>     not correct because build log says:
> -----------------------------------------------------------
> -DLOCALEDIR=\"/usr/share/xblast/locale\"
> -----------------------------------------------------------
>     This should be moved to %{_datadir}/locale (well, some
>     messages are corrupted on both fr_FR and de_DE, perhaps
>     due to ISO-8859 style vs UTF-8 style).

I will fix the location as soon as the other points are clear. And I'll look
into the encodings, but that shouldn't be a problem as .po /.mo files should
have the encoding specified in their header and on the fly conversion will be
done by gettext when nescesarry. Did you see these problems with xblast-sdl,
xblast-x11 or both?




-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.




More information about the Fedora-package-review mailing list