[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Review request for current packaging of Red Eclipse (upstream release imminent)



Hello,
Thanks for the review! :)
I have fixed a bunch of the things pointed out, and tried to justify
those I have not.

I have marked items which I consider incomplete yet with "###", it's
either things that I am unsure how to fix and have posed questions for,
or things that I am in the process of fixing.

I have not re-uploaded to mentors yet, if you want to I can do so as
well, the current updated debian/ dirs are available in git:
http://anonscm.debian.org/gitweb/?p=pkg-games/cube2font.git
http://anonscm.debian.org/gitweb/?p=pkg-games/redeclipse.git
http://anonscm.debian.org/gitweb/?p=pkg-games/redeclipse-data.git

On Wed, 2011-12-21 at 17:30 +0800, Paul Wise wrote:
> The Cube 2 engine is also used by Sauerbraten. Both games should share
> the same engine.
> 
I have asked and confirmed upstream that cube2 in RE is not compatible
with cube2 in Sauer, it has a lot of specialised changes and would not
work as a shared build-dependency: "no, engine is heavily modified and
deviated evolutionary-wise".

> You might want to run wrap-and-sort -s.
Done

> 
> cube2font-dbg doesn't really need to give that much duplicated detail.
Fixed

> 
> There is an extra space in debian/watch
Fixed

> 
> cube2font doesn't appear to be compiled with -g, which is needed for
> debug symbols?
Added the patch from the redeclipse package.

> 
> I'm not sure if every package should have a README.Debian explaining
> how to get a backtrace from gdb.
Probably not, narrowified.

> 
> The quilt stuff in redeclipse/debian/README.source is not needed for
> dpkg-source v3 packages.
Okay, nuked.

> 
> It is possible to use wildcards in debian/*install files, for example:
> 
> usr/share/icons/hicolor/*/apps/redeclipse.png
Ah, implemented.

> 
> Please explain the technical reasons for that upstream prefers to embed enet.
> 
###
My IRC conversation regarding the matter went as follows:
15:47 < arand> Hmm, enet 1.3.1 made it into testing... I need to figure
out how to make RE build without embedded enet in a sane way...
15:50 < eihrul_> don't
15:50 < eihrul_> build with enet included in RE
15:50 < arand> Is there a large diff?
15:50 < eihrul_> not currently, but there has been in the past and may
be in the future
15:51 <@Hirato> so you can choose, have the debian security guys hate
you, or the rest of the community hate you :P
15:51 < arand> Pretty much yea..
15:52 < arand> Fun fun
15:55 < eihrul_> the debian security guys don't understand what enet is
15:55 < eihrul_> it is a part of the sauerbraten engine
15:55 < eihrul_> the fact that it has also been isolated and release as
a separate library for the benefit of others is irrelevant
15:57 < arand> Ok, well I at least have some arguments when asked
then... I guess

I am hesitant to go against upstream on this, does the
Debian-best-practice weight heavier in this case?

> Please get the patches, manual pages and desktop file included upstream.
Manual pages are already upstream, I'm patching in the debian-specifics
(since upstream runs and compiles in the source dir, I did not forward
the changes specific to installation in debian).
The debug building is intentionally not switched on by default upstream.
Fullscreen mode by default is intentional upstream, DGT likes windowed
mode though.

> 
> redeclipse needs to go to contrib if redeclipse-data will be going to non-free.
Oups :( fixed.
> 
> non-free packages need to state why they are non-free in debian/copyright:
> 
> http://www.debian.org/doc/debian-policy/ch-docs.html#s-copyrightfile
Added.

> 
> Some files appear to be not distributable:
> 
> Files: src/site/bits/redeclipse-large.png src/site/bits/redeclipse-regular.png
>        src/site/bits/redeclipse-small.png src/site/bits/relogo*
>        src/site/bits/sprite.png src/site/bits/wallpaper*
>        data/textures/bloodbath.png data/textures/carnage.png
>        data/textures/critical.png data/textures/dominate.png
>        data/textures/firstblood.png data/textures/headshot.png
>        data/textures/massacre.png data/textures/multi.png
>        data/textures/revenge.png data/textures/slaughter.png
>        data/textures/triple.png
>        data/textures/logo.png data/luckystrike/pub2* data/luckystrike/pub1*
> License: Akashi-Font
It seems we agreed in IRC that this is good enough for non-free at
least.

> 
> Files: data/particles/smoke.png data/particles/explosion.png
> data/textures/lava.jpg
> License:
>  Created by RaZgRiZ Made with Filter Forge II Beta 3
>  Licensed publicly for everyone to use and modify as long as the author is
>  credited for the original work.
> 
> Files: data/particles/blood.png
> License:
>  Created by FischKopF, source: http://www.quadropolis.us/node/2693
>  "Feel free to edit, claim it's yours, use it in own projects"
> 
> Files: forest*.ogg cycadas.ogg
> License:
>  Provided that you mention (somewhere, somehow) that the original samples are
>  archived at freesound.org you are granted permission to modify/use, even
>  commercially. It's the only prerequisite I impose.
>  .
>  Furthermore, providing a link to the original samples at freesound may be
>  helpful to other developers, but this is just a personal opinion, not a
>  prerequisite.
>  .
>  The original samples are archived at freesound.org, specifically:
>  http://www.freesound.org/samplesViewSingle.php?id=34580
>  http://www.freesound.org/samplesViewSingle.php?id=7292
> Comment:
>  Special permission for relicensing under above license have been granted by
>  the author.
###
Ack, I figured use implied distribute (as is obviously intended), I'm
off to pester third parties then, again :(

> 
> 
> You are missing the copyright/license for the embedded JavaScript code
> copies in redeclipse-data.
I have nuked the whole site/ from orbit (seems like one of the bits
there were NoDerivs 2.5 anyways, ick!)

> 
> data/models/actors/turret, data/models/weapons/plasma,
> data/models/weapons/flamer,  are under CC-BY-SA-3.0-AU not regular
> CC-BY-SA.
Fixed (yay for unwieldy d/copyright)

> 
> Similarly ./data/torley/ is the USA version of CC-BY-SA. It also has a
> weird no selling clause.
Right, I have noted this down as a custom license in d/copyright.

> 
> Some of the files have no known filetype (according to file(1)) or are
> gzip compressed but the data inside that has no known filetype. Could
> you get them included in file if appropriate? Some of the text-based
> formats could probably use adding to file too. Also, do we have tools
> to create/modify them in Debian?
###
I'm not sure which one's you refer to so I'm guessing:
All .cfg files are cubescript.
.mpz files are map files, RE has a builtin editor for them.
.md5anim .md5mesh .iqm are all various types of model/animation files, I
do not know the intricacies, but they should all be creatable via
blender, the preferred way to edit those would be to have the
original .blend file, which due to size and some not being available at
all, are not included in the upstream source.

I have no idea how one would go about implementing this in the file
utility, I'm afraid.

> 
> The comments in some of the audio files point at this software, I
> guess the audio is one of the things we don't have source for?
> 
> http://www.image-line.com/documents/edison.html
> https://en.wikipedia.org/wiki/Cool_Edit
Indeed, similar to the models above, upstream does not supply source for
music, due to size, and if they did, I'm guessing it would be in a
proprietary format anyways.

> 
> ogginfo -q claims that some of the Ogg files are corrupted (holes).
###
Running
$ for i in $(find . -iname *.ogg); do ogginfo -q $i; done
yeilds me no such warnings, which files in particular did you get  this
for?

> 
> Some of the readme.txt files say that source .psd files are available
> on the author's website.
I am inclined to leave it at that as well, for interested users this
seems to be the sane way to direct them to the sources, no? Should I be
collecting all of those hints in Debian.source?

> 
> There are a small number of duplicate files in the data package, you
> might want to inform upstream about that.
> 
> dpkg-shlibdeps: warning: dependency on libstdc++.so.6 could be avoided
> if "debian/cube2font/usr/bin/cube2font" were not uselessly linked
> against it (they use none of its symbols).
> dpkg-shlibdeps: warning: dependency on libz.so.1 could be avoided if
> "debian/cube2font/usr/bin/cube2font" were not uselessly linked against
> it (they use none of its symbols).
> dpkg-shlibdeps: warning: dependency on libgcc_s.so.1 could be avoided
> if "debian/cube2font/usr/bin/cube2font" were not uselessly linked
> against it (they use none of its symbols).
> dpkg-shlibdeps: warning: dependency on libgcc_s.so.1 could be avoided
> if "debian/redeclipse/usr/lib/games/redeclipse/redeclipse" were not
> uselessly linked against it (they use none of its symbols).
> dpkg-shlibdeps: warning: dependency on libgcc_s.so.1 could be avoided
> if "debian/redeclipse-server/usr/lib/games/redeclipse/redeclipse-server"
> were not uselessly linked against it
###
I Asked in #debian-mentors about libgcc_s, and want told that
"-Wl,--as-needed" can (and does) get rid of it, however the benefits of
using it may not be all that big, and that it may cause issues, so I
decided not to.

Likewise for cube2font it gets rid of libstdc++ and libgcc_s, but for
the above reason, and that I don't know much about the details I decided
not to try to mess with it.

The dependency on libz appears to be unavoidable without modifying the
included (and required) /usr/include/png.h whicj explicitly #include
"zlib.h". Is there a sane way to force-avoid it?

> 
> 
> dpkg-gencontrol: warning: Depends field of package cube2font-dbg:
> unknown substitution variable ${shlibs:Depends}
> dpkg-gencontrol: warning: Depends field of package redeclipse-dbg:
> unknown substitution variable ${shlibs:Depends}
> dpkg-gencontrol: warning: Depends field of package
> redeclipse-server-dbg: unknown substitution variable ${shlibs:Depends}
###
Hmm, should I simply be removing it for these specific packages where it
seems to be unnecessary?

> 
> 
> P: cube2font source: unversioned-copyright-format-uri
> http://dep.debian.net/deps/dep5/
> P: cube2font-dbg: no-upstream-changelog
> P: cube2font: no-upstream-changelog
> P: redeclipse source: unversioned-copyright-format-uri
> http://dep.debian.net/deps/dep5/
> I: redeclipse-server-dbg: extended-description-is-probably-too-short
> I: redeclipse-dbg: extended-description-is-probably-too-short
These warning were hopefully addressed in my original post, please tell
if any of my reasoning there is insufficient.

> 
> -- 
> bye,
> pabs
> 
> http://wiki.debian.org/PaulWise
> 
> 

Thanks
-- 
Martin Erik Werner <martinerikwerner@gmail.com>

Attachment: signature.asc
Description: This is a digitally signed message part


Reply to: