Bug 185721 - Review Request: yadex - Doom level/wad editor
Review Request: yadex - Doom level/wad editor
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-03-16 23:39 EST by Wart
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-04-10 19:53:18 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Patch fixing gcc41 compile (1.26 KB, patch)
2006-03-17 05:57 EST, Hans de Goede
no flags Details | Diff

  None (edit)
Description Wart 2006-03-16 23:39:32 EST
Spec Name or Url: http://www.kobold.org/~wart/fedora/yadex.spec
SRPM Name or Url: http://www.kobold.org/~wart/fedora/yadex-1.7.0-1.src.rpm
Description: 
A Doom/DoomII/Heretic level editor
Comment 1 Wart 2006-03-16 23:41:46 EST
I just realized the .desktop file is missing.  I'll add it shortly.
Comment 3 Hans de Goede 2006-03-17 05:43:09 EST
Question: Why the Requires: freedoom?

Problem on FC-5:
src/wadlist.cc: In member function 'void Wad_list::del()':
src/wadlist.cc:178: error: no match for 'operator=' in
'((Wad_list*)this)->Wad__list::priv->Wad_list_priv::iter = 0'
/usr/lib/gcc/x86_64-redhat-linux/4.1.0/../../../../include/c++/4.1.0/bits/stl_list.h:112:
note: candidates are: std::_List_iterator<boost::shared_ptr<Wad_file> >&)

I'll see if I can fix this, also some 64 bit related warnings to check out
(printf argument size).

Comment 4 Hans de Goede 2006-03-17 05:57:51 EST
Created attachment 126273 [details]
Patch fixing gcc41 compile

I might find more problems, I justed wanted to share this one asap, so that we
don't do double work.
Comment 5 Wart 2006-03-17 11:18:12 EST
(In reply to comment #3)
> Question: Why the Requires: freedoom?

Yadex won't run without an iwad, so I modified the default configuration to use
the freedoom iwad.  Without an iwad yadex will complain and quit, and I
guarantee that will result in a bug report the first time someone tries to use
it.  :)  Best to avoid the issue and Requires: an iwad.
Comment 6 Wart 2006-03-17 13:30:58 EST
(In reply to comment #3)
> Problem on FC-5:
> src/wadlist.cc: In member function 'void Wad_list::del()':
> src/wadlist.cc:178: error: no match for 'operator=' in
> '((Wad_list*)this)->Wad__list::priv->Wad_list_priv::iter = 0'
>
/usr/lib/gcc/x86_64-redhat-linux/4.1.0/../../../../include/c++/4.1.0/bits/stl_list.h:112:
> note: candidates are: std::_List_iterator<boost::shared_ptr<Wad_file> >&)

I've got to be more careful about testing the build on multiple platforms before
submitting these things.

I'll add your patch and do some testing on FC-5 i386 before updating the package.
Comment 7 Hans de Goede 2006-03-18 04:07:24 EST
I thought it would be a good idea to start the full review while waiting for an
updated version with my patch included. This review is done with my patch
included, otherwise I wouldn't ger very far.


MUST
====
* rpmlint output clean
* Package named correctly
* GPL license OK.
* spec file legible, in Am. English
* Source matches upstream
* Successfully compiles and builds on at least one platform (FC-5 x86_64)
* no locale data, shared libraries, or static libraries
* No excessive Requires: or BR:
* Summary and description ok
* macro use consistent
* package owns the directories that it creates.
* Not relocatable
* %doc does not affect runtime

MUSTFIX
=======
* Does not compile on FC-5 without the attached patch
Comment 8 Wart 2006-03-18 15:13:03 EST
Patch included.  The sprite viewer doesn't seem to work on FC4-x86_64 or
FC5-i386.  Type "v" at the yadex command prompt to view sprites and you'll get a
new window that hangs.

http://www.kobold.org/~wart/fedora/yadex-1.7.0-3.src.rpm
http://www.kobold.org/~wart/fedora/yadex.spec
Comment 9 Hans de Goede 2006-03-20 15:08:48 EST
Hmm,

I though I could reproduce this, but after a recompile with some debugging
printf's added its gone, and now even a clean compile doesn't do it anymore,
maybe a compilerbug which got fixed in one of the last updates?
Comment 10 Wart 2006-03-22 00:47:35 EST
I'll investigate on my end once I've finished reconfiguring my desktop with
FC-5.  Don't expect any updates for about a week.
Comment 11 Hans de Goede 2006-04-08 02:54:14 EDT
Any progress on this?
Comment 12 Wart 2006-04-09 21:25:55 EDT
User error.  I was trying to use the mouse in the "view texture" mode, when it
seems that it only responds to up/down arrow.  The sprite and texture viewers
don't seem to properly refresh the display on Expose events, but it's not a
critical problem.

Right now I'm trying to figure out why it fails to build in mock but not on my
desktop.
Comment 13 Wart 2006-04-10 02:25:33 EDT
It was a local mock configuration error.  Everything should be good now.
Comment 14 Hans de Goede 2006-04-10 05:15:29 EDT
Ok,

I'll try to todo a full review soon then. Unfortunatly I _really_ should start
doing some stuff for my work right now. I know I just submitted a package for
review myself, thats because a Dutch language pack was recently released for an
educational game, so no my little one can play it once packaged. Thus I couldn't
help myself :)

I'll also take a look at the bsd-games setgid stuff when I find the time.
Comment 15 Hans de Goede 2006-04-10 05:55:03 EDT
I just saw I already started a full review, so its easy to finish it here we go
again:

MUST
====
* rpmlint output OK:
E: yadex configure-without-libdir-spec
W: yadex patch-not-applied Patch1:
http://glbsp.sourceforge.net/yadex/Yadex_170_Hexen.diff
Which are both ok, for the patch warning see the comment in the spec file, the
other is no problem since there are no files installed to %{_libdir} and there
is a good reason to not use %configure (see comment in spec)
* Package named correctly
* GPL license OK.
* spec file legible, in Am. English
* Source matches upstream
* Successfully compiles and builds on at least one platform (devel x86_64 & i386)
* no locale data, shared libraries, or static libraries
* No excessive Requires: or BR:
* Summary and description ok
* macro use consistent
* package owns the directories that it creates.
* Not relocatable
* %doc does not affect runtime

Approved!
Comment 16 Wart 2006-04-10 19:53:18 EDT
Imported and built.  Thanks!

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