* x86_64 build fixed:
   - Koji like it:
     http://koji.fedoraproject.org/koji/taskinfo?taskID=729360
   - The relevant patch was sent upstream.
   - FIXME: Need to test runtime (no x86_64 box for testing yet).

Still waiting for sponsor (hint, hint...):
 * New upstream version. All my patches merged.
 * Clean koji scratch build on all architectures:
   http://koji.fedoraproject.org/koji/taskinfo?taskID=732368

Please post the URLs of your newest spec/srpm. For me 0.10.11 fails to build

(Note: please don't use the URLs of the result place for scratch builds for
preserving
       your spec/srpm to be reviewed. Scratch build will be deleted after about
one week)

OK, uploaded the revised package to:
SRPM URL: http://oron.fedorapeople.org/libhocr/libhocr-0.10.13-1.fc9.src.rpm
SPEC URL: http://oron.fedorapeople.org/libhocr/libhocr.spec (updated).

Thanks,

Well, for 0.10.13-1:

* Explicit library dependency
-----------------------------------------------------------
On main package:
Requires:	fftw, hspell

On -gtk subpackage:
Requires:	gtk2
-----------------------------------------------------------
  - Please explain why you want to write these explicit library related 
    dependencies
      - Generally speaking, rpmbuild automatically detects library related
        dependencies and adds the dependency to binary rpms, so these explicit
        requires must be removed.
        However, if the rpm explicitly depends on the binaries in the
        required rpm, you can leave the explicit Requires (in that case,
        write as a comment why you want to leave the explicit requires)

** Requires
*  Requires for -devel subpackage
   * Every package which ships pkgconfig .pc files must have
     "Requires: pkgconfig"

   * Also please pkgconfig files themselfs to check dependencies for
     -devel subpackage

     A. %_libdir/pkgconfig/libhocr-gtk.pc contains:
---------------------------------------------------------------
Requires: gtk+-2.0
---------------------------------------------------------------
        This means that libhocr-devel subpackage must have "Requires: gtk2-devel"

     B. %_libdir/pkgconfig/libhocr.pc contains:
---------------------------------------------------------------
Libs: -lhocr  -lhspell -lz  -ltiff -lm
---------------------------------------------------------------
        This means that libhocr-devel must have 
        "Requires: hspell-devel zlib-devel libtiff-devel" (because of
        -lhspell -lz -ltiff)
        However all these 3 linkage seem unneeded, because none of the
        header files in libhocr-devel needs header files from 
        fftw-devel, hspell-devel, libtiff-devel.
        I guess "@ld_hspell@ @ld_tiff@" in libhocr.pc.in should be removed.

* Requires for -gtk subpackage
  * Please recheck the dependency for -gtk subpackage.
    For example, /usr/bin/hocr-gtk contains
----------------------------------------------------------------
    28  import pygtk
    29  pygtk.require('2.0')
----------------------------------------------------------------
    This means that libhocr-gtk must have "Requires: pygtk2".

* Timestamps
  - To keep timestamps on installed files, please consider to use
----------------------------------------------------------------
make install \
	INSTALL="install -p" \
	DESTDIR=%{buildroot}		\
.....
-----------------------------------------------------------------
    This method usually works for based Makefiles based on recent
    autotools

* Desktop files
  - Category "X-Fedora" is deprecated and should be removed.

* %doc %_mandir
  - Files under %_mandir are automatically marked under %doc

* Directory ownership issue
  - Please make it sure that all directories created when installing a
    rpm are correctly created by the rpm
    For example, %_datadir/hocr-gtk is not owned by any packages.

    Note:
    When you write:
-----------------------------------------------------------------
%files
%{_datadir}/hocr-gtk
-----------------------------------------------------------------
    This contains the directory %_datadir/hocr-gtk and all files/directories/etc
    under %{_datadir}/gtk, while when you write
-----------------------------------------------------------------
%files
%dir %{_datadir}/hocr-gtk
-----------------------------------------------------------------
    contains the directory %{_datadir}/hocr-gtk only.

* %files entry on -python subpackage
  - On i386/ppc %{python_sitelib} and %{python_sitearch} are the same,
    so on these archs %files entries are duplicate as:
-----------------------------------------------------------------
  1131  warning: File listed twice: /usr/lib/python2.5/site-packages/_hocr.so
  1132  warning: File listed twice: /usr/lib/python2.5/site-packages/hocr.py
  1133  warning: File listed twice: /usr/lib/python2.5/site-packages/hocr.pyc
  1134  warning: File listed twice: /usr/lib/python2.5/site-packages/hocr.pyo
-----------------------------------------------------------------
    Writing %files entry a bit more verbosely can resolve this issue.

* 64 bits issue
  - libhocr{-gtk}.pc.in contains:
-----------------------------------------------------------------
     2  exec_prefix=${prefix}
     3  libdir=${exec_prefix}/lib
-----------------------------------------------------------------
    So libdir is expanded as /usr/lib even on 64 bits archs, but this is wrong
    as on 64 bits archs libdir must be expanded as /usr/lib64.
    libdir=@libdir@ is correct.

(In reply to comment #5)
> Well, for 0.10.13-1:

First, thanks for the excellent and detailed comments.
Since I'm working with upstream about some other small
issues, I'm merging the fixes and hope to upload an updated
package within a day or two.

A short checklist:
> * Explicit library dependency
> -----------------------------------------------------------
> On main package:
> Requires:       fftw, hspell

fftw - removed (yes it should be delegated to RPM)
hspell - move to hocr-gtk subpackage. This package
         uses gtktextbuffer (python bindings) which uses
         gtkspell which test at *runtime* for backend spellcheckers.
         Added a comment about that in the .spec file

> On -gtk subpackage:
> Requires:       gtk2

fixed.

> ** Requires
> *  Requires for -devel subpackage
>    * Every package which ships pkgconfig .pc files must have
>      "Requires: pkgconfig"

Fixed. I didn't check .pc files at all. Looks like I should write
a small script to test these things (supplement rpmlint...)

>    * Also please pkgconfig files themselfs to check dependencies for
>      -devel subpackage
> 
>      A. %_libdir/pkgconfig/libhocr-gtk.pc contains:
> ---------------------------------------------------------------
> Requires: gtk+-2.0

Fixed.

> ---------------------------------------------------------------
>         This means that libhocr-devel subpackage must have "Requires: gtk2-devel"
> 
>      B. %_libdir/pkgconfig/libhocr.pc contains:
> ---------------------------------------------------------------
> Libs: -lhocr  -lhspell -lz  -ltiff -lm
> ---------------------------------------------------------------
>         This means that libhocr-devel must have 
>         "Requires: hspell-devel zlib-devel libtiff-devel" (because of
>         -lhspell -lz -ltiff)
>         However all these 3 linkage seem unneeded, because none of the
>         header files in libhocr-devel needs header files from 
>         fftw-devel, hspell-devel, libtiff-devel.
>         I guess "@ld_hspell@ @ld_tiff@" in libhocr.pc.in should be removed.

Their inclusion in the .pc file is correct since they are used
by the applications, either explicitly (tiffio.h and fftw3.h) or
implicitly (hspell via gtkspell). However, I'll have to digg deeper
to make sure *which* subpackage requires *what* (TODO)

> * Requires for -gtk subpackage
>   * Please recheck the dependency for -gtk subpackage.
>     For example, /usr/bin/hocr-gtk contains
> ----------------------------------------------------------------
>     28  import pygtk
>     29  pygtk.require('2.0')
> ----------------------------------------------------------------
>     This means that libhocr-gtk must have "Requires: pygtk2".

Fixed.

> * Timestamps
>   - To keep timestamps on installed files, please consider to use
> ----------------------------------------------------------------
> make install \
>         INSTALL="install -p" \
>         DESTDIR=%{buildroot}            \
> .....
> -----------------------------------------------------------------
>     This method usually works for based Makefiles based on recent
>     autotools

Hmmm... first time I see this consideration. It seems logical
but are these any guideline about it?

If it's a general problem maybe I can use your method as a solution
for this package, but it seems better to fix the AC_PROG_INSTALL
macro upstream (if they accept the same logic).

> * Desktop files
>   - Category "X-Fedora" is deprecated and should be removed.

Done. I must have got its addition from some obsolete guide
somewhere but I cannot find any reference to it now, nevermind.
 
> * %doc %_mandir
>   - Files under %_mandir are automatically marked under %doc

Removed the %doc.
 
> * Directory ownership issue
>   - Please make it sure that all directories created when installing a
>     rpm are correctly created by the rpm
>     For example, %_datadir/hocr-gtk is not owned by any packages.

Fixed. Will recheck other ownerships as well.

> * %files entry on -python subpackage
>   - On i386/ppc %{python_sitelib} and %{python_sitearch} are the same,
>     so on these archs %files entries are duplicate as:
> -----------------------------------------------------------------
>   1131  warning: File listed twice: /usr/lib/python2.5/site-packages/_hocr.so
>   1132  warning: File listed twice: /usr/lib/python2.5/site-packages/hocr.py
>   1133  warning: File listed twice: /usr/lib/python2.5/site-packages/hocr.pyc
>   1134  warning: File listed twice: /usr/lib/python2.5/site-packages/hocr.pyo
> -----------------------------------------------------------------
>     Writing %files entry a bit more verbosely can resolve this issue.

TODO. I need to check it again.

> * 64 bits issue
>   - libhocr{-gtk}.pc.in contains:
> -----------------------------------------------------------------
>      2  exec_prefix=${prefix}
>      3  libdir=${exec_prefix}/lib
> -----------------------------------------------------------------
>     So libdir is expanded as /usr/lib even on 64 bits archs, but this is wrong
>     as on 64 bits archs libdir must be expanded as /usr/lib64.
>     libdir=@libdir@ is correct.

Fixed (changed all assignments to use @...@ so we won't have wrong
assumptions in general and respect ./configure selections even in
non-rpm builds -- e.g: ./configure --prefix=/opt/kuku --includedir=/foobar
Sent upstream.

Updated packages+specs - ASAP (a day or two).

(In reply to comment #6)
> (In reply to comment #5)
> > ** Requires
> > *  Requires for -devel subpackage
> > ---------------------------------------------------------------
> >         This means that libhocr-devel subpackage must have "Requires: gtk2-devel"
> > 
> >      B. %_libdir/pkgconfig/libhocr.pc contains:
> > ---------------------------------------------------------------
> > Libs: -lhocr  -lhspell -lz  -ltiff -lm
> > ---------------------------------------------------------------
> >         This means that libhocr-devel must have 
> >         "Requires: hspell-devel zlib-devel libtiff-devel" (because of
> >         -lhspell -lz -ltiff)
> >         However all these 3 linkage seem unneeded, because none of the
> >         header files in libhocr-devel needs header files from 
> >         fftw-devel, hspell-devel, libtiff-devel.
> >         I guess "@ld_hspell@ @ld_tiff@" in libhocr.pc.in should be removed.
> 
> Their inclusion in the .pc file is correct since they are used
> by the applications, either explicitly (tiffio.h and fftw3.h) or
> implicitly (hspell via gtkspell). 

I guess currently I don't understand what you want to say correctly.
libhocr-devel itself does not need tiffio.h or fftw3.h.

> > * Timestamps
> >   - To keep timestamps on installed files, please consider to use
> > ----------------------------------------------------------------
> > make install \
> >         INSTALL="install -p" \
> >         DESTDIR=%{buildroot}            \
> > .....
> > -----------------------------------------------------------------
> >     This method usually works for based Makefiles based on recent
> >     autotools
> 
> Hmmm... first time I see this consideration. It seems logical
> but are these any guideline about it?

Although in PackagingDrafts, I usually recommend this:
https://fedoraproject.org/wiki/PackagingDrafts/MultilibTricks#Timestamps

> > * Desktop files
> >   - Category "X-Fedora" is deprecated and should be removed.
> 
> Done. I must have got its addition from some obsolete guide
> somewhere but I cannot find any reference to it now, nevermind.

Perhaps this is difficult to find:
http://www.redhat.com/archives/fedora-extras-list/2006-October/msg00725.html

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > ** Requires
> > > *  Requires for -devel subpackage
> > > ---------------------------------------------------------------
> > >         This means that libhocr-devel subpackage must have "Requires: gtk2-devel"
> > > 
> > >      B. %_libdir/pkgconfig/libhocr.pc contains:
> > > ---------------------------------------------------------------
> > > Libs: -lhocr  -lhspell -lz  -ltiff -lm
> > > ---------------------------------------------------------------
> > >         This means that libhocr-devel must have 
> > >         "Requires: hspell-devel zlib-devel libtiff-devel" (because of
> > >         -lhspell -lz -ltiff)
> > >         However all these 3 linkage seem unneeded, because none of the
> > >         header files in libhocr-devel needs header files from 
> > >         fftw-devel, hspell-devel, libtiff-devel.
> > >         I guess "@ld_hspell@ @ld_tiff@" in libhocr.pc.in should be removed.
> > 
> > Their inclusion in the .pc file is correct since they are used
> > by the applications, either explicitly (tiffio.h and fftw3.h) or
> > implicitly (hspell via gtkspell). 
> 
> I guess currently I don't understand what you want to say correctly.
> libhocr-devel itself does not need tiffio.h or fftw3.h.

Sorry, my previous reply wasn't clear enough:
1. You are correct that fftw-devel and libtiff-devel should not be
   required by libhocr, since only libhocr *implementation* uses them.
   (fixed, they are only mentioned in BuildRequires).

2. However, I think that we still need them listed in the .pc files
   for other (future) applications that may want to link against
   libhocr. Such applications would probably have in their build
   something like:
       libhocr_LIBS=`pkgconfig --libs libhocr`
   So the linker would add '-lfftw -ltiff'

3. TODO: libz is also mentioned in libhocr.pc, but I want to fix it
        first with upstream, since the author forgot to check
        for it in its configure.ac (actually, it's a dependency of
        hspell which don't come with pkgconfig file of its own).

4. TODO: hspell should be removed from libhocr.pc (IMO) since its not
           linked explicitly, but rather loaded during runtime
           by gtkspell. I'll discuss it with upstream as well.

5. I added "Requires: hspell" to the gtk subpackage because rpmbuild
   would not find this dependency automatically (as mentioned it is
   detected by gtkspell during runtime).

> > > * Timestamps
> > >   - To keep timestamps on installed files, please consider to use
> > > ----------------------------------------------------------------
> > > make install \
> > >         INSTALL="install -p" \
> > >         DESTDIR=%{buildroot}            \
> > > .....
> > > -----------------------------------------------------------------
> > >     This method usually works for based Makefiles based on recent
> > >     autotools
> > 
> > Hmmm... first time I see this consideration. It seems logical
> > but are these any guideline about it?
> 
> Although in PackagingDrafts, I usually recommend this:
> https://fedoraproject.org/wiki/PackagingDrafts/MultilibTricks#Timestamps

Fixed. [added the link to my bookmarks, as it does not appear in 
        https://fedoraproject.org/wiki/PackagingDrafts].

> > > * Desktop files
> > >   - Category "X-Fedora" is deprecated and should be removed.
> > 
> > Done. I must have got its addition from some obsolete guide
> > somewhere but I cannot find any reference to it now, nevermind.
> 
> Perhaps this is difficult to find:
> http://www.redhat.com/archives/fedora-extras-list/2006-October/msg00725.html

Thanks,

I also noticed we are blocked on rawhide by bug 457502
So now libhocr can be used as a debugging tool as well ;-)

ping?

 * I fixed all pending issues about a week ago:
   SRPM URL: http://oron.fedorapeople.org/libhocr/libhocr-0.10.13-2.fc9.src.rpm
   SPEC URL: http://oron.fedorapeople.org/libhocr/libhocr.spec
 * However, these fixes were not merged upstream yet. This is bad
   since it requires running autoreconf in %prep, which BuildRequires:
   automake and aclocal.
 * Yesterday, Kobi (the upstream author) told me he would try pushing
   a fixed version in the coming weekend. I think it's worth waiting for it.
 * If it's still delayed, maybe it would be acceptable to push this version
   anyway?

[Ah, almost forgot... pong! ... thanks]

For 0.10.13-2:

* pkgconfig file issue
  - Leaving this line
-------------------------------------------------------
Libs: -lhocr  -lhspell -lz  -ltiff -lm
-------------------------------------------------------
    in libhocr.pc means that libhocr-devel must have
    "Requires: hspell-devel zlib-devel libtiff-devel" (among these,
    zlib-devel is Required by gtk2-devel)

** Requires
* For -gtk subpackage
  - %_bindir/hocr-gtk has:
-------------------------------------------------------
    30  
    31  import gtk, gtk.glade
-------------------------------------------------------
    and this means that -gtk subpackage Requires pygtk2-libglade
    (note: pygtk2-libglade Requires pygtk2)

  - Also the my previous comment
(In reply to comment #5)
> * Requires for -gtk subpackage
>   * Please recheck the dependency for -gtk subpackage.
>     For example, /usr/bin/hocr-gtk contains
> ----------------------------------------------------------------
>     28  import pygtk
>     29  pygtk.require('2.0')
> ----------------------------------------------------------------
>     This means that libhocr-gtk must have "Requires: pygtk2".
    is for libhocr-gtk, not for libhocr-python.

* Desktop issue
  - The Category "Application" is also deprecated and should be removed.
  - sane-pygtk.desktop contains:
------------------------------------------------------------------
Icon=xsane
------------------------------------------------------------------
    However I can find the icon with the name "xsane.<suffix>" nowhere.
    (even in xsane rpm)

* Directory ownership issue
  - The directory %_defaultdocdir/%name-%version is not owned by
    any packges.

! Some notes:
  - This is not a blocker issue, however would you try to remove 
    "implicit declaration of function" warning
    (i.e. try to fix codes so that the build succeeds with
------------------------------------------------------------------
%build
export CFLAGS="%optflags -Werror-implicit-function-declaration"
%configure #--disable-static
make # %{?_smp_mflags}
------------------------------------------------------------------
     or ask the upstream developer to remove this warning?
     (please refer to:
      http://www.redhat.com/archives/fedora-devel-list/2008-March/msg02036.html )
     Currently build fails with "-Werror-implicit-function-declaration" as
     http://koji.fedoraproject.org/koji/taskinfo?taskID=775601

Created attachment 314268 [details]
pkg-config files (*.pc) dependency helper

A stupid perl script to scan recursively for pkg-config (*.pc or *.pc.in)
files and output information related to .spec file dependencies.

This would help prevent dependency omissions in the future.

 * Updates:
   SRPM URL: http://oron.fedorapeople.org/libhocr/libhocr-0.10.13-3.fc8.src.rpm
   SPEC URL: http://oron.fedorapeople.org/libhocr/libhocr.spec

 * All new patches sent upstream.

> pkgconfig file issue
Fixed (also attached helper script in
 https://bugzilla.redhat.com/show_bug.cgi?id=455555#c12)

> ...and this means that -gtk subpackage Requires pygtk2-libglade
Fixed.

> This means that libhocr-gtk must have "Requires: pygtk2".
> is for libhocr-gtk, not for libhocr-python.
Fixed.

> * Desktop issue
>   - The Category "Application" is also deprecated and should be removed.
Fixed. Added a desktop-file-validate in %build to verify in the future.

>  - sane-pygtk.desktop contains:
>  ------------------------------------------------------------------
>  Icon=xsane
>  ------------------------------------------------------------------

Changed from 'xsane' to 'sane', we are now sure we have the icon: 
 * sane.png is included in sane-backends rpm (as well original tarball).
 * sane-backends is required by sane-backends-libs.
 * The libsane.so.1 contained in sane-backends-libs is required
   by python-imaging-sane.
 * python-imaging-sane is explicitly "Required" by libhocr-gtk
 * So installing libhocr-gtk would bring the icon as well.
QED ;-)

> The directory %_defaultdocdir/%name-%version is not owned by
> any packges.
Fixed.

> %build
> export CFLAGS="%optflags -Werror-implicit-function-declaration"
 * Added to .spec and fixed the code.
 * Also fixed a bunch of 'unused variable' warnings.
 * While doing this, I noticed the code contains tons of duplication.
   I offered Kobi (upstream author) some help in refactoring this code
   and wait for his reply (don't want to step on his feet).

For -3:

! desktop-file-validate
------------------------------------------------------
find . -name '*.desktop' | while read file; do
	/usr/bin/desktop-file-validate "$file"
done
------------------------------------------------------
  - These lines are redundant because desktop-file-install fails
    is desktop file to be installed is invalid (some warning are
    suppressed with desktop-file-install, though)

* Document entry
------------------------------------------------------
%files
%defattr(-,root,root,-)
%doc %{hocrdocdir}
------------------------------------------------------
  - This contains the directory %hocrdocdir and all files/directories
    under %hocrdocdir, i.e. including %hocrdorcdir/examples, while
    this is also included in -doc subpackage.

By the way:
-------------------------------------------------------------
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:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


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

Fedora services back to normal, so...

First, libhocr:
 * New upstream version (with most of our patches):
   SRPM: http://oron.fedorapeople.org/libhocr/libhocr-0.10.17-1.fc8.src.rpm
   SPEC: http://oron.fedorapeople.org/libhocr/libhocr.spec

 * desktop-file-validate:
   - I tested the previous, wrong desktop files and found that
     desktop-file-install did not issue any warning about them
     while desktop-file-validate caught the problems.
   - I therefore left my desktop-file-validate calls.

 * Document entry:
   - Fixed so both %{hocrdocdir} directory and files are listed
     explicitly.
   - The examples subdir is in the -devel package.
   - No more duplicates.

A first pre-review:
  https://bugzilla.redhat.com/show_bug.cgi?id=435724

More to follow (both pre-reviews and other packages review-requests...)
Thanks,

Well:

* This package itself is now good
* As to your pre-review for bug 435724, you seem to have chosen some not-so-good review request :)
  Your pre-remark seems good for initial comments, however there are still
  some other issues which needs fixing and some of them are rather critical.
  However for this package I will approve now.

----------------------------------------------------------------------------------
             This package (libhocr) is APROVED by mtasaka
----------------------------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account".
At a point a mail should be sent to sponsor members which notifies
that you need a sponsor. At the stage, please also write on
this bug for confirmation that you requested for sponsorship and
your FAS (Fedora Account System) name. Then I will sponsor you.

If you want to import this package into Fedora 8/9, 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.

 * Requested sponsorship
 * FAS account: oron

Now I am sponsoring you. Please follow "Join" wiki again.

New Package CVS Request
=======================
Package Name: libhocr
Short Description: A Hebrew optical character recognition library
Owners: oron
Branches: F-8 F-9
InitialCC: oron

cvs done.

libhocr-0.10.17-1.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/libhocr-0.10.17-1.fc8

libhocr-0.10.17-1.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/libhocr-0.10.17-1.fc9

Now closing.

libhocr-0.10.17-1.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

libhocr-0.10.17-1.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.