Bug 511966 (zbar)

Just some quick comments on your specfile:

- Please take a look at https://fedoraproject.org/wiki/Packaging/Guidelines#StaticLibraries
- 'Requires: pkgconfig' is missing in the -devel package
  https://fedoraproject.org/wiki/Packaging:Guidelines#Pkgconfig_Files
- https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries shows the right syntax for ldconf

Is rpmlint not complaining about the length of the lines in %description?

Hello Fabian,

     Thanks for your review, here a new version based on your comments.

md5sum:
e1ba7b73625a3f0a48f0b84f353f3a95  zbar-0.8-1.fc11.src.rpm
c1df24ddee24b1d1cf5e413bf2589408  zbar.spec

URL to download:
http://dougsland.com/zbar_package/zbar.spec
http://dougsland.com/zbar_package/zbar-0.8-1.fc11.src.rpm

Cheers,
Douglas

Hi Douglas,

Is this your first package? 
I'm unable to find you in packager group nor any other package review request from you.
If yes, please go ahead http://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account
and put FE-NEEDSPONSOR into Blocks field.
if no, what is your FAS username?

Hello Jan,

> Is this your first package? 

Yes

> I'm unable to find you in packager group nor any other package review request
> from you.
> If yes, please go ahead
> http://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account
> and put FE-NEEDSPONSOR into Blocks field.
> if no, what is your FAS username? 

Done! Thanks for your review!
My FAS username is: dougsland

Cheers,
Douglas

spec/srpm seem 404...

Hello Mamoru,

  This morning dougsland.com was down, please try again:

http://dougsland.com/zbar_package/zbar-0.8-1.fc11.src.rpm
http://dougsland.com/zbar_package/zbar.spec

I have build a new spec and src.

cb596a48b5eb0fe5559a541ee3653f64  zbar-0.8-1.fc11.src.rpm
b05e48c859c8ebf7fec40eaab33839ac  zbar.spec

Cheers,
Douglas

Just a note:

   Thanks also to Itamar Reis Peixoto <itamar.br> for reviewing the package.

(In reply to comment #7)

I don't have sponsor power, Mamoru Tasaka is the guy and have the power to sponsor you and review your package,  

I recommend you to use fedorapeople to upload your .spec and src.rpm files.

look ->

http://fedoraproject.org/wiki/Infrastructure/fedorapeople.org

zbar results from koji:

http://koji.fedoraproject.org/koji/taskinfo?taskID=1495552

Cheers,
Douglas

Initial comments:

* License
  - As far as I checked the whole source code, the
    license tag should be "LGPLv2+".

* Description
  - You should not repeat the same description already
    written in the main package in every subpackage.

* About Requires/BuildRequires
  - Usually "BuildRequires: pkgconfig" is redundant.
    Any -devel package containing .pc pkgconfig files should
    have "Requires: pkgconfig" or "Requires: /usr/bin/pkgconfig"
    and for example gtk2-devel has this.

  - Please explain why you want to add version specific dependency
    for ImageMagick-c++. Currently Fedora supports F-10/11/12
    and the lowest version of ImageMagick in these branches
    is 6.4.0.10, ref:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires

  - (Related to the above "Explicit Requires"),
    "Requires: gtk2" for -gtk subpackage, and "Requires: qt > 4" for -qt
    subpackages are redundant (and should be removed).

  - Usually the dependency between packages rebuilt from the
    same srpm should be exact EVR (Epoch-Version-Release) specific
    ( for example -gtk should have "Requires: %{name} = %{version}-%{release}")
    ref:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package

  - -pygtk subpackage should depend on -gtk subpackage
---------------------------------------------------------------
$ ldd -r ./zbarpygtk.so 2>/dev/null | grep zbargtk 
	libzbargtk.so.0 => not found
---------------------------------------------------------------

   - The following line has typo.
---------------------------------------------------------------
Provides: libzbarqt-static = {%version}-%{release}
---------------------------------------------------------------
    (should be %{version}) (but for static archives
    see below)

* Conditional BR
  - build.log shows:
---------------------------------------------------------------
   161  checking for xmlto... no
   189  checking for X11/extensions/Xvlib.h... no
---------------------------------------------------------------
    * Please check if "BuildRequires: xmlto" is not needed.
    * Please consider to add "BuildRequires: libXv-devel"

* Timestamps
  - Please consider to use
---------------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
---------------------------------------------------------------
    to keep timestamps on installed files as much as possible.
    This method usually works for Makefiles generated from
    recent autotools.

* %files
  https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries
  - Please explain why you want to ship static archives (i.e. *.a
    files). Fedora strongly discourages this. Expecially
    as this package provides shared libraries currently I cannot
    figure out why these .a files are needed, ref:

  - libtool .la files must be removed (see the URL above)

  - Please use %{_mandir} instead of %{_datadir}/man :
    https://fedoraproject.org/wiki/Packaging/RPMMacros

  - Usually the file "INSTALL" is for people who want to build/install
    the package by themselves and not needed for people using rpm:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Documentation

  - You don't have to install the document files already in main
    package also to every subpackage.

  - -devel subpackage should own %{_includedir}/zbar directory:
    https://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership
    https://fedoraproject.org/wiki/Packaging:UnownedDirectories

  - For -python subpackage, please don't use "%{_libdir}/python*" but
    use %{python_sitearch}, see:
    https://fedoraproject.org/wiki/Packaging/Python#System_Architecture

By the way please change the release number of your spec file
every time you modify your package to avoid confusion.

Hello Mamoru,

     First of all, thanks for you review!

New version base on your comments available at:

http://dougsland.fedorapeople.org/packages/zbar/F11/v0.2/

diff-spec-between-v0.1-and-v0.2 25-Jul-2009 02:08  7.9K  
zbar-0.8-1.fc11.src.rpm         25-Jul-2009 02:06  472K  
zbar-0.8.tar.bz2                25-Jul-2009 00:50  469K  
zbar.spec                       25-Jul-2009 01:14  4.6K  

Results from koji available at:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1505485

Please let me know if you have any additional comment.

Cheers,
Douglas

As I said in the  previous comment, please change the release
number of your spec file every time you modify the spec file
to avoid confusion. Also please actually write in %changelog
what was modified.

Hello Mamoru,

> Comment #12 From  Mamoru Tasaka (mtasaka.u-tokyo.ac.jp)  2009-07-25 
> 13:26:59 EDT  
>
> As I said in the  previous comment, please change the release
> number of your spec file every time you modify the spec file
> to avoid confusion. Also please actually write in %changelog
> what was modified.  

Sure, they are now fixed.

New spec, srpm, source and diff are available at:
http://dougsland.fedorapeople.org/packages/zbar/F11/v0.3/

Results from Koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1516980

Thanks for your review, let me know if you have any additional comment.

Cheers,
Douglas

For -3:

* SourceURL
  - Missed in the previous comment, however for sourceforge hosted
    source, please follow:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

* Requires / BuildRequires
  - "BuildRequires: libX11-devel" is redundant because
    gtk2-devel requires this (and this is also in BuildRequires).

  https://fedoraproject.org/wiki/Packaging/Guidelines#Requires
  - Now "Requires: ImageMagick-c++" on the main package is redundant
    (and should be removed) as the dependencies for libraries automatically
    detected by rpmbuild should pull in this dependency:
    
  - And again "Requires: gtk2" on -gtk subpackage, and "Requires: qt >= 4"
    on -qt subpackage are redundant (and should be removed).

  - "Requires: %{name} = %{version}-%{release}" on -pygtk subpackage
    is redundant because -pygtk subpackage requires -gtk subpackage and
    -gtk subpackage requires main package.

* Provides
  - "Provides: libzbar-static" on -devel subpackage must be removed
    because no static archives are now included (and same on other
    devel related subpackages)

* Directory ownership issue
  - The %files entry
---------------------------------------------------------
%files
%{_includedir}/zbar
---------------------------------------------------------
    contains the directory %_includedir/zbar itself and all 
    files/directories/etc under this directory (please check
    what $ rpm -ql zbar-devel returns), while
---------------------------------------------------------
%files
%dir %{_includedir}/zbar
---------------------------------------------------------
    contains the directory %_includedir/zbar only.

* Macros in %changelog
  - In changelog, please use %% instead of % to keep macros from
    being expanded ($ rpm -q --changelog zbar
    shows what is actually occuring)

Hello Mamoru,

Here new version based on your last comments:

http://dougsland.fedorapeople.org/packages/zbar/F11/v0.4
diff-from-v0-3-to-v0-4  27-Jul-2009 18:50  5.3K  
zbar-0.8-4.fc10.src.rpm 27-Jul-2009 18:48  474K  
zbar-0.8.tar.bz2        25-Jul-2009 00:50  469K  
zbar.spec               27-Jul-2009 18:49  6.4K 

Let me know if you have any additional comment.

Thanks again!

Cheers,
Douglas

For -4:

* Very misc issues
  - SourceURL does not follow the following yet:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net
    ( your SourceURL seems to work, too, however we now recommend to
      use the URL style written in the above URL )

* Scriptlets
  - Calling /sbin/ldconfig on %post{,un} is not needed for -devel
    subpackages.

* Directory ownership issue
  - The directory %{python_sitearch} _itself_ is owned by python
    and zbar-pygtk should not own this directory.

Now:
------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on my wiki page:
http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets
(Check "No one is reviewing")

Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines

(In reply to comment #16)
> B. Do a "pre-review" of other person's review request
>    (at the time you are not sponsored, you cannot do
>    a formal review)
> 


pre-review welcome here ->

https://bugzilla.redhat.com/show_bug.cgi?id=477979

Hello Mamoru,

> For -4:
>
> * Very misc issues
>  - SourceURL does not follow the following yet:
>    https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net
>    ( your SourceURL seems to work, too, however we now recommend to
>      use the URL style written in the above URL )
>
>* Scriptlets
>  - Calling /sbin/ldconfig on %post{,un} is not needed for -devel
>    subpackages.
>
> * Directory ownership issue
>  - The directory %{python_sitearch} _itself_ is owned by python
>    and zbar-pygtk should not own this directory.

All changes requested are done.

http://dougsland.fedorapeople.org/packages/zbar/F11/v0.5/
diff-from-v0_4_to_v0_5  28-Jul-2009 23:57  2.4K  
zbar-0.8-5.fc10.src.rpm 28-Jul-2009 23:45  475K  
zbar-0.8.tar.bz2        25-Jul-2009 00:50  469K  
zbar.spec               28-Jul-2009 23:45  6.5K  

Results from koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1555848

Thanks
Douglas

Hello Mamoru,

Pre-review available at:
https://bugzilla.redhat.com/show_bug.cgi?id=497833

Cheers,
Douglas

Hello Itamar,

>Comment #17 From  Itamar Reis Peixoto (itamar.br)  2009-07-28 
>
>pre-review welcome here ->
>
>https://bugzilla.redhat.com/show_bug.cgi?id=477979  

Your package looks sane to my eyes (pre-review).

Cheers,
Douglas

Well,
- for tuna review (bug 497833), if I review this ticket (currently
  I am not sure if I can make time to review tuna review formally...)
  I have something to comment on tuna review request, however I
  will accept your initial comments for pre-review of tuna.

-------------------------------------------------------------------
   This package (zbar) is APPROVED by mtasaka
-------------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji)".

Now I am sponsoring you.

If you want to import this package into Fedora 9/10, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Removing NEEDSPONSOR.

Hello Mamoru,

     Thanks attention and for your amazing review! 

Cheers,
Douglas

New Package CVS Request
=======================
Package Name: zbar
Short Description: Bar code reader
Owners: dougsland
Branches: F-10 F-11 EL-5
InitialCC: dougsland

CVS done.

Thanks Jason!

imo the spec url could have been http://zbar.sourceforge.net/ which is actual project website.

Hello Juha,

> imo the spec url could have been http://zbar.sourceforge.net/ which is actual
> project website. 

Agreed, applied!

Thanks
Douglas

Now closing.