Bug 435433

Summary: Review Request: ocrad -- An Optical Character Recognition program
Product: [Fedora] Fedora Reporter: Tomas Smetana <tsmetana>
Component: Package ReviewAssignee: Parag AN(पराग) <panemade>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: low Docs Contact:
Priority: low    
Version: rawhideCC: fedora-package-review, notting
Target Milestone: ---Flags: panemade: 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: 2008-03-05 08:35: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:

Description Tomas Smetana 2008-02-29 10:40:07 UTC
Spec URL: http://tsmetana.fedorapeople.org/ocrad/ocrad.spec
SRPM URL: http://tsmetana.fedorapeople.org/ocrad/ocrad-0.17-1.fc8.src.rpm
Description: 
GNU Ocrad is an OCR (Optical Character Recognition) program based on a feature
extraction method. It reads images in pbm (bitmap), pgm (greyscale) or ppm
(color) formats and produces text in byte (8-bit) or UTF-8 formats.
Also includes a layout analyser able to separate the columns or blocks of text
normally found on printed pages.
Ocrad can be used as a stand-alone console application, or as a backend to
other programs.

Comment 1 Parag AN(पराग) 2008-02-29 11:12:33 UTC
1)make command should be using parallel make
make %{?_smp_mflags}

2)your build section should be
%configure 
make CXXFLAGS="$RPM_OPT_FLAGS" %{?_smp_mflags}

3) make install should be called as
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"

4) %files section should look like
%defattr(-,root,root,-)
%doc AUTHORS COPYING ChangeLog NEWS README TODO
%{_bindir}/ocrad
%{_mandir}/man1/*
%{_infodir}/ocrad.info.gz

5) missing requires
Requires(post): /sbin/install-info
Requires(preun): /sbin/install-info


Comment 2 Parag AN(पराग) 2008-02-29 11:17:19 UTC
also,
  build is failing with error
common.cc: In member function 'bool Charset::enable(const char*)':
common.cc:84: error: 'strcmp' is not a member of 'std'



Comment 3 Tomas Smetana 2008-02-29 12:20:02 UTC
(In reply to comment #1)
> 1)make command should be using parallel make
> make %{?_smp_mflags}

Right.

> 2)your build section should be
> %configure 
> make CXXFLAGS="$RPM_OPT_FLAGS" %{?_smp_mflags}

This package doesn't use autotools and its configure script accepts compiler
flags as parameters.  What would be the reason no to use that?  I can change it,
of course.

> 3) make install should be called as
> make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"

I found nothing like this in the guidelines.  I might have overlooked it of
course.  Could you point me at the reasoning for this please?

> 4) %files section should look like
> %defattr(-,root,root,-)
> %doc AUTHORS COPYING ChangeLog NEWS README TODO
> %{_bindir}/ocrad
> %{_mandir}/man1/*
> %{_infodir}/ocrad.info.gz

The package installs info and manual pages with +x permission. Therefore I need
the %attr otherwise rpmlint would complain.

> 5) missing requires
> Requires(post): /sbin/install-info
> Requires(preun): /sbin/install-info

Right.

As for the building problem: my rawhide mock failed to update, so I built the
package on F-8... But there are no shiny new gcc-4.3 C++ headers.  This will
require a patch.

Thanks for looking.  I'll post an update after I solve the compilation errors.

Comment 4 Tomas Smetana 2008-02-29 14:03:39 UTC
Please take a look at the updated version:
Spec URL: http://tsmetana.fedorapeople.org/ocrad/ocrad.spec
SRPM URL: http://tsmetana.fedorapeople.org/ocrad/ocrad-0.17-2.fc8.src.rpm

Thanks.

Comment 5 Parag AN(पराग) 2008-03-01 14:08:01 UTC
(In reply to comment #3)
> (In reply to comment #1)
> > 3) make install should be called as
> > make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
> 
> I found nothing like this in the guidelines.  I might have overlooked it of
> course.  Could you point me at the reasoning for this please?
> 
  Yes. thats not mentioned in packaging guidelines but use of 
INSTALL="install -p"
  ensures that timestamps will be kept same as upstream released of installed
files. You can relate this to
http://fedoraproject.org/wiki/Packaging/Guidelines#head-0239576e441f9ef53d175c4aec8c12868dffb5ab


Also, build failed with
error: Installed (but unpackaged) file(s) found:
   /usr/share/info/dir

Comment 6 Tomas Smetana 2008-03-03 07:10:57 UTC
(In reply to comment #5)
> Also, build failed with
> error: Installed (but unpackaged) file(s) found:
>    /usr/share/info/dir

I don't understand how comes my mock builds don't catch that...

Should be fixed:
Spec URL: http://tsmetana.fedorapeople.org/ocrad/ocrad.spec
SRPM URL: http://tsmetana.fedorapeople.org/ocrad/ocrad-0.17-3.fc8.src.rpm

Comment 7 Parag AN(पराग) 2008-03-03 10:18:58 UTC
Review:
+ package builds in mock (rawhide i386).
koji build => http://koji.fedoraproject.org/koji/taskinfo?taskID=486448
+ rpmlint is silent for SRPM and for RPM.
+ source files match upstream.
687c213b3334d5a6c2dcef97805c5882  ocrad-0.17.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 files present.
+ BuildRequires are proper.
+ Compiler flags are honored correctly.
+ defattr usage is correct.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code.
+ no static libraries.
+ no .pc file present.
+ no -devel subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ no scriptlets are used.
+ Not a GUI app.

APPROVED.

Comment 8 Tomas Smetana 2008-03-03 10:33:32 UTC
New Package CVS Request
=======================
Package Name: ocrad
Short Description: An Optical Character Recognition program
Owners: tsmetana
Branches: F-7 F-8
InitialCC: 
Cvsextras Commits: yes

Comment 9 Kevin Fenzi 2008-03-03 20:08:45 UTC
cvs done.

Comment 10 Tomas Smetana 2008-03-05 08:35:35 UTC
Thank you.