Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 229476
Summary: | Review Request: xblast - Lay bombs and Blast the other players of the field (SDL version) | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Hans de Goede <hdegoede> | ||||||
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> | ||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | medium | ||||||||
Version: | rawhide | Flags: | mtasaka:
fedora-review+
dennis: fedora-cvs+ |
||||||
Target Milestone: | --- | ||||||||
Target Release: | --- | ||||||||
Hardware: | All | ||||||||
OS: | Linux | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2007-03-03 06:59:19 UTC | Type: | --- | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Bug Depends On: | |||||||||
Bug Blocks: | 163779 | ||||||||
Attachments: |
|
Description
Hans de Goede
2007-02-21 13:15:45 UTC
Notice that the review request for the required xblast-data is in bug 229477 I will review this later. By the way I will appreciate it if you would review my mecab related review request (bug 229927 and bug 229929). Thanks! 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) * 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). - Please specify the URL of xblast.png if possible. * 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. * Documentation - Perhaps the following files can be used. ---------------------------------------------------------- ./xblast.man ---------------------------------------------------------- * Functionality - xblast-x11 cannot be launched for me. ---------------------------------------------------------- [tasaka1@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 ----------------------------------------------------------- * 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. - 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). (In reply to comment #2) > I will review this later. > By the way I will appreciate it if you would review my > mecab related review request (bug 229927 and bug 229929). > Thanks! Well me reviewing some packages in return is only fare, so sure I'll take a stab. But tomorrow I'm going to fosdem 2007 in Brussels, so don't expect a full review this weekend. Probably somewhere next week. (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@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? Well,
* x11 version problem
----------------------------------------------------------
[tasaka1@localhost xblast]$ xblast-x11
could not load font 24
could not load font 18
could not load font 14
-----------------------------------------------------------
- turns out that this is because x11 version is not compiled
with "-DMINI_XBLAST" (check x11_config.c). x11 version still
requests vera font, however vera font is not in the font
search path.
BYW according to x11_config.c, x11 version requires some font
from xorg-x11-fonts-ISO8859???? (note: I think that these
font packages are not so usual for non-American/European users
like me).
* gettext translation character corruption
- well, x11 version does not use gettext, so the sentences
are always English and have no problem.
- some character corruption occurs on -sdl version (currently
I created a link as /usr/share/xblast/locale -> ../locale)
on both de_DE and fr_FR.
* Directory ownership:
> 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.
- Okay. I catched what you mean. You want to remove
all packages related to xblast by "yum remove xblast", right?
Your opinition is reasonable and for this I respect your
choice.
Oops... not BYW but BTW (by the way) (In reply to comment #6) > Well, > > * x11 version problem > ---------------------------------------------------------- > [tasaka1@localhost xblast]$ xblast-x11 > could not load font 24 > could not load font 18 > could not load font 14 > ----------------------------------------------------------- > - turns out that this is because x11 version is not compiled > with "-DMINI_XBLAST" (check x11_config.c). x11 version still > requests vera font, however vera font is not in the font > search path. > BYW according to x11_config.c, x11 version requires some font > from xorg-x11-fonts-ISO8859???? (note: I think that these > font packages are not so usual for non-American/European users > like me). > Using -DMINI_XBLAST causes xblast to be compiled for low res screens and thus draw everything in a twice as low resolution, I don't think we want that. The proper fix would be to add a Requires: xorg-x11-fonts-ISO8859-1-75dpi > * gettext translation character corruption > - well, x11 version does not use gettext, so the sentences > are always English and have no problem. > - some character corruption occurs on -sdl version (currently > I created a link as /usr/share/xblast/locale -> ../locale) > on both de_DE and fr_FR. > I've tried this, but I see no problems on my 64 bit rawhide machine, can you give some examples? Regards, Hans (In reply to comment #8) > (In reply to comment #6) > > Well, > > > > * x11 version problem > > ---------------------------------------------------------- > > [tasaka1@localhost xblast]$ xblast-x11 > > could not load font 24 > > could not load font 18 > > could not load font 14 > > ----------------------------------------------------------- > > - turns out that this is because x11 version is not compiled > > with "-DMINI_XBLAST" (check x11_config.c). x11 version still > > requests vera font, however vera font is not in the font > > search path. > > BYW according to x11_config.c, x11 version requires some font > > from xorg-x11-fonts-ISO8859???? (note: I think that these > > font packages are not so usual for non-American/European users > > like me). > > > > Using -DMINI_XBLAST causes xblast to be compiled for low res screens and thus > draw everything in a twice as low resolution, I don't think we want that. > The proper fix would be to add a Requires: xorg-x11-fonts-ISO8859-1-75dpi I have already installed xorg-x11-fonts-ISO8859-1-75dpi. Does xblast-x11 really work for you? For me it seems this is due that -x11 version want to use vera font, still vera font is not in the font search path. Would you check x11_config.c? > > > * gettext translation character corruption > > - well, x11 version does not use gettext, so the sentences > > are always English and have no problem. > > - some character corruption occurs on -sdl version (currently > > I created a link as /usr/share/xblast/locale -> ../locale) > > on both de_DE and fr_FR. > > > > I've tried this, but I see no problems on my 64 bit rawhide machine, can you > give some examples? I will attach a screenshot. Mamoru Created attachment 148774 [details]
screenshot of xblast-sdl on LANG=fr_FR
screenshot on
LANG=fr_FR xblast-sdl
(In reply to comment #9) > > The proper fix would be to add a Requires: xorg-x11-fonts-ISO8859-1-75dpi > > I have already installed xorg-x11-fonts-ISO8859-1-75dpi. > Does xblast-x11 really work for you? > For me it seems this is due that -x11 version want to use > vera font, still vera font is not in the font search path. > Would you check x11_config.c? > I've checked it and the relevant lines are: "-*-helvetica-bold-r-*-*-14-*-*-*-*-*-iso8859-*", /* small */ "-*-helvetica-bold-r-*-*-18-*-*-*-*-*-iso8859-*", /* medium */ "-*-helvetica-bold-r-*-*-24-*-*-*-*-*-iso8859-*", /* large */ Note that that is convoluted X11 font naming, now if you look at: /usr/share/X11/fonts/75dpi/fonts.dir You will find in there: helvB24-ISO8859-1.pcf.gz -adobe-helvetica-bold-r-normal--24-240-75-75-p-138-iso8859-1 And rpm -qf /usr/share/X11/fonts/75dpi/helvB24-ISO8859-1.pcf.gz gives: xorg-x11-fonts-ISO8859-1-75dpi-7.1-1.noarch Unfortunately we cannot use strace here as this is going through X. Maybe something is busted with your X-setup? : * Does /etc/X11/fs/config contain: /usr/share/X11/fonts/75dpi:unscaled in the catalogue = ... lines? * And does /etc/X11/xorg.conf contain: FontPath "unix/:7100" In the Files section? Can you try "entering" "-*-helvetica-bold-r-*-*-24-*-*-*-*-*-iso8859-*" into xfontsel (from xorg-x11-utils package), maybe you've got more then one font matching "-*-helvetica-bold-r-*-*-24-*-*-*-*-*-iso8859-*" and maybe that is the problem? > > I've tried this, but I see no problems on my 64 bit rawhide machine, can you > > give some examples? > I will attach a screenshot. > I see your problem, and I don't have it I'll attach a screenshot of my version, I'm using "export LANG=fr_FR.UTF-8" to test, are you also using UTF-8 ? Created attachment 148785 [details]
Working French screenshot
* For garbage character, it disappeared when I type: "LANG=de_DE.UTF-8 xblast-sdl". Sorry for noises. (In reply to comment #11) > I've checked it and the relevant lines are: > "-*-helvetica-bold-r-*-*-14-*-*-*-*-*-iso8859-*", /* small */ > "-*-helvetica-bold-r-*-*-18-*-*-*-*-*-iso8859-*", /* medium */ > "-*-helvetica-bold-r-*-*-24-*-*-*-*-*-iso8859-*", /* large */ > > You will find in there: > helvB24-ISO8859-1.pcf.gz > -adobe-helvetica-bold-r-normal--24-240-75-75-p-138-iso8859-1 >> Maybe something is busted with your X-setup? : > * Does /etc/X11/fs/config contain: > /usr/share/X11/fonts/75dpi:unscaled > in the catalogue = ... lines? > * And does /etc/X11/xorg.conf contain: > FontPath "unix/:7100" > In the Files section? Well, FontPath is okay, font search path are: ---------------------------------------------------- catalogue = /usr/share/X11/fonts/misc:unscaled, /usr/share/X11/fonts/75dpi:unscaled, /usr/share/X11/fonts/100dpi:unscaled, /usr/share/X11/fonts/Type1, /usr/share/fonts/default/Type1, /usr/share/fonts/default/ghostscript, , /usr/share/fonts/korean/misc:unscaled, /usr/share/fonts/korean/misc, /usr/share/fonts/korean/TrueType, /usr/share/fonts/chinese/misc:unscaled, /usr/share/fonts/chinese/misc, /usr/share/fonts/chinese/TrueType, /usr/share/fonts/japanese/misc:unscaled, /usr/share/fonts/japanese/misc, /usr/share/fonts/japanese/TrueType/S2G, /usr/share/fonts/japanese/TrueType/mikachan, /usr/share/fonts/japanese/TrueType/neuropol, /usr/share/fonts/japanese/TrueType, /usr/share/fonts/japanese/efont-unicode-bdf, /usr/share/X11/fonts/75dpi:unscaled, /usr/share/X11/fonts/TTF, /usr/share/X11/fonts/75dpi ---------------------------------------------------- > > Can you try "entering" "-*-helvetica-bold-r-*-*-24-*-*-*-*-*-iso8859-*" into > xfontsel (from xorg-x11-utils package), maybe you've got more then one font > matching "-*-helvetica-bold-r-*-*-24-*-*-*-*-*-iso8859-*" and maybe that is the > problem? For me, with xfontsel -------------------------------------------------------- "-*-helvetica-bold-r-*-*-14-*-*-*-*-*-iso8859-*", 12 names match "-*-helvetica-bold-r-*-*-18-*-*-*-*-*-iso8859-*", 9 names match "-*-helvetica-bold-r-*-*-24-*-*-*-*-*-iso8859-*", 9 names match --------------------------------------------------------- Well I just tried installing all the xorg-x11-fonts-ISO8859* and xblast-x11 still works fine :| So I'm afraid that there is something broken with your setup. I've tried it on my work machine too (some days ago) and it ran fine there too. Both are rawhide machines though. I believe that adding a Requires: xorg-x11-fonts-ISO8859-1-75dpi should be sufficient normally. Why it isn't working on your machine I don't understand. Well, I tried on FC5 system and the result was ... no problem ... Also for ISO8859 fonts: ------------------------------------------ [tasaka1@localhost ~]$ grep ISO8859 /var/log/rpmpkgs xorg-x11-fonts-ISO8859-1-100dpi-7.0-3.noarch.rpm xorg-x11-fonts-ISO8859-1-75dpi-7.0-3.noarch.rpm ------------------------------------------ Only two rpms are installed but it was okay... So for now I assume that -x11 should work normally. So would you update srpm/spec with the left issues fixed? (In reply to comment #15) > So for now I assume that -x11 should work normally. So > would you update srpm/spec with the left issues fixed? Done, new version here: * Thu Mar 1 2007 Hans de Goede <j.w.r.degoede> 2.10.4-2 - Use sf.net sourceforge URL from the Guidelines (bz 229476) - Keep timestamp while installing the wrapper (bz 229476) - Sanitize and install the manpage (bz 229476) - Add "Requires: xorg-x11-fonts-ISO8859-1-75dpi" to xblast-x11 (bz 229476) - Make xblast look for translations under /usr/share/locale (bz 229476) Spec URL: http://people.atrpms.net/~hdegoede/xblast.spec SRPM URL: http://people.atrpms.net/~hdegoede/xblast-2.10.4-2.fc7.src.rpm Well, A. for xblast-2.10.4-2: * Timestamps --------------------------------------------- install -m 644 %{name}.man $RPM_BUILD_ROOT%{_mandir}/man6/%{name}.6 --------------------------------------------- - Please use "install -p" (gzip keeps timestamp, so the timestamp of this file is finally kept). Other things are okay. ------------------------------------------------------ This package (xblast) is APPROVED by me. ------------------------------------------------------ (In reply to comment #17) > Well, > > A. for xblast-2.10.4-2: > * Timestamps > --------------------------------------------- > install -m 644 %{name}.man $RPM_BUILD_ROOT%{_mandir}/man6/%{name}.6 > --------------------------------------------- > - Please use "install -p" (gzip keeps timestamp, so the timestamp > of this file is finally kept). That file gets patched and thus the timestamp gets set to the build time, so there is no use in using -p. New Package CVS Request ======================= Package Name: xblast Short Description: Lay bombs and Blast the other players of the field Owners: j.w.r.degoede Branches: FC-6 devel InitialCC: empty (In reply to comment #18) > (In reply to comment #17) > > A. for xblast-2.10.4-2: > > * Timestamps > > --------------------------------------------- > > install -m 644 %{name}.man $RPM_BUILD_ROOT%{_mandir}/man6/%{name}.6 > > --------------------------------------------- > > - Please use "install -p" (gzip keeps timestamp, so the timestamp > > of this file is finally kept). > > > That file gets patched and thus the timestamp gets set to the build time, so > there is no use in using -p. Okay. CVS done Imported and build, as usual thanks for the review! Closing. |