Bug 432033
Summary: | Review Request: crystalspace - Crystal Space a free 3D engine | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Hans de Goede <hdegoede> | ||||
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> | ||||
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | fedora-package-review, mtasaka, notting | ||||
Target Milestone: | --- | Flags: | mtasaka:
fedora-review+
kevin: fedora-cvs+ |
||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2008-02-20 10:38:05 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Bug Depends On: | 432185, 432335 | ||||||
Bug Blocks: | 432034 | ||||||
Attachments: |
|
Description
Hans de Goede
2008-02-08 14:55:37 UTC
Only just tried to build, but after some long time it failed only on ppc?? http://koji.fedoraproject.org/koji/taskinfo?taskID=403509 Hmm, it fails with an assembler syntax error while assembling code generated by g++ -> toolchain bug. I've filed bug 432185 for this. If possible I would like to continue with the review of this, if bug 432185 doesn't have a fix in sight when the review is completed I'll just add ExcludeArch ppc for now. Well, I want to review this, however if you want me to review this package please wait until I write comments on other (about) 5-6 review requests... (and perhaps I will go to bed in one or two hours) BTW I am reviewing bug 426492 , however this fails to rebuild with gcc43 on many points and I would appreciate it if you would help me fixing the source codes. (In reply to comment #4) > BTW I am reviewing bug 426492 , however this fails to rebuild > with gcc43 on many points and I would appreciate it if you would > help me fixing the source codes. Okay, I've downloaded the srpm (link in the review is broken btw) and I'm working on fixing this with gcc-4.3 now. Feel free to drop requests like this to me more often, I think you're doing an _awesome_ job with all those reviews and I'm always happy to help out where I can. Hans, I noticed today that: (note: I always enable "koji static-repos" repo) [root@localhost ~]# yum check-update .... skip ........ xerces-c.i386 2.8.0-1.fc9 koji-rawhide xerces-c-devel.i386 2.8.0-1.fc9 koji-rawhide .................... [root@localhost ~]# Resolving Dependencies --> Running transaction check ---> Package xerces-c-devel.i386 0:2.8.0-1.fc9 set to be updated ---> Package xerces-c.i386 0:2.8.0-1.fc9 set to be updated --> Processing Dependency: libxerces-c.so.27 for package: cegui --> Finished Dependency Resolution cegui-0.5.0b-6.fc8.i386 from installed has depsolving problems --> Missing Dependency: libxerces-c.so.27 is needed by package cegui-0.5.0b-6.fc8.i386 (installed) Error: Missing Dependency: libxerces-c.so.27 is needed by package cegui-0.5.0b-6.fc8.i386 (installed) We have to update cegui first (as this package has cegui-devel as BR) Currently cegui build on rawhide fails as: http://koji.fedoraproject.org/koji/taskinfo?taskID=411388 For 1.2-2: A. spec file issue * scriptlet output - I guess that (although I could not find where it is written) Fedora requests that scriptlet output must be quiet. For now -utils %post scriptlets shows a lot of output messages. If you want to keep these messages IMO these should be redirected to some log file. * Directory ownership issue - My directory check shows (I have not installed -doc subpackage) ---------------------------------------------------------------- [tasaka1@localhost crystalspace]$ grep package DIR-check.log | grep any /usr/share/doc/crystalspace-1.2/LICENSE crystalspace-1.2-2.1.fc9 file /usr/share/doc/crystalspace-1.2 is not owned by any package /usr/share/doc/crystalspace-1.2/README crystalspace-1.2-2.1.fc9 file /usr/share/doc/crystalspace-1.2 is not owned by any package /usr/share/doc/crystalspace-1.2/history.old crystalspace-1.2-2.1.fc9 file /usr/share/doc/crystalspace-1.2 is not owned by any package /usr/share/doc/crystalspace-1.2/history.txt crystalspace-1.2-2.1.fc9 file /usr/share/doc/crystalspace-1.2 is not owned by any package /usr/share/doc/crystalspace-1.2/history_1.2.txt crystalspace-1.2-2.1.fc9 file /usr/share/doc/crystalspace-1.2 is not owned by any package ---------------------------------------------------------------- * Multilib issue - I don't know how we should deal with multilib issue (no, I REALLY don't know!!), however at least %_bindir/cs-config-1.2 causes multilib conflict. ---------------------------------------------------------------- [tasaka1@localhost bin]$ diff -u /usr/bin/cs-config-1.2 ./cs-config-1.2 --- /usr/bin/cs-config-1.2 2008-02-14 13:47:02.000000000 +0900 +++ ./cs-config-1.2 2008-02-14 13:41:13.000000000 +0900 @@ -26,7 +26,7 @@ do prefix="${p}" exec_prefix="${prefix}" - makeout="./out/linuxx86/debug" + makeout="./out/linux/debug" version="1.2" longversion="crystalspace 1.2" newincdir="" @@ -92,7 +92,7 @@ case $1 in -lcrystalspace-1.2) DEPS=" -lpthread -lpthread -lz" ;; -lcrystalspace_opengl-1.2) DEPS=" -lcrystalspace-1.2 -lGL -lXext -lX11 -lX11 -lXext -lpthread -lm" ;; - -lcrystalspace_python-1.2) DEPS=" -lcrystalspace-1.2 -L/usr/lib/python2.5 -lpython2.5 -lpthread -ldl -lutil -lm -lpthread" ;; + -lcrystalspace_python-1.2) DEPS=" -lcrystalspace-1.2 -L/usr/lib64/python2.5 -lpython2.5 -lpthread -ldl -lutil -lm -lpthread" ;; -lcrystalspace_staticplugins-1.2) DEPS="" ;; *) ----------------------------------------------------------------- Someone says that on -devel subpackage multilib confilct must be avoided. * Dependency for -devel subpackage - Please check the dependency for -devel subpackage. Example: - From /usr/include/crystalspace-1.2/csplugincommon/opengl/glcommon2d.h : ----------------------------------------------------------------- 26 #if defined(CS_OPENGL_PATH) 27 #include CS_HEADER_GLOBAL(CS_OPENGL_PATH,gl.h) 28 #else 29 #include <GL/gl.h> 30 #endif ----------------------------------------------------------------- - /usr/include/crystalspace-1.2/ivideo/wxwin.h has #include <wx/wx.h> - And some others. ? Timestamps - This rpm installs many "non-built" files and keeping timestamps on them are generally desirable. Would you try to keep timestamps on installed files as much as possible? (usually adding INSTALL="install -p" works, at least on recent autotool-based Makefiles) !! For license issues, I will check it later (as the tarball contains more than 10000 files, which is the largest number I have ever reviewed) !! Again currently crystalspace does not build due to missing dep for hal :( http://koji.fedoraproject.org/koji/taskinfo?taskID=425708 Simply rebuilding hal against new libsmbios seems good http://koji.fedoraproject.org/koji/taskinfo?taskID=425729 (In reply to comment #7) > For 1.2-2: > > A. spec file issue > * scriptlet output > - I guess that (although I could not find where it is written) > Fedora requests that scriptlet output must be quiet. > > For now -utils %post scriptlets shows a lot of output messages. > If you want to keep these messages IMO these should be redirected > to some log file. > I didn't silence this to check it went ok during development, silenced now. > * Directory ownership issue > - My directory check shows (I have not installed -doc subpackage) Fixed > * Multilib issue > - I don't know how we should deal with multilib issue (no, I REALLY > don't know!!), however > at least %_bindir/cs-config-1.2 causes multilib conflict. Fixed > * Dependency for -devel subpackage > - Please check the dependency for -devel subpackage. > Example: > - From /usr/include/crystalspace-1.2/csplugincommon/opengl/glcommon2d.h : > ----------------------------------------------------------------- > 26 #if defined(CS_OPENGL_PATH) > 27 #include CS_HEADER_GLOBAL(CS_OPENGL_PATH,gl.h) > 28 #else > 29 #include <GL/gl.h> > 30 #endif > ----------------------------------------------------------------- > - /usr/include/crystalspace-1.2/ivideo/wxwin.h has #include <wx/wx.h> Hmm, thats optional better to not frag in wx for people who use crystalspace without wx. Otherwise Fixed. > ? Timestamps > - This rpm installs many "non-built" files and keeping timestamps on > them are generally desirable. Would you try to keep timestamps on > installed files as much as possible? > (usually adding INSTALL="install -p" works, at least on recent > autotool-based Makefiles) > This does not use regular makefiles but jam, which I've been fighting all the way to stop it from using custom CFLAGS, so I see no sane way to fix this. New version: Spec URL: http://people.atrpms.net/~hdegoede/crystalspace.spec SRPM URL: http://people.atrpms.net/~hdegoede/crystalspace-1.2-3.fc9.src.rpm For 1.2-3: * As I wrote some missing dependency for -devel subpackage as "Example", it seems that there are some more missing dependencies for -devel package - cegui-devel - zlib-devel (I guess adding more 2 BR listed above should be enough) * It seems that adding INSTALL="install -p" to make install actually works http://koji.fedoraproject.org/koji/taskinfo?taskID=426911 ! This weekend I will try to check license issues (if any) for this package (more than 10000 files needs checking) Created attachment 295066 [details]
License check list for crystalspace
(For non-legal issue, please see my previous comments)
Note:
Unless you specify license tag for subpackages, subpackages
inherits license tag from main package.
For example, currently -doc subpackage has
"License: LGPLv2+ and GPLv2+ and GPLv2".
Legal issue:
GPLv2:
GPLv2/apps/import/maya2spr/binarytree.cpp
-> maya2spr (in -utils) is GPLv2
GPLv2/scripts/max/exportcsp.mcr
GPLv2/scripts/max/exportlights.mcr
GPLv2/scripts/max/exportsprite.mcr
GPLv2/scripts/max/fixmaterials.mcr
GPLv2/scripts/max/sanitycheck.ms
GPLv2/scripts/max/showMap.mcr
-> all these files (in -utils) are GPLv2
GPLv2+:
GPLv2+/apps/tests/ceguitest/ceguitest.cpp
GPLv2+/apps/tests/ceguitest/ceguitest.h
-> ceguitest (in -demos) is GPLv2+
GPLv2+/libs/csplugincommon/sndsys/
-> libcrystalspace-1.2.so (in main) is GPLv2+ (not LGPLv2+)
-----------------------------------------------------------------------------
1810 echo './out/linuxx86/debug/libs/cstool/memory_pen.o@'
'./out/linuxx86/debug/libs/cstool/csfxscr.o@'
'./out/linuxx86/debug/libs/csplugincommon/canvas/scrshot.o@'
'./out/linuxx86/debug/libs/csplugincommon/canvas/x11-keys.o@'
'./out/linuxx86/debug/libs/csplugincommon/canvas/cursorconvert.o@'
'./out/linuxx86/debug/libs/csplugincommon/canvas/softfontcache.o@'
'./out/linuxx86/debug/libs/csplugincommon/canvas/fontcache.o@'
'./out/linuxx86/debug/libs/csplugincommon/canvas/graph2d.o@'
'./out/linuxx86/debug/libs/csplugincommon/imageloader/commonimagefile.o@'
'./out/linuxx86/debug/libs/csplugincommon/imageloader/optionsparser.o@'
'./out/linuxx86/debug/libs/csplugincommon/particlesys/partgen.o@'
'./out/linuxx86/debug/libs/csplugincommon/render3d/txtmgr.o@'
'./out/linuxx86/debug/libs/csplugincommon/render3d/normalizationcube.o@'
'./out/linuxx86/debug/libs/csplugincommon/renderstep/parserenderstep.o@'
'./out/linuxx86/debug/libs/csplugincommon/renderstep/basesteptype.o@'
'./out/linuxx86/debug/libs/csplugincommon/renderstep/basesteploader.o@'
'./out/linuxx86/debug/libs/csplugincommon/script/scriptcommon.o@'
'./out/linuxx86/debug/libs/csplugincommon/shader/shaderprogram.o@'
'./out/linuxx86/debug/libs/csplugincommon/sndsys/convert.o@'
'./out/linuxx86/debug/libs/csplugincommon/sndsys/cyclicbuf.o@'
'./out/linuxx86/debug/libs/csplugincommon/sndsys/sndstream.o@'
'./out/linuxx86/debug/libs/csplugincommon/sndsys/snddata.o@' | sed 's/@ /@/g' |
tr '@' '
1811 ' >> ./out/linuxx86/debug/libs/libcrystalspace-1.2.so.resp
1812
1813
1814 g++ -Wl,--as-needed -shared -o
./out/linuxx86/debug/libs/libcrystalspace-1.2.so
-Wl,@./out/linuxx86/debug/libs/libcrystalspace-1.2.so.resp -lm -ldl -lnsl
-Wl,-z,defs -Wl,--warn-unresolved-symbols -Wl,-E -g3 -lpthread -lpthread -lz
-lm -ldl -lnsl -Wl,-z,defs -Wl,--warn-unresolved-symbols -Wl,-E -g3 \
1815 -Wl,-soname,libcrystalspace-1.2.so
-----------------------------------------------------------------------------
GPLv2+/plugins/sndsys/element/
-> sndsysXXX.so (in main) is GPLv2+
GPLv2+/plugins/utilities/movierecorder/
-> movierecorder.so (in main) is GPLv2+ (not GPLv2)
GPLv2+/scripts/jamtemplate/createproject.sh
-> createproject.sh (in -devel) is GPLv2+
Check for GPLv2+ pollution
root:
GPLv2+/include/csplugincommon/sndsys/snddata.h ->
LGPLv2+/include/csplugincommon.h:#include "csplugincommon/sndsys/snddata.h" ->
LGPLv2+/include/crystalspace.h:#include "csplugincommon.h" ->
File_C_or_C++/plugins/cscript/pycscegui/cs_cegui.cpp:#include "crystalspace.h"
File_C_or_C++/plugins/cscript/csperl5/cswigpl5.cpp:#include "crystalspace.h"
LGPLv2+/apps/tools/genmeshify/converter.cpp:#include "crystalspace.h"
LGPLv2+/apps/tools/genmeshify/genmeshify.cpp:#include "crystalspace.h"
LGPLv2+/apps/tools/genmeshify/processor.cpp:#include "crystalspace.h"
LGPLv2+/apps/tools/genmeshify/stdloadercontext.cpp:#include "crystalspace.h"
LGPLv2+/apps/tools/startme/startme.h:#include <crystalspace.h>
LGPLv2+/apps/tools/lighter2/common.h:#include "crystalspace.h"
LGPLv2+/apps/tools/basemapgen/basemapgen.h:#include "crystalspace.h"
LGPLv2+/apps/tools/heightmapgen/heightmapgen.h:#include "crystalspace.h"
LGPLv2+/apps/tools/partconv/partconv.cpp:#include "crystalspace.h"
( -----------------
LGPLv2+/apps/walktest/walktest.cpp:#include "crystalspace.h"
LGPLv2+/apps/pysimp/pysimp.h:#include <crystalspace.h>
LGPLv2+/apps/tests/imptest/imptest.h:#include <crystalspace.h>
LGPLv2+/apps/tests/eventtest/eventtest.h:#include <crystalspace.h>
LGPLv2+/apps/tests/sndtest/sndtest.h:#include <crystalspace.h>
LGPLv2+/apps/tutorial/pathtut/pathtut.h:#include <crystalspace.h>
LGPLv2+/apps/tutorial/simpcd/simpcd.h:#include <crystalspace.h>
LGPLv2+/apps/tutorial/simple2/simple2.h:#include <crystalspace.h>
LGPLv2+/apps/tutorial/mazing/gamestuff.cpp:#include <crystalspace.h>
LGPLv2+/apps/tutorial/mazing/appmazing.h:#include <crystalspace.h>
LGPLv2+/apps/tutorial/simplept/simplept.h:#include <crystalspace.h>
LGPLv2+/apps/tutorial/phystut/phystut.h:#include <crystalspace.h>
LGPLv2+/apps/tutorial/simpmap/simpmap.h:#include <crystalspace.h>
LGPLv2+/apps/tutorial/simpvs/simpvs.h:#include <crystalspace.h>
LGPLv2+/apps/tutorial/simple1/simple1.h:#include <crystalspace.h>
------------- can be killed )
-> _pycscegui.so (in crystalspace-devel) is polluted
-> Some files in -utils are also polluted
GPLv2+/include/csplugincommon/sndsys/sndstream.h ->
LGPLv2+/include/csplugincommon.h:#include "csplugincommon/sndsys/sndstream.h"
(-> same above)
GPLv2+/include/ivaria/movierecorder.h ->
LGPLv2+/include/ivaria.h:#include "ivaria/movierecorder.h" ->
(-> not pollute any more)
Summary
main : LGPLv2+ and GPLv2+
-utils: LGPLv2+ and GPLv2+ and GPLv2
-demos: LGPLv2+ and GPLv2+
-devel: LGPLv2+ and GPLv2+
-doc: ????
Ah... as libcrystalspace-1.2.so is polluted by GPLv2+, the tag "LGPLv2+" seems almost nonsense. (In reply to comment #9) > For 1.2-3: > > * As I wrote some missing dependency for -devel subpackage as "Example", > it seems that there are some more missing dependencies for -devel package > - cegui-devel > - zlib-devel > (I guess adding more 2 BR listed above should be enough) > Sorry, I missunderstood. I did a full check and couldn't find any other headers besides the 2 above which I've added > * It seems that adding INSTALL="install -p" to make install actually works > http://koji.fedoraproject.org/koji/taskinfo?taskID=426911 > Strange as the makefile is justb a skeleton calling jam, but if it works thats good! Added. > ! This weekend I will try to check license issues (if any) for this > package (more than 10000 files needs checking) Okay, about your analysis, I agree, except that the movierecorder plugin really is GPLv2 and not GPLv2+, as it includes (and uses) the nuppelvideo.h file which says: /* This file is from the NuppelVideo project: * * (c) Roman Hochleitner roman.ac.at * NuppelVideo is distributed under the GNU GENERAL PUBLIC LICENSE version 2 */ This makes the list: main : GPLv2+ and GPLv2 -utils: GPLv2+ and GPLv2 -demos: GPLv2+ -devel: GPLv2+ -doc: ???? So to make things easier, esp the ????, I've just added a License tag of: "GPLv2+ and GPLv2" to the main package, and let all the subpackages inherent this. I've added a large comment above the main License tag explaining why it is what it is. Also I will contact upstream about this, as I believe they intend the core of crystalspace to be LGPL not GPL. Here is a new (hopefully the last) version: Spec URL: http://people.atrpms.net/~hdegoede/crystalspace.spec SRPM URL: http://people.atrpms.net/~hdegoede/crystalspace-1.2-4.fc9.src.rpm Thanks I will check this after rebuilding this on koji again. By the way I will appreciate it if you would review my review request bug 433182 (IMO very simple) (In reply to comment #12) > Also I will contact upstream about this, as I believe they intend > the core of crystalspace to be LGPL not GPL. I guess so. Okay. ------------------------------------------------------------------------ This package (crystalspace) is APPROVED by me ------------------------------------------------------------------------ Thanks for the review! New Package CVS Request ======================= Package Name: crystalspace Short Description: Crystal Space a free 3D engine Owners: jwrdegoede Branches: F-8 InitialCC: Cvsextras Commits: Yes cvs done. Imported, build and Fedora 8 update requested in bodhi, closing. |