On Wed, 2011-12-21 at 14:56 +0100, Martin Erik Werner wrote:
> 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.
>
> >
[1]
> > 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.
> >
[2]
> > 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.
>
> >
[3]
> > 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?
>
> >
[4]
> > 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?
>
> >
> >
[5]
> > 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
Hello again, upstream has now released Red Eclipse 1.2 and hence this is
partly a RFS, partly a re-review request.
The latest version is available in git (now with full gbp+pristine.tar
layout):
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
Or from debexpo/mentors:
http://mentors.debian.net/package/cube2font
http://mentors.debian.net/debian/pool/main/c/cube2font/cube2font_1.2-1.dsc
http://mentors.debian.net/package/redeclipse
http://mentors.debian.net/debian/pool/contrib/r/redeclipse/redeclipse_1.2-1.dsc
http://mentors.debian.net/package/redeclipse-data
http://mentors.debian.net/debian/pool/non-free/r/redeclipse-data/redeclipse-data_1.2-1.dsc
From previous reviewing, there are some outstanding points [1-5] above
that I have questions about (the ones I've left out I consider done or in-the-future items).
[1]
Is this motivation good enough for not using stand-alone Enet?
[2]
I have argumented that we should treat this as Public Domain, but gotten
no clear answer if it is good enough for Debian, see
http://lists.debian.org/debian-legal/2012/01/msg00004.html
I have contacted the author (no response yet) with a request for CC0
clarification, but is this really a necessity?
[3]
I've tried adding at least the compressed maps types to file(1), see
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=652988 , the other
things I'm unsure how I would do, cubescript in particular has no good
indicator, apart from a .cfg file ending :/
[4]
List of duplicates have been forwarded, but it's mostly a wontfix since
linking isn't as easy on windows.
[5]
I have removed ${shlibs:Depends} from the -dbg packages, is this
correct?
Thanks
--
Martin Erik Werner ("arand") <martinerikwerner@gmail.com>
Attachment:
signature.asc
Description: This is a digitally signed message part