Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=715180 Rich Mattes <richmattes@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |richmattes@xxxxxxxxx --- Comment #8 from Rich Mattes <richmattes@xxxxxxxxx> 2011-07-13 19:32:24 EDT --- I have a few comments as well. 1. The boost build dep isn't doing anything in its current form. FreeMat is looking for the math_c99 component, and CMake can't find it. The latest SVN version has a fix in CMakeLists.txt that doesn't specify the math_c99 component on UNIX systems. This should probably be ported, or else a newer snapshot should be used. 2. This RPM doesn't install. None of the included shared libraries that are built end up in the final RPM, as they all lack INSTALL sections in CMakeLists.txt (or link everything statically, depending on the choice below) 3. The library names are very generic, especially libGraphics.so. If you do end up installing them to %{_libdir}, you might want to prefix their names with something unique like freemat. Or, you can do what Mandriva seems to be doing, and use the flag -DBUILD_SHARED_LIBS=OFF to link all of the libs into the FreeMat executable 4. The package is in violation of the package naming guidelines at http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages Since the SVN trunk is a pre-release of version 4.1 (according to the root CMakeLists.txt) the package version should be called freemat-4.1-0.3.20110620svn Also, according to the guidelines, the package name should match the upstream project and tarball name, both of which are "FreeMat" The binary being installed is also capitalized. I think it should be capitalized, even if it seems like the mandriva package uses an all lowercase "freemat" 5. This package would benefit greatly from a .desktop file since it's installing a graphical application. You can add one and submit it upstream for consideration. 6. It looks like blas.ini is required at runtime to figure out which BLAS implementation should be used (you'll want to use ATLAS). Mandriva has a patch that installs blas.ini to /usr/share/freemat and changes the source to look there for it. I believe we should do the same. 7. I don't think it's necessary to separate the .m files in the toolbox into a separate -data package. The freemat language is interpreted and depends on the functions in the .m files for basic functionality. I guess it's not hurting anything since the main package requires the -data package, but the .m files are vital for the program's basic functionality. 8. You might want to copy the PDF Source1 instead of moving it. At least in my case, usually run rpmbuild -ba from ~/rpmbuild/SPECS, and moving the source file generates a file not found error after the first build attempt. For reference, when I talk about the mandriva package, I'm looking at the SRPM at http://svn.mandriva.com/cgi-bin/viewvc.cgi/packages/cooker/freemat/current/ There's also a discussion thread at http://groups.google.com/group/freemat-devel/browse_thread/thread/b9f3b48ac200aec5# that might be relevant -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review