Spec URL: http://palango.fedorapeople.org/tao/taoframework.spec SRPM URL: http://palango.fedorapeople.org/tao/taoframework-2.1.0-1.fc11.src.rpm Description: The Tao Framework for .NET is a collection of bindings to facilitate cross-platform media application development utilizing the .NET and Mono platforms.
Before I do the full review here are some a issues I've just seen: 1. Source0: Please use the full URL of the official tarball (looks like this the generic sf.net URL). "spectool -g taoframework.spec" should work and retrieve the source file. 2. The koji build fails for x86_64: https://koji.fedoraproject.org/koji/taskinfo?taskID=1736656 3. Please ask upstream whether they are interested in Patch1 and add a comment about the upstream status of it. Otherwise the package looks good!
Thanks for looking at this Christian. on 1. The URL is http://sourceforge.net/projects/taoframework/files/The%20Tao%20Framework/2.1.0/taoframework-2.1.0.tar.gz/download This doesn't work, so I removed the ending '/download'. But this doesn't work with spectool too. 2. I fixed that, see: http://koji.fedoraproject.org/koji/taskinfo?taskID=1737616 3. Will do this.
(In reply to comment #2) > on 1. > The URL is > http://sourceforge.net/projects/taoframework/files/The%20Tao%20Framework/2.1.0/taoframework-2.1.0.tar.gz/download > > This doesn't work, so I removed the ending '/download'. But this doesn't work > with spectool too. Please try the URL suggested by http://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net : http://downloads.sourceforge.net/taoframework/files/The Tao Framework/%{version}/%{name}-%{version}.tar.gz > 2. I fixed that, see: > http://koji.fedoraproject.org/koji/taskinfo?taskID=1737616 > 3. Will do this. Looking forward to an updated package with the mentioned changes to do the full review!
Corrected to URL and submitted the patched 0 and 2 upstream. Think this is ready for review now, thanks Christian for reviewing!
Here is the complete review - sorry that it took a little bit longer than expected: * rpmlint: OK rpmlint RPMS/i386/taoframework-* SRPMS/taoframework-2.1.0-1.fc10.src.rpm SPECS/taoframework.spec taoframework-cg.i386: W: only-non-binary-in-usr-lib taoframework-cg.i386: W: no-documentation taoframework-cg-devel.i386: W: no-documentation taoframework-devil.i386: W: only-non-binary-in-usr-lib taoframework-devil.i386: W: no-documentation taoframework-devil-devel.i386: W: no-documentation taoframework-doc.i386: W: only-non-binary-in-usr-lib taoframework-doc.i386: W: no-documentation taoframework-ffmpeg.i386: W: only-non-binary-in-usr-lib taoframework-ffmpeg.i386: W: no-documentation taoframework-ffmpeg-devel.i386: W: no-documentation taoframework-freeglut.i386: W: only-non-binary-in-usr-lib taoframework-freeglut.i386: W: no-documentation taoframework-freeglut-devel.i386: W: no-documentation taoframework-freetype.i386: W: only-non-binary-in-usr-lib taoframework-freetype.i386: W: no-documentation taoframework-freetype-devel.i386: W: no-documentation taoframework-glfw.i386: W: only-non-binary-in-usr-lib taoframework-glfw.i386: W: no-documentation taoframework-glfw-devel.i386: W: no-documentation taoframework-lua.i386: W: only-non-binary-in-usr-lib taoframework-lua.i386: W: no-documentation taoframework-lua-devel.i386: W: no-documentation taoframework-ode.i386: W: only-non-binary-in-usr-lib taoframework-ode.i386: W: no-documentation taoframework-ode-devel.i386: W: no-documentation taoframework-openal.i386: W: only-non-binary-in-usr-lib taoframework-openal.i386: W: no-documentation taoframework-openal-devel.i386: W: no-documentation taoframework-opengl.i386: W: only-non-binary-in-usr-lib taoframework-opengl.i386: W: no-documentation taoframework-opengl-devel.i386: W: no-documentation taoframework-physfs.i386: W: only-non-binary-in-usr-lib taoframework-physfs.i386: W: no-documentation taoframework-physfs-devel.i386: W: no-documentation taoframework-sdl.i386: W: only-non-binary-in-usr-lib taoframework-sdl.i386: W: no-documentation taoframework-sdl-devel.i386: W: no-documentation 26 packages and 1 specfiles checked; 0 errors, 38 warnings. All of the reported warnings are false-positive: W: only-non-binary-in-usr-lib: mono assemblies are not recognized as binaries by rpmlint W: no-documentation: the API documentation is entirely located in the doc subpackage * naming: OK - name matches upstream - spec file name matches package name * sources: OK - md5sum: a48240aabbb46194a2c347ec262588e2 taoframework-2.1.0.tar.gz - sources matches upstream - Source0 tag ok - spectool -g works * binaries in upstream sources: TODO - the upstream tarball contains a bunch of pre-built binaries (*.exe, *.dll) - since all delivered content must be built out of the sources, please delete all binaries in the %prep step just to be on the safe side: find -name '*.dll' -exec rm -f {} \; find -name '*.exe' -exec rm -f {} \; * License: OK - License in spec file does match the actual license - MIT license acceptable for Fedora - one sub-project (Tao.GlBindGen) is licensed under the GPL, but it seems only to be used to generate the OpenGL C# binding which is done upstream and not during the build process; the project even builds fine without this subdirectory - so for now it doesn't look like a license issue here - however, if any pieces of this directory are used in an update of taoframework, the license of this package may be affected * package containing *.pc files must "Requires: pkgconfig": OK * spec file written in English and legible: OK * compilation: TODO - please check whether the package does support a parallel build, if not please add a short comment in the %build section - builds fine in koji: F13, F12, F11 * BuildRequires: OK * locales handling: OK (n/a) * ldconfig in %post and %postun: OK (n/a) * package owns all directories that it creates: OK * %files section: TODO In the subpackages you explicitly include the directory with %dir and then all contained files manually - e.g.: %dir %{_libdir}/mono/tao-glfw-2.6 %{_libdir}/mono/tao-glfw-2.6/Tao.Glfw.dll %{_libdir}/mono/tao-glfw-2.6/Tao.Glfw.dll.config If there is no reason for doing this, I would suggest to use: %{_libdir}/mono/tao-glfw-2.6 * no files listed twice in %files: OK * file permissions: OK - %defattr used - actual permissions in packages ok * %clean section: OK * macro usage: OK * code vs. content: OK (only code besides some pictures/textures in examples/Data which are not installed) * main package (or here: subpackages) should not contain development related parts: OK * large documentation into subpackage: OK * header files in -devel subpackage: OK (n/a) * static libraries in -static package: OK (n/a) * *.so link in -devel package: OK (n/a) * devel package requires base package using fully versioned dependency: OK - done for all subpackages * packages must not contain *.la files: OK * GUI applications must provide *.desktop file: OK (n/a) * packages must not own files/dirs already owned by other packages: OK * rm -rf $RPM_BUILD_ROOT at the beginning of %install: OK * all filenames UTF-8: OK * functional test: n/a * debuginfo sub-package: OK (n/a) * dependencies: TODO All C# bindings of the taoframework use a quite loose binding to the wrapped shared objects files by using C#'s dllImport feature together with a mapping to the actual *.so.* name in the dll.config files - e.g.: taoframework-opengl uses: libGLU.so.1 and libGL.so.1 This means that if the taoframework's opengl C# binding is used, and the mono runtime doesn't find the appropriate libGL.so.1, the program will most likely crash or is aborted due to an exception. To make use of the dependency resolution algorithms of the package manager, it would be better if each subpackage would require the appropriate package which provides the library (if it is available in Fedora's official package repositories).
New spec-file is online. http://palango.fedorapeople.org/tao/taoframework.spec However one thing is strange. The new spec-file builds fine on x86_64 but fails to build on x86. This worked before and fails at a position where no problem is shown on x86_64. Any ideas on this?
Created attachment 369274 [details] changes in this version diff from last revision to latest. what I forgot: The build I'm referring to is here: http://koji.fedoraproject.org/koji/taskinfo?taskID=1803729
Forget the last 2 comments. I thought I tested parallel build - but i apparently didn't. The diff is more or less the same. only killed the parallel build. The new files are here: http://palango.fedorapeople.org/tao/
I've reviewed the new package - here are the some minor remaining issues: - very minor issue: please add a comment in the %build section that parallel build doesn't work - thanks for adding the dependencies - most of them are OK, but for some of them there are some inconsistencies between the *.config files and the provided libraries: I've seen that sometimes the required libraries are not included in Fedora but only in one of the rpmfusion repositories. I'm a little bit unsure, whether it is OK if a Fedora package requires an rpmfusion package, but on the other hand IMHO there is more harm done if the user tries to use the taoframework subpackage (without having the required native library installed) and the mono runtime will just abort and give backtraces. So I would vote for adding the requirements even if they are not in the official Fedora repository. Sure, the package can't be installed then on a Fedora-only system, but it doesn't matter because it wouldn't work anyway. ;-) Additionally sometimes the major version of the libraries differ between the required ones in the config files and the ones provided in the system. Is there any possibility to test, whether a respective binding works? - ffmpeg the config files maps the following libraries: <dllentry os="linux" dll="libavcodec.so.1d" /> <dllentry os="linux" dll="libavformat.so.1d" /> <dllentry os="linux" dll="libavutil.so.1d" /> I haven't found them in any fedora or rpmfusion package. Probably the name is mis-spelled? - lua the config file claims to use the following shared object <dllentry os="linux" dll="liblua5.1.so.0" /> however, lua in Fedora provides only: /usr/lib/liblua-5.1.so Please adjust the config file. - ode the config files maps the following library: <dllentry os="linux" dll="libode.so.0debian1" /> but in it is named in Fedora: /usr/lib/libode.so.1 Please adjust the config file. - physfs: <dllentry os="linux" dll="libphysfs-1.0.so.0" /> vs. /usr/lib/libphysfs-1.0.so.1 Please adjust the config file. - SDL: <dllentry os="linux" dll="libsmpeg-0.4.so.0" /> requires smpeg-libs from rpmfusion-free, I suggest to add smpeg-libs to the requirements mismatch in SDL_gfx: <dllentry os="linux" dll="libSDL_gfx.so.4" /> vs /usr/lib/libSDL_gfx.so.0 Please adjust the config file. - cg: requires /usr/lib/Cg.so and libCgGL.so which are provided by libCg in rpmfusion-nonfree - glfw: <dllentry os="linux" dll=""libglfw.so" /> should be better <dllentry os="linux" dll="libglfw.so.2.6" />
I think it would be wrong to add dependencies for packages only available on rpmfusion. I'm not sure about it, but I think it would be impossible to install this package without having enabled rpmfusion. I think it's better to make the dependencies as good as possible. If there are any products which use this packages they should care about the required dependencies. Fixed config files: - lua - ode - physfs - SDL_gfx - glfw don't fixed because of rpmfusion requirement: - cg - sdl: libsmpeg - ffmpeg
(In reply to comment #10) > I think it would be wrong to add dependencies for packages only available on > rpmfusion. I'm not sure about it, but I think it would be impossible to install > this package without having enabled rpmfusion. Yes, I fully agree now.... ;-) > I think it's better to make the dependencies as good as possible. If there are > any products which use this packages they should care about the required > dependencies. Hm, I think I disagree with this statement. IMHO the packages in Fedora should follow these rules: - the requirements of a package should include all prerequisites which are necessary to use the package (this includes e.g. the native libraries which are used by the late binding in C#) - if a package would not be usable without another one, it must be included in the requirements - a Fedora package must be installable on a Fedora-only system Given these two rules, there would be only the one solution to exclude these bindings which need native libs from rpmfusion completely and - if needed - package them in rpmfusion. I know, this would somehow "split" the taoframework package - but IMHO this would be the best solution. Do you think this would be a reasonable way? Otherwise the package looks good.
I fully agree with you! Only including binding for libraries that we support in fedora sound like the best way. Is there a way to exclude some subpackages from being built?
(In reply to comment #12) > I fully agree with you! Only including binding for libraries that we support in > fedora sound like the best way. > > Is there a way to exclude some subpackages from being built? I don't know of any other way than just to comment out all references to the unwanted sub-packages in the spec file. Additionally it is necessary to get rid of the unwanted bindings during build and install. The best way would be to make a patch against sources/src/Makefile.am to exclude the sub-directories which are not needed.
Sorry it took so long to update. Patched the Makefile and commented the unwanted parts in the specfile out.
(In reply to comment #14) > Sorry it took so long to update. I have to apologize for my long response time as well... > Patched the Makefile and commented the unwanted parts in the specfile out. I've checked the new package and everything looks fine now besides one small issue: In file /usr/lib/mono/gac/Tao.Glfw/2.6.0.0__2bb092b6587e4402/Tao.Glfw.dll.config there is a line: <dllentry os="linux" dll=""libglfw.so.2.6" /> The 2 double quotes one after the other seem to be wrong. Sure, it looks like that it is upstream broken as well, but we should fix it anyway. Once this is fixed, the package can be approved.
Thanks for your review! Updated package with fixed double quote is here. http://palango.fedorapeople.org/tao/
Thanks for the fix! All issues are resolved now -> APPROVED.
New Package CVS Request ======================= Package Name: taoframework Short Description: Multimedia bindings for Mono Owners: palango Branches: F-12 InitialCC:
CVS done (by process-cvs-requests.py). You may want to consider making the non fredora parts conditional, so others can rebuild this if they desire.
Done. Thanks for reviewing Christian!