[Bug 436704] Review Request: mapnik - a Free toolkit for developing mapping applications

Sun Jul 6 16:56:15 UTC 2008

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 at rdsor.ro  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 at rdsor.ro> - 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.