Bug 480050 - Review Request: libchamplain - Map view for Clutter
Summary: Review Request: libchamplain - Map view for Clutter
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Denis Leroy
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 480056
TreeView+ depends on / blocked
 
Reported: 2009-01-14 18:32 UTC by Debarshi Ray
Modified: 2009-03-13 18:39 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-02-20 05:01:35 UTC
denis: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Debarshi Ray 2009-01-14 18:32:33 UTC
Spec URL: http://rishi.fedorapeople.org/libchamplain.spec
SRPM URL: http://rishi.fedorapeople.org/libchamplain-0.2.8-1.fc10.src.rpm

Description:
Libchamplain is a C library aimed to provide a ClutterActor to display
rasterized maps.

Comment 1 Debarshi Ray 2009-01-14 18:32:48 UTC
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1053523

Comment 2 Jochen Schmitt 2009-01-14 20:04:33 UTC
Good:
+ Basename of spec file match with package name
+ Package name fits naming guidelines
+ Source tar ball could downloaded with spectool
+ Tar ball in package matches with upstream
(md5sum: fa4a620efa1a1c1036b6701b7d4dafe1)
+ Package contains a valid OSS license on license tag
+ Package contains a verbatin copy of the license text
+ Rpmlinkt ok with source package.
+ Rpm macros are used consitently
+ BUILDROOT will be clean on the beginning of the %install and %clean stanza
+ Package contains ldconfig scriptlet
+ Package contains several subpackages
+ devel subpackage Requires maiin package
+ %doc stanza is small
+ Koji build works fine
+ Rpmlint is quite on binary packages
+ Rpmlint is quite on debugin package
+ Local install works fine
+ Rpmlint is quite on instlled package
+ Local remove of the package works fine
+ files of the packages doesn't belongs to other packages
+ no duplicate files are listed in the %file stanza
+ All listed files are owned by the package
+ %doc stanza is small so we need no doc subpackage
+ Package contains valid %changelog

Bad.
- Local build fails with:
gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -o .libs/libchamplain-scan .libs/libchamplain-scan.o
-pthread -Wl,--export-dynamic  -lgdk-x11-2.0 -lclutter-cairo-0.8 -lclutter-glx-0.8 -lpangocairo-1.0 -lgthread-2.0 -lrt -lgdk_pixbuf_xlib-2.0 -lpango-1.0 -lgdk_pixbuf-2.0 -lcairo
 -lsoup-2.4 -lgio-2.0 -lgobject-2.0 -lgmodule-2.0 -lglib-2.0 ../../champlain/.libs/libchamplain-0.2.so  -Wl,--rpath -Wl,/usr/lib64
creating libchamplain-scan
gtk-doc: Running scanner libchamplain-scan
Scan failed:
make[2]: *** [scan-build.stamp] Error 255
make[2]: Leaving directory `/home/s4504kr/rpmbuild/BUILD/libchamplain-0.2.8/docs/reference'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/s4504kr/rpmbuild/BUILD/libchamplain-0.2.8'
make: *** [all] Error 2

ToDO.

you can wrote

%{_includedir}/%{name}-0.2/

instead of

%dir %{_includedir}/%{name}-0.2
%{_includedir}/%{name}-0.2/champlain

and 

%{_datadir}/champlain/

instead

%dir %{_datadir}/champlain
%{_datadir}/champlain/error.svg

Comment 3 Debarshi Ray 2009-01-16 19:58:40 UTC
(In reply to comment #2)

> - Local build fails with:
> gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
> --param=ssp-buffer-size=4 -m64 -mtune=generic -o .libs/libchamplain-scan
> .libs/libchamplain-scan.o
> -pthread -Wl,--export-dynamic  -lgdk-x11-2.0 -lclutter-cairo-0.8
> -lclutter-glx-0.8 -lpangocairo-1.0 -lgthread-2.0 -lrt -lgdk_pixbuf_xlib-2.0
> -lpango-1.0 -lgdk_pixbuf-2.0 -lcairo
>  -lsoup-2.4 -lgio-2.0 -lgobject-2.0 -lgmodule-2.0 -lglib-2.0
> ../../champlain/.libs/libchamplain-0.2.so  -Wl,--rpath -Wl,/usr/lib64
> creating libchamplain-scan
> gtk-doc: Running scanner libchamplain-scan
> Scan failed:
> make[2]: *** [scan-build.stamp] Error 255
> make[2]: Leaving directory
> `/home/s4504kr/rpmbuild/BUILD/libchamplain-0.2.8/docs/reference'
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory `/home/s4504kr/rpmbuild/BUILD/libchamplain-0.2.8'
> make: *** [all] Error 2

It is strange, but I tried this on an updated Fedora 10 x86_64 system and on Koji against the dist-f11 tag and both passed. Here is the relevant portion from a rpmbuild on my Fedora 10 x86_64 system:
[...]
gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -o .libs/libchamplain-scan .libs/libchamplain-scan.o -pthread -Wl,--export-dynamic  -lgdk-x11-2.0 -lclutter-cairo-0.8 -lclutter-glx-0.8 -lpangocairo-1.0 -lgthread-2.0 -lrt -lgdk_pixbuf_xlib-2.0 -lpango-1.0 -lgdk_pixbuf-2.0 -lcairo -lsoup-2.4 -lgio-2.0 -lgobject-2.0 -lgmodule-2.0 -lglib-2.0 ../../champlain/.libs/libchamplain-0.2.so  -Wl,--rpath -Wl,/usr/lib64
creating libchamplain-scan
gtk-doc: Running scanner libchamplain-scan
touch scan-build.stamp
gtk-doc: Rebuilding template files
[...]

Which version of Fedora did you try?

> ToDO.
> 
> you can wrote
> 
> %{_includedir}/%{name}-0.2/
>
> [...]
>
> and 
> 
> %{_datadir}/champlain/

Actually, I do not do that with directories created by the package to avoid being suprised by the addition or removal of an important sub-directory in a new upstream release. In a way this is merely a personal preference which I follow in almost all my packages. I hope that it goes well with the guidelines.

Comment 4 Jochen Schmitt 2009-01-18 16:43:47 UTC
(In reply to comment #3)

> Which version of Fedora did you try?

F-10 (x86_64) full updated.

Comment 5 Debarshi Ray 2009-01-18 17:55:32 UTC
I tried a Koji scratch build against dist-f10-updates-candidate and it finished successfully: http://koji.fedoraproject.org/koji/taskinfo?taskID=1064274

Any idea what is going wrong?

Comment 6 Jochen Schmitt 2009-01-18 18:10:26 UTC
Unfortunately No.

Comment 7 Rakesh Pandit 2009-01-19 08:33:05 UTC
I can confirm that package builds fine on my machine (F10 x86_64) as well as i686 (my test box): I also did a all arch build on koji which was successful:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1065454

Comment 8 Peter Robinson 2009-01-20 07:42:37 UTC
Also builds fine on all dist-f11 rawhide platforms 
http://koji.fedoraproject.org/koji/taskinfo?taskID=1068024

Comment 9 Denis Leroy 2009-01-25 21:15:46 UTC
Compiles fine on my F-10 system.

Jochen, looks like something's borked on your setup. It's also not a compile failure, it's an error from gtkdoc-scan. Anyways, this shouldn't block the review.

Jochen, if you want I can take over the review until you figure out what's going on on your end...

Comment 10 Jochen Schmitt 2009-01-25 21:22:22 UTC
OK, yuo make take over this review, because I don't want to block it.

Comment 11 Denis Leroy 2009-01-26 09:55:06 UTC
Looks solid. Pretty much agree with Jochen's review here. Minor request :

- fold %dir entries in %files, as suggestde by Jochen
- maybe fold 3 consecutive %doc lines into 1 ?
- devel package gtk-doc Require not necessary (is pulled in by gtk2-devel anyways)
- devel package Require on clutter-devel won't be necessary for F-11

Otherwise package looks good. Demo code "launcher.c" won't compile as-is because it unnecessarily include "config.h", probably should notify upstream about that. If you have time, can you sed out that line from it ? (not a review blocker)

Comment 12 Debarshi Ray 2009-01-28 20:39:45 UTC
(In reply to comment #11)

Spec: http://rishi.fedorapeople.org/libchamplain.spec
SRPM: http://rishi.fedorapeople.org/libchamplain-0.2.8-2.fc9.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1089611

> - fold %dir entries in %files, as suggestde by Jochen
> - maybe fold 3 consecutive %doc lines into 1 ?

Merely personal preferences as I explained earlier (comment #3).

> - devel package gtk-doc Require not necessary (is pulled in by gtk2-devel
> anyways)

That is true, but I had explicitly mentioned it because libchamplain installs files in directories owned by gtk-doc and it serves as a kind of reminder.

Hope you do not mind. :-)

> - devel package Require on clutter-devel won't be necessary for F-11

Fixed. Could not test this because my Fedora 10 machine broke down, and libchamplain does not build on Fedora 9.
 
> Otherwise package looks good. Demo code "launcher.c" won't compile as-is
> because it unnecessarily include "config.h", probably should notify upstream
> about that. If you have time, can you sed out that line from it ?

Fixed. Will notify upstream also.

Comment 13 Denis Leroy 2009-01-29 10:11:18 UTC
Great. APPROVED.

Comment 14 Debarshi Ray 2009-01-29 13:01:01 UTC
New Package CVS Request
=======================
Package Name: libchamplain
Short Description: Map view for Clutter
Owners: rishi
Branches: F-10
InitialCC:

Comment 15 Kevin Fenzi 2009-01-30 06:17:42 UTC
cvs done.

Comment 16 Debarshi Ray 2009-02-20 05:01:35 UTC
Tagged and built for F-10 and devel.

Comment 17 Fedora Update System 2009-03-13 18:39:53 UTC
libchamplain-0.2.8-2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.