Bug 253909 - Review Request: libpano13 - Library for manipulating panoramic images
Review Request: libpano13 - Library for manipulating panoramic images
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-08-22 16:49 EDT by Bruno Postle
Modified: 2007-11-30 17:12 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-10-02 17:02:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Bruno Postle 2007-08-22 16:49:37 EDT
Spec URL: http://bugbear.blackfish.org.uk/~bruno/apt/SPECS/libpano13.spec
SRPM URL: http://bugbear.blackfish.org.uk/~bruno/apt/fedora/linux/7/x86_64/SRPMS.panorama/libpano13-2.9.12-3.fc7.src.rpm
Description: Helmut Dersch's Panorama Tools library.  Provides very high quality
manipulation, correction and stitching of panoramic photographs.

Note: I already package libpano12 for fedora, libpano13 is a fork
of this libpano12, but is designed to be installed in parallel.

Currently they are very similar and the SPEC is almost identical, the
differences being that:

1. various command-line tools have been moved from pano12 to pano13
2. pano12 is in the process of being relicensed to LGPLv3+ (from GPLv2+)

libpano13 is required for the next hugin release, after that libpano12 can be dropped from fedora.
Comment 1 Parag AN(पराग) 2007-09-28 02:29:06 EDT
I will pick this for review.


1)do you need following files in -doc as rpmlint reported them as errors?
blitz-doc.i386: E: zero-length
/usr/share/doc/blitz-doc-0.9/html/classListInitializer__coll__graph.map
blitz-doc.i386: E: zero-length
/usr/share/doc/blitz-doc-0.9/html/classListInitializationSwitch__coll__graph.map
blitz-doc.i386: E: zero-length
/usr/share/doc/blitz-doc-0.9/html/classF__coll__graph.map
blitz-doc.i386: E: zero-length /usr/share/doc/blitz-doc-0.9/html/minmax_8h__incl.map
blitz-doc.i386: E: zero-length
/usr/share/doc/blitz-doc-0.9/html/classReduceMaxIndexVector__coll__graph.png
blitz-doc.i386: E: zero-length
/usr/share/doc/blitz-doc-0.9/html/classVectorIterConst__coll__graph.map
blitz-doc.i386: E: zero-length
/usr/share/doc/blitz-doc-0.9/html/classVectorIter__coll__graph.map
blitz-doc.i386: E: zero-length
/usr/share/doc/blitz-doc-0.9/html/classIRNGWrapper_3_01IRNG_00_01sharedState_01_4__coll__graph.map
blitz-doc.i386: E: zero-length
/usr/share/doc/blitz-doc-0.9/html/classIRNGWrapper_3_01IRNG_00_01independentState_01_4__coll__graph.map
blitz-doc.i386: E: zero-length
/usr/share/doc/blitz-doc-0.9/html/wrap-climits_8h__incl.map
blitz-doc.i386: E: zero-length
/usr/share/doc/blitz-doc-0.9/html/classReduceMaxIndexVector__coll__graph.map


2) Change %defattr(-,root,root) to %defattr(-,root,root,-) for all %files sections.

3)Change preun section as suggested in 
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-47896da5fb2662d75deefeb9ba75145a398515db

4) Add COPYING, README and LICENSE to all %doc lines in each %files sections

5) Good to have examples directory installed as part of -devel package 
  Add examples to %doc of -devel

6) Remove following from SPEC
Requires(post): /sbin/ldconfig
Requires(postun): /sbin/ldconfig

7) Source URL should be
  Source0: http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz

8) Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for
directory ownership and usability).

9)Artistic 1.0 license is not valid now in Fedora. Upstream should move to
Artistic 2.0 license
Comment 2 Parag AN(पराग) 2007-09-28 03:00:05 EDT
oops wrongly pasted above comment which should be in other package review which
I did before. Now its your turn.
here is your preliminary review.

1) Good to use following syntax in SPEC
Requires: %{name} = %{version}-%{release} for -devel and -tools sub packages.

2) Use make %{?_smp_mflags}

3)Change %defattr(-,root,root) to %defattr(-,root,root,-) for all %files sections.
Comment 3 Bruno Postle 2007-09-30 16:34:40 EDT
(In reply to comment #2)
 
> 1) Good to use following syntax in SPEC
> Requires: %{name} = %{version}-%{release} for -devel and -tools sub packages.

Fine, done.

> 2) Use make %{?_smp_mflags}

Should be ok, though I can't test it, done.

> 3)Change %defattr(-,root,root) to %defattr(-,root,root,-) for all %files sections.

Ok, can't find where this extra field is described, but it seems to be common
enough, done.

New SPEC and SRPM:
http://fedorapeople.org/~bpostle/reviews/libpano13/libpano13.spec
http://fedorapeople.org/~bpostle/reviews/libpano13/libpano13-2.9.12-4.fc7.src.rpm
Comment 4 Parag AN(पराग) 2007-09-30 23:27:09 EDT
(In reply to comment #3)
> (In reply to comment #2)
>  
> > 1) Good to use following syntax in SPEC
> > Requires: %{name} = %{version}-%{release} for -devel and -tools sub packages.
> 
> Fine, done.
> 
> > 2) Use make %{?_smp_mflags}
> 
> Should be ok, though I can't test it, done.
> 
> > 3)Change %defattr(-,root,root) to %defattr(-,root,root,-) for all %files
sections.
> 
> Ok, can't find where this extra field is described, but it seems to be common
> enough, done.
  You can find more info on this at
http://docs.fedoraproject.org/developers-guide/ch-rpm-building.html
> 
> New SPEC and SRPM:
> http://fedorapeople.org/~bpostle/reviews/libpano13/libpano13.spec
> http://fedorapeople.org/~bpostle/reviews/libpano13/libpano13-2.9.12-4.fc7.src.rpm

Comment 5 Parag AN(पराग) 2007-10-01 00:05:59 EDT
here comes my review
Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and RPM.
+ source files match upstream.
76b64d3e9bd9b9422bcb4dae4008555a  libpano13-2.9.12.tar.bz2
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc is small so no need of -doc subpackage.
+ BuildRequires are proper.
+ Compiler flags are honoured correctly.
+ defattr usage is correct.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code.
+ no static libraries.
+ no .pc files are present.
+ -devel and -tools subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ ldconfig scriptlets are used.
+ package libpano13-2.9.12-4.fc8 ->
  Provides: libpano13.so.0
  Requires: libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1)
libc.so.6(GLIBC_2.1.3) libc.so.6(GLIBC_2.3) libc.so.6(GLIBC_2.3.4)
libc.so.6(GLIBC_2.4) libjpeg.so.62 libpano13.so.0 libpng12.so.0
libpng12.so.0(PNG12_0) libtiff.so.3 libz.so.1 rtld(GNU_HASH)
+ package libpano13-devel-2.9.12-4.fc8 ->
  Requires: libgcj-devel libjpeg-devel libpano13 = 2.9.12-3.fc8 libpano13.so.0
libpng-devel libtiff-devel zlib-devel
+ package libpano13-tools-2.9.12-4.fc8 ->
  Requires: libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1)
libc.so.6(GLIBC_2.3.4) libc.so.6(GLIBC_2.4) libjpeg.so.62 libpano13 =
2.9.12-3.fc8 libpano13.so.0 libpng12.so.0 libtiff.so.3 libz.so.1 rtld(GNU_HASH)
+ Not a GUI app.
Comment 6 Parag AN(पराग) 2007-10-01 00:15:35 EDT
APPROVED.
Comment 7 Bruno Postle 2007-10-01 16:46:28 EDT
New Package CVS Request
=======================
Package Name: libpano13
Short Description: Library for manipulating panoramic images
Owners: bpostle
Branches: F-7
InitialCC:
Cvsextras Commits: yes
Comment 8 Kevin Fenzi 2007-10-02 12:16:38 EDT
cvs done.

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