Bug 480050

Summary: Review Request: libchamplain - Map view for Clutter
Product: [Fedora] Fedora Reporter: Debarshi Ray <debarshir>
Component: Package ReviewAssignee: Denis Leroy <denis>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: denis, fedora-package-review, jochen, notting, pbrobinson, rpandit
Target Milestone: ---Flags: denis: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-02-20 05:01:35 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 480056    

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.