Bug 230726 - Review Request: xmoto-edit - X-Moto level editor
Review Request: xmoto-edit - X-Moto level editor
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michał Bentkowski
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-03-02 10:18 EST by Gwyn Ciesla
Modified: 2008-03-07 11:25 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-21 10:08:50 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mr.ecik: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Gwyn Ciesla 2007-03-02 10:18:25 EST
Spec URL: http://zanoni.jcomserv.net/extras/xmoto-exit/xmoto-edit.spec
SRPM URL: http://zanoni.jcomserv.net/extras/xmoto-exit/xmoto-edit-0.2.4-1.src.rpm
Description: Level editor or xmoto.  With the last release, this was split out of the upstream tarball.  The spec is basically a modified xmoto.spec.  rpmlint is clean for srpm, rpm and debuginfo.

I am already sponsored.
Comment 2 Michał Bentkowski 2007-03-02 12:27:34 EST
What's the %{_mandir}/mang/xmoto-edit.6.gz?  Why not to use man6 instead?
Comment 3 Gwyn Ciesla 2007-03-02 12:57:54 EST
Looks like a typo.  Should be man6.  I see it in xmoto, too.  I'll fix both. 
Should provide that back here?  Do you see anything else out of sorts?
Comment 4 Gwyn Ciesla 2007-03-02 13:01:51 EST
On second look, it's set that way in the Makefile.  Should I patch it and send
it upstream?
Comment 5 Michał Bentkowski 2007-03-02 13:11:28 EST
(In reply to comment #3)
> Do you see anything else out of sorts?

I haven't even chcecked whether package's working, but at first sight I see 
that you ought to remove X-Fedora category and, if it exists, get rid of 
Application category as well. Also, if you put icons in %{_datadir}/icons/
hicolor you should add Requires: hicolor-icon-theme (some time ago it was 
discussed on list). Later I'm going to try to build this package and hopefully 
make the full review.

(In reply to comment #4)
> On second look, it's set that way in the Makefile.  Should I patch it and send
> it upstream?

IMHO you should.
Comment 6 Gwyn Ciesla 2007-03-02 13:46:08 EST
I've created the patch and removed X-Fedora.  I'll send the patch upstream after
approval.

Spec URL: http://zanoni.jcomserv.net/extras/xmoto-edit/xmoto-edit.spec
SRPM URL: http://zanoni.jcomserv.net/extras/xmoto-edit/xmoto-edit-0.2.4-2.src.rpm
Comment 7 Michał Bentkowski 2007-03-02 18:40:42 EST
REVIEW: 
** it seems that package has some unneeded dependencies, it built fine in mock 
with: SDL_mixer-devel, libGL-devel, libjpeg-devel, zlib-devel and libpng-devel. 
So you can get rid of curl-devel, ode-devel, lua-devel, libGLU-devel and 
replace bzip2-devel with zlib-devel (if there's something wrong with this set 
of dependencies, please let me know)

** does it make sense not to require main xmoto package? I understand that 
there's no need xmoto to be installed to make levels for it, but there's a 
"Play Level" button in xmoto-edit which ends up with very ugly "sh: xmoto: 
command not found". Thus add xmoto dependency.

** also, you have to add versioned xmoto dependency because of that: "file /usr/
bin/xmoto-edit from install of xmoto-edit-0.2.4-2.fc7 conflicts with file from 
package xmoto-0.2.2-2.fc6". Now I don't know whether it would be better to have 
xmoto >= 0.2.4 dependency or xmoto = %{version}. If xmoto and xmoto-edit 
versions always match, the second solution will be much more sane.

** rpmlint complain about W: xmoto-edit mixed-use-of-spaces-and-tabs (spaces: 
line 1, tab: line 12) so get rid of that

** [ecik@ecik ~]$ desktop-file-validate /usr/share/applications/fedora-xmoto-
edit.desktop
/usr/share/applications/fedora-xmoto-edit.desktop: warning: The 'Application' 
category is not defined by the desktop entry specification.  Please use one of 
"AudioVideo", "Audio", "Video", "Development", "Education", "Game", "Graphics", 
"Network", "Office", "Settings", "System", "Utility" instead

So just get rid of Application category in your desktop file. Install the 
newest rawhide version of desktop-file utils if you can't see this error on 
your desktop-file-validate.

** There's /usr/share/xmoto-edit/xmoto.bin file which contains a lot of binary 
(?) data. Is it arch-dependent? If so, you cannot put this file in %{_datadir}. 
Unfortunetaly, I'm unable to identify what kind of data it is, maybe you know 
something more?

** Desktop file contains "Comment=Xmoto LEvel Editor" - just change LEvel→Level

Other things seems ok.
Comment 8 Gwyn Ciesla 2007-03-10 14:43:51 EST
I've cleaned up the BR, spaces and tabs.  I added the xmoto %{version} R. I
fixed the .desktop file.  I also took a quick look at the xmoto.bin.  It's
actually not binary, it's ascii xml, and doesn't look arch-dependant to me.

Spec URL: http://zanoni.jcomserv.net/extras/xmoto-edit/xmoto-edit.spec
SRPM URL: http://zanoni.jcomserv.net/extras/xmoto-edit/xmoto-edit-0.2.4-3.src.rpm
Comment 9 Gwyn Ciesla 2007-03-10 14:58:38 EST
In my cleanup, I broke the post and postun scripts.  My bash-fu is insufficent.
 What should I do?
Comment 10 Michał Bentkowski 2007-03-11 08:01:39 EDT
(In reply to comment #9)
> In my cleanup, I broke the post and postun scripts.  My bash-fu is 
insufficent.
>  What should I do?

Post new spec with fixed post* scripts. However, I noticed another issue 
related with manpage. Is it compressed by Makefile or by rpm? I'm asking 
because it looks broken:

[ecik@ecik ~]$ man xmoto-edit

gunzip: /usr/share/man/man6/xmoto-edit.6.gz: invalid compressed data--crc error

gunzip: /usr/share/man/man6/xmoto-edit.6.gz: invalid compressed data--length 
error

And this manpage isn't readable, e.g.: "XMOTO-EDIT-.iwvel editfor fo xmohyp, a 
2D mohyacros plat fom gram"

If it's automatically created by makefile then turn off that and let rpm do 
this thing.
Comment 11 Gwyn Ciesla 2007-03-11 12:10:49 EDT
Spec URL: http://zanoni.jcomserv.net/extras/xmoto-edit/xmoto-edit.spec
SRPM URL: http://zanoni.jcomserv.net/extras/xmoto-edit/xmoto-edit-0.2.4-4.src.rpm

Fixed the scripts.

The man page comes compressed right out of the tarball, before ./configure even.
 It's still unreadable.  Should I notify upstream and onmit for now?
Comment 12 Michał Bentkowski 2007-03-20 13:41:12 EDT
(In reply to comment #11)
> The man page comes compressed right out of the tarball, before ./configure 
even.
>  It's still unreadable.  Should I notify upstream and onmit for now?

Sorry that you had to wait...
Yes, it is obviously the best solution. 
Get rid of the man page and I'll be able to aprove this package.
Comment 13 Gwyn Ciesla 2007-03-20 14:00:35 EDT
I've actually got a fixed version from upstream, but they're not willing to do
an entire release just to push it out.  Should I just do a SOURCEN:
URL://to-upstream-svn-web-viewer?
Comment 14 Michał Bentkowski 2007-03-20 14:08:09 EDT
(In reply to comment #13)
> I've actually got a fixed version from upstream, but they're not willing to do
> an entire release just to push it out.  Should I just do a SOURCEN:
> URL://to-upstream-svn-web-viewer?

Sounds good.
Comment 16 Michał Bentkowski 2007-03-20 14:39:28 EDT
Looks good.
Approved.
Comment 17 Gwyn Ciesla 2007-03-20 14:47:15 EDT
New Package CVS Request
=======================
Package Name: xmoto-edit
Short Description: x-moto level editor
Owners: limb@jcomserv.net
Branches: FC-5 FC-6
InitialCC: 
Comment 18 Gwyn Ciesla 2007-03-21 10:08:50 EDT
Build.  Thank you.
Comment 19 Vegard Nossum 2008-03-07 09:53:53 EST
Hello. xmoto-edit runs fine, but it is not possible to play games from within
the editor. When F4 is pressed within the editor, xmoto should start the level
currently being edited. When I press F4, this appears on the console:

syntax error : Invalid option "-level"
X-Moto 0.3.4
usage:  xmoto [options]
options:
...


It seems that this option should be --level instead (it is listed in xmoto help
screen).
Comment 20 Gwyn Ciesla 2008-03-07 10:13:50 EST
A. Thank you for bringing this to my attention.

B. Please file a new bug, this does not belong on the review bug.

C. What version of Fedora is this?  The review bug is for rawhide, but your
error mentions xmoto 0.3.4, and the version in rawhide is 0.4.1.
Comment 21 Vegard Nossum 2008-03-07 10:57:11 EST
(In reply to comment #20)
> A. Thank you for bringing this to my attention.
> 
> B. Please file a new bug, this does not belong on the review bug.

Sorry, I found the link on your Fedora Wiki page and didn't know what review meant.

> C. What version of Fedora is this?  The review bug is for rawhide, but your
> error mentions xmoto 0.3.4, and the version in rawhide is 0.4.1.

It's Fedora 8.
Comment 22 Gwyn Ciesla 2008-03-07 11:25:03 EST
I see.  Click here and fill out what you can. .. 

https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora

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