Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: mapnik - a Free toolkit for developing mapping applications https://bugzilla.redhat.com/show_bug.cgi?id=436704 ------- Additional Comments From rezso@xxxxxxxx 2008-07-06 12:56 EST ------- Mr. Christopher, If you dont mind I wold like to take this package for maintainership with yyour permission, as I already own core GIS ones. My interest is a comercial one, i would like to push some quality into this stuff. First of all let me propose to further review it. http://openrisc.rdsor.ro/mapnik.spec http://openrisc.rdsor.ro/mapnik-0.5.1-1.fc9.src.rpm (In reply to comment #11) > For 0.5.0-0.2 > > * License > - The license of mapnik is LGPLv2+. done. > > * Release number > - If this tarball is created from svn repo, IMO it is better > to include not date but svn revision number for Release tag. > done. Its stable upstream for this time. > * Explicit library dependency > ----------------------------------------------------------- > Requires: boost > Requires: zlib > Requires: freetype > Requires: proj > Requires: gdal > Requires: cairo > Requires: cairomm > ----------------------------------------------------------- > - These library related requires should be catched by > find_require.sh and these type of explicit Requries must be > removed (except for some cases such as mono/java related > packages) > done. > ----------------------------------------------------------- > Requires: python > ----------------------------------------------------------- > - This requires is not needed and must be removed. done. > > * Requires for subpackages > - Requires for -devel subpackage are not added automatically > and you have to find out and add proper Requires. > * Example: > %_includedir/%name/jpeg_io.hpp contains > ----------------------------------------------------------- > 25 extern "C" > 26 { > 27 #include <jpeglib.h> > 28 } > ----------------------------------------------------------- > This means that mapnik-devel should have > "Requires: libjpeg-devel". > The following command is useful for detecting such dependency. > ----------------------------------------------------------- > $ rpm -ql mapnik-devel | grep /usr/include | xargs grep -h 'include ' | sort | uniq done. > ----------------------------------------------------------- > > - Similarly, please check the dependency for -python subpackage > by > ----------------------------------------------------------- > $ rpm -ql mapnik-python | grep python | xargs grep -h 'import ' | sort | uniq > ----------------------------------------------------------- done. > > * Fedora specific compilation flags > - This is not yet correctly honored. done. > * Use of system wide libraries > - build.log shows > ----------------------------------------------------------- > 76 g++ -o agg/src/agg_vcgen_dash.o -c -O3 -fPIC -DNDEBUG -Iagg/include > agg/src/agg_vcgen_dash.cpp > 101 ar rc agg/libagg.a agg/src/agg_line_profile_aa.o ...... > 190 g++ -o src/libmapnik.so .... -L/usr/local/lib -lagg -lfreetype .... > ----------------------------------------------------------- > Here libmapnik.so uses internal libagg.a, not libagg.so provided > by agg-devel. > Please apply patches so that libmapnik.so uses system-wide > libagg.so > - Also > ------------------------------------------------------------ > 166 g++ -o tinyxml/tinystr.os .... tinyxml/tinystr.cpp > 190 g++ -o src/libmapnik.so .... tinyxml/tinystr.os ... > ------------------------------------------------------------ > Here libmapnik.so uses internal tinyxml, however Fedora has > tinyxml-devel so please use system-wide tinyxml. > - By the way Fedora's optimation level is -O2 and -O3 is not > allowed. I got rid of local tinyxml, libxml2-devel external replaces it. I got rid of local agg, external agg-devel replaces it. > > * Macros > - Please use macros. For example, /usr must be %{_prefix}. done. > > * Fonts > - Patch1 shows > ------------------------------------------------------------- > 19 datasource_cache::instance()->register_datasources(mapnik_dir + > "/lib/mapnik/input/"); > 20 - freetype_engine::register_font(mapnik_dir + > "/lib/mapnik/fonts/DejaVuSans.ttf"); > 21 + freetype_engine::register_font(mapnik_dir + > "/usr/share/fonts/dejavu/DejaVuSans.ttf"); > ------------------------------------------------------------- > However /usr/share/fonts/dejavu/DejaVuSans.ttf does not exist on > my system. > * By the way is 'mapnik_dir + "/usr/...."' correct? > - Also if you want to use dejavu fonts, it must be added to > Requires (I am not talking about BuildRequires here). fixed. Olso other liftups, see from changelog: %changelog * Sun Jul 06 2008 Balint Cristian <rezso@xxxxxxxx> - 0.5.1-1 - the license of mapnik is LGPLv2+ - release is now 0.5.1 from upstream stable - fix explicit library dependency requirement - fix library dependency for -devel requirement - fix library dependency for -python requirement - fix to use fedora specific compile flag - fix to use external agg-devel library as shared - make sure get rid of internal tinyxml and agg chunks - use libxml2 by default instead of tinyxml - use macros everywhere in specfile - use external fedora dejavu fonts insted, get rid of local one - place tool binaries in _bindir - add check section and run testsuite, they should pass - add one python tool - build and add doxygen docs - fix multilib issue - fix UTF-8 and some spurious permission - include local copied web license of some demo data - rpmlint should print zarro bugs rpmlint output: mapnik-utils.x86_64: W: no-documentation 6 packages and 0 specfiles checked; 0 errors, 1 warnings. complete koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=699146 Tasaka, I parsed all review guidelines, probably I missed one, its very possible since I did a lot of things, something might be skew from my eye. //cristian -- 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, or are watching someone who is. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review