Bug 201418 - Review Request: widelands - GPL Settlers II clone
Review Request: widelands - GPL Settlers II clone
Status: CLOSED DUPLICATE of bug 238270
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-DEADREVIEW
  Show dependency treegraph
 
Reported: 2006-08-04 19:27 EDT by Nikolai
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-18 15:32:29 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
PATCH: compile + 64 bit fixes for latest CVS (2.67 KB, patch)
2006-11-23 16:10 EST, Hans de Goede
no flags Details | Diff

  None (edit)
Description Nikolai 2006-08-04 19:27:57 EDT
All the files are hosted at http://nikolai.thecodergeek.com/widelands.

The update time on the bottom of the page is incorrect, it will be fixed when I
re-upload the files.
Comment 1 Wart 2006-08-04 19:32:24 EDT
Please try to use the Package Review template when submitting new packages for
review so that the various bug fields get set appropriately:

http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
Comment 2 Wart 2006-08-04 20:07:31 EDT
There's a few things that need to be addressed before this can be approved.

* Is there a reason you want to package files from CVS instead of the official
file release?

* Source1: is a tarball with a single .desktop file.  Don't bother making a
tarball for a single file.  Just use "Source1: widelands.desktop". This will
remove the need for the egregious tar command in %prep

* Reduce the Summary: to a single sentence, without the leading 'A'

* In your %install scriptlet, try using 'cp -a' instead of 'cp -r' to preserve
timestamps

* Remove the CVS directories in %prep, not in %install

* Never refer to '/usr/src/redhat' in the spec file.  It is highly unlikely that
this directory will actually be used when the package is built.  You can refer
to your desktop file as %{SOURCED1} instead.

* Don't use '/usr' in your build command.  Use %{_prefix} instead.  Also,
'share/widelands' should be '%{_datadir}/%{name}'

I'd suggest reading through the packaging guidelines to become more familiar
with the Fedora packaging rules:  http://fedoraproject.org/wiki/Packaging/Guidelines
Comment 3 Nikolai 2006-08-05 07:15:04 EDT
I'm packaging the files from CVS because the official release is REALLY old, 
and they're developing rather slowly. All the remaining will be fixed and 
uploaded today or tomorrow. And I'd like to know if there is a 
recursive "install" command, or if I need to use "for i in *" or just 
recursive "cp".

And I didn't find the link to the "Package Review Request" and didn't have the 
time to look for it, so I just guessed the rest of the info. Sorry for the 
haste, I'm having bad hardware problems. It will be fixed soon.
Comment 4 Nikolai 2006-08-05 09:56:13 EDT
The binaries are up and the syntax highlighted spec file too. As I'm on Windows,
I didn't update the spec and recreated the SRPM. They will be fixed and uploaded
tomorrow.
Comment 5 Hans de Goede 2006-08-10 16:40:00 EDT
MUST:
=====
* rpmlint output is:
E: widelands description-line-too-long Your objective is not to send hordes of
warriors to destroy a enemy, but to build a strong economy.
E: widelands description-line-too-long Your objective is not to send hordes of
warriors to destroy a enemy, but to build a strong economy.
W: widelands no-documentation
E: widelands version-control-internal-file
/usr/share/widelands/tribes/empire/buildings/lumberjack/CVS/Root
E: widelands version-control-internal-file
/usr/share/widelands/worlds/greenland/bobs/tree1/CVS/Repository
E: widelands version-control-internal-file
/usr/share/widelands/tribes/barbarians/buildings/tavern/CVS/Tag
E: widelands version-control-internal-file
/usr/share/widelands/tribes/barbarians/buildings/deep_oremine/CVS/Entries
E: widelands version-control-internal-file
/usr/share/widelands/worlds/blackland/bobs/pebble6/CVS/Root
E: widelands version-control-internal-file
/usr/share/widelands/worlds/greenland/bobs/tree1/CVS/Tag
E: widelands version-control-internal-file
/usr/share/widelands/tribes/barbarians/buildings/warmill/CVS/Repository
And then tons more og the "version-control-internal-file" errors.
These must all be fixed.
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License (GPL) ok, license file included
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on devel-x86_64
* BR: ok
* No locales
* No shared libraries
* Not relocatable
* Package owns / or requires all dirs
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code and permissible content
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* no -devel package needed, no libs / .la files.
* .desktop file as required and properly installed

MUST FIX:
=========
* During build desktop-file-install gives this message:
  widelands.desktop: missing encoding  (guessed UTF-8)
* rpmlint output:
E: widelands description-line-too-long Your objective is not to send hordes of
warriors to destroy a enemy, but to build a strong economy.
E: widelands description-line-too-long Your objective is not to send hordes of
warriors to destroy a enemy, but to build a strong economy.
W: widelands no-documentation
E: widelands version-control-internal-file
/usr/share/widelands/tribes/empire/buildings/lumberjack/CVS/Root
E: widelands version-control-internal-file
/usr/share/widelands/worlds/greenland/bobs/tree1/CVS/Repository
E: widelands version-control-internal-file
/usr/share/widelands/tribes/barbarians/buildings/tavern/CVS/Tag
E: widelands version-control-internal-file
/usr/share/widelands/tribes/barbarians/buildings/deep_oremine/CVS/Entries
E: widelands version-control-internal-file
/usr/share/widelands/worlds/blackland/bobs/pebble6/CVS/Root
E: widelands version-control-internal-file
/usr/share/widelands/worlds/greenland/bobs/tree1/CVS/Tag
E: widelands version-control-internal-file
/usr/share/widelands/tribes/barbarians/buildings/warmill/CVS/Repository
And then tons more og the "version-control-internal-file" errors.
These must all be fixed. Hint for the version-control-internal-file" errors do a 
rm -fr `find -name CVS` on your CVS co before creating the tarbal, this way you
get a substantial smaller tarbal too.
* Drop the "A Settlers II OpenSource clone. " from the summary, this may be
  inspired by Settlers II bu advertising it as a clone is asking for legal 
  trouble.
* As already said don't put the desktop file into a tarbal, just refer to it
  directly. Also don't copy it to the build-dir instead just refer to it as
  %{SOURCE1} in the desktop-file-install command.
* Remove the bogus "#make %{?_smp_mflags}" line from the spec, this package
  uses sconstruct, so that line makes no sense at all.
* There ARE usuable docs, for example the COPYING file and probably others too.
* Please add an icon file according to the freedesktop.org icon standard, it
  should go under:
  %{_datadir}/icons/hicolor/32x32/apps
  Where 32x32 is the size of the icon, please do ls /usr/share/icons/hicolor/
  to see the available valid sizes, if the icon doesn't match any pick the 
  closest. 
* Once the icon is in the proper place you must add %post(un) script to update 
  the icon-cache see:
http://fedoraproject.org/wiki/ScriptletSnippets#head-fc74f078205565f961f6d836b77c3428619c689d
* RPM_OPT_FLAGS do not get used during the build, good luck with fixing this as
  this can be a bitch with SConstruct builds, let me know if you need any help 
  with this.

Questions
=========
* Why the build=debug?
Comment 6 Nikolai 2006-08-11 06:28:15 EDT
About the "build=debug" this is needed to enforce a build with "-g", otherwise
the package is not built with debugging info and the -debuginfo package starts
crying.

I am not currently on Fedora (I like KDE best, and the RH artwork loooks ugly on
it), but I'm configuring the system to receive RPM builds. When I'm done with
this, I will rebuild the package. While I don't finish it, I'll only update the
spec file. I'm looking forward to UnleashKDE's approval :)
Comment 7 Hans de Goede 2006-09-28 10:40:41 EDT
ping?
Comment 8 Hans de Goede 2006-10-25 03:57:31 EDT
ping again?
Comment 9 Nikolai 2006-10-25 15:46:06 EDT
Sorry, my monitor at home isn't working and I can't install any Linux at the PC
I'm using, so I'm stuck for now :/

I'm trying to hurry my father to fix the monitor, but he will probably only be
able to do so once he's back to work. He's not working due to medical issues and
Brazil's social security sucks, so money is short.

I'm at least downloading FC6, so as soon as I fix the monitor I'll be back to work.
Comment 10 Hans de Goede 2006-10-26 09:22:40 EDT
Okay, no problem and good to hear from you. Next time please keep in touch, even
it is just to say that you currently cannot work on a package for whatever reasons.
Comment 11 Hans de Goede 2006-11-19 02:18:00 EST
Any luck with fixing the monitor? This is becoming a rather long story and you
were very unresponsive in the glob2 review too. So I'm seriously concidering
closing both the glob2 review and this on as dead reviews. SO that someone else
can pick them up and resubmit them for inclusion, esp. glob2 is a very popular
game and its a shame its being kept out of FE by all this/

Comment 12 Nikolai 2006-11-19 04:23:46 EST
The monitor arrived yesterday. It still has a few glitches, but at least it works.

I'm installing the tools needed and I'll probably have a new build for glob2 and
widelands today or tomorrow.
Comment 13 Nikolai 2006-11-21 14:41:26 EST
Some schoolwork prevented me from working on the package yesterday. I'm
compiling the widelands package right now, with most of the changes implemented.

The glob2 package is currently unbuildable due to changes on the freetype-devel
package on FC6. It looks like it now doesn't include the
'/usr/include/freetype2/freetype/internal' directory like it did before, and I
don't know how to fix that on the code to make a patch. Filed a bug report on
glob2's bugzilla since it shouldn't be using an internal header.
Comment 14 Nikolai 2006-11-21 15:38:35 EST
I'm uploading the new SRPM. I don't know how to make desktop-file-install stop
complaining about the encoding, so I'd like some hints.

As widelands is growing, the package is now about 25 megs. If I need to split it
into widelands{,-data}, I need some info on how to do it and if it is worthy.
Comment 15 Hans de Goede 2006-11-21 15:51:51 EST
(In reply to comment #14)
> I'm uploading the new SRPM. I don't know how to make desktop-file-install stop
> complaining about the encoding, so I'd like some hints.
> 

Does the desktop file include a line like this? :
Encoding=UTF-8
And is it actually a UTF-8 file?

> As widelands is growing, the package is now about 25 megs. If I need to split it
> into widelands{,-data}, I need some info on how to do it and if it is worthy.

A seperate -data package is only usefull if upstream provides a seperate tarbal
for data and there can thus be 2 seperate SRPMS. Otherwise there is no use in
having a seperate -data package ( I know the Games SIG page claims otherwise,
but its wrong).
Comment 16 Nikolai 2006-11-21 16:06:29 EST
It does not include any Encoding line and the freedesktop.org's specifications
don't say that Encoding is a valid line.
Comment 17 Nikolai 2006-11-21 16:13:48 EST
I tested the game and it is segfaulting when you enter a map. I will upload only
the new specfile and upload the rest when the game is "less unstable".
Comment 18 Hans de Goede 2006-11-23 15:36:01 EST
(In reply to comment #13)
> The glob2 package is currently unbuildable due to changes on the freetype-devel
> package on FC6. It looks like it now doesn't include the
> '/usr/include/freetype2/freetype/internal' directory like it did before, and I
> don't know how to fix that on the code to make a patch. Filed a bug report on
> glob2's bugzilla since it shouldn't be using an internal header.

I just tried with upstreams latest release 0.8.21 and that builds fine with the
new freetype! Also please put glob2 commments in the glob2 review.
Comment 19 Hans de Goede 2006-11-23 16:10:51 EST
Created attachment 142021 [details]
PATCH: compile + 64 bit fixes for latest CVS

About widelands, according to upstream a stable release is expected soon, so I
would expect cvs to be in pretty good shape, maybe you just picked a bad date
todo a checkout?

I've taken a look at CVS from today and that needs the attached patch to
compile and the patch also fixes some 64 bit issues. Please send this patch
upstream.

With this patch applied the checkmate and eleven forests maps both load fine,
so I guess the crahs bug is fixed now. Please make a new srpm available for
review.
Comment 20 Hans de Goede 2006-11-26 13:47:35 EST
Hello, ping?

I'm getting kinda tired of this. So its time for some new rules of engagement, I
expect you to respond (not resolve, but respond) to any of my comments within a
week from now on, or otherwise I'm going to close this and the glob2 review as
FE-DEADREVIEW (see: http://fedoraproject.org/wiki/Extras/Policy/StalledReviews )
and its game over for you as far as getting sponsored by me is concered.
Comment 21 Nikolai 2006-11-26 16:31:51 EST
I am checking out the latest revision and will (hopefully) compile and run it
today. If successful, I will upload the new SRPM. I will also see if any
developers have already applied this patch or not and submit it if needed. Why
do developers keep putting extra qualification on C++ code?

I'm hoping that upstream will release a new stable really soon; it's not very
good to work with CVS/SVN.

Due to some studies I'm having even less time to work on this. I will keep
working on it while I'm able to, but I may mark the bug as FE-DEADREVIEW if I
get even more busy. Sorry to keep you guys waiting.
Comment 22 Nikolai 2006-11-26 19:27:04 EST
I don't have time to upload the SRPM because my connection is kinda slow and I
need to turn off the computer.

I uploaded the specfile, the desktop and icon files as a tarball and a script to
checkout SVN and generate the code tarball to the following URLs:

http://nikolai.thecodergeek.com/widelands/widelands.spec
http://nikolai.thecodergeek.com/widelands/widelands-desktop-and-icon.tar.bz2
http://nikolai.thecodergeek.com/widelands/checkout-widelands.sh

The tarball generated by the script should have a md5sum of
"73633e94809d994521441c154b06f718", unless the script is buggy or tar messes
with the file order.
Comment 23 Nikolai 2006-12-03 04:20:26 EST
Something is wrong with my RPM. When I try to install the new build so I can
test it, I get the following error:

sudo rpm -ivh widelands-1824-2.i386.rpm
rpmdb: PANIC: fatal region error detected; run recovery
error: db4 error(-30977) from dbenv->open: DB_RUNRECOVERY: Fatal error, run
database recovery
error: cannot open Packages index using db3 -  (-30977)
error: cannot open Packages database in /var/lib/rpm

How can I fix it?

Anyway, I'm uploading the SRPM now.
Comment 24 Hans de Goede 2006-12-03 12:42:37 EST
That is most likely not a problem with your RPM, but with your local RPM
database, to fix it remove rpm's database cache files:
rm /var/lib/rpm/__db.*

And the try again. I'm currently really really busy (moving from one home to
another), so don't expect a review of the SRPM soon, I will take a look as time
permits.
Comment 25 Hans de Goede 2006-12-13 07:00:35 EST
I'm afraid this time it is me taking a long time to respond. I'm currently
moving from one home to another and thus I'm both very busy and without internet
at home. I will get back to this as time permits, but for now I have to ask for
some patience.
Comment 26 Hans de Goede 2007-03-18 15:32:29 EDT
Marking as dead review, the reporter has been pinged several times in a related
bug report. With his current lack of response behaviour I'm not going to sponsor
him, see: bug 225010 and bug 191005. Closing as WONTFIX and blocking FEDEADREVIEW>
Comment 27 Karol Trzcionka 2007-04-28 13:06:15 EDT

*** This bug has been marked as a duplicate of 238270 ***

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